From c06edf74370d3b26a98b5da85e884a1808a850e6 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Mon, 5 Dec 2022 21:15:20 -0800 Subject: [PATCH 01/29] Implement AssignImage mutator Signed-off-by: davis-haba --- .../unversioned/assignimage_types.go | 90 +++++ .../unversioned/zz_generated.deepcopy.go | 125 ++++++ apis/mutations/v1alpha1/assignimage_types.go | 98 +++++ .../v1alpha1/zz_generated.conversion.go | 180 +++++++++ .../v1alpha1/zz_generated.deepcopy.go | 125 ++++++ cmd/build/helmify/kustomization.yaml | 6 + .../mutations.gatekeeper.sh_assignimage.yaml | 332 ++++++++++++++++ .../mutations.gatekeeper.sh_assignimages.yaml | 330 ++++++++++++++++ config/crd/kustomization.yaml | 2 + .../assignimage-customresourcedefinition.yaml | 241 ++++++++++++ manifest_staging/deploy/gatekeeper.yaml | 238 +++++++++++ .../mutators/core/reconciler_test.go | 2 +- .../mutators/instances/mutator_controllers.go | 26 +- pkg/expansion/fixtures/fixtures.go | 32 ++ pkg/expansion/system_test.go | 29 ++ pkg/gator/expand/expand.go | 14 + .../mutators/assign/assign_mutator.go | 108 +---- .../assignimage/assignimage_mutator.go | 206 ++++++++++ .../assignimage_mutator_benchmark_test.go | 104 +++++ .../assignimage/assignimage_mutator_test.go | 339 ++++++++++++++++ .../mutators/assignimage/imageparser.go | 114 ++++++ .../mutators/assignimage/imageparser_test.go | 372 ++++++++++++++++++ .../mutators/assignmeta/assignmeta_mutator.go | 4 +- pkg/mutation/mutators/conversion.go | 9 +- pkg/mutation/mutators/core/mutator.go | 143 +++++++ .../mutators/modifyset/modify_set_mutator.go | 84 +--- .../mutators/testhelpers/dummy_mutator.go | 2 +- pkg/mutation/schema/node.go | 2 +- pkg/mutation/schema/node_test.go | 22 ++ pkg/mutation/schema/schema.go | 4 +- pkg/mutation/schema/schema_test.go | 2 +- pkg/mutation/schema/terminal_types.go | 3 + pkg/mutation/system_errors_test.go | 2 +- pkg/mutation/system_test.go | 2 +- pkg/mutation/types/mutator.go | 4 +- pkg/webhook/policy.go | 20 + test/bats/test.bats | 9 + test/bats/tests/mutations/assign_image.yaml | 19 + test/bats/tests/mutations/nginx_pod.yaml | 10 + test/mutations/mutations.yaml | 39 ++ 40 files changed, 3302 insertions(+), 191 deletions(-) create mode 100644 apis/mutations/unversioned/assignimage_types.go create mode 100644 apis/mutations/v1alpha1/assignimage_types.go create mode 100644 config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml create mode 100644 config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml create mode 100644 manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml create mode 100644 pkg/mutation/mutators/assignimage/assignimage_mutator.go create mode 100644 pkg/mutation/mutators/assignimage/assignimage_mutator_benchmark_test.go create mode 100644 pkg/mutation/mutators/assignimage/assignimage_mutator_test.go create mode 100644 pkg/mutation/mutators/assignimage/imageparser.go create mode 100644 pkg/mutation/mutators/assignimage/imageparser_test.go create mode 100644 pkg/mutation/mutators/core/mutator.go create mode 100644 test/bats/tests/mutations/assign_image.yaml create mode 100644 test/bats/tests/mutations/nginx_pod.yaml create mode 100644 test/mutations/mutations.yaml diff --git a/apis/mutations/unversioned/assignimage_types.go b/apis/mutations/unversioned/assignimage_types.go new file mode 100644 index 00000000000..4e4b2a9dea0 --- /dev/null +++ b/apis/mutations/unversioned/assignimage_types.go @@ -0,0 +1,90 @@ +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package unversioned + +import ( + "github.com/open-policy-agent/gatekeeper/apis/status/v1beta1" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// AssignImageSpec defines the desired state of AssignImage. +type AssignImageSpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // ApplyTo lists the specific groups, versions and kinds a mutation will be applied to. + // This is necessary because every mutation implies part of an object schema and object + // schemas are associated with specific GVKs. + ApplyTo []match.ApplyTo `json:"applyTo,omitempty"` + + // Match allows the user to limit which resources get mutated. + // Individual match criteria are AND-ed together. An undefined + // match criteria matches everything. + Match match.Match `json:"match,omitempty"` + + // Location describes the path to be mutated, for example: `spec.containers[name: main].image`. + Location string `json:"location,omitempty"` + + // Parameters define the behavior of the mutator. + Parameters AssignImageParameters `json:"parameters,omitempty"` +} + +type AssignImageParameters struct { + PathTests []PathTest `json:"pathTests,omitempty"` + + // AssignDomain sets the domain component on an image string. The trailing + // slash should not be included. + AssignDomain string `json:"assignDomain,omitempty"` + + // AssignPath sets the domain component on an image string. + AssignPath string `json:"assignPath,omitempty"` + + // AssignImage sets the image component on an image string. It must start + // with a `:` or `@`. + AssignTag string `json:"assignTag,omitempty"` +} + +// AssignImageStatus defines the observed state of AssignImage. +type AssignImageStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file + + ByPod []v1beta1.MutatorPodStatusStatus `json:"byPod,omitempty"` +} + +// +kubebuilder:object:root=true + +// AssignImage is the Schema for the assign API. +type AssignImage struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec AssignImageSpec `json:"spec,omitempty"` + Status AssignImageStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// AssignImageList contains a list of AssignImage. +type AssignImageList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []AssignImage `json:"items"` +} diff --git a/apis/mutations/unversioned/zz_generated.deepcopy.go b/apis/mutations/unversioned/zz_generated.deepcopy.go index 14832664613..296356771d7 100644 --- a/apis/mutations/unversioned/zz_generated.deepcopy.go +++ b/apis/mutations/unversioned/zz_generated.deepcopy.go @@ -82,6 +82,131 @@ func (in *AssignField) DeepCopy() *AssignField { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AssignImage) DeepCopyInto(out *AssignImage) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImage. +func (in *AssignImage) DeepCopy() *AssignImage { + if in == nil { + return nil + } + out := new(AssignImage) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AssignImage) 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 *AssignImageList) DeepCopyInto(out *AssignImageList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]AssignImage, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageList. +func (in *AssignImageList) DeepCopy() *AssignImageList { + if in == nil { + return nil + } + out := new(AssignImageList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AssignImageList) 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 *AssignImageParameters) DeepCopyInto(out *AssignImageParameters) { + *out = *in + if in.PathTests != nil { + in, out := &in.PathTests, &out.PathTests + *out = make([]PathTest, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageParameters. +func (in *AssignImageParameters) DeepCopy() *AssignImageParameters { + if in == nil { + return nil + } + out := new(AssignImageParameters) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AssignImageSpec) DeepCopyInto(out *AssignImageSpec) { + *out = *in + if in.ApplyTo != nil { + in, out := &in.ApplyTo, &out.ApplyTo + *out = make([]match.ApplyTo, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + in.Match.DeepCopyInto(&out.Match) + in.Parameters.DeepCopyInto(&out.Parameters) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageSpec. +func (in *AssignImageSpec) DeepCopy() *AssignImageSpec { + if in == nil { + return nil + } + out := new(AssignImageSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AssignImageStatus) DeepCopyInto(out *AssignImageStatus) { + *out = *in + if in.ByPod != nil { + in, out := &in.ByPod, &out.ByPod + *out = make([]v1beta1.MutatorPodStatusStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageStatus. +func (in *AssignImageStatus) DeepCopy() *AssignImageStatus { + if in == nil { + return nil + } + out := new(AssignImageStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AssignList) DeepCopyInto(out *AssignList) { *out = *in diff --git a/apis/mutations/v1alpha1/assignimage_types.go b/apis/mutations/v1alpha1/assignimage_types.go new file mode 100644 index 00000000000..5939225453c --- /dev/null +++ b/apis/mutations/v1alpha1/assignimage_types.go @@ -0,0 +1,98 @@ +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "github.com/open-policy-agent/gatekeeper/apis/status/v1beta1" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// AssignImageSpec defines the desired state of AssignImage. +type AssignImageSpec struct { + // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster + // Important: Run "make" to regenerate code after modifying this file + + // ApplyTo lists the specific groups, versions and kinds a mutation will be applied to. + // This is necessary because every mutation implies part of an object schema and object + // schemas are associated with specific GVKs. + ApplyTo []match.ApplyTo `json:"applyTo,omitempty"` + + // Match allows the user to limit which resources get mutated. + // Individual match criteria are AND-ed together. An undefined + // match criteria matches everything. + Match match.Match `json:"match,omitempty"` + + // Location describes the path to be mutated, for example: `spec.containers[name: main].image`. + Location string `json:"location,omitempty"` + + // Parameters define the behavior of the mutator. + Parameters AssignImageParameters `json:"parameters,omitempty"` +} + +type AssignImageParameters struct { + PathTests []PathTest `json:"pathTests,omitempty"` + + // AssignDomain sets the domain component on an image string. The trailing + // slash should not be included. + AssignDomain string `json:"assignDomain,omitempty"` + + // AssignPath sets the domain component on an image string. + AssignPath string `json:"assignPath,omitempty"` + + // AssignImage sets the image component on an image string. It must start + // with a `:` or `@`. + AssignTag string `json:"assignTag,omitempty"` +} + +// AssignImageStatus defines the observed state of AssignImage. +type AssignImageStatus struct { + // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster + // Important: Run "make" to regenerate code after modifying this file + + ByPod []v1beta1.MutatorPodStatusStatus `json:"byPod,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:resource:path="assignimage" +// +kubebuilder:resource:scope="Cluster" +// +kubebuilder:subresource:status +// +kubebuilder:storageversion + +// AssignImage is the Schema for the assignimage API. +type AssignImage struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec AssignImageSpec `json:"spec,omitempty"` + Status AssignImageStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// AssignImageList contains a list of AssignImage. +type AssignImageList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []AssignImage `json:"items"` +} + +func init() { + SchemeBuilder.Register(&AssignImage{}, &AssignImageList{}) +} diff --git a/apis/mutations/v1alpha1/zz_generated.conversion.go b/apis/mutations/v1alpha1/zz_generated.conversion.go index 8e4eb5e6ad6..e9bf7c06af0 100644 --- a/apis/mutations/v1alpha1/zz_generated.conversion.go +++ b/apis/mutations/v1alpha1/zz_generated.conversion.go @@ -58,6 +58,56 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*AssignImage)(nil), (*unversioned.AssignImage)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_AssignImage_To_unversioned_AssignImage(a.(*AssignImage), b.(*unversioned.AssignImage), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*unversioned.AssignImage)(nil), (*AssignImage)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_unversioned_AssignImage_To_v1alpha1_AssignImage(a.(*unversioned.AssignImage), b.(*AssignImage), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*AssignImageList)(nil), (*unversioned.AssignImageList)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_AssignImageList_To_unversioned_AssignImageList(a.(*AssignImageList), b.(*unversioned.AssignImageList), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*unversioned.AssignImageList)(nil), (*AssignImageList)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_unversioned_AssignImageList_To_v1alpha1_AssignImageList(a.(*unversioned.AssignImageList), b.(*AssignImageList), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*AssignImageParameters)(nil), (*unversioned.AssignImageParameters)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_AssignImageParameters_To_unversioned_AssignImageParameters(a.(*AssignImageParameters), b.(*unversioned.AssignImageParameters), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*unversioned.AssignImageParameters)(nil), (*AssignImageParameters)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_unversioned_AssignImageParameters_To_v1alpha1_AssignImageParameters(a.(*unversioned.AssignImageParameters), b.(*AssignImageParameters), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*AssignImageSpec)(nil), (*unversioned.AssignImageSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_AssignImageSpec_To_unversioned_AssignImageSpec(a.(*AssignImageSpec), b.(*unversioned.AssignImageSpec), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*unversioned.AssignImageSpec)(nil), (*AssignImageSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_unversioned_AssignImageSpec_To_v1alpha1_AssignImageSpec(a.(*unversioned.AssignImageSpec), b.(*AssignImageSpec), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*AssignImageStatus)(nil), (*unversioned.AssignImageStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_AssignImageStatus_To_unversioned_AssignImageStatus(a.(*AssignImageStatus), b.(*unversioned.AssignImageStatus), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*unversioned.AssignImageStatus)(nil), (*AssignImageStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_unversioned_AssignImageStatus_To_v1alpha1_AssignImageStatus(a.(*unversioned.AssignImageStatus), b.(*AssignImageStatus), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*AssignList)(nil), (*unversioned.AssignList)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha1_AssignList_To_unversioned_AssignList(a.(*AssignList), b.(*unversioned.AssignList), scope) }); err != nil { @@ -297,6 +347,136 @@ func Convert_unversioned_AssignField_To_v1alpha1_AssignField(in *unversioned.Ass return autoConvert_unversioned_AssignField_To_v1alpha1_AssignField(in, out, s) } +func autoConvert_v1alpha1_AssignImage_To_unversioned_AssignImage(in *AssignImage, out *unversioned.AssignImage, s conversion.Scope) error { + out.ObjectMeta = in.ObjectMeta + if err := Convert_v1alpha1_AssignImageSpec_To_unversioned_AssignImageSpec(&in.Spec, &out.Spec, s); err != nil { + return err + } + if err := Convert_v1alpha1_AssignImageStatus_To_unversioned_AssignImageStatus(&in.Status, &out.Status, s); err != nil { + return err + } + return nil +} + +// Convert_v1alpha1_AssignImage_To_unversioned_AssignImage is an autogenerated conversion function. +func Convert_v1alpha1_AssignImage_To_unversioned_AssignImage(in *AssignImage, out *unversioned.AssignImage, s conversion.Scope) error { + return autoConvert_v1alpha1_AssignImage_To_unversioned_AssignImage(in, out, s) +} + +func autoConvert_unversioned_AssignImage_To_v1alpha1_AssignImage(in *unversioned.AssignImage, out *AssignImage, s conversion.Scope) error { + out.ObjectMeta = in.ObjectMeta + if err := Convert_unversioned_AssignImageSpec_To_v1alpha1_AssignImageSpec(&in.Spec, &out.Spec, s); err != nil { + return err + } + if err := Convert_unversioned_AssignImageStatus_To_v1alpha1_AssignImageStatus(&in.Status, &out.Status, s); err != nil { + return err + } + return nil +} + +// Convert_unversioned_AssignImage_To_v1alpha1_AssignImage is an autogenerated conversion function. +func Convert_unversioned_AssignImage_To_v1alpha1_AssignImage(in *unversioned.AssignImage, out *AssignImage, s conversion.Scope) error { + return autoConvert_unversioned_AssignImage_To_v1alpha1_AssignImage(in, out, s) +} + +func autoConvert_v1alpha1_AssignImageList_To_unversioned_AssignImageList(in *AssignImageList, out *unversioned.AssignImageList, s conversion.Scope) error { + out.ListMeta = in.ListMeta + out.Items = *(*[]unversioned.AssignImage)(unsafe.Pointer(&in.Items)) + return nil +} + +// Convert_v1alpha1_AssignImageList_To_unversioned_AssignImageList is an autogenerated conversion function. +func Convert_v1alpha1_AssignImageList_To_unversioned_AssignImageList(in *AssignImageList, out *unversioned.AssignImageList, s conversion.Scope) error { + return autoConvert_v1alpha1_AssignImageList_To_unversioned_AssignImageList(in, out, s) +} + +func autoConvert_unversioned_AssignImageList_To_v1alpha1_AssignImageList(in *unversioned.AssignImageList, out *AssignImageList, s conversion.Scope) error { + out.ListMeta = in.ListMeta + out.Items = *(*[]AssignImage)(unsafe.Pointer(&in.Items)) + return nil +} + +// Convert_unversioned_AssignImageList_To_v1alpha1_AssignImageList is an autogenerated conversion function. +func Convert_unversioned_AssignImageList_To_v1alpha1_AssignImageList(in *unversioned.AssignImageList, out *AssignImageList, s conversion.Scope) error { + return autoConvert_unversioned_AssignImageList_To_v1alpha1_AssignImageList(in, out, s) +} + +func autoConvert_v1alpha1_AssignImageParameters_To_unversioned_AssignImageParameters(in *AssignImageParameters, out *unversioned.AssignImageParameters, s conversion.Scope) error { + out.PathTests = *(*[]unversioned.PathTest)(unsafe.Pointer(&in.PathTests)) + out.AssignDomain = in.AssignDomain + out.AssignPath = in.AssignPath + out.AssignTag = in.AssignTag + return nil +} + +// Convert_v1alpha1_AssignImageParameters_To_unversioned_AssignImageParameters is an autogenerated conversion function. +func Convert_v1alpha1_AssignImageParameters_To_unversioned_AssignImageParameters(in *AssignImageParameters, out *unversioned.AssignImageParameters, s conversion.Scope) error { + return autoConvert_v1alpha1_AssignImageParameters_To_unversioned_AssignImageParameters(in, out, s) +} + +func autoConvert_unversioned_AssignImageParameters_To_v1alpha1_AssignImageParameters(in *unversioned.AssignImageParameters, out *AssignImageParameters, s conversion.Scope) error { + out.PathTests = *(*[]PathTest)(unsafe.Pointer(&in.PathTests)) + out.AssignDomain = in.AssignDomain + out.AssignPath = in.AssignPath + out.AssignTag = in.AssignTag + return nil +} + +// Convert_unversioned_AssignImageParameters_To_v1alpha1_AssignImageParameters is an autogenerated conversion function. +func Convert_unversioned_AssignImageParameters_To_v1alpha1_AssignImageParameters(in *unversioned.AssignImageParameters, out *AssignImageParameters, s conversion.Scope) error { + return autoConvert_unversioned_AssignImageParameters_To_v1alpha1_AssignImageParameters(in, out, s) +} + +func autoConvert_v1alpha1_AssignImageSpec_To_unversioned_AssignImageSpec(in *AssignImageSpec, out *unversioned.AssignImageSpec, s conversion.Scope) error { + out.ApplyTo = *(*[]match.ApplyTo)(unsafe.Pointer(&in.ApplyTo)) + out.Match = in.Match + out.Location = in.Location + if err := Convert_v1alpha1_AssignImageParameters_To_unversioned_AssignImageParameters(&in.Parameters, &out.Parameters, s); err != nil { + return err + } + return nil +} + +// Convert_v1alpha1_AssignImageSpec_To_unversioned_AssignImageSpec is an autogenerated conversion function. +func Convert_v1alpha1_AssignImageSpec_To_unversioned_AssignImageSpec(in *AssignImageSpec, out *unversioned.AssignImageSpec, s conversion.Scope) error { + return autoConvert_v1alpha1_AssignImageSpec_To_unversioned_AssignImageSpec(in, out, s) +} + +func autoConvert_unversioned_AssignImageSpec_To_v1alpha1_AssignImageSpec(in *unversioned.AssignImageSpec, out *AssignImageSpec, s conversion.Scope) error { + out.ApplyTo = *(*[]match.ApplyTo)(unsafe.Pointer(&in.ApplyTo)) + out.Match = in.Match + out.Location = in.Location + if err := Convert_unversioned_AssignImageParameters_To_v1alpha1_AssignImageParameters(&in.Parameters, &out.Parameters, s); err != nil { + return err + } + return nil +} + +// Convert_unversioned_AssignImageSpec_To_v1alpha1_AssignImageSpec is an autogenerated conversion function. +func Convert_unversioned_AssignImageSpec_To_v1alpha1_AssignImageSpec(in *unversioned.AssignImageSpec, out *AssignImageSpec, s conversion.Scope) error { + return autoConvert_unversioned_AssignImageSpec_To_v1alpha1_AssignImageSpec(in, out, s) +} + +func autoConvert_v1alpha1_AssignImageStatus_To_unversioned_AssignImageStatus(in *AssignImageStatus, out *unversioned.AssignImageStatus, s conversion.Scope) error { + out.ByPod = *(*[]v1beta1.MutatorPodStatusStatus)(unsafe.Pointer(&in.ByPod)) + return nil +} + +// Convert_v1alpha1_AssignImageStatus_To_unversioned_AssignImageStatus is an autogenerated conversion function. +func Convert_v1alpha1_AssignImageStatus_To_unversioned_AssignImageStatus(in *AssignImageStatus, out *unversioned.AssignImageStatus, s conversion.Scope) error { + return autoConvert_v1alpha1_AssignImageStatus_To_unversioned_AssignImageStatus(in, out, s) +} + +func autoConvert_unversioned_AssignImageStatus_To_v1alpha1_AssignImageStatus(in *unversioned.AssignImageStatus, out *AssignImageStatus, s conversion.Scope) error { + out.ByPod = *(*[]v1beta1.MutatorPodStatusStatus)(unsafe.Pointer(&in.ByPod)) + return nil +} + +// Convert_unversioned_AssignImageStatus_To_v1alpha1_AssignImageStatus is an autogenerated conversion function. +func Convert_unversioned_AssignImageStatus_To_v1alpha1_AssignImageStatus(in *unversioned.AssignImageStatus, out *AssignImageStatus, s conversion.Scope) error { + return autoConvert_unversioned_AssignImageStatus_To_v1alpha1_AssignImageStatus(in, out, s) +} + func autoConvert_v1alpha1_AssignList_To_unversioned_AssignList(in *AssignList, out *unversioned.AssignList, s conversion.Scope) error { out.ListMeta = in.ListMeta out.Items = *(*[]unversioned.Assign)(unsafe.Pointer(&in.Items)) diff --git a/apis/mutations/v1alpha1/zz_generated.deepcopy.go b/apis/mutations/v1alpha1/zz_generated.deepcopy.go index 4ac3428f126..701df5a8670 100644 --- a/apis/mutations/v1alpha1/zz_generated.deepcopy.go +++ b/apis/mutations/v1alpha1/zz_generated.deepcopy.go @@ -82,6 +82,131 @@ func (in *AssignField) DeepCopy() *AssignField { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AssignImage) DeepCopyInto(out *AssignImage) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImage. +func (in *AssignImage) DeepCopy() *AssignImage { + if in == nil { + return nil + } + out := new(AssignImage) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AssignImage) 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 *AssignImageList) DeepCopyInto(out *AssignImageList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]AssignImage, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageList. +func (in *AssignImageList) DeepCopy() *AssignImageList { + if in == nil { + return nil + } + out := new(AssignImageList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AssignImageList) 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 *AssignImageParameters) DeepCopyInto(out *AssignImageParameters) { + *out = *in + if in.PathTests != nil { + in, out := &in.PathTests, &out.PathTests + *out = make([]PathTest, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageParameters. +func (in *AssignImageParameters) DeepCopy() *AssignImageParameters { + if in == nil { + return nil + } + out := new(AssignImageParameters) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AssignImageSpec) DeepCopyInto(out *AssignImageSpec) { + *out = *in + if in.ApplyTo != nil { + in, out := &in.ApplyTo, &out.ApplyTo + *out = make([]match.ApplyTo, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + in.Match.DeepCopyInto(&out.Match) + in.Parameters.DeepCopyInto(&out.Parameters) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageSpec. +func (in *AssignImageSpec) DeepCopy() *AssignImageSpec { + if in == nil { + return nil + } + out := new(AssignImageSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AssignImageStatus) DeepCopyInto(out *AssignImageStatus) { + *out = *in + if in.ByPod != nil { + in, out := &in.ByPod, &out.ByPod + *out = make([]v1beta1.MutatorPodStatusStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AssignImageStatus. +func (in *AssignImageStatus) DeepCopy() *AssignImageStatus { + if in == nil { + return nil + } + out := new(AssignImageStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AssignList) DeepCopyInto(out *AssignList) { *out = *in diff --git a/cmd/build/helmify/kustomization.yaml b/cmd/build/helmify/kustomization.yaml index 2d94aad7994..19d21e2418d 100644 --- a/cmd/build/helmify/kustomization.yaml +++ b/cmd/build/helmify/kustomization.yaml @@ -46,6 +46,12 @@ patchesJson6902: kind: CustomResourceDefinition name: assignmetadata.mutations.gatekeeper.sh path: labels_patch.yaml + - target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: assignimage.mutations.gatekeeper.sh + path: labels_patch.yaml - target: group: apiextensions.k8s.io version: v1 diff --git a/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml b/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml new file mode 100644 index 00000000000..1a4e442a60a --- /dev/null +++ b/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml @@ -0,0 +1,332 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: assignimage.mutations.gatekeeper.sh +spec: + group: mutations.gatekeeper.sh + names: + kind: AssignImage + listKind: AssignImageList + plural: assignimage + singular: assignimage + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: AssignImage is the Schema for the assignimage API. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AssignImageSpec defines the desired state of AssignImage. + properties: + applyTo: + description: ApplyTo lists the specific groups, versions and kinds + a mutation will be applied to. This is necessary because every mutation + implies part of an object schema and object schemas are associated + with specific GVKs. + items: + description: ApplyTo determines what GVKs items the mutation should + apply to. Globs are not allowed. + properties: + groups: + items: + type: string + type: array + kinds: + items: + type: string + type: array + versions: + items: + type: string + type: array + type: object + type: array + location: + description: 'Location describes the path to be mutated, for example: + `spec.containers[name: main].image`.' + type: string + match: + description: Match allows the user to limit which resources get mutated. + Individual match criteria are AND-ed together. An undefined match + criteria matches everything. + properties: + excludedNamespaces: + description: 'ExcludedNamespaces is a list of namespace names. + If defined, a constraint only applies to resources not in a + listed namespace. ExcludedNamespaces also supports a prefix + or suffix based glob. For example, `excludedNamespaces: [kube-*]` + matches both `kube-system` and `kube-public`, and `excludedNamespaces: + [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or + end. Ex: "kube-*" will match "kube-system" or "kube-public", + "*-system" will match "kube-system" or "gatekeeper-system". The + asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + kinds: + items: + description: Kinds accepts a list of objects with apiGroups + and kinds fields that list the groups/kinds of objects to + which the mutation will apply. If multiple groups/kinds objects + are specified, only one match is needed for the resource to + be in scope. + properties: + apiGroups: + description: APIGroups is the API groups the resources belong + to. '*' is all groups. If '*' is present, the length of + the slice must be one. Required. + items: + type: string + type: array + kinds: + items: + type: string + type: array + type: object + type: array + labelSelector: + description: 'LabelSelector is the combination of two optional + fields: `matchLabels` and `matchExpressions`. These two fields + provide different methods of selecting or excluding k8s objects + based on the label keys and values included in object metadata. All + selection expressions from both sections are ANDed to determine + if an object meets the cumulative requirements of the selector.' + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector + that contains values, a key, and an operator that relates + the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: operator represents a key's relationship + to a set of values. Valid operators are In, NotIn, + Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If + the operator is In or NotIn, the values array must + be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced + during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A + single {key,value} in the matchLabels map is equivalent + to an element of matchExpressions, whose key field is "key", + the operator is "In", and the values array contains only + "value". The requirements are ANDed. + type: object + type: object + name: + description: 'Name is the name of an object. If defined, it will + match against objects with the specified name. Name also supports + a prefix or suffix glob. For example, `name: pod-*` would match + both `pod-a` and `pod-b`, and `name: *-pod` would match both + `a-pod` and `b-pod`.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + namespaceSelector: + description: NamespaceSelector is a label selector against an + object's containing namespace or the object itself, if the object + is a namespace. + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector + that contains values, a key, and an operator that relates + the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: operator represents a key's relationship + to a set of values. Valid operators are In, NotIn, + Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If + the operator is In or NotIn, the values array must + be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced + during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A + single {key,value} in the matchLabels map is equivalent + to an element of matchExpressions, whose key field is "key", + the operator is "In", and the values array contains only + "value". The requirements are ANDed. + type: object + type: object + namespaces: + description: 'Namespaces is a list of namespace names. If defined, + a constraint only applies to resources in a listed namespace. Namespaces + also supports a prefix or suffix based glob. For example, `namespaces: + [kube-*]` matches both `kube-system` and `kube-public`, and + `namespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or + end. Ex: "kube-*" will match "kube-system" or "kube-public", + "*-system" will match "kube-system" or "gatekeeper-system". The + asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + scope: + description: Scope determines if cluster-scoped and/or namespaced-scoped + resources are matched. Accepts `*`, `Cluster`, or `Namespaced`. + (defaults to `*`) + type: string + source: + description: Source determines whether generated or original resources + are matched. Accepts `Generated`|`Original`|`All` (defaults + to `All`). A value of `Generated` will only match generated + resources, while `Original` will only match regular resources. + enum: + - All + - Generated + - Original + type: string + type: object + parameters: + description: Parameters define the behavior of the mutator. + properties: + assignDomain: + description: AssignDomain sets the domain component on an image + string. The trailing slash should not be included. + type: string + assignPath: + description: AssignPath sets the domain component on an image + string. + type: string + assignTag: + description: AssignImage sets the image component on an image + string. It must start with a `:` or `@`. + type: string + pathTests: + items: + description: "PathTest allows the user to customize how the + mutation works if parent paths are missing. It traverses the + list in order. All sub paths are tested against the provided + condition, if the test fails, the mutation is not applied. + All `subPath` entries must be a prefix of `location`. Any + glob characters will take on the same value as was used to + expand the matching glob in `location`. \n Available Tests: + * MustExist - the path must exist or do not mutate * MustNotExist + - the path must not exist or do not mutate." + properties: + condition: + description: Condition describes whether the path either + MustExist or MustNotExist in the original object + enum: + - MustExist + - MustNotExist + type: string + subPath: + type: string + type: object + type: array + type: object + type: object + status: + description: AssignImageStatus defines the observed state of AssignImage. + properties: + byPod: + items: + description: MutatorPodStatusStatus defines the observed state of + MutatorPodStatus. + properties: + enforced: + type: boolean + errors: + items: + description: MutatorError represents a single error caught + while adding a mutator to a system. + properties: + message: + type: string + type: + description: Type indicates a specific class of error + for use by controller code. If not present, the error + should be treated as not matching any known type. + type: string + required: + - message + type: object + type: array + id: + type: string + mutatorUID: + description: Storing the mutator UID allows us to detect drift, + such as when a mutator has been recreated after its CRD was + deleted out from under it, interrupting the watch + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml b/config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml new file mode 100644 index 00000000000..27bd982ccc3 --- /dev/null +++ b/config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml @@ -0,0 +1,330 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + creationTimestamp: null + name: assignimages.mutations.gatekeeper.sh +spec: + group: mutations.gatekeeper.sh + names: + kind: AssignImage + listKind: AssignImageList + plural: assignimages + singular: assignimage + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: AssignImage is the Schema for the assignimage API. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AssignImageSpec defines the desired state of AssignImage. + properties: + applyTo: + description: ApplyTo lists the specific groups, versions and kinds + a mutation will be applied to. This is necessary because every mutation + implies part of an object schema and object schemas are associated + with specific GVKs. + items: + description: ApplyTo determines what GVKs items the mutation should + apply to. Globs are not allowed. + properties: + groups: + items: + type: string + type: array + kinds: + items: + type: string + type: array + versions: + items: + type: string + type: array + type: object + type: array + location: + description: 'Location describes the path to be mutated, for example: + `spec.containers[name: main].image`.' + type: string + match: + description: Match allows the user to limit which resources get mutated. + Individual match criteria are AND-ed together. An undefined match + criteria matches everything. + properties: + excludedNamespaces: + description: 'ExcludedNamespaces is a list of namespace names. + If defined, a constraint only applies to resources not in a + listed namespace. ExcludedNamespaces also supports a prefix + or suffix based glob. For example, `excludedNamespaces: [kube-*]` + matches both `kube-system` and `kube-public`, and `excludedNamespaces: + [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or + end. Ex: "kube-*" will match "kube-system" or "kube-public", + "*-system" will match "kube-system" or "gatekeeper-system". The + asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + kinds: + items: + description: Kinds accepts a list of objects with apiGroups + and kinds fields that list the groups/kinds of objects to + which the mutation will apply. If multiple groups/kinds objects + are specified, only one match is needed for the resource to + be in scope. + properties: + apiGroups: + description: APIGroups is the API groups the resources belong + to. '*' is all groups. If '*' is present, the length of + the slice must be one. Required. + items: + type: string + type: array + kinds: + items: + type: string + type: array + type: object + type: array + labelSelector: + description: 'LabelSelector is the combination of two optional + fields: `matchLabels` and `matchExpressions`. These two fields + provide different methods of selecting or excluding k8s objects + based on the label keys and values included in object metadata. All + selection expressions from both sections are ANDed to determine + if an object meets the cumulative requirements of the selector.' + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector + that contains values, a key, and an operator that relates + the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: operator represents a key's relationship + to a set of values. Valid operators are In, NotIn, + Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If + the operator is In or NotIn, the values array must + be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced + during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A + single {key,value} in the matchLabels map is equivalent + to an element of matchExpressions, whose key field is "key", + the operator is "In", and the values array contains only + "value". The requirements are ANDed. + type: object + type: object + name: + description: 'Name is the name of an object. If defined, it will + match against objects with the specified name. Name also supports + a prefix or suffix glob. For example, `name: pod-*` would match + both `pod-a` and `pod-b`, and `name: *-pod` would match both + `a-pod` and `b-pod`.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + namespaceSelector: + description: NamespaceSelector is a label selector against an + object's containing namespace or the object itself, if the object + is a namespace. + properties: + matchExpressions: + description: matchExpressions is a list of label selector + requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector + that contains values, a key, and an operator that relates + the key and values. + properties: + key: + description: key is the label key that the selector + applies to. + type: string + operator: + description: operator represents a key's relationship + to a set of values. Valid operators are In, NotIn, + Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If + the operator is In or NotIn, the values array must + be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced + during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A + single {key,value} in the matchLabels map is equivalent + to an element of matchExpressions, whose key field is "key", + the operator is "In", and the values array contains only + "value". The requirements are ANDed. + type: object + type: object + namespaces: + description: 'Namespaces is a list of namespace names. If defined, + a constraint only applies to resources in a listed namespace. Namespaces + also supports a prefix or suffix based glob. For example, `namespaces: + [kube-*]` matches both `kube-system` and `kube-public`, and + `namespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or + end. Ex: "kube-*" will match "kube-system" or "kube-public", + "*-system" will match "kube-system" or "gatekeeper-system". The + asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + scope: + description: Scope determines if cluster-scoped and/or namespaced-scoped + resources are matched. Accepts `*`, `Cluster`, or `Namespaced`. + (defaults to `*`) + type: string + source: + description: Source determines whether generated or original resources + are matched. Accepts `Generated`|`Original`|`All` (defaults + to `All`). A value of `Generated` will only match generated + resources, while `Original` will only match regular resources. + enum: + - All + - Generated + - Original + type: string + type: object + parameters: + description: Parameters define the behavior of the mutator. + properties: + assignDomain: + description: AssignDomain sets the domain component on an image + string. The trailing slash should not be included. + type: string + assignPath: + description: AssignPath sets the domain component on an image + string. + type: string + assignTag: + description: AssignImage sets the image component on an image + string. It must start with a `:` or `@`. + type: string + pathTests: + items: + description: "PathTest allows the user to customize how the + mutation works if parent paths are missing. It traverses the + list in order. All sub paths are tested against the provided + condition, if the test fails, the mutation is not applied. + All `subPath` entries must be a prefix of `location`. Any + glob characters will take on the same value as was used to + expand the matching glob in `location`. \n Available Tests: + * MustExist - the path must exist or do not mutate * MustNotExist + - the path must not exist or do not mutate." + properties: + condition: + description: Condition describes whether the path either + MustExist or MustNotExist in the original object + enum: + - MustExist + - MustNotExist + type: string + subPath: + type: string + type: object + type: array + type: object + type: object + status: + description: AssignImageStatus defines the observed state of AssignImage. + properties: + byPod: + items: + description: MutatorPodStatusStatus defines the observed state of + MutatorPodStatus. + properties: + enforced: + type: boolean + errors: + items: + description: MutatorError represents a single error caught + while adding a mutator to a system. + properties: + message: + type: string + type: + description: Type indicates a specific class of error + for use by controller code. If not present, the error + should be treated as not matching any known type. + type: string + required: + - message + type: object + type: array + id: + type: string + mutatorUID: + description: Storing the mutator UID allows us to detect drift, + such as when a mutator has been recreated after its CRD was + deleted out from under it, interrupting the watch + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array + type: object + type: object + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 2d096889dd6..1262dbf3462 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -7,6 +7,7 @@ resources: - bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml - bases/status.gatekeeper.sh_mutatorpodstatuses.yaml - bases/mutations.gatekeeper.sh_assign.yaml +- bases/mutations.gatekeeper.sh_assignimages.yaml - bases/mutations.gatekeeper.sh_assignmetadata.yaml - bases/mutations.gatekeeper.sh_modifyset.yaml - bases/expansion.gatekeeper.sh_expansiontemplate.yaml @@ -46,6 +47,7 @@ patchesStrategicMerge: #- patches/max_name_size_for_modifyset.yaml #- patches/max_name_size_for_assign.yaml #- patches/max_name_size_for_assignmetadata.yaml +#- patches/max_name_size_for_assignimages.yaml # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD #- patches/webhook_in_configs.yaml diff --git a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml new file mode 100644 index 00000000000..046415921be --- /dev/null +++ b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml @@ -0,0 +1,241 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + labels: + app: '{{ template "gatekeeper.name" . }}' + chart: '{{ template "gatekeeper.name" . }}' + gatekeeper.sh/system: "yes" + heritage: '{{ .Release.Service }}' + release: '{{ .Release.Name }}' + name: assignimages.mutations.gatekeeper.sh +spec: + group: mutations.gatekeeper.sh + names: + kind: AssignImage + listKind: AssignImageList + plural: assignimages + singular: assignimage + preserveUnknownFields: false + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: AssignImage is the Schema for the assignimage API. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AssignImageSpec defines the desired state of AssignImage. + properties: + applyTo: + description: ApplyTo lists the specific groups, versions and kinds a mutation will be applied to. This is necessary because every mutation implies part of an object schema and object schemas are associated with specific GVKs. + items: + description: ApplyTo determines what GVKs items the mutation should apply to. Globs are not allowed. + properties: + groups: + items: + type: string + type: array + kinds: + items: + type: string + type: array + versions: + items: + type: string + type: array + type: object + type: array + location: + description: 'Location describes the path to be mutated, for example: `spec.containers[name: main].image`.' + type: string + match: + description: Match allows the user to limit which resources get mutated. Individual match criteria are AND-ed together. An undefined match criteria matches everything. + properties: + excludedNamespaces: + description: 'ExcludedNamespaces is a list of namespace names. If defined, a constraint only applies to resources not in a listed namespace. ExcludedNamespaces also supports a prefix or suffix based glob. For example, `excludedNamespaces: [kube-*]` matches both `kube-system` and `kube-public`, and `excludedNamespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or end. Ex: "kube-*" will match "kube-system" or "kube-public", "*-system" will match "kube-system" or "gatekeeper-system". The asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + kinds: + items: + description: Kinds accepts a list of objects with apiGroups and kinds fields that list the groups/kinds of objects to which the mutation will apply. If multiple groups/kinds objects are specified, only one match is needed for the resource to be in scope. + properties: + apiGroups: + description: APIGroups is the API groups the resources belong to. '*' is all groups. If '*' is present, the length of the slice must be one. Required. + items: + type: string + type: array + kinds: + items: + type: string + type: array + type: object + type: array + labelSelector: + description: 'LabelSelector is the combination of two optional fields: `matchLabels` and `matchExpressions`. These two fields provide different methods of selecting or excluding k8s objects based on the label keys and values included in object metadata. All selection expressions from both sections are ANDed to determine if an object meets the cumulative requirements of the selector.' + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + name: + description: 'Name is the name of an object. If defined, it will match against objects with the specified name. Name also supports a prefix or suffix glob. For example, `name: pod-*` would match both `pod-a` and `pod-b`, and `name: *-pod` would match both `a-pod` and `b-pod`.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + namespaceSelector: + description: NamespaceSelector is a label selector against an object's containing namespace or the object itself, if the object is a namespace. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + namespaces: + description: 'Namespaces is a list of namespace names. If defined, a constraint only applies to resources in a listed namespace. Namespaces also supports a prefix or suffix based glob. For example, `namespaces: [kube-*]` matches both `kube-system` and `kube-public`, and `namespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or end. Ex: "kube-*" will match "kube-system" or "kube-public", "*-system" will match "kube-system" or "gatekeeper-system". The asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + scope: + description: Scope determines if cluster-scoped and/or namespaced-scoped resources are matched. Accepts `*`, `Cluster`, or `Namespaced`. (defaults to `*`) + type: string + source: + description: Source determines whether generated or original resources are matched. Accepts `Generated`|`Original`|`All` (defaults to `All`). A value of `Generated` will only match generated resources, while `Original` will only match regular resources. + enum: + - All + - Generated + - Original + type: string + type: object + parameters: + description: Parameters define the behavior of the mutator. + properties: + assignDomain: + description: AssignDomain sets the domain component on an image string. The trailing slash should not be included. + type: string + assignPath: + description: AssignPath sets the domain component on an image string. + type: string + assignTag: + description: AssignImage sets the image component on an image string. It must start with a `:` or `@`. + type: string + pathTests: + items: + description: "PathTest allows the user to customize how the mutation works if parent paths are missing. It traverses the list in order. All sub paths are tested against the provided condition, if the test fails, the mutation is not applied. All `subPath` entries must be a prefix of `location`. Any glob characters will take on the same value as was used to expand the matching glob in `location`. \n Available Tests: * MustExist - the path must exist or do not mutate * MustNotExist - the path must not exist or do not mutate." + properties: + condition: + description: Condition describes whether the path either MustExist or MustNotExist in the original object + enum: + - MustExist + - MustNotExist + type: string + subPath: + type: string + type: object + type: array + type: object + type: object + status: + description: AssignImageStatus defines the observed state of AssignImage. + properties: + byPod: + items: + description: MutatorPodStatusStatus defines the observed state of MutatorPodStatus. + properties: + enforced: + type: boolean + errors: + items: + description: MutatorError represents a single error caught while adding a mutator to a system. + properties: + message: + type: string + type: + description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type. + type: string + required: + - message + type: object + type: array + id: + type: string + mutatorUID: + description: Storing the mutator UID allows us to detect drift, such as when a mutator has been recreated after its CRD was deleted out from under it, interrupting the watch + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array + type: object + type: object + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 3ecd842fedc..4fc5a342b65 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -795,6 +795,244 @@ metadata: controller-gen.kubebuilder.io/version: v0.10.0 labels: gatekeeper.sh/system: "yes" + name: assignimages.mutations.gatekeeper.sh +spec: + group: mutations.gatekeeper.sh + names: + kind: AssignImage + listKind: AssignImageList + plural: assignimages + singular: assignimage + preserveUnknownFields: false + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: AssignImage is the Schema for the assignimage API. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AssignImageSpec defines the desired state of AssignImage. + properties: + applyTo: + description: ApplyTo lists the specific groups, versions and kinds a mutation will be applied to. This is necessary because every mutation implies part of an object schema and object schemas are associated with specific GVKs. + items: + description: ApplyTo determines what GVKs items the mutation should apply to. Globs are not allowed. + properties: + groups: + items: + type: string + type: array + kinds: + items: + type: string + type: array + versions: + items: + type: string + type: array + type: object + type: array + location: + description: 'Location describes the path to be mutated, for example: `spec.containers[name: main].image`.' + type: string + match: + description: Match allows the user to limit which resources get mutated. Individual match criteria are AND-ed together. An undefined match criteria matches everything. + properties: + excludedNamespaces: + description: 'ExcludedNamespaces is a list of namespace names. If defined, a constraint only applies to resources not in a listed namespace. ExcludedNamespaces also supports a prefix or suffix based glob. For example, `excludedNamespaces: [kube-*]` matches both `kube-system` and `kube-public`, and `excludedNamespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or end. Ex: "kube-*" will match "kube-system" or "kube-public", "*-system" will match "kube-system" or "gatekeeper-system". The asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + kinds: + items: + description: Kinds accepts a list of objects with apiGroups and kinds fields that list the groups/kinds of objects to which the mutation will apply. If multiple groups/kinds objects are specified, only one match is needed for the resource to be in scope. + properties: + apiGroups: + description: APIGroups is the API groups the resources belong to. '*' is all groups. If '*' is present, the length of the slice must be one. Required. + items: + type: string + type: array + kinds: + items: + type: string + type: array + type: object + type: array + labelSelector: + description: 'LabelSelector is the combination of two optional fields: `matchLabels` and `matchExpressions`. These two fields provide different methods of selecting or excluding k8s objects based on the label keys and values included in object metadata. All selection expressions from both sections are ANDed to determine if an object meets the cumulative requirements of the selector.' + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + name: + description: 'Name is the name of an object. If defined, it will match against objects with the specified name. Name also supports a prefix or suffix glob. For example, `name: pod-*` would match both `pod-a` and `pod-b`, and `name: *-pod` would match both `a-pod` and `b-pod`.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + namespaceSelector: + description: NamespaceSelector is a label selector against an object's containing namespace or the object itself, if the object is a namespace. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. The requirements are ANDed. + items: + description: A label selector requirement is a selector that contains values, a key, and an operator that relates the key and values. + properties: + key: + description: key is the label key that the selector applies to. + type: string + operator: + description: operator represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: values is an array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels map is equivalent to an element of matchExpressions, whose key field is "key", the operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + namespaces: + description: 'Namespaces is a list of namespace names. If defined, a constraint only applies to resources in a listed namespace. Namespaces also supports a prefix or suffix based glob. For example, `namespaces: [kube-*]` matches both `kube-system` and `kube-public`, and `namespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' + items: + description: 'A string that supports globbing at its front or end. Ex: "kube-*" will match "kube-system" or "kube-public", "*-system" will match "kube-system" or "gatekeeper-system". The asterisk is required for wildcard matching.' + pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ + type: string + type: array + scope: + description: Scope determines if cluster-scoped and/or namespaced-scoped resources are matched. Accepts `*`, `Cluster`, or `Namespaced`. (defaults to `*`) + type: string + source: + description: Source determines whether generated or original resources are matched. Accepts `Generated`|`Original`|`All` (defaults to `All`). A value of `Generated` will only match generated resources, while `Original` will only match regular resources. + enum: + - All + - Generated + - Original + type: string + type: object + parameters: + description: Parameters define the behavior of the mutator. + properties: + assignDomain: + description: AssignDomain sets the domain component on an image string. The trailing slash should not be included. + type: string + assignPath: + description: AssignPath sets the domain component on an image string. + type: string + assignTag: + description: AssignImage sets the image component on an image string. It must start with a `:` or `@`. + type: string + pathTests: + items: + description: "PathTest allows the user to customize how the mutation works if parent paths are missing. It traverses the list in order. All sub paths are tested against the provided condition, if the test fails, the mutation is not applied. All `subPath` entries must be a prefix of `location`. Any glob characters will take on the same value as was used to expand the matching glob in `location`. \n Available Tests: * MustExist - the path must exist or do not mutate * MustNotExist - the path must not exist or do not mutate." + properties: + condition: + description: Condition describes whether the path either MustExist or MustNotExist in the original object + enum: + - MustExist + - MustNotExist + type: string + subPath: + type: string + type: object + type: array + type: object + type: object + status: + description: AssignImageStatus defines the observed state of AssignImage. + properties: + byPod: + items: + description: MutatorPodStatusStatus defines the observed state of MutatorPodStatus. + properties: + enforced: + type: boolean + errors: + items: + description: MutatorError represents a single error caught while adding a mutator to a system. + properties: + message: + type: string + type: + description: Type indicates a specific class of error for use by controller code. If not present, the error should be treated as not matching any known type. + type: string + required: + - message + type: object + type: array + id: + type: string + mutatorUID: + description: Storing the mutator UID allows us to detect drift, such as when a mutator has been recreated after its CRD was deleted out from under it, interrupting the watch + type: string + observedGeneration: + format: int64 + type: integer + operations: + items: + type: string + type: array + type: object + type: array + type: object + type: object + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.8.0 + labels: + gatekeeper.sh/system: "yes" name: assignmetadata.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh diff --git a/pkg/controller/mutators/core/reconciler_test.go b/pkg/controller/mutators/core/reconciler_test.go index 1cdc07cd59a..c86616e31dc 100644 --- a/pkg/controller/mutators/core/reconciler_test.go +++ b/pkg/controller/mutators/core/reconciler_test.go @@ -254,7 +254,7 @@ func (m *fakeMutator) HasDiff(right mutationtypes.Mutator) bool { m.path.String() != other.path.String() } -func (m *fakeMutator) UsesExternalData() bool { +func (m *fakeMutator) MustTerminate() bool { return false } diff --git a/pkg/controller/mutators/instances/mutator_controllers.go b/pkg/controller/mutators/instances/mutator_controllers.go index b5074ea380c..e2bc0ce3f71 100644 --- a/pkg/controller/mutators/instances/mutator_controllers.go +++ b/pkg/controller/mutators/instances/mutator_controllers.go @@ -7,6 +7,7 @@ import ( "github.com/open-policy-agent/frameworks/constraint/pkg/externaldata" mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" mutationsv1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1" + "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" "github.com/open-policy-agent/gatekeeper/pkg/controller/mutators/core" "github.com/open-policy-agent/gatekeeper/pkg/expansion" "github.com/open-policy-agent/gatekeeper/pkg/mutation" @@ -82,11 +83,34 @@ func (a *Adder) Add(mgr manager.Manager) error { Events: events, EventsSource: eventsSource, } - if err := modifySet.Add(mgr); err != nil { return err } + assignImage := core.Adder{ + Tracker: a.Tracker, + GetPod: a.GetPod, + MutationSystem: a.MutationSystem, + Kind: "AssignImage", + NewMutationObj: func() client.Object { return &v1alpha1.AssignImage{} }, + MutatorFor: func(obj client.Object) (types.Mutator, error) { + // The type is provided by the `NewObj` function above. If we + // are fed the wrong type, this is a non-recoverable error and we + // may as well crash for visibility + assignImage := obj.(*v1alpha1.AssignImage) // nolint:forcetypeassert + unversioned := &mutationsunversioned.AssignImage{} + if err := scheme.Convert(assignImage, unversioned, nil); err != nil { + return nil, err + } + return mutators.MutatorForAssignImage(unversioned) + }, + Events: events, + EventsSource: eventsSource, + } + if err := assignImage.Add(mgr); err != nil { + return err + } + assignMetadata := core.Adder{ Tracker: a.Tracker, GetPod: a.GetPod, diff --git a/pkg/expansion/fixtures/fixtures.go b/pkg/expansion/fixtures/fixtures.go index 94dbafb6649..3d88b5d58d9 100644 --- a/pkg/expansion/fixtures/fixtures.go +++ b/pkg/expansion/fixtures/fixtures.go @@ -120,6 +120,23 @@ spec: - containerPort: '80' ` + PodMutateImage = ` +apiVersion: v1 +kind: Pod +metadata: + labels: + app: nginx + namespace: default +spec: + containers: + - args: + - "/bin/sh" + image: nginx:v2 + name: nginx + ports: + - containerPort: '80' +` + PodImagePullMutateAnnotated = ` apiVersion: v1 kind: Pod @@ -205,6 +222,21 @@ spec: kinds: [] ` + AssignImage = ` +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: AssignImage +metadata: + name: tag-v2 +spec: + applyTo: + - groups: [""] + kinds: ["Pod"] + versions: ["v1"] + location: "spec.containers[name:nginx].image" + parameters: + assignTag: ":v2" +` + AssignHostnameSourceOriginal = ` apiVersion: mutations.gatekeeper.sh/v1alpha1 kind: Assign diff --git a/pkg/expansion/system_test.go b/pkg/expansion/system_test.go index 92696217f04..6251ff8f950 100644 --- a/pkg/expansion/system_test.go +++ b/pkg/expansion/system_test.go @@ -13,6 +13,7 @@ import ( "github.com/open-policy-agent/gatekeeper/pkg/mutation" "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignimage" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignmeta" "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" "gopkg.in/yaml.v3" @@ -770,6 +771,20 @@ func TestExpand(t *testing.T) { {Obj: loadFixture(fixtures.ResultantPurr, t), EnforcementAction: "warn", TemplateName: "expand-cats-purr"}, }, }, + { + name: "1 mutator deployment expands pod with AssignImage", + generator: loadFixture(fixtures.DeploymentNginx, t), + ns: &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}}, + mutators: []types.Mutator{ + loadAssignImage(fixtures.AssignImage, t), + }, + templates: []*expansionunversioned.ExpansionTemplate{ + loadTemplate(fixtures.TempExpDeploymentExpandsPods, t), + }, + want: []*Resultant{ + {Obj: loadFixture(fixtures.PodMutateImage, t), EnforcementAction: "", TemplateName: "expand-deployments"}, + }, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -874,6 +889,20 @@ func loadAssign(f string, t *testing.T) types.Mutator { return mut } +func loadAssignImage(f string, t *testing.T) types.Mutator { + u := loadFixture(f, t) + a := &mutationsunversioned.AssignImage{} + err := convertUnstructuredToTyped(u, a) + if err != nil { + t.Fatalf("error converting assign: %s", err) + } + mut, err := assignimage.MutatorForAssignImage(a) + if err != nil { + t.Fatalf("error creating assignimage: %s", err) + } + return mut +} + func loadAssignMeta(f string, t *testing.T) types.Mutator { u := loadFixture(f, t) a := &mutationsunversioned.AssignMetadata{} diff --git a/pkg/gator/expand/expand.go b/pkg/gator/expand/expand.go index 74084b9ae99..c5a82c51419 100644 --- a/pkg/gator/expand/expand.go +++ b/pkg/gator/expand/expand.go @@ -10,6 +10,7 @@ import ( "github.com/open-policy-agent/gatekeeper/pkg/expansion" "github.com/open-policy-agent/gatekeeper/pkg/mutation" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignimage" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignmeta" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/modifyset" "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" @@ -23,6 +24,7 @@ var mutatorKinds = map[string]bool{ "Assign": true, "AssignMetadata": true, "ModifySet": true, + "AssignImage": true, } type Expander struct { @@ -151,6 +153,12 @@ func (er *Expander) addMutator(mut *unstructured.Unstructured) error { return err } m, mutErr = modifyset.MutatorForModifySet(ms) + case "AssignImage": + a, err := convertAssignImage(mut) + if err != nil { + return err + } + m, mutErr = assignimage.MutatorForAssignImage(a) default: return fmt.Errorf("cannot convert mutator of kind %q", mut.GetKind()) } @@ -246,6 +254,12 @@ func convertModifySet(u *unstructured.Unstructured) (*mutationsunversioned.Modif return ms, err } +func convertAssignImage(u *unstructured.Unstructured) (*mutationsunversioned.AssignImage, error) { + ai := &mutationsunversioned.AssignImage{} + err := convertUnstructuredToTyped(u, ai) + return ai, err +} + func convertNamespace(u *unstructured.Unstructured) (*corev1.Namespace, error) { ns := &corev1.Namespace{} err := convertUnstructuredToTyped(u, ns) diff --git a/pkg/mutation/mutators/assign/assign_mutator.go b/pkg/mutation/mutators/assign/assign_mutator.go index e06fee11cea..7b1e82d2eb2 100644 --- a/pkg/mutation/mutators/assign/assign_mutator.go +++ b/pkg/mutation/mutators/assign/assign_mutator.go @@ -3,13 +3,11 @@ package assign import ( "fmt" "reflect" - "sort" "github.com/google/go-cmp/cmp" mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" mutationsv1beta1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1beta1" "github.com/open-policy-agent/gatekeeper/pkg/logging" - "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/core" "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" patht "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" @@ -39,21 +37,11 @@ type Mutator struct { var _ schema.MutatorWithSchema = &Mutator{} func (m *Mutator) Matches(mutable *types.Mutable) bool { - gvk := mutable.Object.GetObjectKind().GroupVersionKind() - if !match.AppliesTo(m.assign.Spec.ApplyTo, gvk) { - return false - } - target := &match.Matchable{ - Object: mutable.Object, - Namespace: mutable.Namespace, - Source: mutable.Source, - } - matches, err := match.Matches(&m.assign.Spec.Match, target) + res, err := core.MatchWithApplyTo(mutable, m.assign.Spec.ApplyTo, &m.assign.Spec.Match) if err != nil { log.Error(err, "Matches failed for assign", "assign", m.assign.Name) - return false } - return matches + return res } func (m *Mutator) TerminalType() parser.NodeType { @@ -68,7 +56,7 @@ func (m *Mutator) Mutate(mutable *types.Mutable) (bool, error) { return core.Mutate(m.Path(), m.tester, core.NewDefaultSetter(value), mutable.Object) } -func (m *Mutator) UsesExternalData() bool { +func (m *Mutator) MustTerminate() bool { return m.assign.Spec.Parameters.Assign.ExternalData != nil } @@ -128,8 +116,7 @@ func (m *Mutator) String() string { return fmt.Sprintf("%s/%s/%s:%d", m.id.Kind, m.id.Namespace, m.id.Name, m.assign.GetGeneration()) } -// MutatorForAssign returns an mutator built from -// the given assign instance. +// MutatorForAssign returns a mutator built from the given assign instance. func MutatorForAssign(assign *mutationsunversioned.Assign) (*Mutator, error) { // This is not always set by the kubernetes API server assign.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "Assign"}) @@ -143,7 +130,7 @@ func MutatorForAssign(assign *mutationsunversioned.Assign) (*Mutator, error) { return nil, fmt.Errorf("assign %s can't change metadata", assign.GetName()) } - err = checkKeyNotChanged(path) + err = core.CheckKeyNotChanged(path) if err != nil { return nil, err } @@ -168,29 +155,17 @@ func MutatorForAssign(assign *mutationsunversioned.Assign) (*Mutator, error) { } } - id := types.MakeID(assign) - - pathTests, err := gatherPathTests(assign) + tester, err := core.NewTester(assign.GetName(), "Assign", path, assign.Spec.Parameters.PathTests) if err != nil { return nil, err } - tester, err := patht.New(path, pathTests) + gvks, err := core.NewValidatedBindings(assign.GetName(), "Assign", assign.Spec.ApplyTo) if err != nil { return nil, err } - for _, applyTo := range assign.Spec.ApplyTo { - if len(applyTo.Groups) == 0 || len(applyTo.Versions) == 0 || len(applyTo.Kinds) == 0 { - return nil, fmt.Errorf("invalid applyTo for Assign mutator %s, all of group, version and kind must be specified", assign.GetName()) - } - } - - gvks := getSortedGVKs(assign.Spec.ApplyTo) - if len(gvks) == 0 { - return nil, fmt.Errorf("applyTo required for Assign mutator %s", assign.GetName()) - } return &Mutator{ - id: id, + id: types.MakeID(assign), assign: assign.DeepCopy(), bindings: gvks, path: path, @@ -198,19 +173,6 @@ func MutatorForAssign(assign *mutationsunversioned.Assign) (*Mutator, error) { }, nil } -func gatherPathTests(assign *mutationsunversioned.Assign) ([]patht.Test, error) { - pts := assign.Spec.Parameters.PathTests - var pathTests []patht.Test - for _, pt := range pts { - p, err := parser.Parse(pt.SubPath) - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("problem parsing sub path `%s` for Assign %s", pt.SubPath, assign.GetName())) - } - pathTests = append(pathTests, patht.Test{SubPath: p, Condition: pt.Condition}) - } - return pathTests, nil -} - // IsValidAssign returns an error if the given assign object is not // semantically valid. func IsValidAssign(assign *mutationsunversioned.Assign) error { @@ -231,39 +193,6 @@ func hasMetadataRoot(path parser.Path) bool { return false } -// checkKeyNotChanged does not allow to change the key field of -// a list element. A path like foo[name: bar].name is rejected. -func checkKeyNotChanged(p parser.Path) error { - if len(p.Nodes) == 0 { - return errors.New("empty path") - } - if len(p.Nodes) < 2 { - return nil - } - lastNode := p.Nodes[len(p.Nodes)-1] - secondLastNode := p.Nodes[len(p.Nodes)-2] - - if secondLastNode.Type() != parser.ListNode { - return nil - } - if lastNode.Type() != parser.ObjectNode { - return fmt.Errorf("invalid path format: child of a list can't be a list") - } - addedObject, ok := lastNode.(*parser.Object) - if !ok { - return errors.New("failed converting an ObjectNodeType to Object") - } - listNode, ok := secondLastNode.(*parser.List) - if !ok { - return errors.New("failed converting a ListNodeType to List") - } - - if addedObject.Reference == listNode.KeyField { - return fmt.Errorf("invalid path format: changing the item key is not allowed") - } - return nil -} - func validateObjectAssignedToList(p parser.Path, value interface{}) error { if len(p.Nodes) == 0 { return errors.New("empty path") @@ -291,24 +220,3 @@ func validateObjectAssignedToList(p parser.Path, value interface{}) error { return nil } - -func getSortedGVKs(bindings []match.ApplyTo) []runtimeschema.GroupVersionKind { - // deduplicate GVKs - gvksMap := map[runtimeschema.GroupVersionKind]struct{}{} - for _, binding := range bindings { - for _, gvk := range binding.Flatten() { - gvksMap[gvk] = struct{}{} - } - } - - var gvks []runtimeschema.GroupVersionKind - for gvk := range gvksMap { - gvks = append(gvks, gvk) - } - - // we iterate over the map in a stable order so that - // unit tests won't be flaky. - sort.Slice(gvks, func(i, j int) bool { return gvks[i].String() < gvks[j].String() }) - - return gvks -} diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator.go b/pkg/mutation/mutators/assignimage/assignimage_mutator.go new file mode 100644 index 00000000000..313d08dd461 --- /dev/null +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator.go @@ -0,0 +1,206 @@ +package assignimage + +import ( + "fmt" + + "github.com/google/go-cmp/cmp" + mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" + mutationsv1beta1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1beta1" + "github.com/open-policy-agent/gatekeeper/pkg/logging" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/core" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" + patht "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/schema" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" + "github.com/pkg/errors" + runtimeschema "k8s.io/apimachinery/pkg/runtime/schema" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation", logging.Mutator, "assignimage") + +// Mutator is a mutator object built out of an AssignImage instance. +type Mutator struct { + id types.ID + assignImage *mutationsunversioned.AssignImage + + path parser.Path + + // bindings are the set of GVKs this Mutator applies to. + bindings []runtimeschema.GroupVersionKind + tester *patht.Tester +} + +// Mutator implements mutatorWithSchema. +var _ schema.MutatorWithSchema = &Mutator{} + +func (m *Mutator) Matches(mutable *types.Mutable) bool { + res, err := core.MatchWithApplyTo(mutable, m.assignImage.Spec.ApplyTo, &m.assignImage.Spec.Match) + if err != nil { + log.Error(err, "Matches failed for modify set", "modifyset", m.assignImage.Name) + } + return res +} + +func (m *Mutator) TerminalType() parser.NodeType { + return schema.String +} + +func (m *Mutator) Mutate(mutable *types.Mutable) (bool, error) { + p := m.assignImage.Spec.Parameters + s := setter{tag: p.AssignTag, domain: p.AssignDomain, path: p.AssignPath} + return core.Mutate(m.Path(), m.tester, s, mutable.Object) +} + +func (m *Mutator) MustTerminate() bool { + return true +} + +func (m *Mutator) ID() types.ID { + return m.id +} + +func (m *Mutator) SchemaBindings() []runtimeschema.GroupVersionKind { + return m.bindings +} + +func (m *Mutator) HasDiff(mutator types.Mutator) bool { + toCheck, ok := mutator.(*Mutator) + if !ok { // different types, different + return true + } + + if !cmp.Equal(toCheck.id, m.id) { + return true + } + if !cmp.Equal(toCheck.path, m.path) { + return true + } + if !cmp.Equal(toCheck.bindings, m.bindings) { + return true + } + + // any difference in spec may be enough + if !cmp.Equal(toCheck.assignImage.Spec, m.assignImage.Spec) { + return true + } + + return false +} + +func (m *Mutator) Path() parser.Path { + return m.path +} + +func (m *Mutator) DeepCopy() types.Mutator { + res := &Mutator{ + id: m.id, + assignImage: m.assignImage.DeepCopy(), + path: parser.Path{ + Nodes: make([]parser.Node, len(m.path.Nodes)), + }, + bindings: make([]runtimeschema.GroupVersionKind, len(m.bindings)), + } + + copy(res.path.Nodes, m.path.Nodes) + copy(res.bindings, m.bindings) + res.tester = m.tester.DeepCopy() + return res +} + +func (m *Mutator) String() string { + return fmt.Sprintf("%s/%s/%s:%d", m.id.Kind, m.id.Namespace, m.id.Name, m.assignImage.GetGeneration()) +} + +// MutatorForAssign returns an mutator built from +// the given assignImage instance. +func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Mutator, error) { + // This is not always set by the kubernetes API server + assignImage.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "Assign"}) + + path, err := parser.Parse(assignImage.Spec.Location) + if err != nil { + return nil, errors.Wrapf(err, "invalid location format `%s` for Assign %s", assignImage.Spec.Location, assignImage.GetName()) + } + + if core.HasMetadataRoot(path) { + return nil, fmt.Errorf("assignImage %s can't change metadata", assignImage.GetName()) + } + + if hasListTerminal(path) { + return nil, fmt.Errorf("assignImage %s can't change a list without a key", assignImage.GetName()) + } + + err = core.CheckKeyNotChanged(path) + if err != nil { + return nil, err + } + + p := assignImage.Spec.Parameters + if err := validateImageParts(p.AssignDomain, p.AssignPath, p.AssignTag); err != nil { + return nil, fmt.Errorf("assignImage %s has invalid parameters: %w", assignImage.GetName(), err) + } + + tester, err := core.NewTester(assignImage.GetName(), "AssignImage", path, p.PathTests) + if err != nil { + return nil, err + } + + gvks, err := core.NewValidatedBindings(assignImage.GetName(), "AssignImage", assignImage.Spec.ApplyTo) + if err != nil { + return nil, err + } + + return &Mutator{ + id: types.MakeID(assignImage), + assignImage: assignImage.DeepCopy(), + bindings: gvks, + path: path, + tester: tester, + }, nil +} + +func hasListTerminal(path parser.Path) bool { + if len(path.Nodes) == 0 { + return false + } + return path.Nodes[len(path.Nodes)-1].Type() == parser.ListNode +} + +var _ core.Setter = setter{} + +type setter struct { + tag string + domain string + path string +} + +func (s setter) KeyedListOkay() bool { return false } + +func (s setter) KeyedListValue() (map[string]interface{}, error) { + panic("assignimage setter does not handle keyed lists") +} + +func (s setter) SetValue(obj map[string]interface{}, key string) error { + val, exists := obj[key] + strVal := "" + if exists { + val, ok := val.(string) + if !ok { + return fmt.Errorf("expected value at AssignImage location to be a string, got %v of type %T", val, val) + } + strVal = val + } + + obj[key] = mutateImage(s.domain, s.path, s.tag, strVal) + return nil +} + +// IsValidAssignImage returns an error if the given assignImage object is not +// semantically valid. +func IsValidAssignImage(assignImage *mutationsunversioned.AssignImage) error { + if _, err := MutatorForAssignImage(assignImage); err != nil { + return err + } + return nil +} diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_benchmark_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_benchmark_test.go new file mode 100644 index 00000000000..f3824dd730c --- /dev/null +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_benchmark_test.go @@ -0,0 +1,104 @@ +package assignimage + +import ( + "fmt" + "strings" + "testing" + + "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func assignImage(domain, path, tag, location string) *unversioned.AssignImage { + result := &unversioned.AssignImage{ + Spec: unversioned.AssignImageSpec{ + ApplyTo: []match.ApplyTo{{ + Groups: []string{"*"}, + Versions: []string{"*"}, + Kinds: []string{"*"}, + }}, + Location: location, + Parameters: unversioned.AssignImageParameters{ + AssignDomain: domain, + AssignPath: path, + AssignTag: tag, + }, + }, + } + + return result +} + +func benchmarkAssignImageMutator(b *testing.B, n int) { + ai := assignImage("a.b.c", "lib/repo", ":latest", "spec"+strings.Repeat(".spec", n-1)) + mutator, err := MutatorForAssignImage(ai) + if err != nil { + b.Fatal(err) + } + + obj := &unstructured.Unstructured{ + Object: make(map[string]interface{}), + } + p := make([]string, n) + for i := 0; i < n; i++ { + p[i] = "spec" + } + _, err = mutator.Mutate(&types.Mutable{Object: obj}) + if err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = mutator.Mutate(&types.Mutable{Object: obj}) + } +} + +func benchmarkNoAssignImageMutator(b *testing.B, n int) { + location := "spec" + strings.Repeat(".spec", n-1) + a := assignImage("a.b.c", "lib/repo", ":latest", location) + a.Spec.Parameters.PathTests = []unversioned.PathTest{{ + SubPath: location, + Condition: tester.MustNotExist, + }} + mutator, err := MutatorForAssignImage(a) + if err != nil { + b.Fatal(err) + } + + obj := &unstructured.Unstructured{ + Object: make(map[string]interface{}), + } + p := make([]string, n) + for i := 0; i < n; i++ { + p[i] = "spec" + } + _, err = mutator.Mutate(&types.Mutable{Object: obj}) + if err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = mutator.Mutate(&types.Mutable{Object: obj}) + } +} + +func BenchmarkAssignImageMutator_Mutate(b *testing.B) { + ns := []int{1, 2, 5, 10, 20} + + for _, n := range ns { + b.Run(fmt.Sprintf("always mutate %d-depth", n), func(b *testing.B) { + benchmarkAssignImageMutator(b, n) + }) + } + + for _, n := range ns { + b.Run(fmt.Sprintf("never mutate %d-depth", n), func(b *testing.B) { + benchmarkNoAssignImageMutator(b, n) + }) + } +} diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go new file mode 100644 index 00000000000..94f792dacf5 --- /dev/null +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -0,0 +1,339 @@ +package assignimage + +import ( + "fmt" + "testing" + + mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" +) + +type aiTestConfig struct { + domain string + path string + tag string + + location string + pathTests []mutationsunversioned.PathTest + applyTo []match.ApplyTo +} + +func newAiMutator(cfg *aiTestConfig) *Mutator { + m := newAi(cfg) + m2, err := MutatorForAssignImage(m) + if err != nil { + panic(err) + } + return m2 +} + +func newAi(cfg *aiTestConfig) *mutationsunversioned.AssignImage { + m := &mutationsunversioned.AssignImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Foo", + }, + } + m.Spec.Parameters.AssignDomain = cfg.domain + m.Spec.Parameters.AssignPath = cfg.path + m.Spec.Parameters.AssignTag = cfg.tag + m.Spec.Location = cfg.location + m.Spec.Parameters.PathTests = cfg.pathTests + m.Spec.ApplyTo = cfg.applyTo + return m +} + +func newPod(imageVal, name string) *unstructured.Unstructured { + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: name, + Image: imageVal, + }, + }, + }, + } + + u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + if err != nil { + panic(fmt.Sprintf("converting pod to unstructured: %v", err)) + } + return &unstructured.Unstructured{Object: u} +} + +func podTest(wantImage string) func(*unstructured.Unstructured) error { + return func(u *unstructured.Unstructured) error { + var pod corev1.Pod + err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &pod) + if err != nil { + return err + } + + if len(pod.Spec.Containers) != 1 { + return fmt.Errorf("incorrect number of containers: %d", len(pod.Spec.Containers)) + } + + c := pod.Spec.Containers[0] + if c.Image != wantImage { + return fmt.Errorf("image incorrect, got: %q wanted %v", c.Image, wantImage) + } + + return nil + } +} + +func TestMutate(t *testing.T) { + tests := []struct { + name string + obj *unstructured.Unstructured + cfg *aiTestConfig + fn func(*unstructured.Unstructured) error + }{ + { + name: "mutate tag", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + tag: ":new", + }, + obj: newPod("library/busybox:v1", "foo"), + fn: podTest("library/busybox:new"), + }, + { + name: "mutate path", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + path: "new/repo", + }, + obj: newPod("library/busybox:v1", "foo"), + fn: podTest("new/repo:v1"), + }, + { + name: "mutate domain", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + domain: "myreg.io", + }, + obj: newPod("docker.io/library/busybox:v1", "foo"), + fn: podTest("myreg.io/library/busybox:v1"), + }, + { + name: "add domain", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + domain: "myreg.io", + }, + obj: newPod("library/busybox:v1", "foo"), + fn: podTest("myreg.io/library/busybox:v1"), + }, + { + name: "add tag", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + tag: ":latest", + }, + obj: newPod("myreg.io/library/busybox", "foo"), + fn: podTest("myreg.io/library/busybox:latest"), + }, + { + name: "add digest", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + tag: "@sha256:12345678901234567890123456789012", + }, + obj: newPod("myreg.io/library/busybox", "foo"), + fn: podTest("myreg.io/library/busybox@sha256:12345678901234567890123456789012"), + }, + { + name: "mutate all field", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + domain: "myreg.io", + path: "newlib/newbox", + tag: ":v2", + }, + obj: newPod("docker.io/library/busybox:v1", "foo"), + fn: podTest("myreg.io/newlib/newbox:v2"), + }, + { + name: "mutate path, domain not set", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + path: "newlib/newbox", + }, + obj: newPod("library/busybox:v1", "foo"), + fn: podTest("newlib/newbox:v1"), + }, + { + name: "mutate path and tag, no domain set", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + path: "newlib/newbox", + tag: ":latest", + }, + obj: newPod("library/busybox:v1", "foo"), + fn: podTest("newlib/newbox:latest"), + }, + { + name: "mutate tag to digest", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + tag: "@sha256:12345678901234567890123456789012", + }, + obj: newPod("library/busybox:v1", "foo"), + fn: podTest("library/busybox@sha256:12345678901234567890123456789012"), + }, + { + name: "mutate domain with bad imageref with no domain still converges", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + domain: "myreg.io", + }, + obj: newPod("this/not.good:ABC123_//lib.com.repo//localhost@blah101", "foo"), + fn: podTest("myreg.io/this/not.good:ABC123_//lib.com.repo//localhost@blah101"), + }, + { + name: "mutate domain with bad imageref with domain still converges", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + domain: "myreg.io", + }, + obj: newPod("a.b.c:5000//not.good:ABC123_//lib.com.repo//localhost@blah101", "foo"), + fn: podTest("myreg.io//not.good:ABC123_//lib.com.repo//localhost@blah101"), + }, + { + name: "mutate path to domain-like string with domain set", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + domain: "myreg.io", + path: "my.special.repo/a.b/c", + }, + obj: newPod("a.b:latest", "foo"), + fn: podTest("myreg.io/my.special.repo/a.b/c:latest"), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mutator := newAiMutator(test.cfg) + obj := test.obj.DeepCopy() + _, err := mutator.Mutate(&types.Mutable{Object: obj}) + if err != nil { + t.Fatalf("failed mutation: %s", err) + } + if err := test.fn(obj); err != nil { + t.Errorf("failed test: %v", err) + } + }) + } +} + +func TestMutatorForAssignImage(t *testing.T) { + tests := []struct { + name string + cfg *aiTestConfig + wantErr bool + }{ + { + name: "valid assignImage", + cfg: &aiTestConfig{ + domain: "a.b.c", + path: "new/app", + tag: ":latest", + location: "spec.containers[name:foo].image", + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + }, + }, + { + name: "metadata root returns err", + cfg: &aiTestConfig{ + domain: "a.b.c", + path: "new/app", + tag: ":latest", + location: "metadata.labels.image", + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + }, + wantErr: true, + }, + { + name: "terminal list returns err", + cfg: &aiTestConfig{ + domain: "a.b.c", + path: "new/app", + tag: ":latest", + location: "spec.containers[name:foo]", + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + }, + wantErr: true, + }, + { + name: "syntactically invalid location returns err", + cfg: &aiTestConfig{ + domain: "a.b.c", + path: "new/app", + tag: ":latest", + location: "/x/y/zx[)", + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + }, + wantErr: true, + }, + { + name: "bad assigns return err", + cfg: &aiTestConfig{ + domain: "", + path: "a.b.c/repo", + tag: ":latest", + location: "spec.containers[name:foo].image", + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + }, + wantErr: true, + }, + { + name: "metadata root returns err", + cfg: &aiTestConfig{ + domain: "a.b.c", + path: "new/app", + tag: ":latest", + location: "metadata.labels.image", + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mut, err := MutatorForAssignImage(newAi(tc.cfg)) + if err != nil && mut != nil { + t.Errorf("returned non-nil mutator but got err: %s", err) + } + if !tc.wantErr && err != nil { + t.Errorf("did not want error but got: %v", err) + } + if tc.wantErr && err == nil { + t.Error("wanted error but got nil") + } + }) + } +} diff --git a/pkg/mutation/mutators/assignimage/imageparser.go b/pkg/mutation/mutators/assignimage/imageparser.go new file mode 100644 index 00000000000..3ea3ffab4ed --- /dev/null +++ b/pkg/mutation/mutators/assignimage/imageparser.go @@ -0,0 +1,114 @@ +package assignimage + +import ( + "fmt" + "regexp" + "strings" +) + +type image struct { + domain string + path string + tag string +} + +var ( + // domainRegexp defines a schema for a domain component. Primarily, this is to + // avoid the possibility of injecting `/` tokens, which could cause part of + // the domain component to "leak" over to the location component, ultimately + // causing non convergence. + domainRegexp = regexp.MustCompile(`^\w[\w.\-_]*(:\d+)?$`) + + // pathRegexp defines a schema for a location component. It follows the convention + // specified in the docker distribution reference. The regex restricts + // location-components to start with an alphanumeric character, with following + // parts able to be separated by a separator (one period, one or two + // underscore and multiple dashes). + pathRegexp = regexp.MustCompile(`^[a-z0-9]+(?:(?:(?:[._/]|__|[-]*)[a-z0-9]+)+)?`) + + // tagRegexp defines a schema for a tag component. It must start with `:` or `@`. + tagRegexp = regexp.MustCompile(`(^:[\w][\w.-]{0,127}$)|(^@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:][0-9A-Fa-f]{32,}$)`) +) + +func mutateImage(domain, path, tag, mutableImgRef string) string { + oldImg := newImage(mutableImgRef) + newImg := oldImg.newMutatedImage(domain, path, tag) + return newImg.fullRef() +} + +func newImage(imageRef string) image { + domain, remainder := splitDomain(imageRef) + var pt string + tag := "" + if tagSep := strings.IndexAny(remainder, ":@"); tagSep > -1 { + pt = remainder[:tagSep] + tag = remainder[tagSep:] + } else { + pt = remainder + } + + return image{domain: domain, path: pt, tag: tag} +} + +func (img image) newMutatedImage(domain, path, tag string) image { + return image{ + domain: ignoreUnset(img.domain, domain), + path: ignoreUnset(img.path, path), + tag: ignoreUnset(img.tag, tag), + } +} + +// ignoreUnset returns `new` if `new` is set, otherwise it returns `old`. +func ignoreUnset(old, new string) string { + if new != "" { + return new + } + return old +} + +func (img image) fullRef() string { + domain := img.domain + if domain != "" { + domain += "/" + } + return domain + img.path + img.tag +} + +func splitDomain(name string) (domain, remainder string) { + i := strings.IndexRune(name, '/') + if i == -1 || (!strings.ContainsAny(name[:i], ".:") && name[:i] != "localhost") { + return "", name + } + return name[:i], name[i+1:] +} + +func validateImageParts(domain, path, tag string) error { + if domain == "" && path == "" && tag == "" { + return fmt.Errorf("at least one of [assignDomain, assignPath, assignTag] must be set") + } + if domain != "" && !domainRegexp.MatchString(domain) { + return fmt.Errorf("assignDomain %q does not match pattern %q", domain, domainRegexp.String()) + } + // match the whole string for path (anchoring with `$` is tricky here) + if path != "" && path != pathRegexp.FindString(path) { + return fmt.Errorf("assignPath %q does not match pattern %q", path, pathRegexp.String()) + } + if tag != "" && !tagRegexp.MatchString(tag) { + return fmt.Errorf("assignTag %q does not match pattern %q", tag, tagRegexp.String()) + } + + // Check if the path looks like a domain string, and the domain is not set. + // This prevents part of the path field from "leaking" to the domain, causing + // nonconvergent behavior. + // For example, suppose: domain="", path="gcr.io/repo", tag="" + // Suppose no value is currently set on the mutable, so the result is + // just "gcr.io/repo". When this value mutated again, "gcr.io" is parsed into + // the domain component, so the result would be "gcr.io/gcr.io/repo". + if domain == "" { + if d, _ := splitDomain(path); d != "" { + return fmt.Errorf("assignDomain must be set if assignPath %q can be interpretted as part of a domain", path) + } + } + + return nil +} diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go new file mode 100644 index 00000000000..843fec9335c --- /dev/null +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -0,0 +1,372 @@ +package assignimage + +import ( + "testing" +) + +func TestNewImage(t *testing.T) { + tests := []struct { + name string + imageRef string + want image + }{ + { + name: "full image with tag", + imageRef: "gcr.io/some-image/hello:latest", + want: image{ + domain: "gcr.io", + path: "some-image/hello", + tag: ":latest", + }, + }, + { + name: "full image with hash", + imageRef: "some-image/hello@sha256:abcde", + want: image{ + domain: "", + path: "some-image/hello", + tag: "@sha256:abcde", + }, + }, + { + name: "slash in location with tag", + imageRef: "some-image/hello:latest", + want: image{ + domain: "", + path: "some-image/hello", + tag: ":latest", + }, + }, + { + name: "only location", + imageRef: "some-image/hello", + want: image{ + domain: "", + path: "some-image/hello", + tag: "", + }, + }, + { + name: "no slash in location", + imageRef: "some-image:tag123", + want: image{ + domain: "", + path: "some-image", + tag: ":tag123", + }, + }, + { + name: "just location", + imageRef: "alpine", + want: image{ + domain: "", + path: "alpine", + tag: "", + }, + }, + { + name: "leading underscore", + imageRef: "_/alpine", + want: image{ + domain: "", + path: "_/alpine", + tag: "", + }, + }, + { + name: "leading underscore with tag", + imageRef: "_/alpine:latest", + want: image{ + domain: "", + path: "_/alpine", + tag: ":latest", + }, + }, + { + name: "no domain, location has /", + imageRef: "library/busybox:v9", + want: image{ + domain: "", + path: "library/busybox", + tag: ":v9", + }, + }, + { + name: "dots in domain", + imageRef: "this.that.com/repo/alpine:1.23", + want: image{ + domain: "this.that.com", + path: "repo/alpine", + tag: ":1.23", + }, + }, + { + name: "port and dots in domain", + imageRef: "this.that.com:5000/repo/alpine:latest", + want: image{ + domain: "this.that.com:5000", + path: "repo/alpine", + tag: ":latest", + }, + }, + { + name: "localhost with port", + imageRef: "localhost:5000/repo/alpine:latest", + want: image{ + domain: "localhost:5000", + path: "repo/alpine", + tag: ":latest", + }, + }, + { + name: "dots in location", + imageRef: "x.y.z/gcr.io/repo:latest", + want: image{ + domain: "x.y.z", + path: "gcr.io/repo", + tag: ":latest", + }, + }, + { + name: "dot in domain", + imageRef: "gcr.io/repo:latest", + want: image{ + domain: "gcr.io", + path: "repo", + tag: ":latest", + }, + }, + { + name: "invalid ref still parses", + imageRef: "x.io/get/ready4.//this/not.good:404@yikes/bad/string", + want: image{ + domain: "x.io", + path: "get/ready4.//this/not.good", + tag: ":404@yikes/bad/string", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := newImage(tc.imageRef) + if got != tc.want { + t.Errorf("got: %v, want %v", got, tc.want) + } + }) + } +} + +func TestValidateImageParts(t *testing.T) { + tests := []struct { + name string + domain string + path string + tag string + wantErr bool + }{ + { + name: "all valid components", + domain: "my.register.io:5000", + path: "lib/stuff/app", + tag: ":latest", + }, + { + name: "no fields set returns err", + wantErr: true, + }, + { + name: "valid domain with port", + domain: "localhost:5000", + }, + { + name: "valid domain no port", + domain: "localhost", + }, + { + name: "valid domain with .", + domain: "a.b.c", + }, + { + name: "valid domain with - . and port", + domain: "a-b-c.com:5000", + }, + { + name: "valid domain with . and port", + domain: "a.b.c:123", + }, + { + name: "valid domain with _", + domain: "a_b_c:123", + }, + { + name: "invalid domain with leading /", + domain: "/not.ok.io:2000", + wantErr: true, + }, + { + name: "invalid domain with trailing /", + domain: "not.ok.io:2000/", + wantErr: true, + }, + { + name: "invalid domain with middle /", + domain: "not/ok/io", + wantErr: true, + }, + { + name: "invalid domain port start with alpha", + domain: "my.reg.io:abc2000", + wantErr: true, + }, + { + name: "invalid domain with multiple :", + domain: "my.reg.io:2000:", + wantErr: true, + }, + { + name: "invalid domain with repeat :", + domain: "my.reg.io::2000", + wantErr: true, + }, + { + name: "invalid domain with tag", + domain: "my.reg.io:latest", + wantErr: true, + }, + { + name: "invalid domain with digest", + domain: "my.reg.io@sha256:abcde123456789", + wantErr: true, + }, + { + name: "invalid domain with bad character", + domain: ";!234.com", + wantErr: true, + }, + { + name: "valid path", + path: "lib/stuff", + }, + { + name: "domain-like path with domain", + domain: "my.reg.io:5000", + path: "a.b.c/stuff", + }, + { + name: "domain-like path without domain returns err", + path: "a.b.c/stuff", + tag: ":latest", + wantErr: true, + }, + { + name: "valid path . and -", + path: "lib/stuff-app__thing/a/b--c/e", + }, + { + name: "invalid path ending / returns err", + path: "lib/stuff/app/", + wantErr: true, + }, + { + name: "invalid path with leading / returns err", + path: "/lib/stuff/app", + wantErr: true, + }, + { + name: "invalid path with leading : returns err", + domain: "my.register.io:5000", + path: ":lib/stuff/app", + wantErr: true, + }, + { + name: "invalid path with leading @ returns err", + domain: "my.register.io:5000", + path: "@lib/stuff/app", + wantErr: true, + }, + { + name: "invalid path with trailing : returns err", + domain: "my.register.io:5000", + path: "lib/stuff/app:", + wantErr: true, + }, + { + name: "invalid path with trailing @ returns err", + domain: "my.register.io:5000", + path: "lib/stuff/app@", + wantErr: true, + }, + { + name: "invalid path : in middle returns err", + domain: "my.register.io:5000", + path: "lib/stuff:things/app", + wantErr: true, + }, + { + name: "test valid tag", + tag: ":latest", + }, + { + name: "test valid digest", + tag: "@sha256:12345678901234567890123456789012", + }, + { + name: "invalid tag no leading :", + tag: "latest", + wantErr: true, + }, + { + name: "invalid digest no leading @", + tag: "sha256:12345678901234567890123456789012", + wantErr: true, + }, + { + name: "invalid digest hash too short", + tag: "@sha256:123456", + wantErr: true, + }, + { + name: "invalid digest not base 16", + tag: "@sha256:1XYZ5678901234567890123456789012", + wantErr: true, + }, + { + name: "invalid tag leading /", + tag: "/:latest", + wantErr: true, + }, + { + name: "invalid tag trailing /", + tag: ":latest/", + wantErr: true, + }, + { + name: "invalid tag double :", + tag: "::latest", + wantErr: true, + }, + { + name: "invalid digest double @", + tag: "@@sha256:1XYZ5678901234567890123456789012", + wantErr: true, + }, + { + name: "invalid tag @ and :", + tag: "@:latest", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateImageParts(tc.domain, tc.path, tc.tag) + if !tc.wantErr && err != nil { + t.Errorf("did not want error but got: %v", err) + } + if tc.wantErr && err == nil { + t.Error("wanted error but got nil") + } + }) + } +} diff --git a/pkg/mutation/mutators/assignmeta/assignmeta_mutator.go b/pkg/mutation/mutators/assignmeta/assignmeta_mutator.go index 21ee16902aa..e4a72e5a91f 100644 --- a/pkg/mutation/mutators/assignmeta/assignmeta_mutator.go +++ b/pkg/mutation/mutators/assignmeta/assignmeta_mutator.go @@ -80,7 +80,7 @@ func (m *Mutator) Mutate(mutable *types.Mutable) (bool, error) { return core.Mutate(m.path, m.tester, core.NewDefaultSetter(value), mutable.Object) } -func (m *Mutator) UsesExternalData() bool { +func (m *Mutator) MustTerminate() bool { return m.assignMetadata.Spec.Parameters.Assign.ExternalData != nil } @@ -123,7 +123,7 @@ func (m *Mutator) String() string { return fmt.Sprintf("%s/%s/%s:%d", m.id.Kind, m.id.Namespace, m.id.Name, m.assignMetadata.GetGeneration()) } -// MutatorForAssignMetadata builds an Mutator from the given AssignMetadata object. +// MutatorForAssignMetadata builds a Mutator from the given AssignMetadata object. func MutatorForAssignMetadata(assignMeta *mutationsunversioned.AssignMetadata) (*Mutator, error) { // This is not always set by the kubernetes API server assignMeta.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignMetadata"}) diff --git a/pkg/mutation/mutators/conversion.go b/pkg/mutation/mutators/conversion.go index b421c06e15f..15168a4fb75 100644 --- a/pkg/mutation/mutators/conversion.go +++ b/pkg/mutation/mutators/conversion.go @@ -3,6 +3,7 @@ package mutators import ( mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignimage" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignmeta" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/modifyset" ) @@ -18,7 +19,13 @@ func MutatorForAssignMetadata(assignMeta *mutationsunversioned.AssignMetadata) ( return assignmeta.MutatorForAssignMetadata(assignMeta) } -// MutatorForModifySet builds an AssignMetadataMutator from the given ModifySet object. +// MutatorForModifySet builds an ModifySetMutator from the given ModifySet object. func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*modifyset.Mutator, error) { return modifyset.MutatorForModifySet(modifySet) } + +// MutatorForAssignImage builds an AssignImageMutator from the given AssignImage +// object. +func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*assignimage.Mutator, error) { + return assignimage.MutatorForAssignImage(assignImage) +} diff --git a/pkg/mutation/mutators/core/mutator.go b/pkg/mutation/mutators/core/mutator.go new file mode 100644 index 00000000000..9bf295dc12b --- /dev/null +++ b/pkg/mutation/mutators/core/mutator.go @@ -0,0 +1,143 @@ +package core + +import ( + "fmt" + "reflect" + "sort" + + "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" + patht "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// NewTester returns a patht.Tester for the given object name, kind path and +// pathtests. +func NewTester(name string, kind string, path parser.Path, ptests []unversioned.PathTest) (*patht.Tester, error) { + pathTests, err := gatherPathTests(name, kind, ptests) + if err != nil { + return nil, err + } + tester, err := patht.New(path, pathTests) + if err != nil { + return nil, err + } + + return tester, nil +} + +// NewValidatedBindings returns a slice of gvks from the given applies, or an +// error if the applies are invalid. +func NewValidatedBindings(name string, kind string, applies []match.ApplyTo) ([]schema.GroupVersionKind, error) { + for _, applyTo := range applies { + if len(applyTo.Groups) == 0 || len(applyTo.Versions) == 0 || len(applyTo.Kinds) == 0 { + return nil, fmt.Errorf("invalid applyTo for %s mutator %s, all of group, version and kind must be specified", kind, name) + } + } + + gvks := getSortedGVKs(applies) + if len(gvks) == 0 { + return nil, fmt.Errorf("applyTo required for %s mutator %s", kind, name) + } + + return gvks, nil +} + +func gatherPathTests(mutName string, mutKind string, pts []unversioned.PathTest) ([]patht.Test, error) { + var pathTests []patht.Test + for _, pt := range pts { + p, err := parser.Parse(pt.SubPath) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("problem parsing sub path `%s` for %s %s", pt.SubPath, mutKind, mutName)) + } + pathTests = append(pathTests, patht.Test{SubPath: p, Condition: pt.Condition}) + } + + return pathTests, nil +} + +func getSortedGVKs(bindings []match.ApplyTo) []schema.GroupVersionKind { + // deduplicate GVKs + gvksMap := map[schema.GroupVersionKind]struct{}{} + for _, binding := range bindings { + for _, gvk := range binding.Flatten() { + gvksMap[gvk] = struct{}{} + } + } + + var gvks []schema.GroupVersionKind + for gvk := range gvksMap { + gvks = append(gvks, gvk) + } + + // we iterate over the map in a stable order so that + // unit tests won't be flaky. + sort.Slice(gvks, func(i, j int) bool { return gvks[i].String() < gvks[j].String() }) + + return gvks +} + +// HasMetadataRoot returns true if the root node at given path references the +// metadata field. +func HasMetadataRoot(path parser.Path) bool { + if len(path.Nodes) == 0 { + return false + } + return reflect.DeepEqual(path.Nodes[0], &parser.Object{Reference: "metadata"}) +} + +// CheckKeyNotChanged does not allow to change the key field of +// a list element. A path like foo[name: bar].name is rejected. +func CheckKeyNotChanged(p parser.Path) error { + if len(p.Nodes) == 0 { + return errors.New("empty path") + } + if len(p.Nodes) < 2 { + return nil + } + lastNode := p.Nodes[len(p.Nodes)-1] + secondLastNode := p.Nodes[len(p.Nodes)-2] + + if secondLastNode.Type() != parser.ListNode { + return nil + } + if lastNode.Type() != parser.ObjectNode { + return fmt.Errorf("invalid path format: child of a list can't be a list") + } + addedObject, ok := lastNode.(*parser.Object) + if !ok { + return errors.New("failed converting an ObjectNodeType to Object") + } + listNode, ok := secondLastNode.(*parser.List) + if !ok { + return errors.New("failed converting a ListNodeType to List") + } + + if addedObject.Reference == listNode.KeyField { + return fmt.Errorf("invalid path format: changing the item key is not allowed") + } + + return nil +} + +func MatchWithApplyTo(mut *types.Mutable, applies []match.ApplyTo, mat *match.Match) (bool, error) { + gvk := mut.Object.GetObjectKind().GroupVersionKind() + if !match.AppliesTo(applies, gvk) { + return false, nil + } + + target := &match.Matchable{ + Object: mut.Object, + Namespace: mut.Namespace, + Source: mut.Source, + } + matches, err := match.Matches(mat, target) + if err != nil { + return false, err + } + + return matches, nil +} diff --git a/pkg/mutation/mutators/modifyset/modify_set_mutator.go b/pkg/mutation/mutators/modifyset/modify_set_mutator.go index ee7aa845c26..6be2a422fb4 100644 --- a/pkg/mutation/mutators/modifyset/modify_set_mutator.go +++ b/pkg/mutation/mutators/modifyset/modify_set_mutator.go @@ -2,14 +2,11 @@ package modifyset import ( "fmt" - "reflect" - "sort" "github.com/google/go-cmp/cmp" mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" mutationsv1beta1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1beta1" "github.com/open-policy-agent/gatekeeper/pkg/logging" - "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/core" "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" patht "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" @@ -40,21 +37,11 @@ type Mutator struct { var _ schema.MutatorWithSchema = &Mutator{} func (m *Mutator) Matches(mutable *types.Mutable) bool { - gvk := mutable.Object.GetObjectKind().GroupVersionKind() - if !match.AppliesTo(m.modifySet.Spec.ApplyTo, gvk) { - return false - } - target := &match.Matchable{ - Object: mutable.Object, - Namespace: mutable.Namespace, - Source: mutable.Source, - } - matches, err := match.Matches(&m.modifySet.Spec.Match, target) + res, err := core.MatchWithApplyTo(mutable, m.modifySet.Spec.ApplyTo, &m.modifySet.Spec.Match) if err != nil { log.Error(err, "Matches failed for modify set", "modifyset", m.modifySet.Name) - return false } - return matches + return res } func (m *Mutator) TerminalType() parser.NodeType { @@ -75,7 +62,7 @@ func (m *Mutator) Mutate(mutable *types.Mutable) (bool, error) { ) } -func (m *Mutator) UsesExternalData() bool { +func (m *Mutator) MustTerminate() bool { // modify set doesn't use external data return false } @@ -147,7 +134,7 @@ func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, e return nil, errors.Wrapf(err, "invalid location format `%s` for ModifySet %s", modifySet.Spec.Location, modifySet.GetName()) } - if hasMetadataRoot(path) { + if core.HasMetadataRoot(path) { return nil, fmt.Errorf("modifyset %s can't change metadata", modifySet.GetName()) } @@ -155,29 +142,17 @@ func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, e return nil, fmt.Errorf("final node in a modifyset location cannot be a keyed list") } - id := types.MakeID(modifySet) - - pathTests, err := gatherPathTests(modifySet) + tester, err := core.NewTester(modifySet.GetName(), "ModifySet", path, modifySet.Spec.Parameters.PathTests) if err != nil { return nil, err } - tester, err := patht.New(path, pathTests) + gvks, err := core.NewValidatedBindings(modifySet.GetName(), "ModifySet", modifySet.Spec.ApplyTo) if err != nil { return nil, err } - for _, applyTo := range modifySet.Spec.ApplyTo { - if len(applyTo.Groups) == 0 || len(applyTo.Versions) == 0 || len(applyTo.Kinds) == 0 { - return nil, fmt.Errorf("invalid applyTo for ModifySet mutator %s, all of group, version and kind must be specified", modifySet.GetName()) - } - } - - gvks := getSortedGVKs(modifySet.Spec.ApplyTo) - if len(gvks) == 0 { - return nil, fmt.Errorf("applyTo required for ModifySet mutator %s", modifySet.GetName()) - } return &Mutator{ - id: id, + id: types.MakeID(modifySet), modifySet: modifySet.DeepCopy(), bindings: gvks, path: path, @@ -185,19 +160,6 @@ func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, e }, nil } -func gatherPathTests(modifySet *mutationsunversioned.ModifySet) ([]patht.Test, error) { - pts := modifySet.Spec.Parameters.PathTests - var pathTests []patht.Test - for _, pt := range pts { - p, err := parser.Parse(pt.SubPath) - if err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("problem parsing sub path `%s` for ModifySet %s", pt.SubPath, modifySet.GetName())) - } - pathTests = append(pathTests, patht.Test{SubPath: p, Condition: pt.Condition}) - } - return pathTests, nil -} - // IsValidModifySet returns an error if the given modifyset object is not // semantically valid. func IsValidModifySet(modifySet *mutationsunversioned.ModifySet) error { @@ -207,38 +169,6 @@ func IsValidModifySet(modifySet *mutationsunversioned.ModifySet) error { return nil } -func hasMetadataRoot(path parser.Path) bool { - if len(path.Nodes) == 0 { - return false - } - - if reflect.DeepEqual(path.Nodes[0], &parser.Object{Reference: "metadata"}) { - return true - } - return false -} - -func getSortedGVKs(bindings []match.ApplyTo) []runtimeschema.GroupVersionKind { - // deduplicate GVKs - gvksMap := map[runtimeschema.GroupVersionKind]struct{}{} - for _, binding := range bindings { - for _, gvk := range binding.Flatten() { - gvksMap[gvk] = struct{}{} - } - } - - var gvks []runtimeschema.GroupVersionKind - for gvk := range gvksMap { - gvks = append(gvks, gvk) - } - - // we iterate over the map in a stable order so that - // unit tests won't be flaky. - sort.Slice(gvks, func(i, j int) bool { return gvks[i].String() < gvks[j].String() }) - - return gvks -} - var _ core.Setter = setter{} type setter struct { diff --git a/pkg/mutation/mutators/testhelpers/dummy_mutator.go b/pkg/mutation/mutators/testhelpers/dummy_mutator.go index 12cc4b79745..cfa7661453f 100644 --- a/pkg/mutation/mutators/testhelpers/dummy_mutator.go +++ b/pkg/mutation/mutators/testhelpers/dummy_mutator.go @@ -50,7 +50,7 @@ func (d *DummyMutator) Mutate(mutable *types.Mutable) (bool, error) { return core.Mutate(d.Path(), t, core.NewDefaultSetter(d.value), mutable.Object) } -func (d *DummyMutator) UsesExternalData() bool { +func (d *DummyMutator) MustTerminate() bool { return false } diff --git a/pkg/mutation/schema/node.go b/pkg/mutation/schema/node.go index b68515b7982..7516426f83e 100644 --- a/pkg/mutation/schema/node.go +++ b/pkg/mutation/schema/node.go @@ -14,7 +14,7 @@ type node struct { // ReferencedBy tracks the Mutations which reference this part of the schema tree. ReferencedBy IDSet - // Children is the set of child Nodes a this location in the schema. + // Children is the set of child Nodes at this location in the schema. // Each node defines a distinct child definition. If multiple Nodes are defined // for the same child, then there is a schema conflict. Children map[string]map[parser.NodeType]node diff --git a/pkg/mutation/schema/node_test.go b/pkg/mutation/schema/node_test.go index 2ddcd0f38e6..5e513ba52ad 100644 --- a/pkg/mutation/schema/node_test.go +++ b/pkg/mutation/schema/node_test.go @@ -132,6 +132,28 @@ func TestNode_Add(t *testing.T) { id("set"): true, }, }, + { + name: "string vs. set conflict", + before: []idPath{ + ipt("set", "spec.containers[name: foo].images", Set), + }, + add: ipt("string", "spec.containers[name: foo].images", String), + want: IDSet{ + id("set"): true, + id("string"): true, + }, + }, + { + name: "string vs. set conflict at nonterminal node", + before: []idPath{ + ipt("string", "spec.containers.image", String), + }, + add: ipt("set", "spec.containers", Set), + want: IDSet{ + id("set"): true, + id("string"): true, + }, + }, { name: "obj vs. set conflict", before: []idPath{ diff --git a/pkg/mutation/schema/schema.go b/pkg/mutation/schema/schema.go index bb1b5b130ac..b09d8607050 100644 --- a/pkg/mutation/schema/schema.go +++ b/pkg/mutation/schema/schema.go @@ -106,7 +106,7 @@ func (db *DB) upsert(mutator MutatorWithSchema) error { s = &node{} db.schemas[gvk] = s } - newConflicts := s.Add(id, path.Nodes, mutator.TerminalType(), mutator.UsesExternalData()) + newConflicts := s.Add(id, path.Nodes, mutator.TerminalType(), mutator.MustTerminate()) conflicts = merge(conflicts, newConflicts) } @@ -143,7 +143,7 @@ func (db *DB) remove(id types.ID) { log.Error(nil, "mutator associated with missing schema", "mutator", id, "schema", gvk) panic(fmt.Sprintf("mutator %v associated with missing schema %v", id, gvk)) } - s.Remove(id, cachedMutator.Path().Nodes, cachedMutator.TerminalType(), cachedMutator.UsesExternalData()) + s.Remove(id, cachedMutator.Path().Nodes, cachedMutator.TerminalType(), cachedMutator.MustTerminate()) db.schemas[gvk] = s if len(s.ReferencedBy) == 0 { diff --git a/pkg/mutation/schema/schema_test.go b/pkg/mutation/schema/schema_test.go index 82063d7d575..11380a23ca4 100644 --- a/pkg/mutation/schema/schema_test.go +++ b/pkg/mutation/schema/schema_test.go @@ -42,7 +42,7 @@ func (m *fakeMutator) Mutate(*types.Mutable) (bool, error) { panic("should not be called") } -func (m *fakeMutator) UsesExternalData() bool { +func (m *fakeMutator) MustTerminate() bool { return m.usesExternalData } diff --git a/pkg/mutation/schema/terminal_types.go b/pkg/mutation/schema/terminal_types.go index c535af4cf00..10f3c474818 100644 --- a/pkg/mutation/schema/terminal_types.go +++ b/pkg/mutation/schema/terminal_types.go @@ -8,3 +8,6 @@ const Unknown = parser.NodeType("Unknown") // Set represents a list populated by unique values. const Set = parser.NodeType("Set") + +// String represents a string element. +const String = parser.NodeType("String") diff --git a/pkg/mutation/system_errors_test.go b/pkg/mutation/system_errors_test.go index ce50a2e4847..f808ccb2fd8 100644 --- a/pkg/mutation/system_errors_test.go +++ b/pkg/mutation/system_errors_test.go @@ -46,7 +46,7 @@ func (e errorMutator) Mutate(*types.Mutable) (bool, error) { return false, e.err } -func (e errorMutator) UsesExternalData() bool { +func (e errorMutator) MustTerminate() bool { return false } diff --git a/pkg/mutation/system_test.go b/pkg/mutation/system_test.go index 438977e1735..94b318cf6fb 100644 --- a/pkg/mutation/system_test.go +++ b/pkg/mutation/system_test.go @@ -65,7 +65,7 @@ func (m *fakeMutator) Mutate(mutable *types.Mutable) (bool, error) { return true, nil } -func (m *fakeMutator) UsesExternalData() bool { +func (m *fakeMutator) MustTerminate() bool { return false } diff --git a/pkg/mutation/types/mutator.go b/pkg/mutation/types/mutator.go index 0bfaafd69b4..044fb62eb09 100644 --- a/pkg/mutation/types/mutator.go +++ b/pkg/mutation/types/mutator.go @@ -51,8 +51,8 @@ type Mutator interface { Matches(mutable *Mutable) bool // Mutate applies the mutation to the given object Mutate(mutable *Mutable) (bool, error) - // UsesExternalData returns true if the mutation uses external data. - UsesExternalData() bool + // MustTerminate returns true if the mutator requires its path to terminate + MustTerminate() bool // ID returns the id of the current mutator. ID() ID // HasDiff tells if the mutator has meaningful differences diff --git a/pkg/webhook/policy.go b/pkg/webhook/policy.go index 95663e692e1..24d628803fc 100644 --- a/pkg/webhook/policy.go +++ b/pkg/webhook/policy.go @@ -41,6 +41,7 @@ import ( "github.com/open-policy-agent/gatekeeper/pkg/logging" "github.com/open-policy-agent/gatekeeper/pkg/mutation" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignimage" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignmeta" "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/modifyset" mutationtypes "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" @@ -340,6 +341,8 @@ func (h *validationHandler) validateGatekeeperResources(ctx context.Context, req return h.validateAssign(req) case req.AdmissionRequest.Kind.Group == mutationsGroup && req.AdmissionRequest.Kind.Kind == "ModifySet": return h.validateModifySet(req) + case req.AdmissionRequest.Kind.Group == mutationsGroup && req.AdmissionRequest.Kind.Kind == "AssignImage": + return h.validateAssignImage(req) case req.AdmissionRequest.Kind.Group == externalDataGroup && req.AdmissionRequest.Kind.Kind == "Provider": return h.validateProvider(req) } @@ -452,6 +455,23 @@ func (h *validationHandler) validateAssign(req *admission.Request) (bool, error) return false, nil } +func (h *validationHandler) validateAssignImage(req *admission.Request) (bool, error) { + obj, _, err := deserializer.Decode(req.AdmissionRequest.Object.Raw, nil, nil) + if err != nil { + return false, err + } + unversioned := &mutationsunversioned.AssignImage{} + if err := runtimeScheme.Convert(obj, unversioned, nil); err != nil { + return false, err + } + err = assignimage.IsValidAssignImage(unversioned) + if err != nil { + return true, err + } + + return false, nil +} + func (h *validationHandler) validateModifySet(req *admission.Request) (bool, error) { obj, _, err := deserializer.Decode(req.AdmissionRequest.Object.Raw, nil, nil) if err != nil { diff --git a/test/bats/test.bats b/test/bats/test.bats index 24bf49a8720..f8dbe3f867a 100644 --- a/test/bats/test.bats +++ b/test/bats/test.bats @@ -52,6 +52,8 @@ teardown_file() { @test "mutation crds are established" { wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assign.mutations.gatekeeper.sh" wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assignmetadata.mutations.gatekeeper.sh" + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/modifyset.mutations.gatekeeper.sh" + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assignimages.mutations.gatekeeper.sh" } @test "waiting for validating webhook" { @@ -82,9 +84,16 @@ teardown_file() { run kubectl get svc mutate-svc -o jsonpath="{.metadata.annotations.gatekeeper\.sh\/mutations}" assert_equal 'Assign//k8sexternalip:1' "${output}" + kubectl apply -f ${BATS_TESTS_DIR}/mutations/assign_image.yaml + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "mutator_enforced AssignImage add-domain-digest" + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl apply -f ${BATS_TESTS_DIR}/mutations/nginx_pod.yaml" + run kubectl get svc mutate-svc -o jsonpath="{.spec.containers[name:test].image}" + assert_equal "foocorp.org/nginx@sha256:abcde67890123456789abc345678901a" "${output}" + kubectl delete --ignore-not-found svc mutate-svc kubectl delete --ignore-not-found assignmetadata k8sownerlabel kubectl delete --ignore-not-found assign k8sexternalip + kubectl delete --ignore-not-found assignimage add-domain-digest } @test "applying sync config" { diff --git a/test/bats/tests/mutations/assign_image.yaml b/test/bats/tests/mutations/assign_image.yaml new file mode 100644 index 00000000000..f54f2fa359b --- /dev/null +++ b/test/bats/tests/mutations/assign_image.yaml @@ -0,0 +1,19 @@ +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: AssignImage +metadata: + name: add-domain-digest +spec: + applyTo: + - groups: [ "" ] + kinds: [ "Pod" ] + versions: [ "v1" ] + location: "spec.containers[name:*].image" + parameters: + assignDomain: "foocorp.org" + assignTag: "@sha256:abcde67890123456789abc345678901a" + match: + source: "All" + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" ] \ No newline at end of file diff --git a/test/bats/tests/mutations/nginx_pod.yaml b/test/bats/tests/mutations/nginx_pod.yaml new file mode 100644 index 00000000000..b9b346392dc --- /dev/null +++ b/test/bats/tests/mutations/nginx_pod.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Pod +metadata: + name: nginx-test + labels: + role: myrole +spec: + containers: + - name: test + image: nginx:latest \ No newline at end of file diff --git a/test/mutations/mutations.yaml b/test/mutations/mutations.yaml new file mode 100644 index 00000000000..1b7b885ba49 --- /dev/null +++ b/test/mutations/mutations.yaml @@ -0,0 +1,39 @@ +apiVersion: mutations.gatekeeper.sh/v1beta1 +kind: Assign +metadata: + # since mutation is applied alphabetically, we can force to run this mutation first + # by prepending the name with a lowercase letter + name: a-sidecar-injection +spec: + applyTo: + - groups: [ "" ] + kinds: [ "Pod" ] + versions: [ "v1" ] + match: + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" ] + location: "spec.containers[name:networking]" + parameters: + assign: + value: + name: networking + image: busybox +--- +apiVersion: mutations.gatekeeper.sh/v1beta1 +kind: AssignMetadata +metadata: + name: add-env-label +spec: + match: + source: Generated + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" ] + location: "metadata.labels.env" + parameters: + assign: + value: "prod" +--- From acab9fe57daa78fbaf2b6b5b6d87a9eb27a7b3f5 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 6 Dec 2022 13:08:56 -0800 Subject: [PATCH 02/29] fix tests Signed-off-by: davis-haba --- apis/mutations/v1alpha1/assignimage_types.go | 1 - cmd/build/helmify/kustomization.yaml | 2 +- .../assignimage-customresourcedefinition.yaml | 4 -- pkg/mutation/mutators/conversion.go | 2 +- test/mutations/mutations.yaml | 39 ------------------- 5 files changed, 2 insertions(+), 46 deletions(-) delete mode 100644 test/mutations/mutations.yaml diff --git a/apis/mutations/v1alpha1/assignimage_types.go b/apis/mutations/v1alpha1/assignimage_types.go index 5939225453c..9c8ca6c3863 100644 --- a/apis/mutations/v1alpha1/assignimage_types.go +++ b/apis/mutations/v1alpha1/assignimage_types.go @@ -73,7 +73,6 @@ type AssignImageStatus struct { // +kubebuilder:resource:path="assignimage" // +kubebuilder:resource:scope="Cluster" // +kubebuilder:subresource:status -// +kubebuilder:storageversion // AssignImage is the Schema for the assignimage API. type AssignImage struct { diff --git a/cmd/build/helmify/kustomization.yaml b/cmd/build/helmify/kustomization.yaml index 19d21e2418d..f4bd7cda12c 100644 --- a/cmd/build/helmify/kustomization.yaml +++ b/cmd/build/helmify/kustomization.yaml @@ -50,7 +50,7 @@ patchesJson6902: group: apiextensions.k8s.io version: v1 kind: CustomResourceDefinition - name: assignimage.mutations.gatekeeper.sh + name: assignimages.mutations.gatekeeper.sh path: labels_patch.yaml - target: group: apiextensions.k8s.io diff --git a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml index 046415921be..10374702dc5 100644 --- a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml @@ -4,11 +4,7 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.8.0 labels: - app: '{{ template "gatekeeper.name" . }}' - chart: '{{ template "gatekeeper.name" . }}' gatekeeper.sh/system: "yes" - heritage: '{{ .Release.Service }}' - release: '{{ .Release.Name }}' name: assignimages.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh diff --git a/pkg/mutation/mutators/conversion.go b/pkg/mutation/mutators/conversion.go index 15168a4fb75..7b83bceb544 100644 --- a/pkg/mutation/mutators/conversion.go +++ b/pkg/mutation/mutators/conversion.go @@ -19,7 +19,7 @@ func MutatorForAssignMetadata(assignMeta *mutationsunversioned.AssignMetadata) ( return assignmeta.MutatorForAssignMetadata(assignMeta) } -// MutatorForModifySet builds an ModifySetMutator from the given ModifySet object. +// MutatorForModifySet builds a ModifySetMutator from the given ModifySet object. func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*modifyset.Mutator, error) { return modifyset.MutatorForModifySet(modifySet) } diff --git a/test/mutations/mutations.yaml b/test/mutations/mutations.yaml deleted file mode 100644 index 1b7b885ba49..00000000000 --- a/test/mutations/mutations.yaml +++ /dev/null @@ -1,39 +0,0 @@ -apiVersion: mutations.gatekeeper.sh/v1beta1 -kind: Assign -metadata: - # since mutation is applied alphabetically, we can force to run this mutation first - # by prepending the name with a lowercase letter - name: a-sidecar-injection -spec: - applyTo: - - groups: [ "" ] - kinds: [ "Pod" ] - versions: [ "v1" ] - match: - scope: Namespaced - kinds: - - apiGroups: [ "*" ] - kinds: [ "Pod" ] - location: "spec.containers[name:networking]" - parameters: - assign: - value: - name: networking - image: busybox ---- -apiVersion: mutations.gatekeeper.sh/v1beta1 -kind: AssignMetadata -metadata: - name: add-env-label -spec: - match: - source: Generated - scope: Namespaced - kinds: - - apiGroups: [ "*" ] - kinds: [ "Pod" ] - location: "metadata.labels.env" - parameters: - assign: - value: "prod" ---- From cc6b0ad2d5a59854e474b3fcbd31cbbc2bda1ff3 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 6 Dec 2022 15:53:21 -0800 Subject: [PATCH 03/29] fix controller gen setup Signed-off-by: davis-haba --- .../mutations.gatekeeper.sh_assignimages.yaml | 330 ------------------ config/crd/kustomization.yaml | 10 +- .../assignimage-customresourcedefinition.yaml | 16 +- manifest_staging/deploy/gatekeeper.yaml | 12 +- 4 files changed, 30 insertions(+), 338 deletions(-) delete mode 100644 config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml diff --git a/config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml b/config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml deleted file mode 100644 index 27bd982ccc3..00000000000 --- a/config/crd/bases/mutations.gatekeeper.sh_assignimages.yaml +++ /dev/null @@ -1,330 +0,0 @@ ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.8.0 - creationTimestamp: null - name: assignimages.mutations.gatekeeper.sh -spec: - group: mutations.gatekeeper.sh - names: - kind: AssignImage - listKind: AssignImageList - plural: assignimages - singular: assignimage - scope: Namespaced - versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - description: AssignImage is the Schema for the assignimage API. - properties: - apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' - type: string - kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' - type: string - metadata: - type: object - spec: - description: AssignImageSpec defines the desired state of AssignImage. - properties: - applyTo: - description: ApplyTo lists the specific groups, versions and kinds - a mutation will be applied to. This is necessary because every mutation - implies part of an object schema and object schemas are associated - with specific GVKs. - items: - description: ApplyTo determines what GVKs items the mutation should - apply to. Globs are not allowed. - properties: - groups: - items: - type: string - type: array - kinds: - items: - type: string - type: array - versions: - items: - type: string - type: array - type: object - type: array - location: - description: 'Location describes the path to be mutated, for example: - `spec.containers[name: main].image`.' - type: string - match: - description: Match allows the user to limit which resources get mutated. - Individual match criteria are AND-ed together. An undefined match - criteria matches everything. - properties: - excludedNamespaces: - description: 'ExcludedNamespaces is a list of namespace names. - If defined, a constraint only applies to resources not in a - listed namespace. ExcludedNamespaces also supports a prefix - or suffix based glob. For example, `excludedNamespaces: [kube-*]` - matches both `kube-system` and `kube-public`, and `excludedNamespaces: - [*-system]` matches both `kube-system` and `gatekeeper-system`.' - items: - description: 'A string that supports globbing at its front or - end. Ex: "kube-*" will match "kube-system" or "kube-public", - "*-system" will match "kube-system" or "gatekeeper-system". The - asterisk is required for wildcard matching.' - pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ - type: string - type: array - kinds: - items: - description: Kinds accepts a list of objects with apiGroups - and kinds fields that list the groups/kinds of objects to - which the mutation will apply. If multiple groups/kinds objects - are specified, only one match is needed for the resource to - be in scope. - properties: - apiGroups: - description: APIGroups is the API groups the resources belong - to. '*' is all groups. If '*' is present, the length of - the slice must be one. Required. - items: - type: string - type: array - kinds: - items: - type: string - type: array - type: object - type: array - labelSelector: - description: 'LabelSelector is the combination of two optional - fields: `matchLabels` and `matchExpressions`. These two fields - provide different methods of selecting or excluding k8s objects - based on the label keys and values included in object metadata. All - selection expressions from both sections are ANDed to determine - if an object meets the cumulative requirements of the selector.' - properties: - matchExpressions: - description: matchExpressions is a list of label selector - requirements. The requirements are ANDed. - items: - description: A label selector requirement is a selector - that contains values, a key, and an operator that relates - the key and values. - properties: - key: - description: key is the label key that the selector - applies to. - type: string - operator: - description: operator represents a key's relationship - to a set of values. Valid operators are In, NotIn, - Exists and DoesNotExist. - type: string - values: - description: values is an array of string values. If - the operator is In or NotIn, the values array must - be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced - during a strategic merge patch. - items: - type: string - type: array - required: - - key - - operator - type: object - type: array - matchLabels: - additionalProperties: - type: string - description: matchLabels is a map of {key,value} pairs. A - single {key,value} in the matchLabels map is equivalent - to an element of matchExpressions, whose key field is "key", - the operator is "In", and the values array contains only - "value". The requirements are ANDed. - type: object - type: object - name: - description: 'Name is the name of an object. If defined, it will - match against objects with the specified name. Name also supports - a prefix or suffix glob. For example, `name: pod-*` would match - both `pod-a` and `pod-b`, and `name: *-pod` would match both - `a-pod` and `b-pod`.' - pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ - type: string - namespaceSelector: - description: NamespaceSelector is a label selector against an - object's containing namespace or the object itself, if the object - is a namespace. - properties: - matchExpressions: - description: matchExpressions is a list of label selector - requirements. The requirements are ANDed. - items: - description: A label selector requirement is a selector - that contains values, a key, and an operator that relates - the key and values. - properties: - key: - description: key is the label key that the selector - applies to. - type: string - operator: - description: operator represents a key's relationship - to a set of values. Valid operators are In, NotIn, - Exists and DoesNotExist. - type: string - values: - description: values is an array of string values. If - the operator is In or NotIn, the values array must - be non-empty. If the operator is Exists or DoesNotExist, - the values array must be empty. This array is replaced - during a strategic merge patch. - items: - type: string - type: array - required: - - key - - operator - type: object - type: array - matchLabels: - additionalProperties: - type: string - description: matchLabels is a map of {key,value} pairs. A - single {key,value} in the matchLabels map is equivalent - to an element of matchExpressions, whose key field is "key", - the operator is "In", and the values array contains only - "value". The requirements are ANDed. - type: object - type: object - namespaces: - description: 'Namespaces is a list of namespace names. If defined, - a constraint only applies to resources in a listed namespace. Namespaces - also supports a prefix or suffix based glob. For example, `namespaces: - [kube-*]` matches both `kube-system` and `kube-public`, and - `namespaces: [*-system]` matches both `kube-system` and `gatekeeper-system`.' - items: - description: 'A string that supports globbing at its front or - end. Ex: "kube-*" will match "kube-system" or "kube-public", - "*-system" will match "kube-system" or "gatekeeper-system". The - asterisk is required for wildcard matching.' - pattern: ^(\*|\*-)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\*|-\*)?$ - type: string - type: array - scope: - description: Scope determines if cluster-scoped and/or namespaced-scoped - resources are matched. Accepts `*`, `Cluster`, or `Namespaced`. - (defaults to `*`) - type: string - source: - description: Source determines whether generated or original resources - are matched. Accepts `Generated`|`Original`|`All` (defaults - to `All`). A value of `Generated` will only match generated - resources, while `Original` will only match regular resources. - enum: - - All - - Generated - - Original - type: string - type: object - parameters: - description: Parameters define the behavior of the mutator. - properties: - assignDomain: - description: AssignDomain sets the domain component on an image - string. The trailing slash should not be included. - type: string - assignPath: - description: AssignPath sets the domain component on an image - string. - type: string - assignTag: - description: AssignImage sets the image component on an image - string. It must start with a `:` or `@`. - type: string - pathTests: - items: - description: "PathTest allows the user to customize how the - mutation works if parent paths are missing. It traverses the - list in order. All sub paths are tested against the provided - condition, if the test fails, the mutation is not applied. - All `subPath` entries must be a prefix of `location`. Any - glob characters will take on the same value as was used to - expand the matching glob in `location`. \n Available Tests: - * MustExist - the path must exist or do not mutate * MustNotExist - - the path must not exist or do not mutate." - properties: - condition: - description: Condition describes whether the path either - MustExist or MustNotExist in the original object - enum: - - MustExist - - MustNotExist - type: string - subPath: - type: string - type: object - type: array - type: object - type: object - status: - description: AssignImageStatus defines the observed state of AssignImage. - properties: - byPod: - items: - description: MutatorPodStatusStatus defines the observed state of - MutatorPodStatus. - properties: - enforced: - type: boolean - errors: - items: - description: MutatorError represents a single error caught - while adding a mutator to a system. - properties: - message: - type: string - type: - description: Type indicates a specific class of error - for use by controller code. If not present, the error - should be treated as not matching any known type. - type: string - required: - - message - type: object - type: array - id: - type: string - mutatorUID: - description: Storing the mutator UID allows us to detect drift, - such as when a mutator has been recreated after its CRD was - deleted out from under it, interrupting the watch - type: string - observedGeneration: - format: int64 - type: integer - operations: - items: - type: string - type: array - type: object - type: array - type: object - type: object - served: true - storage: true -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 1262dbf3462..c2b8e350482 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -7,7 +7,7 @@ resources: - bases/status.gatekeeper.sh_constrainttemplatepodstatuses.yaml - bases/status.gatekeeper.sh_mutatorpodstatuses.yaml - bases/mutations.gatekeeper.sh_assign.yaml -- bases/mutations.gatekeeper.sh_assignimages.yaml +- bases/mutations.gatekeeper.sh_assignimage.yaml - bases/mutations.gatekeeper.sh_assignmetadata.yaml - bases/mutations.gatekeeper.sh_modifyset.yaml - bases/expansion.gatekeeper.sh_expansiontemplate.yaml @@ -42,12 +42,18 @@ patchesJson6902: kind: CustomResourceDefinition name: modifyset.mutations.gatekeeper.sh path: patches/max_name_size.yaml +- target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: assignimage.mutations.gatekeeper.sh + path: patches/max_name_size.yaml patchesStrategicMerge: #- patches/max_name_size_for_modifyset.yaml #- patches/max_name_size_for_assign.yaml #- patches/max_name_size_for_assignmetadata.yaml -#- patches/max_name_size_for_assignimages.yaml +#- patches/max_name_size_for_assignimage.yaml # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. # patches here are for enabling the conversion webhook for each CRD #- patches/webhook_in_configs.yaml diff --git a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml index 10374702dc5..a8d0982cb7b 100644 --- a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml @@ -4,17 +4,21 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.8.0 labels: + app: '{{ template "gatekeeper.name" . }}' + chart: '{{ template "gatekeeper.name" . }}' gatekeeper.sh/system: "yes" - name: assignimages.mutations.gatekeeper.sh + heritage: '{{ .Release.Service }}' + release: '{{ .Release.Name }}' + name: assignimage.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh names: kind: AssignImage listKind: AssignImageList - plural: assignimages + plural: assignimage singular: assignimage preserveUnknownFields: false - scope: Namespaced + scope: Cluster versions: - name: v1alpha1 schema: @@ -28,6 +32,10 @@ spec: description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: + properties: + name: + maxLength: 63 + type: string type: object spec: description: AssignImageSpec defines the desired state of AssignImage. @@ -229,6 +237,8 @@ spec: type: object served: true storage: true + subresources: + status: {} status: acceptedNames: kind: "" diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 4fc5a342b65..803f87a3207 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -795,16 +795,16 @@ metadata: controller-gen.kubebuilder.io/version: v0.10.0 labels: gatekeeper.sh/system: "yes" - name: assignimages.mutations.gatekeeper.sh + name: assignimage.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh names: kind: AssignImage listKind: AssignImageList - plural: assignimages + plural: assignimage singular: assignimage preserveUnknownFields: false - scope: Namespaced + scope: Cluster versions: - name: v1alpha1 schema: @@ -818,6 +818,10 @@ spec: description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: + properties: + name: + maxLength: 63 + type: string type: object spec: description: AssignImageSpec defines the desired state of AssignImage. @@ -1019,6 +1023,8 @@ spec: type: object served: true storage: true + subresources: + status: {} status: acceptedNames: kind: "" From e6441845076348799ffe0cb6a6999af8b6ef64b3 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 6 Dec 2022 16:02:17 -0800 Subject: [PATCH 04/29] fix helm manifest generation Signed-off-by: davis-haba --- cmd/build/helmify/kustomization.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/build/helmify/kustomization.yaml b/cmd/build/helmify/kustomization.yaml index f4bd7cda12c..19d21e2418d 100644 --- a/cmd/build/helmify/kustomization.yaml +++ b/cmd/build/helmify/kustomization.yaml @@ -50,7 +50,7 @@ patchesJson6902: group: apiextensions.k8s.io version: v1 kind: CustomResourceDefinition - name: assignimages.mutations.gatekeeper.sh + name: assignimage.mutations.gatekeeper.sh path: labels_patch.yaml - target: group: apiextensions.k8s.io From f20bf4f2b49405d0032931322910a94b9ad1d07d Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 6 Dec 2022 18:25:02 -0800 Subject: [PATCH 05/29] WIP assignimage byPod status Signed-off-by: davis-haba --- Makefile | 4 +- .../assignimage-customresourcedefinition.yaml | 4 -- .../mutatorstatus/mutatorstatus_controller.go | 29 ++++++++++- pkg/readiness/ready_tracker.go | 48 ++++++++++++++++++- pkg/readiness/ready_tracker_test.go | 40 ++++++++++++++++ pkg/readiness/testdata/99-assignimage.yaml | 20 ++++++++ pkg/readiness/testdata_test.go | 19 ++++++++ test/bats/test.bats | 2 +- 8 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 pkg/readiness/testdata/99-assignimage.yaml diff --git a/Makefile b/Makefile index f335bb371fa..2664caa82e8 100644 --- a/Makefile +++ b/Makefile @@ -101,7 +101,9 @@ all: lint test manager # Run tests native-test: - GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race -bench . -coverprofile cover.out + # GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race -bench . -coverprofile cover.out + # TODO comment above back in + GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race . -coverprofile cover.out # Hook to run docker tests .PHONY: test diff --git a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml index a8d0982cb7b..33ba628ab4c 100644 --- a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml @@ -4,11 +4,7 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.8.0 labels: - app: '{{ template "gatekeeper.name" . }}' - chart: '{{ template "gatekeeper.name" . }}' gatekeeper.sh/system: "yes" - heritage: '{{ .Release.Service }}' - release: '{{ .Release.Name }}' name: assignimage.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index bb1b422fc16..8e67becc93c 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -25,6 +25,7 @@ import ( constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client" "github.com/open-policy-agent/frameworks/constraint/pkg/externaldata" mutationsv1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1" + mutationsv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" "github.com/open-policy-agent/gatekeeper/apis/status/v1beta1" "github.com/open-policy-agent/gatekeeper/pkg/expansion" "github.com/open-policy-agent/gatekeeper/pkg/logging" @@ -79,8 +80,8 @@ func (a *Adder) Add(mgr manager.Manager) error { // newReconciler returns a new reconcile.Reconciler. func newReconciler( - mgr manager.Manager, - cs *watch.ControllerSwitch, + mgr manager.Manager, + cs *watch.ControllerSwitch, ) reconcile.Reconciler { return &ReconcileMutatorStatus{ // Separate reader and writer because manager's default client bypasses the cache for unstructured resources. @@ -162,6 +163,13 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { if err != nil { return err } + err = c.Watch( + &source.Kind{Type: &mutationsv1alpha1.AssignImage{}}, + handler.EnqueueRequestsFromMapFunc(util.EventPackerMapFuncHardcodeGVK(schema.GroupVersionKind{Group: v1beta1.MutationsGroup, Version: "v1alpha1", Kind: "AssignImage"})), + ) + if err != nil { + return err + } return c.Watch( &source.Kind{Type: &mutationsv1.ModifySet{}}, handler.EnqueueRequestsFromMapFunc(util.EventPackerMapFuncHardcodeGVK(schema.GroupVersionKind{Group: v1beta1.MutationsGroup, Version: "v1", Kind: "ModifySet"})), @@ -197,6 +205,20 @@ func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcil } gvk, unpackedRequest, err := util.UnpackRequest(request) + + // TODO rm debug + fmt.Printf("Reconcile() gvk: %s\n", gvk) + wrongSchema := schema.GroupVersionKind{ + Group: "mutations.gatekeeper.sh", + Version: "v1", + Kind: "AssignImage", + } + if gvk == wrongSchema { + errStr := fmt.Sprintf("Reconcile got wrong GVK for AssignImage: %s, wanted version to be v1alpha1", gvk) + panic(errStr) + } + // END DEBUG + if err != nil { // Unrecoverable, do not retry. log.Error(err, "unpacking request", "request", request) @@ -217,6 +239,9 @@ func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcil if errors.IsNotFound(err) { return reconcile.Result{}, nil } + // TODO rm debug + fmt.Printf("Reconcile problem getting resource, err: %s\n", err) + // END DEBUG return reconcile.Result{}, err } diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index 184ac1b0bc1..7e33836144b 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -27,6 +27,7 @@ import ( "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" configv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/config/v1alpha1" mutationv1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1" + mutationsv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" "github.com/open-policy-agent/gatekeeper/pkg/keys" "github.com/open-policy-agent/gatekeeper/pkg/operations" "github.com/open-policy-agent/gatekeeper/pkg/syncutil" @@ -64,6 +65,7 @@ type Tracker struct { assignMetadata *objectTracker assign *objectTracker modifySet *objectTracker + assignImage *objectTracker externalDataProvider *objectTracker constraints *trackerMap data *trackerMap @@ -97,6 +99,7 @@ func newTracker(lister Lister, mutationEnabled bool, externalDataEnabled bool, f tracker.assignMetadata = newObjTracker(mutationv1.GroupVersion.WithKind("AssignMetadata"), fn) tracker.assign = newObjTracker(mutationv1.GroupVersion.WithKind("Assign"), fn) tracker.modifySet = newObjTracker(mutationv1.GroupVersion.WithKind("ModifySet"), fn) + tracker.assignImage = newObjTracker(mutationsv1alpha1.GroupVersion.WithKind("AssignImage"), fn) } if externalDataEnabled { tracker.externalDataProvider = newObjTracker(externaldatav1beta1.SchemeGroupVersion.WithKind("Provider"), fn) @@ -144,6 +147,11 @@ func (t *Tracker) For(gvk schema.GroupVersionKind) Expectations { return t.modifySet } return noopExpectations{} + case gvk.GroupVersion() == mutationsv1alpha1.GroupVersion && gvk.Kind == "AssignImage": + if t.mutationEnabled { + return t.assignImage + } + return noopExpectations{} } // Avoid new constraint trackers after templates have been populated. @@ -207,12 +215,13 @@ func (t *Tracker) Satisfied() bool { } if t.mutationEnabled { - if !t.assignMetadata.Satisfied() || !t.assign.Satisfied() || !t.modifySet.Satisfied() { + if !t.assignMetadata.Satisfied() || !t.assign.Satisfied() || !t.modifySet.Satisfied() || !t.assignImage.Satisfied() { return false } log.V(1).Info("all expectations satisfied", "tracker", "assignMetadata") log.V(1).Info("all expectations satisfied", "tracker", "assign") log.V(1).Info("all expectations satisfied", "tracker", "modifySet") + log.V(1).Info("all expectations satisfied", "tracker", "assignImage") } if t.externalDataEnabled { @@ -275,6 +284,9 @@ func (t *Tracker) Run(ctx context.Context) error { grp.Go(func() error { return t.trackModifySet(gctx) }) + grp.Go(func() error { + return t.trackAssignImage(gctx) + }) } if t.externalDataEnabled { grp.Go(func() error { @@ -331,7 +343,7 @@ func (t *Tracker) Populated() bool { mutationPopulated := true if t.mutationEnabled { // If !t.mutationEnabled and we call this, it yields a null pointer exception - mutationPopulated = t.assignMetadata.Populated() && t.assign.Populated() && t.modifySet.Populated() + mutationPopulated = t.assignMetadata.Populated() && t.assign.Populated() && t.modifySet.Populated() && t.assignImage.Populated() } externalDataProviderPopulated := true if t.externalDataEnabled { @@ -529,6 +541,31 @@ func (t *Tracker) trackModifySet(ctx context.Context) error { return nil } +func (t *Tracker) trackAssignImage(ctx context.Context) error { + defer func() { + t.assignImage.ExpectationsDone() + log.V(1).Info("AssignImage expectations populated") + _ = t.constraintTrackers.Wait() + }() + + if !t.mutationEnabled { + return nil + } + + assignImageList := &mutationsv1alpha1.AssignImageList{} + lister := retryLister(t.lister, retryAll) + if err := lister.List(ctx, assignImageList); err != nil { + return fmt.Errorf("listing AssignImage: %w", err) + } + log.V(1).Info("setting expectations for AssignImage", "AssignImage Count", len(assignImageList.Items)) + + for index := range assignImageList.Items { + log.V(1).Info("expecting AssignImage", "name", assignImageList.Items[index].GetName()) + t.assignImage.Expect(&assignImageList.Items[index]) + } + return nil +} + func (t *Tracker) trackExternalDataProvider(ctx context.Context) error { defer func() { t.externalDataProvider.ExpectationsDone() @@ -815,6 +852,7 @@ func (t *Tracker) statsPrinter(ctx context.Context) { logUnsatisfiedAssignMetadata(t) logUnsatisfiedAssign(t) logUnsatisfiedModifySet(t) + logUnsatisfiedAssignImage(t) } if t.externalDataEnabled { logUnsatisfiedExternalDataProvider(t) @@ -840,6 +878,12 @@ func logUnsatisfiedModifySet(t *Tracker) { } } +func logUnsatisfiedAssignImage(t *Tracker) { + for _, amKey := range t.assignImage.unsatisfied() { + log.Info("unsatisfied AssignImage", "name", amKey.namespacedName) + } +} + func logUnsatisfiedExternalDataProvider(t *Tracker) { for _, amKey := range t.externalDataProvider.unsatisfied() { log.Info("unsatisfied Provider", "name", amKey.namespacedName) diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go index 3bcff8e9f63..325cbabcf5d 100644 --- a/pkg/readiness/ready_tracker_test.go +++ b/pkg/readiness/ready_tracker_test.go @@ -224,6 +224,46 @@ func Test_ModifySet(t *testing.T) { } } +//func Test_AssignImage(t *testing.T) { +// g := gomega.NewWithT(t) +// +// testutils.Setenv(t, "POD_NAME", "no-pod") +// +// // Apply fixtures *before* the controllers are set up. +// err := applyFixtures("testdata") +// if err != nil { +// t.Fatalf("applying fixtures: %v", err) +// } +// +// // Wire up the rest. +// mgr, wm := setupManager(t) +// opaClient := setupOpa(t) +// +// mutationSystem := mutation.NewSystem(mutation.SystemOpts{}) +// +// if err := setupController(mgr, wm, opaClient, mutationSystem, nil); err != nil { +// t.Fatalf("setupControllers: %v", err) +// } +// +// ctx := context.Background() +// testutils.StartManager(ctx, t, mgr) +// +// g.Eventually(func() (bool, error) { +// ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) +// defer cancel() +// return probeIsReady(ctx) +// }, 20*time.Second, 1*time.Second).Should(gomega.BeTrue()) +// +// // Verify that the AssignImage is present in the cache +// for _, am := range testAssignImage { +// id := mutationtypes.MakeID(am) +// expectedMutator := mutationSystem.Get(id) +// if expectedMutator == nil { +// t.Fatal("want expectedMutator != nil but got nil") +// } +// } +//} + func Test_Assign(t *testing.T) { g := gomega.NewWithT(t) diff --git a/pkg/readiness/testdata/99-assignimage.yaml b/pkg/readiness/testdata/99-assignimage.yaml new file mode 100644 index 00000000000..abb62e84478 --- /dev/null +++ b/pkg/readiness/testdata/99-assignimage.yaml @@ -0,0 +1,20 @@ +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: AssignImage +metadata: + name: demo +spec: + applyTo: + - groups: [ "" ] + kinds: [ "Pod" ] + versions: [ "v1" ] + location: "spec.containers[name:*].image" + parameters: + assignDomain: "barcorp.org" + assignTag: ":latest" + assignPath: "newpath/newrepo" + match: + source: "All" + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" ] diff --git a/pkg/readiness/testdata_test.go b/pkg/readiness/testdata_test.go index 845e1feac36..9c61672030b 100644 --- a/pkg/readiness/testdata_test.go +++ b/pkg/readiness/testdata_test.go @@ -51,6 +51,10 @@ var testModifySet = []*mutationsv1alpha1.ModifySet{ makeModifySet("demo"), } +var testAssignImage = []*mutationsv1alpha1.AssignImage{ + makeAssignImage("demo"), +} + var testAssign = []*mutationsv1alpha1.Assign{ makeAssign("demo"), } @@ -105,6 +109,21 @@ func makeModifySet(name string) *mutationsv1alpha1.ModifySet { } } +func makeAssignImage(name string) *mutationsv1alpha1.AssignImage { + return &mutationsv1alpha1.AssignImage{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "mutations.gatekeeper.sh/v1alpha1", + Kind: "Assign", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: mutationsv1alpha1.AssignImageSpec{ + Location: "spec.containers[name:*].image", + }, + } +} + func makeAssign(name string) *mutationsv1alpha1.Assign { return &mutationsv1alpha1.Assign{ TypeMeta: metav1.TypeMeta{ diff --git a/test/bats/test.bats b/test/bats/test.bats index f8dbe3f867a..40cec166c0a 100644 --- a/test/bats/test.bats +++ b/test/bats/test.bats @@ -53,7 +53,7 @@ teardown_file() { wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assign.mutations.gatekeeper.sh" wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assignmetadata.mutations.gatekeeper.sh" wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/modifyset.mutations.gatekeeper.sh" - wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assignimages.mutations.gatekeeper.sh" + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl wait --for condition=established --timeout=60s crd/assignimage.mutations.gatekeeper.sh" } @test "waiting for validating webhook" { From f02daed744219231eeffb506c8809f71439d8372 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 6 Dec 2022 20:40:57 -0800 Subject: [PATCH 06/29] mutator pod status working for assignimage Signed-off-by: davis-haba --- Makefile | 4 +- .../mutatorstatus/mutatorstatus_controller.go | 31 ++----- pkg/readiness/ready_tracker_test.go | 82 ++++++++++--------- pkg/readiness/testdata/99-assignimage.yaml | 20 ----- test/bats/test.bats | 3 +- test/bats/tests/mutations/nginx_pod.yaml | 2 +- 6 files changed, 55 insertions(+), 87 deletions(-) delete mode 100644 pkg/readiness/testdata/99-assignimage.yaml diff --git a/Makefile b/Makefile index 2664caa82e8..ef73df88b15 100644 --- a/Makefile +++ b/Makefile @@ -101,9 +101,7 @@ all: lint test manager # Run tests native-test: - # GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race -bench . -coverprofile cover.out - # TODO comment above back in - GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race . -coverprofile cover.out + GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race -bench . -coverprofile cover.out # Hook to run docker tests .PHONY: test diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index 8e67becc93c..4f0f744401a 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -34,7 +34,6 @@ import ( "github.com/open-policy-agent/gatekeeper/pkg/readiness" "github.com/open-policy-agent/gatekeeper/pkg/util" "github.com/open-policy-agent/gatekeeper/pkg/watch" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -80,8 +79,8 @@ func (a *Adder) Add(mgr manager.Manager) error { // newReconciler returns a new reconcile.Reconciler. func newReconciler( - mgr manager.Manager, - cs *watch.ControllerSwitch, + mgr manager.Manager, + cs *watch.ControllerSwitch, ) reconcile.Reconciler { return &ReconcileMutatorStatus{ // Separate reader and writer because manager's default client bypasses the cache for unstructured resources. @@ -125,7 +124,12 @@ func PodStatusToMutatorMapper(selfOnly bool, kindMatch string, packerMap handler } } u := &unstructured.Unstructured{} - u.SetGroupVersionKind(schema.GroupVersionKind{Group: v1beta1.MutationsGroup, Version: "v1", Kind: kind}) + // AssignImage is the only mutator in v1alpha1 still + v := "v1" + if kind == "AssignImage" { + v = "v1alpha1" + } + u.SetGroupVersionKind(schema.GroupVersionKind{Group: v1beta1.MutationsGroup, Version: v, Kind: kind}) u.SetName(name) return packerMap(u) } @@ -205,20 +209,6 @@ func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcil } gvk, unpackedRequest, err := util.UnpackRequest(request) - - // TODO rm debug - fmt.Printf("Reconcile() gvk: %s\n", gvk) - wrongSchema := schema.GroupVersionKind{ - Group: "mutations.gatekeeper.sh", - Version: "v1", - Kind: "AssignImage", - } - if gvk == wrongSchema { - errStr := fmt.Sprintf("Reconcile got wrong GVK for AssignImage: %s, wanted version to be v1alpha1", gvk) - panic(errStr) - } - // END DEBUG - if err != nil { // Unrecoverable, do not retry. log.Error(err, "unpacking request", "request", request) @@ -235,11 +225,6 @@ func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcil instance := &unstructured.Unstructured{} instance.SetGroupVersionKind(gvk) if err := r.reader.Get(ctx, unpackedRequest.NamespacedName, instance); err != nil { - // If the mutator does not exist, we are done - if errors.IsNotFound(err) { - return reconcile.Result{}, nil - } - // TODO rm debug fmt.Printf("Reconcile problem getting resource, err: %s\n", err) // END DEBUG return reconcile.Result{}, err diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go index 325cbabcf5d..0ac988b0bb7 100644 --- a/pkg/readiness/ready_tracker_test.go +++ b/pkg/readiness/ready_tracker_test.go @@ -224,45 +224,45 @@ func Test_ModifySet(t *testing.T) { } } -//func Test_AssignImage(t *testing.T) { -// g := gomega.NewWithT(t) -// -// testutils.Setenv(t, "POD_NAME", "no-pod") -// -// // Apply fixtures *before* the controllers are set up. -// err := applyFixtures("testdata") -// if err != nil { -// t.Fatalf("applying fixtures: %v", err) -// } -// -// // Wire up the rest. -// mgr, wm := setupManager(t) -// opaClient := setupOpa(t) -// -// mutationSystem := mutation.NewSystem(mutation.SystemOpts{}) -// -// if err := setupController(mgr, wm, opaClient, mutationSystem, nil); err != nil { -// t.Fatalf("setupControllers: %v", err) -// } -// -// ctx := context.Background() -// testutils.StartManager(ctx, t, mgr) -// -// g.Eventually(func() (bool, error) { -// ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) -// defer cancel() -// return probeIsReady(ctx) -// }, 20*time.Second, 1*time.Second).Should(gomega.BeTrue()) -// -// // Verify that the AssignImage is present in the cache -// for _, am := range testAssignImage { -// id := mutationtypes.MakeID(am) -// expectedMutator := mutationSystem.Get(id) -// if expectedMutator == nil { -// t.Fatal("want expectedMutator != nil but got nil") -// } -// } -//} +func Test_AssignImage(t *testing.T) { + g := gomega.NewWithT(t) + + testutils.Setenv(t, "POD_NAME", "no-pod") + + // Apply fixtures *before* the controllers are set up. + err := applyFixtures("testdata") + if err != nil { + t.Fatalf("applying fixtures: %v", err) + } + + // Wire up the rest. + mgr, wm := setupManager(t) + opaClient := setupOpa(t) + + mutationSystem := mutation.NewSystem(mutation.SystemOpts{}) + + if err := setupController(mgr, wm, opaClient, mutationSystem, nil); err != nil { + t.Fatalf("setupControllers: %v", err) + } + + ctx := context.Background() + testutils.StartManager(ctx, t, mgr) + + g.Eventually(func() (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + return probeIsReady(ctx) + }, 20*time.Second, 1*time.Second).Should(gomega.BeTrue()) + + // Verify that the AssignImage is present in the cache + for _, am := range testAssignImage { + id := mutationtypes.MakeID(am) + expectedMutator := mutationSystem.Get(id) + if expectedMutator == nil { + t.Fatal("want expectedMutator != nil but got nil") + } + } +} func Test_Assign(t *testing.T) { g := gomega.NewWithT(t) @@ -621,6 +621,10 @@ func probeIsReady(ctx context.Context) (bool, error) { return false, err } + // TODO rm debug + fmt.Printf("probeIsReady() got resp code %d", resp.StatusCode) + // END DEBUG + return resp.StatusCode >= 200 && resp.StatusCode < 400, nil } diff --git a/pkg/readiness/testdata/99-assignimage.yaml b/pkg/readiness/testdata/99-assignimage.yaml deleted file mode 100644 index abb62e84478..00000000000 --- a/pkg/readiness/testdata/99-assignimage.yaml +++ /dev/null @@ -1,20 +0,0 @@ -apiVersion: mutations.gatekeeper.sh/v1alpha1 -kind: AssignImage -metadata: - name: demo -spec: - applyTo: - - groups: [ "" ] - kinds: [ "Pod" ] - versions: [ "v1" ] - location: "spec.containers[name:*].image" - parameters: - assignDomain: "barcorp.org" - assignTag: ":latest" - assignPath: "newpath/newrepo" - match: - source: "All" - scope: Namespaced - kinds: - - apiGroups: [ "*" ] - kinds: [ "Pod" ] diff --git a/test/bats/test.bats b/test/bats/test.bats index 40cec166c0a..0511238c6d7 100644 --- a/test/bats/test.bats +++ b/test/bats/test.bats @@ -87,7 +87,8 @@ teardown_file() { kubectl apply -f ${BATS_TESTS_DIR}/mutations/assign_image.yaml wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "mutator_enforced AssignImage add-domain-digest" wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl apply -f ${BATS_TESTS_DIR}/mutations/nginx_pod.yaml" - run kubectl get svc mutate-svc -o jsonpath="{.spec.containers[name:test].image}" + run kubectl get pod nginx-test-pod -o jsonpath="{.spec.containers[0].image}" + echo "got output ${output}" assert_equal "foocorp.org/nginx@sha256:abcde67890123456789abc345678901a" "${output}" kubectl delete --ignore-not-found svc mutate-svc diff --git a/test/bats/tests/mutations/nginx_pod.yaml b/test/bats/tests/mutations/nginx_pod.yaml index b9b346392dc..84c59e28715 100644 --- a/test/bats/tests/mutations/nginx_pod.yaml +++ b/test/bats/tests/mutations/nginx_pod.yaml @@ -1,7 +1,7 @@ apiVersion: v1 kind: Pod metadata: - name: nginx-test + name: nginx-test-pod labels: role: myrole spec: From 6eada2f29282076ef428196cf532df8c7b1bc80c Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 7 Dec 2022 02:45:03 -0800 Subject: [PATCH 07/29] e2e test assignimage mutator deleted Signed-off-by: davis-haba --- .../mutators/assignimage/assignimage_mutator.go | 2 +- test/bats/test.bats | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator.go b/pkg/mutation/mutators/assignimage/assignimage_mutator.go index 313d08dd461..5ae889ed467 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator.go @@ -116,7 +116,7 @@ func (m *Mutator) String() string { // the given assignImage instance. func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Mutator, error) { // This is not always set by the kubernetes API server - assignImage.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "Assign"}) + assignImage.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignImage"}) path, err := parser.Parse(assignImage.Spec.Location) if err != nil { diff --git a/test/bats/test.bats b/test/bats/test.bats index 0511238c6d7..f3e3a05dd6e 100644 --- a/test/bats/test.bats +++ b/test/bats/test.bats @@ -84,17 +84,25 @@ teardown_file() { run kubectl get svc mutate-svc -o jsonpath="{.metadata.annotations.gatekeeper\.sh\/mutations}" assert_equal 'Assign//k8sexternalip:1' "${output}" + # Test AssignImage kubectl apply -f ${BATS_TESTS_DIR}/mutations/assign_image.yaml wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "mutator_enforced AssignImage add-domain-digest" wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl apply -f ${BATS_TESTS_DIR}/mutations/nginx_pod.yaml" run kubectl get pod nginx-test-pod -o jsonpath="{.spec.containers[0].image}" - echo "got output ${output}" assert_equal "foocorp.org/nginx@sha256:abcde67890123456789abc345678901a" "${output}" + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl delete pod nginx-test-pod" + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl delete assignimage add-domain-digest" + + # Test removing the AssignImage does not apply mutation + wait_for_process ${WAIT_TIME} ${SLEEP_TIME} "kubectl apply -f ${BATS_TESTS_DIR}/mutations/nginx_pod.yaml" + run kubectl get pod nginx-test-pod -o jsonpath="{.spec.containers[0].image}" + assert_equal "nginx:latest" "${output}" kubectl delete --ignore-not-found svc mutate-svc kubectl delete --ignore-not-found assignmetadata k8sownerlabel kubectl delete --ignore-not-found assign k8sexternalip kubectl delete --ignore-not-found assignimage add-domain-digest + kubectl delete --ignore-not-found pod nginx-test-pod } @test "applying sync config" { @@ -171,7 +179,7 @@ teardown_file() { } @test "waiting for namespaces to be synced using metrics endpoint" { - kubectl run temp --image=curlimages/curl -- tail -f /dev/null + kubectl run --generator=run-pod/v1 temp --image=curlimages/curl -- tail -f /dev/null kubectl wait --for=condition=Ready --timeout=60s pod temp num_namespaces=$(kubectl get ns -o json | jq '.items | length') From f586335501b0dce48cf067e5e0e92698791bbfab Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 7 Dec 2022 03:01:28 -0800 Subject: [PATCH 08/29] old kubectl run Signed-off-by: davis-haba --- Makefile | 2 +- .../mutatorstatus/mutatorstatus_controller.go | 2 -- pkg/readiness/ready_tracker_test.go | 4 ---- test/bats/test.bats | 2 +- .../input/assignimage_mutator.yaml | 18 ++++++++++++++++++ .../basic-expansion/output/output.yaml | 2 +- 6 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml diff --git a/Makefile b/Makefile index ef73df88b15..f335bb371fa 100644 --- a/Makefile +++ b/Makefile @@ -101,7 +101,7 @@ all: lint test manager # Run tests native-test: - GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race -bench . -coverprofile cover.out + GO111MODULE=on go test -mod vendor ./pkg/... ./apis/... -race -bench . -coverprofile cover.out # Hook to run docker tests .PHONY: test diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index 4f0f744401a..8fafd976b6e 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -225,8 +225,6 @@ func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcil instance := &unstructured.Unstructured{} instance.SetGroupVersionKind(gvk) if err := r.reader.Get(ctx, unpackedRequest.NamespacedName, instance); err != nil { - fmt.Printf("Reconcile problem getting resource, err: %s\n", err) - // END DEBUG return reconcile.Result{}, err } diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go index 0ac988b0bb7..9145bc63456 100644 --- a/pkg/readiness/ready_tracker_test.go +++ b/pkg/readiness/ready_tracker_test.go @@ -621,10 +621,6 @@ func probeIsReady(ctx context.Context) (bool, error) { return false, err } - // TODO rm debug - fmt.Printf("probeIsReady() got resp code %d", resp.StatusCode) - // END DEBUG - return resp.StatusCode >= 200 && resp.StatusCode < 400, nil } diff --git a/test/bats/test.bats b/test/bats/test.bats index f3e3a05dd6e..86a31b4f3d1 100644 --- a/test/bats/test.bats +++ b/test/bats/test.bats @@ -179,7 +179,7 @@ teardown_file() { } @test "waiting for namespaces to be synced using metrics endpoint" { - kubectl run --generator=run-pod/v1 temp --image=curlimages/curl -- tail -f /dev/null + kubectl run temp --image=curlimages/curl -- tail -f /dev/null kubectl wait --for=condition=Ready --timeout=60s pod temp num_namespaces=$(kubectl get ns -o json | jq '.items | length') diff --git a/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml b/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml new file mode 100644 index 00000000000..161cced9f2e --- /dev/null +++ b/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml @@ -0,0 +1,18 @@ +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: AssignImage +metadata: + name: add-tag-latest +spec: + applyTo: + - groups: [ "" ] + kinds: [ "Pod" ] + versions: [ "v1" ] + location: "spec.containers[name:*].image" + parameters: + assignTag: "latest" + match: + source: "All" + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" \ No newline at end of file diff --git a/test/gator/expand/fixtures/basic-expansion/output/output.yaml b/test/gator/expand/fixtures/basic-expansion/output/output.yaml index 676c973a671..6ed41c65b60 100644 --- a/test/gator/expand/fixtures/basic-expansion/output/output.yaml +++ b/test/gator/expand/fixtures/basic-expansion/output/output.yaml @@ -9,7 +9,7 @@ spec: containers: - args: - /bin/sh - image: nginx:1.14.2 + image: nginx:latest imagePullPolicy: Always name: nginx ports: From a3e64d12f304f447c52390cb13b3d7c6869a66bd Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 7 Dec 2022 22:29:48 -0800 Subject: [PATCH 09/29] address comments. domain must have '.' unless localhost Signed-off-by: davis-haba --- .../assignimage/assignimage_mutator.go | 2 +- .../mutators/assignimage/imageparser.go | 39 ++++++------ .../mutators/assignimage/imageparser_test.go | 59 +++++++++++++++++-- test/bats/tests/mutations/assign_image.yaml | 2 +- test/bats/tests/mutations/nginx_pod.yaml | 2 +- 5 files changed, 80 insertions(+), 24 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator.go b/pkg/mutation/mutators/assignimage/assignimage_mutator.go index 5ae889ed467..037800d1c14 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator.go @@ -128,7 +128,7 @@ func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Muta } if hasListTerminal(path) { - return nil, fmt.Errorf("assignImage %s can't change a list without a key", assignImage.GetName()) + return nil, fmt.Errorf("assignImage %s cannot mutate list-type fields", assignImage.GetName()) } err = core.CheckKeyNotChanged(path) diff --git a/pkg/mutation/mutators/assignimage/imageparser.go b/pkg/mutation/mutators/assignimage/imageparser.go index 3ea3ffab4ed..8406e7815cc 100644 --- a/pkg/mutation/mutators/assignimage/imageparser.go +++ b/pkg/mutation/mutators/assignimage/imageparser.go @@ -6,18 +6,17 @@ import ( "strings" ) -type image struct { - domain string - path string - tag string -} - var ( - // domainRegexp defines a schema for a domain component. Primarily, this is to - // avoid the possibility of injecting `/` tokens, which could cause part of - // the domain component to "leak" over to the location component, ultimately - // causing non convergence. - domainRegexp = regexp.MustCompile(`^\w[\w.\-_]*(:\d+)?$`) + // We perform validation on the components of an image string to ensure that + // the user cannot define a mutator which does not converge. This would + // otherwise be possible by injecting tokens we use to split an image string, + // [@:/], into components that would cause that component to be split the next + // time the mutation is applied and "leak" to its neighbor. Some validation is + // done as regex on individual components, and other validation which looks at + // multiple components together is done in code. + + //domainRegexp defines a schema for a domain component. + domainRegexp = regexp.MustCompile(`(^\w[\w\-_]*\.[\w\-_\.]*[\w](:\d+)?$)|(^localhost(:\d+)?$)`) // pathRegexp defines a schema for a location component. It follows the convention // specified in the docker distribution reference. The regex restricts @@ -30,6 +29,12 @@ var ( tagRegexp = regexp.MustCompile(`(^:[\w][\w.-]{0,127}$)|(^@[A-Za-z][A-Za-z0-9]*([-_+.][A-Za-z][A-Za-z0-9]*)*[:][0-9A-Fa-f]{32,}$)`) ) +type image struct { + domain string + path string + tag string +} + func mutateImage(domain, path, tag, mutableImgRef string) string { oldImg := newImage(mutableImgRef) newImg := oldImg.newMutatedImage(domain, path, tag) @@ -87,26 +92,26 @@ func validateImageParts(domain, path, tag string) error { return fmt.Errorf("at least one of [assignDomain, assignPath, assignTag] must be set") } if domain != "" && !domainRegexp.MatchString(domain) { - return fmt.Errorf("assignDomain %q does not match pattern %q", domain, domainRegexp.String()) + return fmt.Errorf("assignDomain %q does not match pattern %s", domain, domainRegexp.String()) } // match the whole string for path (anchoring with `$` is tricky here) if path != "" && path != pathRegexp.FindString(path) { - return fmt.Errorf("assignPath %q does not match pattern %q", path, pathRegexp.String()) + return fmt.Errorf("assignPath %q does not match pattern %s", path, pathRegexp.String()) } if tag != "" && !tagRegexp.MatchString(tag) { - return fmt.Errorf("assignTag %q does not match pattern %q", tag, tagRegexp.String()) + return fmt.Errorf("assignTag %q does not match pattern %s", tag, tagRegexp.String()) } // Check if the path looks like a domain string, and the domain is not set. // This prevents part of the path field from "leaking" to the domain, causing - // nonconvergent behavior. + // non convergent behavior. // For example, suppose: domain="", path="gcr.io/repo", tag="" // Suppose no value is currently set on the mutable, so the result is // just "gcr.io/repo". When this value mutated again, "gcr.io" is parsed into - // the domain component, so the result would be "gcr.io/gcr.io/repo". + // the domain component, so the result would be "gcr.io/gcr.io/repo" and so on. if domain == "" { if d, _ := splitDomain(path); d != "" { - return fmt.Errorf("assignDomain must be set if assignPath %q can be interpretted as part of a domain", path) + return fmt.Errorf("assignDomain must be set if the first part of assignPath %q can be interpretted as part of a domain", path) } } diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index 843fec9335c..df31653fb79 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -196,8 +196,39 @@ func TestValidateImageParts(t *testing.T) { domain: "a.b.c:123", }, { - name: "valid domain with _", - domain: "a_b_c:123", + name: "invalid domain no .", + domain: "foobar", + wantErr: true, + }, + { + name: "invalid domain leading .", + domain: ".foobar", + wantErr: true, + }, + { + name: "invalid domain trailing .", + domain: "foobar.", + wantErr: true, + }, + { + name: "invalid domain . before port", + domain: "foobar.:5000", + wantErr: true, + }, + { + name: "invalid domain / before port", + domain: "foobar/:5000", + wantErr: true, + }, + { + name: "invalid domain leading and trailing .", + domain: ".foobar.", + wantErr: true, + }, + { + name: "invalid domain with _ and port but no .", + domain: "a_b_c:123", + wantErr: true, }, { name: "invalid domain with leading /", @@ -341,6 +372,26 @@ func TestValidateImageParts(t *testing.T) { tag: ":latest/", wantErr: true, }, + { + name: "invalid tag trailing :", + tag: ":latest:", + wantErr: true, + }, + { + name: "invalid tag trailing @", + tag: ":latest@", + wantErr: true, + }, + { + name: "invalid tag : inside", + tag: ":lat:est", + wantErr: true, + }, + { + name: "invalid tag @ inside", + tag: "@sha256:12345678901234567890123456789012@sha256:12345678901234567890123456789012", + wantErr: true, + }, { name: "invalid tag double :", tag: "::latest", @@ -362,10 +413,10 @@ func TestValidateImageParts(t *testing.T) { t.Run(tc.name, func(t *testing.T) { err := validateImageParts(tc.domain, tc.path, tc.tag) if !tc.wantErr && err != nil { - t.Errorf("did not want error but got: %v", err) + t.Errorf("(domain=%s, path=%s, tag=%s) did not want error but got: %v", tc.domain, tc.path, tc.tag, err) } if tc.wantErr && err == nil { - t.Error("wanted error but got nil") + t.Errorf("(domain=%s, path=%s, tag=%s) wanted error but got nil", tc.domain, tc.path, tc.tag) } }) } diff --git a/test/bats/tests/mutations/assign_image.yaml b/test/bats/tests/mutations/assign_image.yaml index f54f2fa359b..c2d2af4ef23 100644 --- a/test/bats/tests/mutations/assign_image.yaml +++ b/test/bats/tests/mutations/assign_image.yaml @@ -16,4 +16,4 @@ spec: scope: Namespaced kinds: - apiGroups: [ "*" ] - kinds: [ "Pod" ] \ No newline at end of file + kinds: [ "Pod" ] diff --git a/test/bats/tests/mutations/nginx_pod.yaml b/test/bats/tests/mutations/nginx_pod.yaml index 84c59e28715..b90135fdc97 100644 --- a/test/bats/tests/mutations/nginx_pod.yaml +++ b/test/bats/tests/mutations/nginx_pod.yaml @@ -7,4 +7,4 @@ metadata: spec: containers: - name: test - image: nginx:latest \ No newline at end of file + image: nginx:latest From 2eac13bf453b05734f8e1429803453e63e069d4a Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 7 Dec 2022 23:01:13 -0800 Subject: [PATCH 10/29] appease linter Signed-off-by: davis-haba --- pkg/mutation/mutators/assignimage/imageparser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/mutation/mutators/assignimage/imageparser.go b/pkg/mutation/mutators/assignimage/imageparser.go index 8406e7815cc..e5f31fff943 100644 --- a/pkg/mutation/mutators/assignimage/imageparser.go +++ b/pkg/mutation/mutators/assignimage/imageparser.go @@ -15,7 +15,7 @@ var ( // done as regex on individual components, and other validation which looks at // multiple components together is done in code. - //domainRegexp defines a schema for a domain component. + // domainRegexp defines a schema for a domain component. domainRegexp = regexp.MustCompile(`(^\w[\w\-_]*\.[\w\-_\.]*[\w](:\d+)?$)|(^localhost(:\d+)?$)`) // pathRegexp defines a schema for a location component. It follows the convention From b9499ab8c037681d6cd6c162af379b5e0eed0ccc Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 7 Dec 2022 23:10:34 -0800 Subject: [PATCH 11/29] fix gator tests Signed-off-by: davis-haba --- .../fixtures/basic-expansion/input/assignimage_mutator.yaml | 4 ++-- test/gator/expand/test.bats | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml b/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml index 161cced9f2e..8043daf3485 100644 --- a/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml +++ b/test/gator/expand/fixtures/basic-expansion/input/assignimage_mutator.yaml @@ -9,10 +9,10 @@ spec: versions: [ "v1" ] location: "spec.containers[name:*].image" parameters: - assignTag: "latest" + assignTag: ":latest" match: source: "All" scope: Namespaced kinds: - apiGroups: [ "*" ] - kinds: [ "Pod" \ No newline at end of file + kinds: [ "Pod" ] diff --git a/test/gator/expand/test.bats b/test/gator/expand/test.bats index 363815578f1..a26e3cd3d9f 100644 --- a/test/gator/expand/test.bats +++ b/test/gator/expand/test.bats @@ -12,9 +12,9 @@ match_yaml_in_dir () { want=$(cat "${BATS_TEST_DIRNAME}"/fixtures/"${match_dir}"/output/output.yaml) if [[ ${yaml_output} != *"$want"* ]]; then - printf "ERROR: resource not found in output\n" - printf "WANT:\n%s\n\n" "$want" - printf "GOT:\n%s\n" "$yaml_output" + echo "ERROR: resource not found in output" + echo "WANT: ${want}" + echo "GOT: ${yaml_output}" echo "DIFF: " diff <( echo "$want" ) <( echo "$yaml_output" ) exit 1 From b5521ff85cbf1600399ba9d86dd4dffec96c72f6 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 7 Dec 2022 23:29:17 -0800 Subject: [PATCH 12/29] add test domain ending in colon still converges Signed-off-by: davis-haba --- .../mutators/assignimage/assignimage_mutator_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go index 94f792dacf5..ca838d22c83 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -221,6 +221,17 @@ func TestMutate(t *testing.T) { obj: newPod("a.b.c:5000//not.good:ABC123_//lib.com.repo//localhost@blah101", "foo"), fn: podTest("myreg.io//not.good:ABC123_//lib.com.repo//localhost@blah101"), }, + { + name: "mutate path and tag colon in imageref's domain still converges", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + path: "repo/app", + tag: ":latest", + }, + obj: newPod("a.b.c:/not.good:ABC123_//lib.com.repo//localhost@blah101", "foo"), + fn: podTest("a.b.c:/repo/app:latest"), + }, { name: "mutate path to domain-like string with domain set", cfg: &aiTestConfig{ From 7bb472f6b667bb6fff292e703c1ddc64f84a6111 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Thu, 8 Dec 2022 00:20:01 -0800 Subject: [PATCH 13/29] docs for assignimage Signed-off-by: davis-haba --- website/docs/mutation.md | 62 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/website/docs/mutation.md b/website/docs/mutation.md index 9012402c99a..4f353a2cf68 100644 --- a/website/docs/mutation.md +++ b/website/docs/mutation.md @@ -13,6 +13,7 @@ Mutation policies are defined using mutation-specific CRDs, called __mutators__: - AssignMetadata - defines changes to the metadata section of a resource - Assign - any change outside the metadata section - ModifySet - adds or removes entries from a list, such as the arguments to a container +- AssignImage - defines changes to the components of an image string The rules for mutating metadata are more strict than for mutating the rest of the resource. The differences are described in more detail below. @@ -214,6 +215,44 @@ spec: - `operation` can be `merge` to insert values into the list if missing, or `prune` to remove values from the list. `merge` is default. +### AssignImage + +AssignImage is a mutator specifically for changing the components of an image +string. Suppose you have an image like `my.registry.io:2000/repo/app:latest`. +`my.registry.io:2000` would be the domain, `repo/app` would be the path, and +`:latest` would be the tag. The domain, path, and tag of an image can be changed +separately or in conjunction. + +For example, to change the whole image to `my.registry.io/repo/app@sha256:abcde67890123456789abc345678901a`: + +```yaml +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: AssignImage +metadata: + name: assign-container-image +spec: + applyTo: + - groups: [ "" ] + kinds: [ "Pod" ] + versions: [ "v1" ] + location: "spec.containers[name:*].image" + parameters: + assignDomain: "my.registry.io" + assignPath: "repo/app" + assignTag: "@sha256:abcde67890123456789abc345678901a" + match: + source: "All" + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" ] +``` + +Only one of `[assignDomain, assignPath, assignTag]` is required. Note that `assignTag` +must start with `:` or `@`. Also, if `assignPath` is set to a value which could potentially +be interpreted as a domain, such as `my.repo.lib/app`, then `assignDomain` must +also be specified. + ## Examples ### Adding an annotation @@ -354,6 +393,29 @@ spec: - 1.2.3.4 ``` +### Setting a Pod's container image to use a specific digest: + +```yaml +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: AssignImage +metadata: + name: add-nginx-digest +spec: + applyTo: + - groups: [ "" ] + kinds: [ "Pod" ] + versions: [ "v1" ] + location: "spec.containers[name:nginx].image" + parameters: + assignTag: "@sha256:abcde67890123456789abc345678901a" + match: + source: "All" + scope: Namespaced + kinds: + - apiGroups: [ "*" ] + kinds: [ "Pod" ] +``` + ### External Data See [External Data For Gatekeeper Mutating Webhook](externaldata.md#external-data-for-gatekeeper-mutating-webhook). From fc2b112144a3eded6cb2cc899355b8e7e8e57357 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Thu, 8 Dec 2022 00:21:19 -0800 Subject: [PATCH 14/29] remove newline Signed-off-by: davis-haba --- website/docs/mutation.md | 1 - 1 file changed, 1 deletion(-) diff --git a/website/docs/mutation.md b/website/docs/mutation.md index 4f353a2cf68..f2cf0e49398 100644 --- a/website/docs/mutation.md +++ b/website/docs/mutation.md @@ -214,7 +214,6 @@ spec: - `spec.parameters.values.fromList` holds the list of values that will be added or removed. - `operation` can be `merge` to insert values into the list if missing, or `prune` to remove values from the list. `merge` is default. - ### AssignImage AssignImage is a mutator specifically for changing the components of an image From fa11b39b4cab4c40a360b731e01f195f15353823 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 13 Dec 2022 17:01:22 -0800 Subject: [PATCH 15/29] address comments Signed-off-by: davis-haba --- .../mutatorstatus/mutatorstatus_controller.go | 5 + .../mutators/assignimage/imageparser_test.go | 294 ++++++++++-------- 2 files changed, 172 insertions(+), 127 deletions(-) diff --git a/pkg/controller/mutatorstatus/mutatorstatus_controller.go b/pkg/controller/mutatorstatus/mutatorstatus_controller.go index 8fafd976b6e..dcda104aaca 100644 --- a/pkg/controller/mutatorstatus/mutatorstatus_controller.go +++ b/pkg/controller/mutatorstatus/mutatorstatus_controller.go @@ -34,6 +34,7 @@ import ( "github.com/open-policy-agent/gatekeeper/pkg/readiness" "github.com/open-policy-agent/gatekeeper/pkg/util" "github.com/open-policy-agent/gatekeeper/pkg/watch" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -225,6 +226,10 @@ func (r *ReconcileMutatorStatus) Reconcile(ctx context.Context, request reconcil instance := &unstructured.Unstructured{} instance.SetGroupVersionKind(gvk) if err := r.reader.Get(ctx, unpackedRequest.NamespacedName, instance); err != nil { + // If the mutator does not exist, we are done + if errors.IsNotFound(err) { + return reconcile.Result{}, nil + } return reconcile.Result{}, err } diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index df31653fb79..198ec7f074c 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -1,6 +1,8 @@ package assignimage import ( + "fmt" + "strings" "testing" ) @@ -157,13 +159,43 @@ func TestNewImage(t *testing.T) { } } +func isDomainError(domain string) func(error) bool { + return func(err error) bool { + return strings.Contains(err.Error(), fmt.Sprintf("assignDomain %q does not match pattern", domain)) + } +} + +func isPathError(path string) func(error) bool { + return func(err error) bool { + return strings.Contains(err.Error(), fmt.Sprintf("assignPath %q does not match pattern", path)) + } +} + +func isTagError(tag string) func(error) bool { + return func(err error) bool { + return strings.Contains(err.Error(), fmt.Sprintf("assignTag %q does not match pattern", tag)) + } +} + +func isEmptyArgsError() func(error) bool { + return func(err error) bool { + return strings.Contains(err.Error(), "at least one of") + } +} + +func isPathlikeDomainError() func(error) bool { + return func(err error) bool { + return strings.Contains(err.Error(), "assignDomain must be set if") + } +} + func TestValidateImageParts(t *testing.T) { tests := []struct { - name string - domain string - path string - tag string - wantErr bool + name string + domain string + path string + tag string + errFn func(error) bool }{ { name: "all valid components", @@ -172,8 +204,8 @@ func TestValidateImageParts(t *testing.T) { tag: ":latest", }, { - name: "no fields set returns err", - wantErr: true, + name: "no fields set returns err", + errFn: isEmptyArgsError(), }, { name: "valid domain with port", @@ -196,84 +228,84 @@ func TestValidateImageParts(t *testing.T) { domain: "a.b.c:123", }, { - name: "invalid domain no .", - domain: "foobar", - wantErr: true, + name: "invalid domain no .", + domain: "foobar", + errFn: isDomainError("foobar"), }, { - name: "invalid domain leading .", - domain: ".foobar", - wantErr: true, + name: "invalid domain leading .", + domain: ".foobar", + errFn: isDomainError(".foobar"), }, { - name: "invalid domain trailing .", - domain: "foobar.", - wantErr: true, + name: "invalid domain trailing .", + domain: "foobar.", + errFn: isDomainError("foobar."), }, { - name: "invalid domain . before port", - domain: "foobar.:5000", - wantErr: true, + name: "invalid domain . before port", + domain: "foobar.:5000", + errFn: isDomainError("foobar.:5000"), }, { - name: "invalid domain / before port", - domain: "foobar/:5000", - wantErr: true, + name: "invalid domain / before port", + domain: "foobar/:5000", + errFn: isDomainError("foobar/:5000"), }, { - name: "invalid domain leading and trailing .", - domain: ".foobar.", - wantErr: true, + name: "invalid domain leading and trailing .", + domain: ".foobar.", + errFn: isDomainError(".foobar."), }, { - name: "invalid domain with _ and port but no .", - domain: "a_b_c:123", - wantErr: true, + name: "invalid domain with _ and port but no .", + domain: "a_b_c:123", + errFn: isDomainError("a_b_c:123"), }, { - name: "invalid domain with leading /", - domain: "/not.ok.io:2000", - wantErr: true, + name: "invalid domain with leading /", + domain: "/not.ok.io:2000", + errFn: isDomainError("/not.ok.io:2000"), }, { - name: "invalid domain with trailing /", - domain: "not.ok.io:2000/", - wantErr: true, + name: "invalid domain with trailing /", + domain: "not.ok.io:2000/", + errFn: isDomainError("not.ok.io:2000/"), }, { - name: "invalid domain with middle /", - domain: "not/ok/io", - wantErr: true, + name: "invalid domain with middle /", + domain: "not/ok/io", + errFn: isDomainError("not/ok/io"), }, { - name: "invalid domain port start with alpha", - domain: "my.reg.io:abc2000", - wantErr: true, + name: "invalid domain port start with alpha", + domain: "my.reg.io:abc2000", + errFn: isDomainError("my.reg.io:abc2000"), }, { - name: "invalid domain with multiple :", - domain: "my.reg.io:2000:", - wantErr: true, + name: "invalid domain with multiple :", + domain: "my.reg.io:2000:", + errFn: isDomainError("my.reg.io:2000:"), }, { - name: "invalid domain with repeat :", - domain: "my.reg.io::2000", - wantErr: true, + name: "invalid domain with repeat :", + domain: "my.reg.io::2000", + errFn: isDomainError("my.reg.io::2000"), }, { - name: "invalid domain with tag", - domain: "my.reg.io:latest", - wantErr: true, + name: "invalid domain with tag", + domain: "my.reg.io:latest", + errFn: isDomainError("my.reg.io:latest"), }, { - name: "invalid domain with digest", - domain: "my.reg.io@sha256:abcde123456789", - wantErr: true, + name: "invalid domain with digest", + domain: "my.reg.io@sha256:abcde123456789", + errFn: isDomainError("my.reg.io@sha256:abcde123456789"), }, { - name: "invalid domain with bad character", - domain: ";!234.com", - wantErr: true, + name: "invalid domain with bad character", + domain: ";!234.com", + errFn: isDomainError(";!234.com"), }, { name: "valid path", @@ -285,54 +317,54 @@ func TestValidateImageParts(t *testing.T) { path: "a.b.c/stuff", }, { - name: "domain-like path without domain returns err", - path: "a.b.c/stuff", - tag: ":latest", - wantErr: true, + name: "domain-like path without domain returns err", + path: "a.b.c/stuff", + tag: ":latest", + errFn: isPathlikeDomainError(), }, { name: "valid path . and -", path: "lib/stuff-app__thing/a/b--c/e", }, { - name: "invalid path ending / returns err", - path: "lib/stuff/app/", - wantErr: true, + name: "invalid path ending / returns err", + path: "lib/stuff/app/", + errFn: isPathError("lib/stuff/app/"), }, { - name: "invalid path with leading / returns err", - path: "/lib/stuff/app", - wantErr: true, + name: "invalid path with leading / returns err", + path: "/lib/stuff/app", + errFn: isPathError("/lib/stuff/app"), }, { - name: "invalid path with leading : returns err", - domain: "my.register.io:5000", - path: ":lib/stuff/app", - wantErr: true, + name: "invalid path with leading : returns err", + domain: "my.register.io:5000", + path: ":lib/stuff/app", + errFn: isPathError(":lib/stuff/app"), }, { - name: "invalid path with leading @ returns err", - domain: "my.register.io:5000", - path: "@lib/stuff/app", - wantErr: true, + name: "invalid path with leading @ returns err", + domain: "my.register.io:5000", + path: "@lib/stuff/app", + errFn: isPathError("@lib/stuff/app"), }, { - name: "invalid path with trailing : returns err", - domain: "my.register.io:5000", - path: "lib/stuff/app:", - wantErr: true, + name: "invalid path with trailing : returns err", + domain: "my.register.io:5000", + path: "lib/stuff/app:", + errFn: isPathError("lib/stuff/app:"), }, { - name: "invalid path with trailing @ returns err", - domain: "my.register.io:5000", - path: "lib/stuff/app@", - wantErr: true, + name: "invalid path with trailing @ returns err", + domain: "my.register.io:5000", + path: "lib/stuff/app@", + errFn: isPathError("lib/stuff/app@"), }, { - name: "invalid path : in middle returns err", - domain: "my.register.io:5000", - path: "lib/stuff:things/app", - wantErr: true, + name: "invalid path : in middle returns err", + domain: "my.register.io:5000", + path: "lib/stuff:things/app", + errFn: isPathError("lib/stuff:things/app"), }, { name: "test valid tag", @@ -343,80 +375,88 @@ func TestValidateImageParts(t *testing.T) { tag: "@sha256:12345678901234567890123456789012", }, { - name: "invalid tag no leading :", - tag: "latest", - wantErr: true, + name: "invalid tag no leading :", + tag: "latest", + errFn: isTagError("latest"), }, { - name: "invalid digest no leading @", - tag: "sha256:12345678901234567890123456789012", - wantErr: true, + name: "invalid digest no leading @", + tag: "sha256:12345678901234567890123456789012", + errFn: isTagError("sha256:12345678901234567890123456789012"), }, { - name: "invalid digest hash too short", - tag: "@sha256:123456", - wantErr: true, + name: "invalid digest hash too short", + tag: "@sha256:123456", + errFn: isTagError("@sha256:123456"), }, { - name: "invalid digest not base 16", - tag: "@sha256:1XYZ5678901234567890123456789012", - wantErr: true, + name: "invalid digest not base 16", + tag: "@sha256:1XYZ5678901234567890123456789012", + errFn: isTagError("@sha256:1XYZ5678901234567890123456789012"), }, { - name: "invalid tag leading /", - tag: "/:latest", - wantErr: true, + name: "invalid tag leading /", + tag: "/:latest", + errFn: isTagError("/:latest"), }, { - name: "invalid tag trailing /", - tag: ":latest/", - wantErr: true, + name: "invalid tag trailing /", + tag: ":latest/", + errFn: isTagError(":latest/"), }, { - name: "invalid tag trailing :", - tag: ":latest:", - wantErr: true, + name: "invalid tag trailing :", + tag: ":latest:", + errFn: isTagError(":latest:"), }, { - name: "invalid tag trailing @", - tag: ":latest@", - wantErr: true, + name: "invalid tag trailing @", + tag: ":latest@", + errFn: isTagError(":latest@"), }, { - name: "invalid tag : inside", - tag: ":lat:est", - wantErr: true, + name: "invalid tag : inside", + tag: ":lat:est", + errFn: isTagError(":lat:est"), }, { - name: "invalid tag @ inside", - tag: "@sha256:12345678901234567890123456789012@sha256:12345678901234567890123456789012", - wantErr: true, + name: "invalid tag @ inside", + tag: "@sha256:12345678901234567890123456789012@sha256:12345678901234567890123456789012", + errFn: isTagError("@sha256:12345678901234567890123456789012@sha256:12345678901234567890123456789012"), }, { - name: "invalid tag double :", - tag: "::latest", - wantErr: true, + name: "invalid tag double :", + tag: "::latest", + errFn: isTagError("::latest"), }, { - name: "invalid digest double @", - tag: "@@sha256:1XYZ5678901234567890123456789012", - wantErr: true, + name: "invalid digest double @", + tag: "@@sha256:1XYZ5678901234567890123456789012", + errFn: isTagError("@@sha256:1XYZ5678901234567890123456789012"), }, { - name: "invalid tag @ and :", - tag: "@:latest", - wantErr: true, + name: "invalid tag @ and :", + tag: "@:latest", + errFn: isTagError("@:latest"), }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { err := validateImageParts(tc.domain, tc.path, tc.tag) - if !tc.wantErr && err != nil { + if tc.errFn == nil && err != nil { t.Errorf("(domain=%s, path=%s, tag=%s) did not want error but got: %v", tc.domain, tc.path, tc.tag, err) } - if tc.wantErr && err == nil { - t.Errorf("(domain=%s, path=%s, tag=%s) wanted error but got nil", tc.domain, tc.path, tc.tag) + if tc.errFn != nil { + if err == nil { + t.Errorf("(domain=%s, path=%s, tag=%s) wanted error but got nil", tc.domain, tc.path, tc.tag) + } else { + if !tc.errFn(err) { + t.Errorf("got error of unexpected type: %s", err) + } + } + } + if tc.errFn != nil && err == nil { } }) } From 8cf83ab59d3553dc95d5b97873a788adeb5822ce Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 13 Dec 2022 17:16:18 -0800 Subject: [PATCH 16/29] appease linter Signed-off-by: davis-haba --- pkg/mutation/mutators/assignimage/imageparser_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index 198ec7f074c..b753822c4cf 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -450,10 +450,8 @@ func TestValidateImageParts(t *testing.T) { if tc.errFn != nil { if err == nil { t.Errorf("(domain=%s, path=%s, tag=%s) wanted error but got nil", tc.domain, tc.path, tc.tag) - } else { - if !tc.errFn(err) { - t.Errorf("got error of unexpected type: %s", err) - } + } else if !tc.errFn(err) { + t.Errorf("got error of unexpected type: %s", err) } } if tc.errFn != nil && err == nil { From f6baca6722727357e571133d29c5ce6db21fbef5 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 13 Dec 2022 17:38:04 -0800 Subject: [PATCH 17/29] cleanup dead code branch Signed-off-by: davis-haba --- pkg/mutation/mutators/assignimage/imageparser_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index b753822c4cf..ee6b7a191b4 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -454,8 +454,6 @@ func TestValidateImageParts(t *testing.T) { t.Errorf("got error of unexpected type: %s", err) } } - if tc.errFn != nil && err == nil { - } }) } } From 4e88c2c68b1ff533b2c62c4f3b3c7693ab355590 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Thu, 15 Dec 2022 12:19:11 -0800 Subject: [PATCH 18/29] validateDomain to use splitDomain Signed-off-by: davis-haba --- .../mutators/assignimage/imageparser.go | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/imageparser.go b/pkg/mutation/mutators/assignimage/imageparser.go index e5f31fff943..42a39f3817b 100644 --- a/pkg/mutation/mutators/assignimage/imageparser.go +++ b/pkg/mutation/mutators/assignimage/imageparser.go @@ -87,12 +87,28 @@ func splitDomain(name string) (domain, remainder string) { return name[:i], name[i+1:] } +func validateDomain(domain string) error { + if domain == "" { + return nil + } + + if !domainRegexp.MatchString(domain) { + return fmt.Errorf("assignDomain %q does not match pattern %s", domain, domainRegexp.String()) + } + + if d, r := splitDomain(domain + "/"); d != domain || r != "" { + return fmt.Errorf("domain %q could not be regognized as a valid domain", domain) + } + + return nil +} + func validateImageParts(domain, path, tag string) error { if domain == "" && path == "" && tag == "" { return fmt.Errorf("at least one of [assignDomain, assignPath, assignTag] must be set") } - if domain != "" && !domainRegexp.MatchString(domain) { - return fmt.Errorf("assignDomain %q does not match pattern %s", domain, domainRegexp.String()) + if err := validateDomain(domain); err != nil { + return err } // match the whole string for path (anchoring with `$` is tricky here) if path != "" && path != pathRegexp.FindString(path) { From adbe835527d9f7e38950531ce603143f47f954e7 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 21 Dec 2022 20:44:39 -0800 Subject: [PATCH 19/29] future-proof validateImageParts. Add custom error types. Signed-off-by: davis-haba --- .../assignimage/assignimage_mutator.go | 8 +- .../assignimage/assignimage_mutator_test.go | 47 ++++++----- pkg/mutation/mutators/assignimage/errors.go | 81 +++++++++++++++++++ .../mutators/assignimage/imageparser.go | 52 +++++++++--- .../mutators/assignimage/imageparser_test.go | 16 ++-- 5 files changed, 160 insertions(+), 44 deletions(-) create mode 100644 pkg/mutation/mutators/assignimage/errors.go diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator.go b/pkg/mutation/mutators/assignimage/assignimage_mutator.go index 037800d1c14..b96b9f640d3 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator.go @@ -112,7 +112,7 @@ func (m *Mutator) String() string { return fmt.Sprintf("%s/%s/%s:%d", m.id.Kind, m.id.Namespace, m.id.Name, m.assignImage.GetGeneration()) } -// MutatorForAssign returns an mutator built from +// MutatorForAssignImage returns a mutator built from // the given assignImage instance. func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Mutator, error) { // This is not always set by the kubernetes API server @@ -120,15 +120,15 @@ func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Muta path, err := parser.Parse(assignImage.Spec.Location) if err != nil { - return nil, errors.Wrapf(err, "invalid location format `%s` for Assign %s", assignImage.Spec.Location, assignImage.GetName()) + return nil, errors.Wrapf(err, "invalid location format `%s` for assignImage %s", assignImage.Spec.Location, assignImage.GetName()) } if core.HasMetadataRoot(path) { - return nil, fmt.Errorf("assignImage %s can't change metadata", assignImage.GetName()) + return nil, newMetadataRootError(assignImage.GetName()) } if hasListTerminal(path) { - return nil, fmt.Errorf("assignImage %s cannot mutate list-type fields", assignImage.GetName()) + return nil, newListTerminalError(assignImage.GetName()) } err = core.CheckKeyNotChanged(path) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go index ca838d22c83..97b93d7303a 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -1,7 +1,9 @@ package assignimage import ( + "errors" "fmt" + "strings" "testing" mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned" @@ -262,9 +264,9 @@ func TestMutate(t *testing.T) { func TestMutatorForAssignImage(t *testing.T) { tests := []struct { - name string - cfg *aiTestConfig - wantErr bool + name string + cfg *aiTestConfig + errFn func(error) bool }{ { name: "valid assignImage", @@ -285,7 +287,10 @@ func TestMutatorForAssignImage(t *testing.T) { location: "metadata.labels.image", applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, - wantErr: true, + errFn: func(err error) bool { + _, ok := err.(metadataRootError) + return ok + }, }, { name: "terminal list returns err", @@ -296,7 +301,10 @@ func TestMutatorForAssignImage(t *testing.T) { location: "spec.containers[name:foo]", applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, - wantErr: true, + errFn: func(err error) bool { + _, ok := err.(listTerminalError) + return ok + }, }, { name: "syntactically invalid location returns err", @@ -307,7 +315,9 @@ func TestMutatorForAssignImage(t *testing.T) { location: "/x/y/zx[)", applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, - wantErr: true, + errFn: func(err error) bool { + return strings.Contains(err.Error(), "invalid location format") + }, }, { name: "bad assigns return err", @@ -318,18 +328,10 @@ func TestMutatorForAssignImage(t *testing.T) { location: "spec.containers[name:foo].image", applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, - wantErr: true, - }, - { - name: "metadata root returns err", - cfg: &aiTestConfig{ - domain: "a.b.c", - path: "new/app", - tag: ":latest", - location: "metadata.labels.image", - applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + errFn: func(err error) bool { + _, ok := errors.Unwrap(err).(domainLikePathError) + return ok }, - wantErr: true, }, } @@ -339,11 +341,12 @@ func TestMutatorForAssignImage(t *testing.T) { if err != nil && mut != nil { t.Errorf("returned non-nil mutator but got err: %s", err) } - if !tc.wantErr && err != nil { - t.Errorf("did not want error but got: %v", err) - } - if tc.wantErr && err == nil { - t.Error("wanted error but got nil") + if tc.errFn != nil { + if err == nil { + t.Errorf("wanted err but got nil") + } else if !tc.errFn(err) { + t.Errorf("got error of unexpected type: %s", err) + } } }) } diff --git a/pkg/mutation/mutators/assignimage/errors.go b/pkg/mutation/mutators/assignimage/errors.go new file mode 100644 index 00000000000..c54906f765c --- /dev/null +++ b/pkg/mutation/mutators/assignimage/errors.go @@ -0,0 +1,81 @@ +package assignimage + +import "fmt" + +type baseError struct { + s string +} + +func (e baseError) Error() string { + return e.s +} + +// Component field (domain|path|tag) errors. +type invalidDomainError struct{ baseError } + +type ( + invalidPathError struct{ baseError } + invalidTagError struct{ baseError } + missingComponentsError struct{ baseError } + domainLikePathError struct{ baseError } +) + +// Location field errors. +type listTerminalError struct{ baseError } +type metadataRootError struct{ baseError } + +func newInvalidDomainError(domain string) invalidDomainError { + return invalidDomainError{ + baseError{ + fmt.Sprintf("assignDomain %q does not match pattern %s", domain, domainRegexp.String()), + }, + } +} + +func newInvalidPathError(path string) invalidPathError { + return invalidPathError{ + baseError{ + fmt.Sprintf("assignPath %q does not match pattern %s", path, pathRegexp.String()), + }, + } +} + +func newInvalidTagError(tag string) invalidTagError { + return invalidTagError{ + baseError{ + fmt.Sprintf("assignTag %q does not match pattern %s", tag, tagRegexp.String()), + }, + } +} + +func newMissingComponentsError() missingComponentsError { + return missingComponentsError{ + baseError{ + "at least one of [assignDomain, assignPath, assignTag] must be set", + }, + } +} + +func newDomainLikePathError(path string) domainLikePathError { + return domainLikePathError{ + baseError{ + fmt.Sprintf("assignDomain must be set if the first part of assignPath %q can be interpretted as part of a domain", path), + }, + } +} + +func newListTerminalError(name string) listTerminalError { + return listTerminalError{ + baseError{ + fmt.Sprintf("assignImage %s cannot mutate list-type fields", name), + }, + } +} + +func newMetadataRootError(name string) metadataRootError { + return metadataRootError{ + baseError{ + fmt.Sprintf("assignImage %s can't change metadata", name), + }, + } +} diff --git a/pkg/mutation/mutators/assignimage/imageparser.go b/pkg/mutation/mutators/assignimage/imageparser.go index 42a39f3817b..26b56f06659 100644 --- a/pkg/mutation/mutators/assignimage/imageparser.go +++ b/pkg/mutation/mutators/assignimage/imageparser.go @@ -13,7 +13,8 @@ var ( // [@:/], into components that would cause that component to be split the next // time the mutation is applied and "leak" to its neighbor. Some validation is // done as regex on individual components, and other validation which looks at - // multiple components together is done in code. + // multiple components together is done in code. All validation for domain and + // tag must be put in validateDomain and validateTag respectively. // domainRegexp defines a schema for a domain component. domainRegexp = regexp.MustCompile(`(^\w[\w\-_]*\.[\w\-_\.]*[\w](:\d+)?$)|(^localhost(:\d+)?$)`) @@ -43,16 +44,22 @@ func mutateImage(domain, path, tag, mutableImgRef string) string { func newImage(imageRef string) image { domain, remainder := splitDomain(imageRef) - var pt string + path, tag := splitTag(remainder) + return image{domain: domain, path: path, tag: tag} +} + +// splitTag separates the path and tag components from a string. +func splitTag(remainder string) (string, string) { + var path string tag := "" if tagSep := strings.IndexAny(remainder, ":@"); tagSep > -1 { - pt = remainder[:tagSep] + path = remainder[:tagSep] tag = remainder[tagSep:] } else { - pt = remainder + path = remainder } - return image{domain: domain, path: pt, tag: tag} + return path, tag } func (img image) newMutatedImage(domain, path, tag string) image { @@ -93,11 +100,32 @@ func validateDomain(domain string) error { } if !domainRegexp.MatchString(domain) { - return fmt.Errorf("assignDomain %q does not match pattern %s", domain, domainRegexp.String()) + return newInvalidDomainError(domain) } + // The error below should theoretically be unreachable, as the regex + // validation should preclude this from happening. This check is included + // anyway to prevent code drift, and ensure that if a domain is validated + // it can also be recognized as a domain. if d, r := splitDomain(domain + "/"); d != domain || r != "" { - return fmt.Errorf("domain %q could not be regognized as a valid domain", domain) + return fmt.Errorf("domain %q could not be recognized as a valid domain", domain) + } + + return nil +} + +func validateTag(tag string) error { + if tag == "" { + return nil + } + + if !tagRegexp.MatchString(tag) { + return newInvalidTagError(tag) + } + + // This error should never happen, but the check is included to prevent drift. + if _, t := splitTag(tag); t != tag { + return fmt.Errorf("tag %q could not be recognized as a valid tag or digest", tag) } return nil @@ -105,17 +133,17 @@ func validateDomain(domain string) error { func validateImageParts(domain, path, tag string) error { if domain == "" && path == "" && tag == "" { - return fmt.Errorf("at least one of [assignDomain, assignPath, assignTag] must be set") + return newMissingComponentsError() } if err := validateDomain(domain); err != nil { return err } // match the whole string for path (anchoring with `$` is tricky here) if path != "" && path != pathRegexp.FindString(path) { - return fmt.Errorf("assignPath %q does not match pattern %s", path, pathRegexp.String()) + return newInvalidPathError(path) } - if tag != "" && !tagRegexp.MatchString(tag) { - return fmt.Errorf("assignTag %q does not match pattern %s", tag, tagRegexp.String()) + if err := validateTag(tag); err != nil { + return err } // Check if the path looks like a domain string, and the domain is not set. @@ -127,7 +155,7 @@ func validateImageParts(domain, path, tag string) error { // the domain component, so the result would be "gcr.io/gcr.io/repo" and so on. if domain == "" { if d, _ := splitDomain(path); d != "" { - return fmt.Errorf("assignDomain must be set if the first part of assignPath %q can be interpretted as part of a domain", path) + return newDomainLikePathError(path) } } diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index ee6b7a191b4..bff316d8249 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -1,7 +1,6 @@ package assignimage import ( - "fmt" "strings" "testing" ) @@ -161,31 +160,36 @@ func TestNewImage(t *testing.T) { func isDomainError(domain string) func(error) bool { return func(err error) bool { - return strings.Contains(err.Error(), fmt.Sprintf("assignDomain %q does not match pattern", domain)) + _, ok := err.(invalidDomainError) + return ok && strings.Contains(err.Error(), domain) } } func isPathError(path string) func(error) bool { return func(err error) bool { - return strings.Contains(err.Error(), fmt.Sprintf("assignPath %q does not match pattern", path)) + _, ok := err.(invalidPathError) + return ok && strings.Contains(err.Error(), path) } } func isTagError(tag string) func(error) bool { return func(err error) bool { - return strings.Contains(err.Error(), fmt.Sprintf("assignTag %q does not match pattern", tag)) + _, ok := err.(invalidTagError) + return ok && strings.Contains(err.Error(), tag) } } func isEmptyArgsError() func(error) bool { return func(err error) bool { - return strings.Contains(err.Error(), "at least one of") + _, ok := err.(missingComponentsError) + return ok } } func isPathlikeDomainError() func(error) bool { return func(err error) bool { - return strings.Contains(err.Error(), "assignDomain must be set if") + _, ok := err.(domainLikePathError) + return ok } } From f553ff2a61893a17d6ffa41116b9a5bd53430a05 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 21 Dec 2022 21:33:42 -0800 Subject: [PATCH 20/29] fix readiness tracker test Signed-off-by: davis-haba --- pkg/readiness/ready_tracker_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go index 9145bc63456..914faadad4b 100644 --- a/pkg/readiness/ready_tracker_test.go +++ b/pkg/readiness/ready_tracker_test.go @@ -240,8 +240,9 @@ func Test_AssignImage(t *testing.T) { opaClient := setupOpa(t) mutationSystem := mutation.NewSystem(mutation.SystemOpts{}) + providerCache := frameworksexternaldata.NewCache() - if err := setupController(mgr, wm, opaClient, mutationSystem, nil); err != nil { + if err := setupController(mgr, wm, opaClient, mutationSystem, providerCache); err != nil { t.Fatalf("setupControllers: %v", err) } From 52fa795bb26ed6932b52348e80b4b7e21fc842e1 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 21 Dec 2022 21:44:49 -0800 Subject: [PATCH 21/29] make manifests Signed-off-by: davis-haba --- config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml | 8 +------- .../crds/assignimage-customresourcedefinition.yaml | 8 +------- manifest_staging/deploy/gatekeeper.yaml | 8 +------- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml b/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml index 1a4e442a60a..1e2d9ec61a5 100644 --- a/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml +++ b/config/crd/bases/mutations.gatekeeper.sh_assignimage.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.10.0 creationTimestamp: null name: assignimage.mutations.gatekeeper.sh spec: @@ -324,9 +324,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml index 33ba628ab4c..bab801672a6 100644 --- a/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/assignimage-customresourcedefinition.yaml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.10.0 labels: gatekeeper.sh/system: "yes" name: assignimage.mutations.gatekeeper.sh @@ -235,9 +235,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/manifest_staging/deploy/gatekeeper.yaml b/manifest_staging/deploy/gatekeeper.yaml index 803f87a3207..e101eeee58b 100644 --- a/manifest_staging/deploy/gatekeeper.yaml +++ b/manifest_staging/deploy/gatekeeper.yaml @@ -1025,18 +1025,12 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.10.0 labels: gatekeeper.sh/system: "yes" name: assignmetadata.mutations.gatekeeper.sh From e4baaaed71ca2b7eb80a908af459209faed77191 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Fri, 30 Dec 2022 01:14:31 -0800 Subject: [PATCH 22/29] validate that splitting a valid tag never returns a path Signed-off-by: davis-haba --- pkg/mutation/mutators/assignimage/imageparser.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/imageparser.go b/pkg/mutation/mutators/assignimage/imageparser.go index 26b56f06659..df00f15718d 100644 --- a/pkg/mutation/mutators/assignimage/imageparser.go +++ b/pkg/mutation/mutators/assignimage/imageparser.go @@ -123,8 +123,10 @@ func validateTag(tag string) error { return newInvalidTagError(tag) } - // This error should never happen, but the check is included to prevent drift. - if _, t := splitTag(tag); t != tag { + // This error should never happen because the regex above prevents it, but the + // check is included to prevent drift. Splitting the tag should return itself, + // and splitting a valid tag should never return a path. + if p, t := splitTag(tag); t != tag || p != "" { return fmt.Errorf("tag %q could not be recognized as a valid tag or digest", tag) } From 9102ee99f13d2bec6846d0669a33f30284a45e1b Mon Sep 17 00:00:00 2001 From: davis-haba Date: Tue, 10 Jan 2023 10:26:45 -0800 Subject: [PATCH 23/29] degenerate cases for unit tests. do not expose regex on image component error. Signed-off-by: davis-haba --- .../assignimage/assignimage_mutator_test.go | 21 ++++++++++++++----- pkg/mutation/mutators/assignimage/errors.go | 6 +++--- .../mutators/assignimage/imageparser_test.go | 9 ++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go index 97b93d7303a..4fb456614d8 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -25,8 +25,8 @@ type aiTestConfig struct { applyTo []match.ApplyTo } -func newAiMutator(cfg *aiTestConfig) *Mutator { - m := newAi(cfg) +func newAIMutator(cfg *aiTestConfig) *Mutator { + m := newAI(cfg) m2, err := MutatorForAssignImage(m) if err != nil { panic(err) @@ -34,7 +34,7 @@ func newAiMutator(cfg *aiTestConfig) *Mutator { return m2 } -func newAi(cfg *aiTestConfig) *mutationsunversioned.AssignImage { +func newAI(cfg *aiTestConfig) *mutationsunversioned.AssignImage { m := &mutationsunversioned.AssignImage{ ObjectMeta: metav1.ObjectMeta{ Name: "Foo", @@ -110,6 +110,17 @@ func TestMutate(t *testing.T) { obj: newPod("library/busybox:v1", "foo"), fn: podTest("library/busybox:new"), }, + { + name: "mutate path and tag with empty image", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + path: "library/busybox", + tag: ":new", + }, + obj: newPod("", "foo"), + fn: podTest("library/busybox:new"), + }, { name: "mutate path", cfg: &aiTestConfig{ @@ -249,7 +260,7 @@ func TestMutate(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - mutator := newAiMutator(test.cfg) + mutator := newAIMutator(test.cfg) obj := test.obj.DeepCopy() _, err := mutator.Mutate(&types.Mutable{Object: obj}) if err != nil { @@ -337,7 +348,7 @@ func TestMutatorForAssignImage(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - mut, err := MutatorForAssignImage(newAi(tc.cfg)) + mut, err := MutatorForAssignImage(newAI(tc.cfg)) if err != nil && mut != nil { t.Errorf("returned non-nil mutator but got err: %s", err) } diff --git a/pkg/mutation/mutators/assignimage/errors.go b/pkg/mutation/mutators/assignimage/errors.go index c54906f765c..6e1fe6d0347 100644 --- a/pkg/mutation/mutators/assignimage/errors.go +++ b/pkg/mutation/mutators/assignimage/errors.go @@ -27,7 +27,7 @@ type metadataRootError struct{ baseError } func newInvalidDomainError(domain string) invalidDomainError { return invalidDomainError{ baseError{ - fmt.Sprintf("assignDomain %q does not match pattern %s", domain, domainRegexp.String()), + fmt.Sprintf("assignDomain %q must be a fully-qualified domain name or localhost", domain), }, } } @@ -35,7 +35,7 @@ func newInvalidDomainError(domain string) invalidDomainError { func newInvalidPathError(path string) invalidPathError { return invalidPathError{ baseError{ - fmt.Sprintf("assignPath %q does not match pattern %s", path, pathRegexp.String()), + fmt.Sprintf("assignPath %q must be a valid docker image path", path), }, } } @@ -43,7 +43,7 @@ func newInvalidPathError(path string) invalidPathError { func newInvalidTagError(tag string) invalidTagError { return invalidTagError{ baseError{ - fmt.Sprintf("assignTag %q does not match pattern %s", tag, tagRegexp.String()), + fmt.Sprintf("assignTag %q must be a valid docker image tag", tag), }, } } diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index bff316d8249..8aee08ac2b3 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -20,6 +20,15 @@ func TestNewImage(t *testing.T) { tag: ":latest", }, }, + { + name: "all empty components", + imageRef: "", + want: image{ + domain: "", + path: "", + tag: "", + }, + }, { name: "full image with hash", imageRef: "some-image/hello@sha256:abcde", From a97be6ad119b338e833fd627e3b3060705a7961c Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 11 Jan 2023 11:23:47 -0800 Subject: [PATCH 24/29] test missing image field. update error copy. Signed-off-by: davis-haba --- .../assignimage/assignimage_mutator_test.go | 33 +++++++++++++++++++ pkg/mutation/mutators/assignimage/errors.go | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go index 4fb456614d8..1819853c2bc 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -72,6 +72,28 @@ func newPod(imageVal, name string) *unstructured.Unstructured { return &unstructured.Unstructured{Object: u} } +func newPodNoImage() *unstructured.Unstructured { + pod := &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Pod", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "foo", + }, + }, + }, + } + + u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) + if err != nil { + panic(fmt.Sprintf("converting pod to unstructured: %v", err)) + } + return &unstructured.Unstructured{Object: u} +} + func podTest(wantImage string) func(*unstructured.Unstructured) error { return func(u *unstructured.Unstructured) error { var pod corev1.Pod @@ -121,6 +143,17 @@ func TestMutate(t *testing.T) { obj: newPod("", "foo"), fn: podTest("library/busybox:new"), }, + { + name: "mutate path and tag with missing image", + cfg: &aiTestConfig{ + applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, + location: `spec.containers[name:foo].image`, + path: "library/busybox", + tag: ":new", + }, + obj: newPodNoImage(), + fn: podTest("library/busybox:new"), + }, { name: "mutate path", cfg: &aiTestConfig{ diff --git a/pkg/mutation/mutators/assignimage/errors.go b/pkg/mutation/mutators/assignimage/errors.go index 6e1fe6d0347..95a4d35f5d9 100644 --- a/pkg/mutation/mutators/assignimage/errors.go +++ b/pkg/mutation/mutators/assignimage/errors.go @@ -43,7 +43,7 @@ func newInvalidPathError(path string) invalidPathError { func newInvalidTagError(tag string) invalidTagError { return invalidTagError{ baseError{ - fmt.Sprintf("assignTag %q must be a valid docker image tag", tag), + fmt.Sprintf("assignTag %q must be a valid docker image tag or digest", tag), }, } } From 88df8e5b75b7da9ad12923450d61e519250872f5 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Wed, 11 Jan 2023 11:27:31 -0800 Subject: [PATCH 25/29] tag error copy Signed-off-by: davis-haba --- pkg/mutation/mutators/assignimage/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/mutation/mutators/assignimage/errors.go b/pkg/mutation/mutators/assignimage/errors.go index 95a4d35f5d9..7dce7ec3d82 100644 --- a/pkg/mutation/mutators/assignimage/errors.go +++ b/pkg/mutation/mutators/assignimage/errors.go @@ -43,7 +43,7 @@ func newInvalidPathError(path string) invalidPathError { func newInvalidTagError(tag string) invalidTagError { return invalidTagError{ baseError{ - fmt.Sprintf("assignTag %q must be a valid docker image tag or digest", tag), + fmt.Sprintf("assignTag %q must be a valid docker image tag or digest starting with ':' or '@'", tag), }, } } From 50d8f16ce990264f6716d4eb86b986152a02ca59 Mon Sep 17 00:00:00 2001 From: Davis Haba <52938648+davis-haba@users.noreply.github.com> Date: Fri, 20 Jan 2023 18:56:15 -0800 Subject: [PATCH 26/29] Update pkg/expansion/system_test.go Co-authored-by: Rita Zhang Signed-off-by: Davis Haba <52938648+davis-haba@users.noreply.github.com> --- pkg/expansion/system_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/expansion/system_test.go b/pkg/expansion/system_test.go index 6251ff8f950..b62d3696e70 100644 --- a/pkg/expansion/system_test.go +++ b/pkg/expansion/system_test.go @@ -894,7 +894,7 @@ func loadAssignImage(f string, t *testing.T) types.Mutator { a := &mutationsunversioned.AssignImage{} err := convertUnstructuredToTyped(u, a) if err != nil { - t.Fatalf("error converting assign: %s", err) + t.Fatalf("error converting assignImage: %s", err) } mut, err := assignimage.MutatorForAssignImage(a) if err != nil { From 82e78eb3722878c8143807ce716cfeeef4d50620 Mon Sep 17 00:00:00 2001 From: Davis Haba <52938648+davis-haba@users.noreply.github.com> Date: Fri, 20 Jan 2023 18:56:27 -0800 Subject: [PATCH 27/29] Update pkg/mutation/mutators/assignimage/assignimage_mutator.go Co-authored-by: Rita Zhang Signed-off-by: Davis Haba <52938648+davis-haba@users.noreply.github.com> --- pkg/mutation/mutators/assignimage/assignimage_mutator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator.go b/pkg/mutation/mutators/assignimage/assignimage_mutator.go index b96b9f640d3..806b2f6dc60 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator.go @@ -37,7 +37,7 @@ var _ schema.MutatorWithSchema = &Mutator{} func (m *Mutator) Matches(mutable *types.Mutable) bool { res, err := core.MatchWithApplyTo(mutable, m.assignImage.Spec.ApplyTo, &m.assignImage.Spec.Match) if err != nil { - log.Error(err, "Matches failed for modify set", "modifyset", m.assignImage.Name) + log.Error(err, "Matches failed for assign image", "assignImage", m.assignImage.Name) } return res } From 09389e3486fdf5d08ee410afce594b3a8ef147a1 Mon Sep 17 00:00:00 2001 From: davis-haba Date: Fri, 20 Jan 2023 19:43:59 -0800 Subject: [PATCH 28/29] errors.As instead of type casting in unit tests Signed-off-by: davis-haba --- .../mutators/assignimage/assignimage_mutator_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go index 1819853c2bc..f6d332b8289 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -332,8 +332,8 @@ func TestMutatorForAssignImage(t *testing.T) { applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, errFn: func(err error) bool { - _, ok := err.(metadataRootError) - return ok + var tarErr *metadataRootError + return errors.As(err, &tarErr) }, }, { @@ -346,8 +346,8 @@ func TestMutatorForAssignImage(t *testing.T) { applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, errFn: func(err error) bool { - _, ok := err.(listTerminalError) - return ok + var tarErr *listTerminalError + return errors.As(err, &tarErr) }, }, { @@ -373,8 +373,8 @@ func TestMutatorForAssignImage(t *testing.T) { applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, errFn: func(err error) bool { - _, ok := errors.Unwrap(err).(domainLikePathError) - return ok + var tarErr *domainLikePathError + return errors.As(err, &tarErr) }, }, } From d2caf9f41ddebf5977df2341552eb8737bb3bcde Mon Sep 17 00:00:00 2001 From: davis-haba Date: Fri, 20 Jan 2023 20:25:39 -0800 Subject: [PATCH 29/29] fix error type checking Signed-off-by: davis-haba --- .../assignimage/assignimage_mutator_test.go | 9 +++------ .../mutators/assignimage/imageparser_test.go | 16 ++++++---------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go index f6d332b8289..11a51b4a365 100644 --- a/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go +++ b/pkg/mutation/mutators/assignimage/assignimage_mutator_test.go @@ -332,8 +332,7 @@ func TestMutatorForAssignImage(t *testing.T) { applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, errFn: func(err error) bool { - var tarErr *metadataRootError - return errors.As(err, &tarErr) + return errors.As(err, &metadataRootError{}) }, }, { @@ -346,8 +345,7 @@ func TestMutatorForAssignImage(t *testing.T) { applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, errFn: func(err error) bool { - var tarErr *listTerminalError - return errors.As(err, &tarErr) + return errors.As(err, &listTerminalError{}) }, }, { @@ -373,8 +371,7 @@ func TestMutatorForAssignImage(t *testing.T) { applyTo: []match.ApplyTo{{Groups: []string{""}, Versions: []string{"v1"}, Kinds: []string{"Foo"}}}, }, errFn: func(err error) bool { - var tarErr *domainLikePathError - return errors.As(err, &tarErr) + return errors.As(err, &domainLikePathError{}) }, }, } diff --git a/pkg/mutation/mutators/assignimage/imageparser_test.go b/pkg/mutation/mutators/assignimage/imageparser_test.go index 8aee08ac2b3..22d63c9e971 100644 --- a/pkg/mutation/mutators/assignimage/imageparser_test.go +++ b/pkg/mutation/mutators/assignimage/imageparser_test.go @@ -1,6 +1,7 @@ package assignimage import ( + "errors" "strings" "testing" ) @@ -169,36 +170,31 @@ func TestNewImage(t *testing.T) { func isDomainError(domain string) func(error) bool { return func(err error) bool { - _, ok := err.(invalidDomainError) - return ok && strings.Contains(err.Error(), domain) + return errors.As(err, &invalidDomainError{}) && strings.Contains(err.Error(), domain) } } func isPathError(path string) func(error) bool { return func(err error) bool { - _, ok := err.(invalidPathError) - return ok && strings.Contains(err.Error(), path) + return errors.As(err, &invalidPathError{}) && strings.Contains(err.Error(), path) } } func isTagError(tag string) func(error) bool { return func(err error) bool { - _, ok := err.(invalidTagError) - return ok && strings.Contains(err.Error(), tag) + return errors.As(err, &invalidTagError{}) && strings.Contains(err.Error(), tag) } } func isEmptyArgsError() func(error) bool { return func(err error) bool { - _, ok := err.(missingComponentsError) - return ok + return errors.As(err, &missingComponentsError{}) } } func isPathlikeDomainError() func(error) bool { return func(err error) bool { - _, ok := err.(domainLikePathError) - return ok + return errors.As(err, &domainLikePathError{}) } }