Repository: knative/pkg
Pull request #2280 (2021-09-07 23:39:03)

Introduce `duckv1.Pod`.
This is mostly for symmetry since many folks that validate PodSpecable types often also want to validate `Pod`, so while `Pod` isn't as often a duck-type, the main value of this is exposing similar mechanisms to #2279 for `corev1.Pod` without folks needing to define their own `corev1.Pod` clone.

/kind enhancement

**Release Note**

<!--
:page_facing_up: If this change has user-visible impact, write a release note in the block
below. Include the string "action required" if additional action is required of
users switching to the new release, for example in case of a breaking change.

Write as if you are speaking to users, not other Knative contributors. If this
change has no user-visible impact, no release-note is needed.
-->
```release-note

```

**Docs**

<!--
:book: If this change has user-visible impact, link to an issue or PR in
https://github.com/knative/docs.

See here for guidance on getting permanent links to files: https://help.github.com/en/articles/getting-permanent-links-to-files

Please use the following format for linking documentation:
- [knative/docs]: <issue or pr link>
- [Feature Track]: <link>
- [Usage]: <link>
- [Other doc]: <link>
-->
```docs

```


knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2280#" title="Author self-approved">mattmoor</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/duck/OWNERS](https://github.com/knative/pkg/blob/main/apis/duck/OWNERS)~~ [mattmoor]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: @mattmoor: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 611910c918fdab0d6a50726a124ad8591f4606b9 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2280/pull-knative-pkg-unit-tests/1435387170369048576) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/2280). [Your PR dashboard](https://gubernator.knative.dev/pr/mattmoor).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
mattmoor: /retest

metrics exporter 🙃 
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
> Merging [#2280](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (611910c) into [main](https://codecov.io/gh/knative/pkg/commit/26db1ba732f60c5fa781789c55f013b8124d4b2b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) (26db1ba) will **increase** coverage by `0.01%`.
> The diff coverage is `95.45%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/2280/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)

```diff
@@            Coverage Diff             @@
##             main    #2280      +/-   ##
==========================================
+ Coverage   64.30%   64.32%   +0.01%     
==========================================
  Files         223      223              
  Lines        9593     9615      +22     
==========================================
+ Hits         6169     6185      +16     
- Misses       3149     3152       +3     
- Partials      275      278       +3     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [apis/duck/v1/podspec\_types.go](https://codecov.io/gh/knative/pkg/pull/2280/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-YXBpcy9kdWNrL3YxL3BvZHNwZWNfdHlwZXMuZ28=) | `95.23% <85.71%> (-4.77%)` | :arrow_down: |
| [apis/duck/v1/podspec\_defaults.go](https://codecov.io/gh/knative/pkg/pull/2280/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-YXBpcy9kdWNrL3YxL3BvZHNwZWNfZGVmYXVsdHMuZ28=) | `100.00% <100.00%> (ø)` | |
| [apis/duck/v1/podspec\_validation.go](https://codecov.io/gh/knative/pkg/pull/2280/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-YXBpcy9kdWNrL3YxL3BvZHNwZWNfdmFsaWRhdGlvbi5nbw==) | `100.00% <100.00%> (ø)` | |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/2280/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `90.32% <0.00%> (-3.23%)` | :arrow_down: |
| [controller/controller.go](https://codecov.io/gh/knative/pkg/pull/2280/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-Y29udHJvbGxlci9jb250cm9sbGVyLmdv) | `86.49% <0.00%> (+0.36%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Last update [26db1ba...611910c](https://codecov.io/gh/knative/pkg/pull/2280?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2280#" title="Author self-approved">mattmoor</a>*, *<a href="https://github.com/knative/pkg/pull/2280#pullrequestreview-748583483" title="Approved">n3wscott</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/duck/OWNERS](https://github.com/knative/pkg/blob/main/apis/duck/OWNERS)~~ [mattmoor,n3wscott]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/apis/duck/v1/podspec_defaults.go b/apis/duck/v1/podspec_defaults.go
index 9fc3f3ceb..1c6acb1e7 100644
--- a/apis/duck/v1/podspec_defaults.go
+++ b/apis/duck/v1/podspec_defaults.go
@@ -30,7 +30,7 @@ func (wp *WithPod) SetDefaults(ctx context.Context) {
 	}
 }
 
-// psvKey is used for associating a PodSpecDefaulter with a context.Context
+// psdKey is used for associating a PodSpecDefaulter with a context.Context
 type psdKey struct{}
 
 func WithPodSpecDefaulter(ctx context.Context, psd PodSpecDefaulter) context.Context {
@@ -45,3 +45,29 @@ func GetPodSpecDefaulter(ctx context.Context) PodSpecDefaulter {
 	}
 	return untyped.(PodSpecDefaulter)
 }
+
+// PodDefaulter is a callback to validate a Pod.
+type PodDefaulter func(context.Context, *Pod)
+
+// SetDefaults implements apis.Defaultable
+func (p *Pod) SetDefaults(ctx context.Context) {
+	if pd := GetPodDefaulter(ctx); pd != nil {
+		pd(ctx, p)
+	}
+}
+
+// pdKey is used for associating a PodDefaulter with a context.Context
+type pdKey struct{}
+
+func WithPodDefaulter(ctx context.Context, pd PodDefaulter) context.Context {
+	return context.WithValue(ctx, pdKey{}, pd)
+}
+
+// GetPodDefaulter extracts the PodDefaulter from the context.
+func GetPodDefaulter(ctx context.Context) PodDefaulter {
+	untyped := ctx.Value(pdKey{})
+	if untyped == nil {
+		return nil
+	}
+	return untyped.(PodDefaulter)
+}
diff --git a/apis/duck/v1/podspec_types.go b/apis/duck/v1/podspec_types.go
index b822a4847..7b1946e50 100644
--- a/apis/duck/v1/podspec_types.go
+++ b/apis/duck/v1/podspec_types.go
@@ -45,9 +45,6 @@ type WithPod struct {
 	Spec WithPodSpec `json:"spec,omitempty"`
 }
 
-var _ apis.Validatable = (*WithPod)(nil)
-var _ apis.Defaultable = (*WithPod)(nil)
-
 // WithPodSpec is the shell around the PodSpecable within WithPod.
 type WithPodSpec struct {
 	Template PodSpecable `json:"template,omitempty"`
@@ -55,6 +52,8 @@ type WithPodSpec struct {
 
 // Verify WithPod resources meet duck contracts.
 var (
+	_ apis.Validatable      = (*WithPod)(nil)
+	_ apis.Defaultable      = (*WithPod)(nil)
 	_ apis.Listable         = (*WithPod)(nil)
 	_ ducktypes.Populatable = (*WithPod)(nil)
 )
@@ -95,3 +94,52 @@ type WithPodList struct {
 
 	Items []WithPod `json:"items"`
 }
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// Pod is a wrapper around Pod-like resource, which supports our interfaces
+// for webhooks
+type Pod struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec corev1.PodSpec `json:"spec,omitempty"`
+}
+
+// Verify Pod resources meet duck contracts.
+var (
+	_ apis.Validatable      = (*Pod)(nil)
+	_ apis.Defaultable      = (*Pod)(nil)
+	_ apis.Listable         = (*Pod)(nil)
+	_ ducktypes.Populatable = (*Pod)(nil)
+)
+
+// GetFullType implements duck.Implementable
+func (p *Pod) GetFullType() ducktypes.Populatable {
+	return &Pod{}
+}
+
+// Populate implements duck.Populatable
+func (p *Pod) Populate() {
+	p.Spec = corev1.PodSpec{
+		Containers: []corev1.Container{{
+			Name:  "container-name",
+			Image: "container-image:latest",
+		}},
+	}
+}
+
+// GetListType implements apis.Listable
+func (p *Pod) GetListType() runtime.Object {
+	return &PodList{}
+}
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// PodList is a list of WithPod resources
+type PodList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata"`
+
+	Items []WithPod `json:"items"`
+}
diff --git a/apis/duck/v1/podspec_validation.go b/apis/duck/v1/podspec_validation.go
index 3f8f28b6c..1ec6ef48d 100644
--- a/apis/duck/v1/podspec_validation.go
+++ b/apis/duck/v1/podspec_validation.go
@@ -48,3 +48,30 @@ func GetPodSpecValidator(ctx context.Context) PodSpecValidator {
 	}
 	return untyped.(PodSpecValidator)
 }
+
+// PodValidator is a callback to validate Pods.
+type PodValidator func(context.Context, *Pod) *apis.FieldError
+
+// Validate implements apis.Validatable
+func (p *Pod) Validate(ctx context.Context) *apis.FieldError {
+	if pv := GetPodValidator(ctx); pv != nil {
+		return pv(ctx, p)
+	}
+	return nil
+}
+
+// pvKey is used for associating a PodValidator with a context.Context
+type pvKey struct{}
+
+func WithPodValidator(ctx context.Context, pv PodValidator) context.Context {
+	return context.WithValue(ctx, pvKey{}, pv)
+}
+
+// GetPodValidator extracts the PodValidator from the context.
+func GetPodValidator(ctx context.Context) PodValidator {
+	untyped := ctx.Value(pvKey{})
+	if untyped == nil {
+		return nil
+	}
+	return untyped.(PodValidator)
+}
diff --git a/apis/duck/v1/zz_generated.deepcopy.go b/apis/duck/v1/zz_generated.deepcopy.go
index ae98f0468..22088b214 100644
--- a/apis/duck/v1/zz_generated.deepcopy.go
+++ b/apis/duck/v1/zz_generated.deepcopy.go
@@ -367,6 +367,66 @@ func (in *KResourceList) DeepCopyObject() runtime.Object {
 	return nil
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *Pod) DeepCopyInto(out *Pod) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
+	in.Spec.DeepCopyInto(&out.Spec)
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Pod.
+func (in *Pod) DeepCopy() *Pod {
+	if in == nil {
+		return nil
+	}
+	out := new(Pod)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *Pod) DeepCopyObject() runtime.Object {
+	if c := in.DeepCopy(); c != nil {
+		return c
+	}
+	return nil
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *PodList) DeepCopyInto(out *PodList) {
+	*out = *in
+	out.TypeMeta = in.TypeMeta
+	in.ListMeta.DeepCopyInto(&out.ListMeta)
+	if in.Items != nil {
+		in, out := &in.Items, &out.Items
+		*out = make([]WithPod, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodList.
+func (in *PodList) DeepCopy() *PodList {
+	if in == nil {
+		return nil
+	}
+	out := new(PodList)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object.
+func (in *PodList) DeepCopyObject() runtime.Object {
+	if c := in.DeepCopy(); c != nil {
+		return c
+	}
+	return nil
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *PodSpecable) DeepCopyInto(out *PodSpecable) {
 	*out = *in


Test patch:
diff --git a/apis/duck/v1/implements_test.go b/apis/duck/v1/implements_test.go
index a786f41f0..f55fc57ba 100644
--- a/apis/duck/v1/implements_test.go
+++ b/apis/duck/v1/implements_test.go
@@ -21,6 +21,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
+	corev1 "k8s.io/api/core/v1"
 
 	"knative.dev/pkg/apis/duck"
 	"knative.dev/pkg/apis/duck/ducktypes"
@@ -33,6 +34,7 @@ func TestTypesImplements(t *testing.T) {
 	}{
 		{instance: &AddressableType{}, iface: &Addressable{}},
 		{instance: &KResource{}, iface: &Conditions{}},
+		{instance: &corev1.Pod{}, iface: &Pod{}},
 	}
 	for _, tc := range testCases {
 		if err := duck.VerifyType(tc.instance, tc.iface); err != nil {
diff --git a/apis/duck/v1/podspec_defaults_test.go b/apis/duck/v1/podspec_defaults_test.go
index ed1b378af..c2db357b7 100644
--- a/apis/duck/v1/podspec_defaults_test.go
+++ b/apis/duck/v1/podspec_defaults_test.go
@@ -97,3 +97,68 @@ func TestPodSpecDefaulting(t *testing.T) {
 		})
 	}
 }
+
+func TestPodDefaulting(t *testing.T) {
+	p := Pod{
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{{
+				Name:  "blah",
+				Image: "busybox",
+			}},
+		},
+	}
+
+	tests := []struct {
+		name string
+		with func(context.Context) context.Context
+		want *Pod
+	}{{
+		name: "no check",
+		with: func(ctx context.Context) context.Context {
+			return ctx
+		},
+		want: p.DeepCopy(),
+	}, {
+		name: "no change",
+		with: func(ctx context.Context) context.Context {
+			return WithPodDefaulter(ctx, func(ctx context.Context, wp *Pod) {
+			})
+		},
+		want: p.DeepCopy(),
+	}, {
+		name: "no busybox",
+		with: func(ctx context.Context) context.Context {
+			return WithPodDefaulter(ctx, func(ctx context.Context, wp *Pod) {
+				for i, c := range wp.Spec.InitContainers {
+					if !strings.Contains(c.Image, "@") {
+						wp.Spec.InitContainers[i].Image = c.Image + "@sha256:deadbeef"
+					}
+				}
+				for i, c := range wp.Spec.Containers {
+					if !strings.Contains(c.Image, "@") {
+						wp.Spec.Containers[i].Image = c.Image + "@sha256:deadbeef"
+					}
+				}
+			})
+		},
+		want: &Pod{
+			Spec: corev1.PodSpec{
+				Containers: []corev1.Container{{
+					Name:  "blah",
+					Image: "busybox@sha256:deadbeef",
+				}},
+			},
+		},
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := test.with(context.Background())
+			got := p.DeepCopy()
+			got.SetDefaults(ctx)
+			if !cmp.Equal(test.want, got) {
+				t.Errorf("SetDefaults (-want, +got) = %s", cmp.Diff(test.want, got))
+			}
+		})
+	}
+}
diff --git a/apis/duck/v1/podspec_validation_test.go b/apis/duck/v1/podspec_validation_test.go
index 933f6efee..41efc2738 100644
--- a/apis/duck/v1/podspec_validation_test.go
+++ b/apis/duck/v1/podspec_validation_test.go
@@ -85,3 +85,61 @@ func TestPodSpecValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestPodValidation(t *testing.T) {
+	tests := []struct {
+		name string
+		with func(context.Context) context.Context
+		want *apis.FieldError
+	}{{
+		name: "no check",
+		with: func(ctx context.Context) context.Context {
+			return ctx
+		},
+		want: nil,
+	}, {
+		name: "no error",
+		with: func(ctx context.Context) context.Context {
+			return WithPodValidator(ctx, func(ctx context.Context, wp *Pod) *apis.FieldError {
+				return nil
+			})
+		},
+		want: nil,
+	}, {
+		name: "no busybox",
+		with: func(ctx context.Context) context.Context {
+			return WithPodValidator(ctx, func(ctx context.Context, wp *Pod) *apis.FieldError {
+				for i, c := range wp.Spec.InitContainers {
+					if c.Image == "busybox" {
+						return apis.ErrInvalidValue(c.Image, "image").ViaFieldIndex("spec.template.spec.initContainers", i)
+					}
+				}
+				for i, c := range wp.Spec.Containers {
+					if c.Image == "busybox" {
+						return apis.ErrInvalidValue(c.Image, "image").ViaFieldIndex("spec.template.spec.containers", i)
+					}
+				}
+				return nil
+			})
+		},
+		want: apis.ErrInvalidValue("busybox", "spec.template.spec.containers[0].image"),
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			p := Pod{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:  "blah",
+						Image: "busybox",
+					}},
+				},
+			}
+			ctx := test.with(context.Background())
+			got := p.Validate(ctx)
+			if test.want.Error() != got.Error() {
+				t.Errorf("Validate() = %v, wanted %v", got, test.want)
+			}
+		})
+	}
+}