diff --git a/Makefile b/Makefile index 887fab3c68a..fc223f4179b 100644 --- a/Makefile +++ b/Makefile @@ -188,10 +188,6 @@ deploy-mutation: patch-image k8s.gcr.io/kustomize/kustomize:v${KUSTOMIZE_VERSION} build \ --load_restrictor LoadRestrictionsNone \ /config/overlays/dev_mutation | kubectl apply -f - - docker run -v $(shell pwd)/config:/config -v $(shell pwd)/vendor:/vendor \ - k8s.gcr.io/kustomize/kustomize:v${KUSTOMIZE_VERSION} build \ - --load_restrictor LoadRestrictionsNone \ - /config/overlays/mutation | kubectl apply -f - # Deploy controller in the configured Kubernetes cluster in ~/.kube/config deploy: patch-image manifests @@ -208,10 +204,6 @@ manifests: __controller-gen paths="./apis/..." \ paths="./pkg/..." \ output:crd:artifacts:config=config/crd/bases - # As mutation CRDs are not ready to be included in our final gatekeeper.yaml, we leave them out of config/crd/kustomization.yaml. - # This makes these files unavailable to the helmify step below. The solve for this was to copy the mutation CRDs into - # config/overlays/mutation_webhook/. To maintain the generation pipeline, we do that copy step here. - cp config/crd/bases/*mutat* config/overlays/mutation_webhook/ rm -rf manifest_staging mkdir -p manifest_staging/deploy mkdir -p manifest_staging/charts/gatekeeper diff --git a/apis/mutations/v1alpha1/assign_types.go b/apis/mutations/v1alpha1/assign_types.go index 023507fc820..69ce84a45f8 100644 --- a/apis/mutations/v1alpha1/assign_types.go +++ b/apis/mutations/v1alpha1/assign_types.go @@ -32,10 +32,22 @@ import ( type AssignSpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster // Important: Run "make" to regenerate code after modifying this file - ApplyTo []match.ApplyTo `json:"applyTo,omitempty"` - Match match.Match `json:"match,omitempty"` - Location string `json:"location,omitempty"` - Parameters Parameters `json:"parameters,omitempty"` + + // 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]`. + Location string `json:"location,omitempty"` + + // Parameters define the behavior of the mutator. + Parameters Parameters `json:"parameters,omitempty"` } type Parameters struct { diff --git a/apis/mutations/v1alpha1/modifyset_types.go b/apis/mutations/v1alpha1/modifyset_types.go new file mode 100644 index 00000000000..0c084cc071f --- /dev/null +++ b/apis/mutations/v1alpha1/modifyset_types.go @@ -0,0 +1,147 @@ +/* + +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" + "k8s.io/apimachinery/pkg/runtime" +) + +// 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. + +// ModifySetSpec defines the desired state of ModifySet. +type ModifySetSpec 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].args`. + Location string `json:"location,omitempty"` + + // Parameters define the behavior of the mutator. + Parameters ModifySetParameters `json:"parameters,omitempty"` +} + +type ModifySetParameters struct { + // PathTests are a series of existence tests that can be checked + // before a mutation is applied + PathTests []PathTest `json:"pathTests,omitempty"` + + // Operation describes whether values should be merged in ("merge"), or pruned ("prune"). Default value is "merge" + // +kubebuilder:validation:Enum=merge;prune + // +kubebuilder:default=merge + Operation Operation `json:"operation,omitempty"` + + // Values describes the values provided to the operation as `values.fromList`. + // +kubebuilder:validation:Schemaless + // +kubebuilder:validation:Type=object + // +kubebuilder:validation:XPreserveUnknownFields + Values Values `json:"values,omitempty"` +} + +type Operation string + +const ( + // MergeOp means that the provided values should be merged with the existing values. + MergeOp Operation = "merge" + + // PruneOp means that the provided values should be removed from the existing values. + PruneOp Operation = "prune" +) + +// Values describes the values provided to the operation. +// +kubebuilder:object:generate=false +type Values struct { + FromList []interface{} `json:"fromList,omitempty"` +} + +func (in *Values) DeepCopy() *Values { + if in == nil { + return nil + } + + var fromList []interface{} + if in.FromList != nil { + fromList = make([]interface{}, len(in.FromList)) + for i := range fromList { + fromList[i] = runtime.DeepCopyJSONValue(in.FromList[i]) + } + } + + return &Values{ + FromList: fromList, + } +} + +func (in *Values) DeepCopyInto(out *Values) { + *in = *out + + if in.FromList != nil { + fromList := make([]interface{}, len(in.FromList)) + for i := range fromList { + fromList[i] = runtime.DeepCopyJSONValue(in.FromList[i]) + } + out.FromList = fromList + } +} + +// ModifySetStatus defines the observed state of ModifySet. +type ModifySetStatus 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="modifyset" +// +kubebuilder:resource:scope="Cluster" +// +kubebuilder:subresource:status + +// ModifySet allows the user to modify non-keyed lists, such as +// the list of arguments to a container. +type ModifySet struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec ModifySetSpec `json:"spec,omitempty"` + Status ModifySetStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// ModifySetList contains a list of ModifySet. +type ModifySetList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ModifySet `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ModifySet{}, &ModifySetList{}) +} diff --git a/apis/mutations/v1alpha1/zz_generated.deepcopy.go b/apis/mutations/v1alpha1/zz_generated.deepcopy.go index 4ce108c468d..e713e7a37f5 100644 --- a/apis/mutations/v1alpha1/zz_generated.deepcopy.go +++ b/apis/mutations/v1alpha1/zz_generated.deepcopy.go @@ -244,6 +244,132 @@ func (in *MetadataParameters) DeepCopy() *MetadataParameters { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ModifySet) DeepCopyInto(out *ModifySet) { + *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 ModifySet. +func (in *ModifySet) DeepCopy() *ModifySet { + if in == nil { + return nil + } + out := new(ModifySet) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ModifySet) 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 *ModifySetList) DeepCopyInto(out *ModifySetList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ModifySet, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModifySetList. +func (in *ModifySetList) DeepCopy() *ModifySetList { + if in == nil { + return nil + } + out := new(ModifySetList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ModifySetList) 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 *ModifySetParameters) DeepCopyInto(out *ModifySetParameters) { + *out = *in + if in.PathTests != nil { + in, out := &in.PathTests, &out.PathTests + *out = make([]PathTest, len(*in)) + copy(*out, *in) + } + in.Values.DeepCopyInto(&out.Values) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModifySetParameters. +func (in *ModifySetParameters) DeepCopy() *ModifySetParameters { + if in == nil { + return nil + } + out := new(ModifySetParameters) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ModifySetSpec) DeepCopyInto(out *ModifySetSpec) { + *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 ModifySetSpec. +func (in *ModifySetSpec) DeepCopy() *ModifySetSpec { + if in == nil { + return nil + } + out := new(ModifySetSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ModifySetStatus) DeepCopyInto(out *ModifySetStatus) { + *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 ModifySetStatus. +func (in *ModifySetStatus) DeepCopy() *ModifySetStatus { + if in == nil { + return nil + } + out := new(ModifySetStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Parameters) DeepCopyInto(out *Parameters) { *out = *in diff --git a/cmd/build/helmify/kustomization.yaml b/cmd/build/helmify/kustomization.yaml index 240db813af0..1f896e91715 100644 --- a/cmd/build/helmify/kustomization.yaml +++ b/cmd/build/helmify/kustomization.yaml @@ -5,7 +5,7 @@ commonLabels: release: "{{ .Release.Name }}" heritage: "{{ .Release.Service }}" bases: - - "../../../config/overlays/mutation_webhook" # calls ../../default + - "../../../config/overlays/mutation" # calls ../../default patchesStrategicMerge: - kustomize-for-helm.yaml - delete-ports.yaml @@ -52,6 +52,12 @@ patchesJson6902: kind: CustomResourceDefinition name: assign.mutations.gatekeeper.sh path: labels_patch.yaml + - target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: modifyset.mutations.gatekeeper.sh + path: labels_patch.yaml # these are defined in the chart values rather than hard-coded - target: kind: Deployment diff --git a/config/crd/bases/mutations.gatekeeper.sh_assign.yaml b/config/crd/bases/mutations.gatekeeper.sh_assign.yaml index bce24f80e6e..289978d5ddd 100644 --- a/config/crd/bases/mutations.gatekeeper.sh_assign.yaml +++ b/config/crd/bases/mutations.gatekeeper.sh_assign.yaml @@ -33,7 +33,7 @@ spec: description: AssignSpec defines the desired state of Assign. properties: applyTo: - description: 'INSERT ADDITIONAL SPEC FIELDS - desired state of cluster Important: Run "make" to regenerate code after modifying this file' + 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: @@ -52,9 +52,10 @@ spec: type: object type: array location: + description: 'Location describes the path to be mutated, for example: `spec.containers[name: main]`.' type: string match: - description: Match selects objects to apply mutations to. + 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: items: @@ -144,6 +145,7 @@ spec: type: string type: object parameters: + description: Parameters define the behavior of the mutator. properties: assign: description: Assign.value holds the value to be assigned diff --git a/config/overlays/mutation_webhook/mutations.gatekeeper.sh_assign.yaml b/config/crd/bases/mutations.gatekeeper.sh_modifyset.yaml similarity index 86% rename from config/overlays/mutation_webhook/mutations.gatekeeper.sh_assign.yaml rename to config/crd/bases/mutations.gatekeeper.sh_modifyset.yaml index bce24f80e6e..5f7313929c1 100644 --- a/config/overlays/mutation_webhook/mutations.gatekeeper.sh_assign.yaml +++ b/config/crd/bases/mutations.gatekeeper.sh_modifyset.yaml @@ -6,20 +6,20 @@ metadata: annotations: controller-gen.kubebuilder.io/version: v0.5.0 creationTimestamp: null - name: assign.mutations.gatekeeper.sh + name: modifyset.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh names: - kind: Assign - listKind: AssignList - plural: assign - singular: assign + kind: ModifySet + listKind: ModifySetList + plural: modifyset + singular: modifyset scope: Cluster versions: - name: v1alpha1 schema: openAPIV3Schema: - description: Assign is the Schema for the assign API. + description: ModifySet allows the user to modify non-keyed lists, such as the list of arguments to a container. 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' @@ -30,10 +30,10 @@ spec: metadata: type: object spec: - description: AssignSpec defines the desired state of Assign. + description: ModifySetSpec defines the desired state of ModifySet. properties: applyTo: - description: 'INSERT ADDITIONAL SPEC FIELDS - desired state of cluster Important: Run "make" to regenerate code after modifying this file' + 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: @@ -52,9 +52,10 @@ spec: type: object type: array location: + description: 'Location describes the path to be mutated, for example: `spec.containers[name: main].args`.' type: string match: - description: Match selects objects to apply mutations to. + 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: items: @@ -144,15 +145,17 @@ spec: type: string type: object parameters: + description: Parameters define the behavior of the mutator. properties: - assign: - description: Assign.value holds the value to be assigned - type: object - x-kubernetes-preserve-unknown-fields: true - assignIf: - description: once https://github.com/kubernetes-sigs/controller-tools/pull/528 is merged, we can use an actual object - type: object + operation: + default: merge + description: Operation describes whether values should be merged in ("merge"), or pruned ("prune"). Default value is "merge" + enum: + - merge + - prune + type: string pathTests: + description: PathTests are a series of existence tests that can be checked before a mutation is applied 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: @@ -166,10 +169,14 @@ spec: type: string type: object type: array + values: + description: Values describes the values provided to the operation as `values.fromList`. + type: object + x-kubernetes-preserve-unknown-fields: true type: object type: object status: - description: AssignStatus defines the observed state of Assign. + description: ModifySetStatus defines the observed state of ModifySet. properties: byPod: items: diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index c225bb767e8..390d33740e2 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -12,6 +12,7 @@ resources: # add the mutation crds to resources: #- bases/mutations.gatekeeper.sh_assigns.yaml #- bases/mutations.gatekeeper.sh_assignmetadata.yaml +#- bases/mutations.gatekeeper.sh_modifyset.yaml bases: - ../../vendor/github.com/open-policy-agent/frameworks/constraint/deploy @@ -24,6 +25,7 @@ patches: path: patches/preserve_unknown_fields_false.yaml patchesStrategicMerge: +#- patches/max_name_size_for_modifyset.yaml #- patches/max_name_size_for_assign.yaml #- patches/max_name_size_for_assignmetadata.yaml # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. diff --git a/config/overlays/dev_mutation/kustomization.yaml b/config/overlays/dev_mutation/kustomization.yaml index 7935531ce2f..0421c270bb4 100644 --- a/config/overlays/dev_mutation/kustomization.yaml +++ b/config/overlays/dev_mutation/kustomization.yaml @@ -2,7 +2,7 @@ namespace: gatekeeper-system resources: - - ../mutation_webhook + - ../mutation patchesStrategicMerge: - manager_image_patch.yaml diff --git a/config/overlays/mutation/kustomization.yaml b/config/overlays/mutation/kustomization.yaml index dfdecbe8521..82e689a7b69 100644 --- a/config/overlays/mutation/kustomization.yaml +++ b/config/overlays/mutation/kustomization.yaml @@ -1,19 +1,17 @@ # Adds namespace to all resources. namespace: gatekeeper-system -# Value of this field is prepended to the -# names of all resources, e.g. a deployment named -# "wordpress" becomes "alices-wordpress". -# Note that it should also match with the prefix (text before '-') of the namespace -# field above. -namePrefix: gatekeeper- - commonLabels: - gatekeeper.sh/system: "yes" + gatekeeper.sh/system: "yes" + +bases: + - ../../default resources: +- webhook.yaml - ../../crd/bases/mutations.gatekeeper.sh_assign.yaml - ../../crd/bases/mutations.gatekeeper.sh_assignmetadata.yaml +- ../../crd/bases/mutations.gatekeeper.sh_modifyset.yaml - ../../crd/bases/status.gatekeeper.sh_mutatorpodstatuses.yaml patches: @@ -36,19 +34,18 @@ patchesJson6902: kind: CustomResourceDefinition name: assignmetadata.mutations.gatekeeper.sh path: ../../crd/patches/max_name_size.yaml - -patchesStrategicMerge: -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix. -# patches here are for enabling the conversion webhook for each CRD -#- ../../crd/patches/webhook_in_assignmetadata.yaml -#- ../../crd/patches/webhook_in_assigns.yaml -# +kubebuilder:scaffold:crdkustomizewebhookpatch - -# [CERTMANAGER] To enable webhook, uncomment all the sections with [CERTMANAGER] prefix. -# patches here are for enabling the CA injection for each CRD -#- ../../crd/patches/cainjection_in_assignmetadata.yaml -#- ../../crd/patches/cainjection_in_assigns.yaml -# +kubebuilder:scaffold:crdkustomizecainjectionpatch +- target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: modifyset.mutations.gatekeeper.sh + path: ../../crd/patches/max_name_size.yaml +- target: + group: rbac.authorization.k8s.io + version: v1 + kind: ClusterRole + name: manager-role + path: webhook_permissions_patch.yaml # the following config is for teaching kustomize how to do kustomization for CRDs. configurations: diff --git a/config/overlays/mutation_webhook/webhook.yaml b/config/overlays/mutation/webhook.yaml similarity index 100% rename from config/overlays/mutation_webhook/webhook.yaml rename to config/overlays/mutation/webhook.yaml diff --git a/config/overlays/mutation_webhook/webhook_permissions_patch.yaml b/config/overlays/mutation/webhook_permissions_patch.yaml similarity index 100% rename from config/overlays/mutation_webhook/webhook_permissions_patch.yaml rename to config/overlays/mutation/webhook_permissions_patch.yaml diff --git a/config/overlays/mutation_webhook/kustomization.yaml b/config/overlays/mutation_webhook/kustomization.yaml deleted file mode 100644 index a4ea0b13a7a..00000000000 --- a/config/overlays/mutation_webhook/kustomization.yaml +++ /dev/null @@ -1,29 +0,0 @@ -# TODO: this is a temporary kustomization for the mutation webhook -# It is kept separate until the mutation feature is stable enough, -# when the mutation webhook should be moved to config/webhooks/manifests.yaml - -namespace: gatekeeper-system - -bases: - - ../../default - -resources: - - webhook.yaml - - mutations.gatekeeper.sh_assign.yaml - - mutations.gatekeeper.sh_assignmetadata.yaml - - status.gatekeeper.sh_mutatorpodstatuses.yaml - -patches: -- target: - group: apiextensions.k8s.io - version: v1 - kind: CustomResourceDefinition - path: ../../crd/patches/preserve_unknown_fields_false.yaml - -patchesJson6902: -- target: - group: rbac.authorization.k8s.io - version: v1 - kind: ClusterRole - name: manager-role - path: webhook_permissions_patch.yaml diff --git a/config/overlays/mutation_webhook/status.gatekeeper.sh_mutatorpodstatuses.yaml b/config/overlays/mutation_webhook/status.gatekeeper.sh_mutatorpodstatuses.yaml deleted file mode 100644 index 30d53bb1d3f..00000000000 --- a/config/overlays/mutation_webhook/status.gatekeeper.sh_mutatorpodstatuses.yaml +++ /dev/null @@ -1,68 +0,0 @@ - ---- -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - annotations: - controller-gen.kubebuilder.io/version: v0.5.0 - creationTimestamp: null - name: mutatorpodstatuses.status.gatekeeper.sh -spec: - group: status.gatekeeper.sh - names: - kind: MutatorPodStatus - listKind: MutatorPodStatusList - plural: mutatorpodstatuses - singular: mutatorpodstatus - scope: Namespaced - versions: - - name: v1beta1 - schema: - openAPIV3Schema: - description: MutatorPodStatus is the Schema for the mutationpodstatuses 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 - status: - 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 - 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: object - served: true - storage: true -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/manifest_staging/charts/gatekeeper/crds/assign-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assign-customresourcedefinition.yaml index 66bae643d11..1ba5a6e8e89 100644 --- a/manifest_staging/charts/gatekeeper/crds/assign-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/assign-customresourcedefinition.yaml @@ -28,12 +28,16 @@ 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: AssignSpec defines the desired state of Assign. properties: applyTo: - description: 'INSERT ADDITIONAL SPEC FIELDS - desired state of cluster Important: Run "make" to regenerate code after modifying this file' + 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: @@ -52,9 +56,10 @@ spec: type: object type: array location: + description: 'Location describes the path to be mutated, for example: `spec.containers[name: main]`.' type: string match: - description: Match selects objects to apply mutations to. + 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: items: @@ -144,6 +149,7 @@ spec: type: string type: object parameters: + description: Parameters define the behavior of the mutator. properties: assign: description: Assign.value holds the value to be assigned diff --git a/manifest_staging/charts/gatekeeper/crds/assignmetadata-customresourcedefinition.yaml b/manifest_staging/charts/gatekeeper/crds/assignmetadata-customresourcedefinition.yaml index 43f960d6ef6..75df3ed1bcc 100644 --- a/manifest_staging/charts/gatekeeper/crds/assignmetadata-customresourcedefinition.yaml +++ b/manifest_staging/charts/gatekeeper/crds/assignmetadata-customresourcedefinition.yaml @@ -28,6 +28,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: AssignMetadataSpec defines the desired state of AssignMetadata. diff --git a/config/overlays/mutation_webhook/mutations.gatekeeper.sh_assignmetadata.yaml b/manifest_staging/charts/gatekeeper/crds/modifyset-customresourcedefinition.yaml similarity index 71% rename from config/overlays/mutation_webhook/mutations.gatekeeper.sh_assignmetadata.yaml rename to manifest_staging/charts/gatekeeper/crds/modifyset-customresourcedefinition.yaml index e737a2d9c75..813f8ef77b1 100644 --- a/config/overlays/mutation_webhook/mutations.gatekeeper.sh_assignmetadata.yaml +++ b/manifest_staging/charts/gatekeeper/crds/modifyset-customresourcedefinition.yaml @@ -1,25 +1,25 @@ - ---- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.5.0 - creationTimestamp: null - name: assignmetadata.mutations.gatekeeper.sh + labels: + gatekeeper.sh/system: "yes" + name: modifyset.mutations.gatekeeper.sh spec: group: mutations.gatekeeper.sh names: - kind: AssignMetadata - listKind: AssignMetadataList - plural: assignmetadata - singular: assignmetadata + kind: ModifySet + listKind: ModifySetList + plural: modifyset + singular: modifyset + preserveUnknownFields: false scope: Cluster versions: - name: v1alpha1 schema: openAPIV3Schema: - description: AssignMetadata is the Schema for the assignmetadata API. + description: ModifySet allows the user to modify non-keyed lists, such as the list of arguments to a container. 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' @@ -28,14 +28,38 @@ 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: AssignMetadataSpec defines the desired state of AssignMetadata. + description: ModifySetSpec defines the desired state of ModifySet. 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].args`.' type: string match: - description: Match selects objects to apply mutations to. + 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: items: @@ -125,18 +149,40 @@ spec: type: string type: object parameters: + description: Parameters define the behavior of the mutator. properties: - assign: - description: Assign.value holds the value to be assigned + operation: + default: merge + description: Operation describes whether values should be merged in ("merge"), or pruned ("prune"). Default value is "merge" + enum: + - merge + - prune + type: string + pathTests: + description: PathTests are a series of existence tests that can be checked before a mutation is applied + 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 + values: + description: Values describes the values provided to the operation as `values.fromList`. type: object x-kubernetes-preserve-unknown-fields: true type: object type: object status: - description: AssignMetadataStatus defines the observed state of AssignMetadata. + description: ModifySetStatus defines the observed state of ModifySet. properties: byPod: - description: 'INSERT ADDITIONAL STATUS FIELD - define observed state of cluster Important: Run "make" to regenerate code after modifying this file' items: description: MutatorPodStatusStatus defines the observed state of MutatorPodStatus. properties: diff --git a/manifest_staging/charts/gatekeeper/templates/gatekeeper-mutating-webhook-configuration-mutatingwebhookconfiguration.yaml b/manifest_staging/charts/gatekeeper/templates/gatekeeper-mutating-webhook-configuration-mutatingwebhookconfiguration.yaml index 8a860a48839..6c94bc2abbb 100644 --- a/manifest_staging/charts/gatekeeper/templates/gatekeeper-mutating-webhook-configuration-mutatingwebhookconfiguration.yaml +++ b/manifest_staging/charts/gatekeeper/templates/gatekeeper-mutating-webhook-configuration-mutatingwebhookconfiguration.yaml @@ -6,6 +6,7 @@ metadata: labels: app: '{{ template "gatekeeper.name" . }}' chart: '{{ template "gatekeeper.name" . }}' + gatekeeper.sh/system: "yes" heritage: '{{ .Release.Service }}' release: '{{ .Release.Name }}' name: gatekeeper-mutating-webhook-configuration diff --git a/pkg/controller/add_modifyset.go b/pkg/controller/add_modifyset.go new file mode 100644 index 00000000000..c1efe3ab1d1 --- /dev/null +++ b/pkg/controller/add_modifyset.go @@ -0,0 +1,24 @@ +/* + +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 controller + +import ( + "github.com/open-policy-agent/gatekeeper/pkg/controller/mutators/modifyset" +) + +func init() { + Injectors = append(Injectors, &modifyset.Adder{}) +} diff --git a/pkg/controller/config/config_controller_test.go b/pkg/controller/config/config_controller_test.go index bab111809cc..3d84d8a5a4b 100644 --- a/pkg/controller/config/config_controller_test.go +++ b/pkg/controller/config/config_controller_test.go @@ -570,10 +570,9 @@ func TestConfig_Retries(t *testing.T) { // Create the Config object and expect the Reconcile to be created ctx = context.Background() - err = c.Create(ctx, instance) - if err != nil { - t.Fatal(err) - } + g.Eventually(func() error { + return c.Create(ctx, instance.DeepCopy()) + }, timeout).Should(gomega.BeNil()) defer func() { ctx := context.Background() diff --git a/pkg/controller/mutators/modifyset/modifyset_controller.go b/pkg/controller/mutators/modifyset/modifyset_controller.go new file mode 100644 index 00000000000..e190f219e90 --- /dev/null +++ b/pkg/controller/mutators/modifyset/modifyset_controller.go @@ -0,0 +1,77 @@ +/* + + +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 modifyset + +import ( + "context" + + opa "github.com/open-policy-agent/frameworks/constraint/pkg/client" + mutationsv1alpha1 "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/mutation" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" + "github.com/open-policy-agent/gatekeeper/pkg/readiness" + "github.com/open-policy-agent/gatekeeper/pkg/watch" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" +) + +type Adder struct { + MutationSystem *mutation.System + Tracker *readiness.Tracker + GetPod func(context.Context) (*corev1.Pod, error) +} + +// Add creates a new ModifySet Controller and adds it to the Manager. The Manager will set fields on the Controller +// and Start it when the Manager is Started. +func (a *Adder) Add(mgr manager.Manager) error { + adder := core.Adder{ + Tracker: a.Tracker, + GetPod: a.GetPod, + MutationSystem: a.MutationSystem, + Kind: "ModifySet", + NewMutationObj: func() client.Object { return &mutationsv1alpha1.ModifySet{} }, + 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 + modifyset := obj.(*mutationsv1alpha1.ModifySet) // nolint:forcetypeassert + return mutators.MutatorForModifySet(modifyset) + }, + } + return adder.Add(mgr) +} + +func (a *Adder) InjectOpa(o *opa.Client) {} + +func (a *Adder) InjectWatchManager(w *watch.Manager) {} + +func (a *Adder) InjectControllerSwitch(cs *watch.ControllerSwitch) {} + +func (a *Adder) InjectTracker(t *readiness.Tracker) { + a.Tracker = t +} + +func (a *Adder) InjectGetPod(getPod func(context.Context) (*corev1.Pod, error)) { + a.GetPod = getPod +} + +func (a *Adder) InjectMutationSystem(mutationSystem *mutation.System) { + a.MutationSystem = mutationSystem +} diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go index 7f29a327aaf..e66263f5aea 100644 --- a/pkg/logging/logging.go +++ b/pkg/logging/logging.go @@ -21,5 +21,6 @@ const ( ResourceName = "resource_name" RequestUsername = "request_username" MutationApplied = "mutation_applied" + Mutator = "mutator" DebugLevel = 2 // r.log.Debug(foo) == r.log.V(logging.DebugLevel).Info(foo) ) diff --git a/pkg/mutation/mutators/assign_mutator.go b/pkg/mutation/mutators/assign/assign_mutator.go similarity index 87% rename from pkg/mutation/mutators/assign_mutator.go rename to pkg/mutation/mutators/assign/assign_mutator.go index faaaf2072e1..ae2b9e94b2c 100644 --- a/pkg/mutation/mutators/assign_mutator.go +++ b/pkg/mutation/mutators/assign/assign_mutator.go @@ -1,4 +1,4 @@ -package mutators +package assign import ( "encoding/json" @@ -24,11 +24,11 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) -var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation") +var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation", logging.Mutator, "assign") -// AssignMutator is a mutator object built out of a +// Mutator is a mutator object built out of an // Assign instance. -type AssignMutator struct { +type Mutator struct { id types.ID assign *mutationsv1alpha1.Assign assignValue interface{} @@ -41,27 +41,31 @@ type AssignMutator struct { valueTest *mutationsv1alpha1.AssignIf } -// AssignMutator implements mutatorWithSchema. -var _ schema.MutatorWithSchema = &AssignMutator{} +// Mutator implements mutatorWithSchema. +var _ schema.MutatorWithSchema = &Mutator{} -func (m *AssignMutator) Matches(obj client.Object, ns *corev1.Namespace) bool { +func (m *Mutator) Matches(obj client.Object, ns *corev1.Namespace) bool { if !match.AppliesTo(m.assign.Spec.ApplyTo, obj) { return false } matches, err := match.Matches(&m.assign.Spec.Match, obj, ns) if err != nil { - log.Error(err, "AssignMutator.Matches failed", "assign", m.assign.Name) + log.Error(err, "Matches failed for assign", "assign", m.assign.Name) return false } return matches } -func (m *AssignMutator) Mutate(obj *unstructured.Unstructured) (bool, error) { - return core.Mutate(m, m.tester, m.testValue, obj) +func (m *Mutator) TerminalType() parser.NodeType { + return schema.Unknown +} + +func (m *Mutator) Mutate(obj *unstructured.Unstructured) (bool, error) { + return core.Mutate(m.Path(), m.tester, m.testValue, core.NewDefaultSetter(m), obj) } // valueTest returns true if it is okay for the mutation func to override the value. -func (m *AssignMutator) testValue(v interface{}, exists bool) bool { +func (m *Mutator) testValue(v interface{}, exists bool) bool { if len(m.valueTest.In) != 0 { ifInMatched := false if !exists { @@ -92,20 +96,20 @@ func (m *AssignMutator) testValue(v interface{}, exists bool) bool { return true } -func (m *AssignMutator) ID() types.ID { +func (m *Mutator) ID() types.ID { return m.id } -func (m *AssignMutator) SchemaBindings() []runtimeschema.GroupVersionKind { +func (m *Mutator) SchemaBindings() []runtimeschema.GroupVersionKind { return m.bindings } -func (m *AssignMutator) Value() (interface{}, error) { +func (m *Mutator) Value() (interface{}, error) { return runtime.DeepCopyJSONValue(m.assignValue), nil } -func (m *AssignMutator) HasDiff(mutator types.Mutator) bool { - toCheck, ok := mutator.(*AssignMutator) +func (m *Mutator) HasDiff(mutator types.Mutator) bool { + toCheck, ok := mutator.(*Mutator) if !ok { // different types, different return true } @@ -128,12 +132,12 @@ func (m *AssignMutator) HasDiff(mutator types.Mutator) bool { return false } -func (m *AssignMutator) Path() parser.Path { +func (m *Mutator) Path() parser.Path { return m.path } -func (m *AssignMutator) DeepCopy() types.Mutator { - res := &AssignMutator{ +func (m *Mutator) DeepCopy() types.Mutator { + res := &Mutator{ id: m.id, assign: m.assign.DeepCopy(), assignValue: runtime.DeepCopyJSONValue(m.assignValue), @@ -150,13 +154,13 @@ func (m *AssignMutator) DeepCopy() types.Mutator { return res } -func (m *AssignMutator) String() string { +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 AssignMutator built from +// MutatorForAssign returns an mutator built from // the given assign instance. -func MutatorForAssign(assign *mutationsv1alpha1.Assign) (*AssignMutator, error) { +func MutatorForAssign(assign *mutationsv1alpha1.Assign) (*Mutator, error) { path, err := parser.Parse(assign.Spec.Location) if err != nil { return nil, errors.Wrapf(err, "invalid location format `%s` for Assign %s", assign.Spec.Location, assign.GetName()) @@ -212,7 +216,7 @@ func MutatorForAssign(assign *mutationsv1alpha1.Assign) (*AssignMutator, error) return nil, fmt.Errorf("applyTo required for Assign mutator %s", assign.GetName()) } - return &AssignMutator{ + return &Mutator{ id: id, assign: assign.DeepCopy(), assignValue: value, diff --git a/pkg/mutation/mutators/assign_mutator_benchmark_test.go b/pkg/mutation/mutators/assign/assign_mutator_benchmark_test.go similarity index 99% rename from pkg/mutation/mutators/assign_mutator_benchmark_test.go rename to pkg/mutation/mutators/assign/assign_mutator_benchmark_test.go index 23942d0c784..e100387ce87 100644 --- a/pkg/mutation/mutators/assign_mutator_benchmark_test.go +++ b/pkg/mutation/mutators/assign/assign_mutator_benchmark_test.go @@ -1,4 +1,4 @@ -package mutators +package assign import ( "fmt" diff --git a/pkg/mutation/mutators/assign_mutator_test.go b/pkg/mutation/mutators/assign/assign_mutator_test.go similarity index 99% rename from pkg/mutation/mutators/assign_mutator_test.go rename to pkg/mutation/mutators/assign/assign_mutator_test.go index a87a8990fce..311845ff068 100644 --- a/pkg/mutation/mutators/assign_mutator_test.go +++ b/pkg/mutation/mutators/assign/assign_mutator_test.go @@ -1,4 +1,4 @@ -package mutators +package assign import ( "encoding/json" @@ -39,7 +39,7 @@ func makeValue(v interface{}) runtime.RawExtension { return runtime.RawExtension{Raw: j} } -func newAssignMutator(cfg *assignTestCfg) *AssignMutator { +func newAssignMutator(cfg *assignTestCfg) *Mutator { m := &mutationsv1alpha1.Assign{ ObjectMeta: metav1.ObjectMeta{ Name: "Foo", diff --git a/pkg/mutation/mutators/assignmeta_mutator.go b/pkg/mutation/mutators/assignmeta/assignmeta_mutator.go similarity index 79% rename from pkg/mutation/mutators/assignmeta_mutator.go rename to pkg/mutation/mutators/assignmeta/assignmeta_mutator.go index 7e6fb352836..4a74f01e1b2 100644 --- a/pkg/mutation/mutators/assignmeta_mutator.go +++ b/pkg/mutation/mutators/assignmeta/assignmeta_mutator.go @@ -1,4 +1,4 @@ -package mutators +package assignmeta import ( "encoding/json" @@ -7,6 +7,7 @@ import ( "github.com/google/go-cmp/cmp" mutationsv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" + "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" @@ -16,8 +17,11 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" ) +var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation", logging.Mutator, "assignmeta") + var ( labelsValidSubPath = []parser.Node{ &parser.Object{ @@ -38,9 +42,9 @@ var ( } ) -// AssignMetadataMutator is a mutator built out of an +// Mutator is a mutator built out of an // AssignMeta instance. -type AssignMetadataMutator struct { +type Mutator struct { id types.ID assignMetadata *mutationsv1alpha1.AssignMetadata assignValue string @@ -50,36 +54,36 @@ type AssignMetadataMutator struct { tester *tester.Tester } -// assignMetadataMutator implements mutator. -var _ types.Mutator = &AssignMetadataMutator{} +// Mutator implements mutator. +var _ types.Mutator = &Mutator{} -func (m *AssignMetadataMutator) Matches(obj client.Object, ns *corev1.Namespace) bool { +func (m *Mutator) Matches(obj client.Object, ns *corev1.Namespace) bool { matches, err := match.Matches(&m.assignMetadata.Spec.Match, obj, ns) if err != nil { - log.Error(err, "AssignMetadataMutator.Matches failed", "assignMeta", m.assignMetadata.Name) + log.Error(err, "Matches failed for assign metadata", "assignMeta", m.assignMetadata.Name) return false } return matches } -func (m *AssignMetadataMutator) Mutate(obj *unstructured.Unstructured) (bool, error) { +func (m *Mutator) Mutate(obj *unstructured.Unstructured) (bool, error) { // Note: Performance here can be improved by ~3x by writing a specialized // function instead of using a generic function. AssignMetadata only ever // mutates metadata.annotations or metadata.labels, and we spend ~70% of // compute covering cases that aren't valid for this Mutator. - return core.Mutate(m, m.tester, nil, obj) + return core.Mutate(m.path, m.tester, nil, core.NewDefaultSetter(m), obj) } -func (m *AssignMetadataMutator) ID() types.ID { +func (m *Mutator) ID() types.ID { return m.id } -func (m *AssignMetadataMutator) Path() parser.Path { +func (m *Mutator) Path() parser.Path { return m.path } -func (m *AssignMetadataMutator) HasDiff(mutator types.Mutator) bool { - toCheck, ok := mutator.(*AssignMetadataMutator) +func (m *Mutator) HasDiff(mutator types.Mutator) bool { + toCheck, ok := mutator.(*Mutator) if !ok { // different types, different return true } @@ -95,8 +99,8 @@ func (m *AssignMetadataMutator) HasDiff(mutator types.Mutator) bool { return false } -func (m *AssignMetadataMutator) DeepCopy() types.Mutator { - res := &AssignMetadataMutator{ +func (m *Mutator) DeepCopy() types.Mutator { + res := &Mutator{ id: m.id, assignMetadata: m.assignMetadata.DeepCopy(), assignValue: m.assignValue, @@ -106,16 +110,16 @@ func (m *AssignMetadataMutator) DeepCopy() types.Mutator { return res } -func (m *AssignMetadataMutator) Value() (interface{}, error) { +func (m *Mutator) Value() (interface{}, error) { return m.assignValue, nil } -func (m *AssignMetadataMutator) String() string { +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 AssignMetadataMutator from the given AssignMetadata object. -func MutatorForAssignMetadata(assignMeta *mutationsv1alpha1.AssignMetadata) (*AssignMetadataMutator, error) { +// MutatorForAssignMetadata builds an Mutator from the given AssignMetadata object. +func MutatorForAssignMetadata(assignMeta *mutationsv1alpha1.AssignMetadata) (*Mutator, error) { path, err := parser.Parse(assignMeta.Spec.Location) if err != nil { return nil, errors.Wrapf(err, "invalid location format for AssignMetadata %s: %s", assignMeta.GetName(), assignMeta.Spec.Location) @@ -145,7 +149,7 @@ func MutatorForAssignMetadata(assignMeta *mutationsv1alpha1.AssignMetadata) (*As return nil, err } - return &AssignMetadataMutator{ + return &Mutator{ id: types.MakeID(assignMeta), assignMetadata: assignMeta.DeepCopy(), assignValue: valueString, diff --git a/pkg/mutation/mutators/assignmeta_mutator_benchmark_test.go b/pkg/mutation/mutators/assignmeta/assignmeta_mutator_benchmark_test.go similarity index 85% rename from pkg/mutation/mutators/assignmeta_mutator_benchmark_test.go rename to pkg/mutation/mutators/assignmeta/assignmeta_mutator_benchmark_test.go index 72a10543cbc..0dd0541e9cb 100644 --- a/pkg/mutation/mutators/assignmeta_mutator_benchmark_test.go +++ b/pkg/mutation/mutators/assignmeta/assignmeta_mutator_benchmark_test.go @@ -1,12 +1,25 @@ -package mutators +package assignmeta import ( + "encoding/json" "testing" "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ) +func makeValue(v interface{}) runtime.RawExtension { + v2 := map[string]interface{}{ + "value": v, + } + j, err := json.Marshal(v2) + if err != nil { + panic(err) + } + return runtime.RawExtension{Raw: j} +} + func assignMetadata(value interface{}, location string) *v1alpha1.AssignMetadata { result := &v1alpha1.AssignMetadata{ Spec: v1alpha1.AssignMetadataSpec{ diff --git a/pkg/mutation/mutators/conversion.go b/pkg/mutation/mutators/conversion.go new file mode 100644 index 00000000000..ed0f423772f --- /dev/null +++ b/pkg/mutation/mutators/conversion.go @@ -0,0 +1,24 @@ +package mutators + +import ( + mutationsv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignmeta" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/modifyset" +) + +// MutatorForAssign returns an AssignMutator built from +// the given assign instance. +func MutatorForAssign(a *mutationsv1alpha1.Assign) (*assign.Mutator, error) { + return assign.MutatorForAssign(a) +} + +// MutatorForAssignMetadata builds an AssignMetadataMutator from the given AssignMetadata object. +func MutatorForAssignMetadata(assignMeta *mutationsv1alpha1.AssignMetadata) (*assignmeta.Mutator, error) { + return assignmeta.MutatorForAssignMetadata(assignMeta) +} + +// MutatorForModifySet builds an AssignMetadataMutator from the given ModifySet object. +func MutatorForModifySet(modifySet *mutationsv1alpha1.ModifySet) (*modifyset.Mutator, error) { + return modifyset.MutatorForModifySet(modifySet) +} diff --git a/pkg/mutation/mutators/core/errors.go b/pkg/mutation/mutators/core/errors.go new file mode 100644 index 00000000000..fc11957244b --- /dev/null +++ b/pkg/mutation/mutators/core/errors.go @@ -0,0 +1,7 @@ +package core + +import "errors" + +// ErrNonKeyedSetter occurs when a setter that doesn't understand keyed lists +// is called against a keyed list. +var ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists") diff --git a/pkg/mutation/mutators/core/mutation_function.go b/pkg/mutation/mutators/core/mutation_function.go index af922571886..3cf79b8e3bd 100644 --- a/pkg/mutation/mutators/core/mutation_function.go +++ b/pkg/mutation/mutators/core/mutation_function.go @@ -14,10 +14,79 @@ import ( var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation") -func Mutate(mutator types.Mutator, tester *path.Tester, valueTest func(interface{}, bool) bool, obj *unstructured.Unstructured) (bool, error) { - s := &mutatorState{mutator: mutator, tester: tester, valueTest: valueTest} - if len(mutator.Path().Nodes) == 0 { - return false, fmt.Errorf("mutator %v has an empty target location", mutator.ID()) +// Setter tells the mutate function what to do once we have found the +// node that needs mutating. +type Setter interface { + // SetValue takes the object that needs mutating and the key of the + // field on that object that should be mutated. It is up to the + // implementor to actually mutate the object. + SetValue(obj map[string]interface{}, key string) error + + // KeyedListOkay returns whether this setter can handle keyed lists. + // If it can't, an attempt to mutate a keyed-list-type field will + // result in an error. + KeyedListOkay() bool + + // KeyedListValue is the value that will be assigned to the + // targeted keyed list entry. Unline SetValue(), this does + // not do mutation directly. + KeyedListValue() (map[string]interface{}, error) +} + +var _ Setter = &DefaultSetter{} + +func NewDefaultSetter(m types.Mutator) *DefaultSetter { + return &DefaultSetter{mutator: m} +} + +// DefaultSetter is a setter that merely sets the value at the specified path +// to the provided value. No special logic, like set merging. +type DefaultSetter struct { + mutator types.Mutator +} + +func (s *DefaultSetter) KeyedListOkay() bool { return true } + +func (s *DefaultSetter) KeyedListValue() (map[string]interface{}, error) { + value, err := s.mutator.Value() + if err != nil { + log.Error(err, "error getting mutator value for mutator %+v", s.mutator) + return nil, err + } + valueAsObject, ok := value.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("assign.value for keyed list mutator %s is not an object", s.mutator.ID()) + } + return valueAsObject, nil +} + +func (s *DefaultSetter) SetValue(obj map[string]interface{}, key string) error { + value, err := s.mutator.Value() + if err != nil { + return err + } + obj[key] = value + return nil +} + +func Mutate( + path parser.Path, + tester *path.Tester, + valueTest func(interface{}, bool) bool, + setter Setter, + obj *unstructured.Unstructured, +) (bool, error) { + if setter == nil { + return false, errors.New("setter must not be nil") + } + s := &mutatorState{ + path: path, + tester: tester, + valueTest: valueTest, + setter: setter, + } + if len(path.Nodes) == 0 { + return false, errors.New("attempting to mutate an empty target location") } if obj == nil { return false, errors.New("attempting to mutate a nil object") @@ -27,17 +96,18 @@ func Mutate(mutator types.Mutator, tester *path.Tester, valueTest func(interface } type mutatorState struct { - mutator types.Mutator - tester *path.Tester + path parser.Path + tester *path.Tester // valueTest takes the input value and whether that value already existed. // It returns true if the value should be mutated valueTest func(interface{}, bool) bool + setter Setter } // mutateInternal mutates the resource recursively. It returns false if there has been no change // to any downstream objects in the tree, indicating that the mutation should not be persisted. func (s *mutatorState) mutateInternal(current interface{}, depth int) (bool, interface{}, error) { - pathEntry := s.mutator.Path().Nodes[depth] + pathEntry := s.path.Nodes[depth] switch castPathEntry := pathEntry.(type) { case *parser.Object: currentAsObject, ok := current.(map[string]interface{}) @@ -55,15 +125,13 @@ func (s *mutatorState) mutateInternal(current interface{}, depth int) (bool, int } } // we have hit the end of our path, this is the base case - if len(s.mutator.Path().Nodes)-1 == depth { + if len(s.path.Nodes)-1 == depth { if s.valueTest != nil && !s.valueTest(next, exists) { return false, nil, nil } - value, err := s.mutator.Value() - if err != nil { + if err := s.setter.SetValue(currentAsObject, castPathEntry.Reference); err != nil { return false, nil, err } - currentAsObject[castPathEntry.Reference] = value return true, currentAsObject, nil } if !exists { // Next element is missing and needs to be added @@ -89,7 +157,11 @@ func (s *mutatorState) mutateInternal(current interface{}, depth int) (bool, int } shallowCopy := make([]interface{}, len(currentAsList)) copy(shallowCopy, currentAsList) - if len(s.mutator.Path().Nodes)-1 == depth { + // base case + if len(s.path.Nodes)-1 == depth { + if !s.setter.KeyedListOkay() { + return false, nil, ErrNonKeyedSetter + } return s.setListElementToValue(shallowCopy, castPathEntry, depth) } @@ -150,15 +222,11 @@ func (s *mutatorState) setListElementToValue(currentAsList []interface{}, listPa if listPathEntry.Glob { return false, nil, fmt.Errorf("last path entry can not be globbed") } - newValue, err := s.mutator.Value() + + newValueAsObject, err := s.setter.KeyedListValue() if err != nil { - log.Error(err, "error getting mutator value for mutator %+v", s.mutator) return false, nil, err } - newValueAsObject, ok := newValue.(map[string]interface{}) - if !ok { - return false, nil, fmt.Errorf("last path entry of type list requires an object value, pathEntry: %+v", listPathEntry) - } key := listPathEntry.KeyField if listPathEntry.KeyValue == nil { @@ -189,8 +257,8 @@ func (s *mutatorState) setListElementToValue(currentAsList []interface{}, listPa func (s *mutatorState) createMissingElement(depth int) (interface{}, error) { var next interface{} - pathEntry := s.mutator.Path().Nodes[depth] - nextPathEntry := s.mutator.Path().Nodes[depth+1] + pathEntry := s.path.Nodes[depth] + nextPathEntry := s.path.Nodes[depth+1] // Create new element of type switch nextPathEntry.(type) { diff --git a/pkg/mutation/mutators/core/mutation_function_setter_test.go b/pkg/mutation/mutators/core/mutation_function_setter_test.go new file mode 100644 index 00000000000..b22487c57b4 --- /dev/null +++ b/pkg/mutation/mutators/core/mutation_function_setter_test.go @@ -0,0 +1,36 @@ +package core + +import ( + "testing" + + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +type notKeyedSetter struct{} + +func (s *notKeyedSetter) KeyedListOkay() bool { return false } + +func (s *notKeyedSetter) KeyedListValue() (map[string]interface{}, error) { + panic("notKeyedSetter setter does not handle keyed lists") +} + +func (s *notKeyedSetter) SetValue(obj map[string]interface{}, key string) error { + panic("NOT IMPLEMENTED") +} + +func TestKeyedListIncompatible(t *testing.T) { + path, err := parser.Parse(`spec.containers[name: "foo"]`) + if err != nil { + t.Fatal(err) + } + obj := &unstructured.Unstructured{Object: map[string]interface{}{}} + m, err := Mutate(path, &tester.Tester{}, nil, ¬KeyedSetter{}, obj) + if err != ErrNonKeyedSetter { + t.Errorf("wanted err = %+v, got %+v", ErrNonKeyedSetter, err) + } + if m != false { + t.Error("expected m=false, got true") + } +} diff --git a/pkg/mutation/mutators/modifyset/modify_set_mutator.go b/pkg/mutation/mutators/modifyset/modify_set_mutator.go new file mode 100644 index 00000000000..1bfc60010bd --- /dev/null +++ b/pkg/mutation/mutators/modifyset/modify_set_mutator.go @@ -0,0 +1,307 @@ +package modifyset + +import ( + "fmt" + "reflect" + "sort" + + "github.com/google/go-cmp/cmp" + mutationsv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" + "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" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/schema" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + runtimeschema "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation", logging.Mutator, "modifyset") + +// Mutator is a mutator object built out of a +// ModifySet instance. +type Mutator struct { + id types.ID + modifySet *mutationsv1alpha1.ModifySet + + 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(obj client.Object, ns *corev1.Namespace) bool { + if !match.AppliesTo(m.modifySet.Spec.ApplyTo, obj) { + return false + } + matches, err := match.Matches(&m.modifySet.Spec.Match, obj, ns) + if err != nil { + log.Error(err, "Matches failed for modify set", "modifyset", m.modifySet.Name) + return false + } + return matches +} + +func (m *Mutator) TerminalType() parser.NodeType { + return schema.Set +} + +func (m *Mutator) Mutate(obj *unstructured.Unstructured) (bool, error) { + return core.Mutate( + m.Path(), + m.tester, + nil, + setter{ + op: m.modifySet.Spec.Parameters.Operation, + values: runtime.DeepCopyJSONValue(m.modifySet.Spec.Parameters.Values.FromList).([]interface{}), + }, + obj, + ) +} + +func (m *Mutator) ID() types.ID { + return m.id +} + +func (m *Mutator) SchemaBindings() []runtimeschema.GroupVersionKind { + return m.bindings +} + +func (m *Mutator) Value() (interface{}, error) { + return runtime.DeepCopyJSONValue(m.modifySet.Spec.Parameters.Values.FromList), nil +} + +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.modifySet.Spec, m.modifySet.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, + modifySet: m.modifySet.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.modifySet.GetGeneration()) +} + +// MutatorForModifySet returns an Mutator built from +// the given modifyset instance. +func MutatorForModifySet(modifySet *mutationsv1alpha1.ModifySet) (*Mutator, error) { + path, err := parser.Parse(modifySet.Spec.Location) + if err != nil { + return nil, errors.Wrapf(err, "invalid location format `%s` for ModifySet %s", modifySet.Spec.Location, modifySet.GetName()) + } + + if hasMetadataRoot(path) { + return nil, fmt.Errorf("modifyset %s can't change metadata", modifySet.GetName()) + } + + if path.Nodes[len(path.Nodes)-1].Type() == parser.ListNode { + return nil, fmt.Errorf("final node in a modifyset location cannot be a keyed list") + } + + id := types.MakeID(modifySet) + + pathTests, err := gatherPathTests(modifySet) + if err != nil { + return nil, err + } + tester, err := patht.New(path, pathTests) + 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, + modifySet: modifySet.DeepCopy(), + bindings: gvks, + path: path, + tester: tester, + }, nil +} + +func gatherPathTests(modifySet *mutationsv1alpha1.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 *mutationsv1alpha1.ModifySet) error { + if _, err := MutatorForModifySet(modifySet); err != nil { + return err + } + 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 { + values []interface{} + op mutationsv1alpha1.Operation +} + +func (s setter) KeyedListOkay() bool { return false } + +func (s setter) KeyedListValue() (map[string]interface{}, error) { + panic("modifyset setter does not handle keyed lists") +} + +func (s setter) SetValue(obj map[string]interface{}, key string) error { + switch s.op { + case mutationsv1alpha1.MergeOp: + return s.setValueMerge(obj, key) + case mutationsv1alpha1.PruneOp: + return s.setValuePrune(obj, key) + default: + return fmt.Errorf("unrecognized operation for modifyset: %s", s.op) + } +} + +func (s setter) setValueMerge(obj map[string]interface{}, key string) error { + val, ok := obj[key] + // missing list => add all values as a new list. + if !ok { + obj[key] = runtime.DeepCopyJSONValue(s.values) + return nil + } + + vals, ok := val.([]interface{}) + if !ok { + return fmt.Errorf("%+v is not a list of values, cannot treat it as a set", val) + } +outer: + for _, v := range s.values { + for _, existing := range vals { + if cmp.Equal(v, existing) { + continue outer + } + } + // Value does not currently exist, add it. + vals = append(vals, v) + } + obj[key] = vals + return nil +} + +func (s setter) setValuePrune(obj map[string]interface{}, key string) error { + val, ok := obj[key] + // missing list => we're done. + if !ok { + return nil + } + + vals, ok := val.([]interface{}) + if !ok { + return fmt.Errorf("%+v is not a list of values, cannot treat it as a set", val) + } +outer: + for _, v := range s.values { + for i, existing := range vals { + if cmp.Equal(v, existing) { + // we are assuming order is important, otherwise this could be done + // more cheaply by swapping values + vals = append(vals[:i], vals[i+1:]...) + continue outer + } + } + } + obj[key] = vals + return nil +} diff --git a/pkg/mutation/mutators/modifyset/modify_set_mutator_benchmark_test.go b/pkg/mutation/mutators/modifyset/modify_set_mutator_benchmark_test.go new file mode 100644 index 00000000000..2ae1787cd1b --- /dev/null +++ b/pkg/mutation/mutators/modifyset/modify_set_mutator_benchmark_test.go @@ -0,0 +1,115 @@ +package modifyset + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/match" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" +) + +func makeValue(v interface{}) runtime.RawExtension { + v2 := map[string]interface{}{ + "value": v, + } + j, err := json.Marshal(v2) + if err != nil { + panic(err) + } + return runtime.RawExtension{Raw: j} +} + +func modifyset(value interface{}, location string) *v1alpha1.ModifySet { + result := &v1alpha1.ModifySet{ + Spec: v1alpha1.ModifySetSpec{ + ApplyTo: []match.ApplyTo{{ + Groups: []string{"*"}, + Versions: []string{"*"}, + Kinds: []string{"*"}, + }}, + Location: location, + Parameters: v1alpha1.ModifySetParameters{ + Values: v1alpha1.Values{ + FromList: []interface{}{makeValue(value)}, + }, + }, + }, + } + + return result +} + +func benchmarkModifySetMutator(b *testing.B, n int) { + mutator, err := MutatorForModifySet(modifyset("foo", "spec"+strings.Repeat(".spec", n-1))) + 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(obj) + if err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = mutator.Mutate(obj) + } +} + +func benchmarkNoModifySetMutator(b *testing.B, n int) { + path := "spec" + strings.Repeat(".spec", n-1) + a := modifyset("foo", path) + a.Spec.Parameters.PathTests = []v1alpha1.PathTest{{ + SubPath: path, + Condition: tester.MustNotExist, + }} + mutator, err := MutatorForModifySet(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(obj) + if err != nil { + b.Fatal(err) + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = mutator.Mutate(obj) + } +} + +func BenchmarkModifySetMutator_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) { + benchmarkModifySetMutator(b, n) + }) + } + + for _, n := range ns { + b.Run(fmt.Sprintf("never mutate %d-depth", n), func(b *testing.B) { + benchmarkNoModifySetMutator(b, n) + }) + } +} diff --git a/pkg/mutation/mutators/modifyset/modify_set_test.go b/pkg/mutation/mutators/modifyset/modify_set_test.go new file mode 100644 index 00000000000..67ecdf58f3f --- /dev/null +++ b/pkg/mutation/mutators/modifyset/modify_set_test.go @@ -0,0 +1,164 @@ +package modifyset + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + mutationsv1alpha1 "github.com/open-policy-agent/gatekeeper/apis/mutations/v1alpha1" +) + +func TestSetterMerge(t *testing.T) { + tcs := []struct { + name string + vals []interface{} + existing interface{} + expected []interface{} + errExpected bool + }{ + { + name: "Error on non-list", + vals: []interface{}{"a"}, + existing: 7, + errExpected: true, + }, + { + name: "Empty vals", + vals: []interface{}{}, + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"a", "b", "c"}, + }, + { + name: "Nil vals", + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"a", "b", "c"}, + }, + { + name: "Duplicate vals", + vals: []interface{}{"a", "b"}, + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"a", "b", "c"}, + }, + { + name: "Overlapping vals", + vals: []interface{}{"a", "b", "d"}, + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"a", "b", "c", "d"}, + }, + { + name: "Nil existing", + vals: []interface{}{"a", "b", "d"}, + expected: []interface{}{"a", "b", "d"}, + }, + { + name: "Empty list existing", + vals: []interface{}{"a", "b", "d"}, + existing: []interface{}{}, + expected: []interface{}{"a", "b", "d"}, + }, + { + name: "Non-standard members", + vals: []interface{}{[]interface{}{"a", "b", "d"}, []interface{}{"z", "y", "q"}}, + existing: []interface{}{[]interface{}{"a", "b", "d"}, []interface{}{"r", "r", "r"}}, + expected: []interface{}{[]interface{}{"a", "b", "d"}, []interface{}{"r", "r", "r"}, []interface{}{"z", "y", "q"}}, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + s := setter{ + values: tc.vals, + op: mutationsv1alpha1.MergeOp, + } + obj := map[string]interface{}{} + if tc.existing != nil { + obj["field"] = tc.existing + } + err := s.SetValue(obj, "field") + if err != nil != tc.errExpected { + t.Errorf("err = %+v, wanted %v", err, tc.errExpected) + } + if tc.expected != nil { + if !cmp.Equal(tc.expected, obj["field"]) { + t.Errorf("got %+v, wanted %+v", obj["field"], tc.expected) + } + } + }) + } +} + +func TestSetterPrune(t *testing.T) { + tcs := []struct { + name string + vals []interface{} + existing interface{} + expected []interface{} + errExpected bool + }{ + { + name: "Error on non-list", + vals: []interface{}{"a"}, + existing: 7, + errExpected: true, + }, + { + name: "Empty vals", + vals: []interface{}{}, + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"a", "b", "c"}, + }, + { + name: "Nil vals", + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"a", "b", "c"}, + }, + { + name: "Duplicate vals", + vals: []interface{}{"a", "b"}, + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"c"}, + }, + { + name: "Overlapping vals", + vals: []interface{}{"a", "b", "d"}, + existing: []interface{}{"a", "b", "c"}, + expected: []interface{}{"c"}, + }, + { + name: "Nil existing", + vals: []interface{}{"a", "b", "d"}, + expected: nil, + }, + { + name: "Empty list existing", + vals: []interface{}{"a", "b", "d"}, + existing: []interface{}{}, + expected: []interface{}{}, + }, + { + name: "Non-standard members", + vals: []interface{}{[]interface{}{"a", "b", "d"}, []interface{}{"z", "y", "q"}}, + existing: []interface{}{[]interface{}{"a", "b", "d"}, []interface{}{"r", "r", "r"}}, + expected: []interface{}{[]interface{}{"r", "r", "r"}}, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + s := setter{ + values: tc.vals, + op: mutationsv1alpha1.PruneOp, + } + obj := map[string]interface{}{} + if tc.existing != nil { + obj["field"] = tc.existing + } + err := s.SetValue(obj, "field") + if err != nil != tc.errExpected { + t.Errorf("err = %+v, wanted %v", err, tc.errExpected) + } + if tc.expected != nil { + if !cmp.Equal(tc.expected, obj["field"]) { + t.Errorf("got %+v, wanted %+v", obj["field"], tc.expected) + } + } + }) + } +} diff --git a/pkg/mutation/mutators/testhelpers/dummy_mutator.go b/pkg/mutation/mutators/testhelpers/dummy_mutator.go index 711a6b30099..00c6d073ffb 100644 --- a/pkg/mutation/mutators/testhelpers/dummy_mutator.go +++ b/pkg/mutation/mutators/testhelpers/dummy_mutator.go @@ -53,7 +53,7 @@ func (d *DummyMutator) Matches(obj client.Object, ns *corev1.Namespace) bool { func (d *DummyMutator) Mutate(obj *unstructured.Unstructured) (bool, error) { t, _ := path.New(parser.Path{}, nil) - return core.Mutate(d, t, func(_ interface{}, _ bool) bool { return true }, obj) + return core.Mutate(d.Path(), t, func(_ interface{}, _ bool) bool { return true }, core.NewDefaultSetter(d), obj) } func (d *DummyMutator) String() string { diff --git a/pkg/mutation/schema/node.go b/pkg/mutation/schema/node.go index be3a2f26532..9830ccae676 100644 --- a/pkg/mutation/schema/node.go +++ b/pkg/mutation/schema/node.go @@ -12,10 +12,6 @@ import ( type idSet map[types.ID]bool -// unknown represents a path element we do not know the type of. -// Elements of type unknown do not conflict with path elements of known types. -const unknown = parser.NodeType("Unknown") - func (c idSet) String() string { var keys []string for k := range c { @@ -47,7 +43,7 @@ type node struct { // // If the returned references is non-nil it contains at least two elements, one // of which is the passed id. -func (n *node) Add(id types.ID, path []parser.Node) idSet { +func (n *node) Add(id types.ID, path []parser.Node, terminalType parser.NodeType) idSet { if n.ReferencedBy == nil { n.ReferencedBy = make(map[types.ID]bool) } @@ -68,7 +64,7 @@ func (n *node) Add(id types.ID, path []parser.Node) idSet { if n.Children[childKey] == nil { n.Children[childKey] = make(map[parser.NodeType]node) } - childType := headType(path) + childType := headType(path, terminalType) if _, exists := n.Children[childKey][childType]; !exists { n.Children[childKey][childType] = node{} } @@ -76,7 +72,7 @@ func (n *node) Add(id types.ID, path []parser.Node) idSet { // Add the remaining path to the appropriate child, collecting any conflicts // found when adding it. child := n.Children[childKey][childType] - conflicts := child.Add(id, path[1:]) + conflicts := child.Add(id, path[1:], terminalType) n.Children[childKey][childType] = child // Detect conflicts at this node. @@ -90,7 +86,7 @@ const ErrNotFound = util.Error("path not found") // Remove removes the id and path from the tree. // Panics if the ID is not defined or was Add()ed with a different path. -func (n *node) Remove(id types.ID, path []parser.Node) { +func (n *node) Remove(id types.ID, path []parser.Node, terminalType parser.NodeType) { // This ID no longer references this node. if _, isReferenced := n.ReferencedBy[id]; isReferenced { delete(n.ReferencedBy, id) @@ -108,7 +104,7 @@ func (n *node) Remove(id types.ID, path []parser.Node) { // The child does not exist. panic(fmt.Errorf("no child for key %q: %w", childKey, ErrNotFound)) } - childType := headType(path) + childType := headType(path, terminalType) if _, found := n.Children[childKey][childType]; !found { // No child of the key and type exists. // This is how we detect that the path for id is incomplete. If the path @@ -118,7 +114,7 @@ func (n *node) Remove(id types.ID, path []parser.Node) { } child := n.Children[childKey][childType] - child.Remove(id, path[1:]) + child.Remove(id, path[1:], terminalType) // Delete the type from the child if it is no longer referenced. if len(child.ReferencedBy) == 0 { @@ -139,14 +135,14 @@ func (n *node) conflicts(childKey string) idSet { // Count the number of distinct types with this key. nTypes := len(n.Children[childKey]) - if _, hasUnknown := n.Children[childKey][unknown]; hasUnknown { + if _, hasUnknown := n.Children[childKey][Unknown]; hasUnknown { // Nodes whose types we are unable to determine do not count against this // check. nTypes-- } for nodeType, child := range n.Children[childKey] { - if nodeType == unknown { + if nodeType == Unknown { // If we don't know the type of a node, we assume it conflicts with nothing. continue } @@ -167,13 +163,13 @@ func (n *node) conflicts(childKey string) idSet { // passed path. // // Returns an error if the path does not exist. -func (n *node) HasConflicts(path []parser.Node) (bool, error) { +func (n *node) HasConflicts(path []parser.Node, terminalType parser.NodeType) (bool, error) { if len(path) == 0 { return false, nil } childKey := key(path[0]) - childType := headType(path) + childType := headType(path, terminalType) if _, found := n.Children[childKey]; !found { // Path has not been added, so there can be no conflicts. return false, ErrNotFound @@ -189,7 +185,7 @@ func (n *node) HasConflicts(path []parser.Node) (bool, error) { return false, ErrNotFound } child := n.Children[childKey][childType] - return child.HasConflicts(path[1:]) + return child.HasConflicts(path[1:], terminalType) } // merge inserts elements from `from` into `into`. Returns `into`, or a @@ -210,10 +206,10 @@ func merge(into, from idSet) idSet { // headType returns the type of the second Node, if it exists. // This is essential for determining whether the current location in a schema // path is a list. -func headType(path []parser.Node) parser.NodeType { +func headType(path []parser.Node, terminalType parser.NodeType) parser.NodeType { if len(path) < 2 { - // Default unknown as we have no information either way. - return unknown + // Default to the terminal type, as we are at the last path node. + return terminalType } return path[1].Type() } diff --git a/pkg/mutation/schema/node_test.go b/pkg/mutation/schema/node_test.go index ca93f30fd45..e21581fe948 100644 --- a/pkg/mutation/schema/node_test.go +++ b/pkg/mutation/schema/node_test.go @@ -13,7 +13,8 @@ import ( type idPath struct { types.ID - path string + path string + terminalType parser.NodeType } func id(name string) types.ID { @@ -29,7 +30,11 @@ func ids(names ...string) idSet { } func ip(name string, path string) idPath { - return idPath{ID: id(name), path: path} + return idPath{ID: id(name), path: path, terminalType: Unknown} +} + +func ipt(name string, path string, terminalType parser.NodeType) idPath { + return idPath{ID: id(name), path: path, terminalType: terminalType} } func gvk(group, version, kind string) schema.GroupVersionKind { @@ -84,6 +89,14 @@ func TestNode_Add(t *testing.T) { add: ip("list", "spec.containers.image"), want: nil, }, + { + name: "no conflict if ambiguous Set", + before: []idPath{ + ip("object", "spec.containers"), + }, + add: ipt("set", "spec.containers", Set), + want: nil, + }, { name: "list vs. object conflict", before: []idPath{ @@ -95,6 +108,28 @@ func TestNode_Add(t *testing.T) { id("list"): true, }, }, + { + name: "list vs. set conflict", + before: []idPath{ + ip("list", "spec.containers[name: foo]"), + }, + add: ipt("set", "spec.containers", Set), + want: map[types.ID]bool{ + id("list"): true, + id("set"): true, + }, + }, + { + name: "obj vs. set conflict", + before: []idPath{ + ip("object", "spec.containers.name"), + }, + add: ipt("set", "spec.containers", Set), + want: map[types.ID]bool{ + id("object"): true, + id("set"): true, + }, + }, { name: "list key field conflict", before: []idPath{ @@ -129,14 +164,14 @@ func TestNode_Add(t *testing.T) { if err != nil { t.Fatal(err) } - root.Add(p.ID, path.Nodes) + root.Add(p.ID, path.Nodes, p.terminalType) } path, err := parser.Parse(tc.add.path) if err != nil { t.Fatal(err) } - conflicts := root.Add(tc.add.ID, path.Nodes) + conflicts := root.Add(tc.add.ID, path.Nodes, tc.add.terminalType) if diff := cmp.Diff(tc.want, conflicts); diff != "" { t.Error(diff) } @@ -192,7 +227,7 @@ func TestNode_RemovePanic(t *testing.T) { if err != nil { t.Fatal(err) } - root.Add(p.ID, path.Nodes) + root.Add(p.ID, path.Nodes, Unknown) } pRemove, err := parser.Parse(tc.toRemove.path) @@ -207,7 +242,7 @@ func TestNode_RemovePanic(t *testing.T) { t.Errorf("expected Remove to succeed but panicked: %v", r) } }() - root.Remove(tc.toRemove.ID, pRemove.Nodes) + root.Remove(tc.toRemove.ID, pRemove.Nodes, Unknown) }) } } @@ -218,6 +253,7 @@ func TestNode_Remove(t *testing.T) { before []idPath toRemove []idPath toCheck string + terminalType parser.NodeType wantConflictBefore bool wantConflictAfter bool }{ @@ -232,6 +268,17 @@ func TestNode_Remove(t *testing.T) { wantConflictBefore: true, wantConflictAfter: false, }, + { + name: "remove set conflict same key", + before: []idPath{ + ip("object", "spec.containers.hello"), + ipt("set", "spec.containers", Set), + }, + toRemove: []idPath{ipt("set", "spec.containers", Set)}, + toCheck: "spec.containers.hello", + wantConflictBefore: true, + wantConflictAfter: false, + }, { name: "remove object conflict different key", before: []idPath{ @@ -254,6 +301,17 @@ func TestNode_Remove(t *testing.T) { wantConflictBefore: true, wantConflictAfter: false, }, + { + name: "remove list-set conflict", + before: []idPath{ + ipt("set", "spec.containers", Set), + ip("list", "spec.containers[name: foo]"), + }, + toRemove: []idPath{ipt("set", "spec.containers", Set)}, + toCheck: "spec.containers[name: foo]", + wantConflictBefore: true, + wantConflictAfter: false, + }, { name: "multiple conflicts", before: []idPath{ @@ -316,14 +374,14 @@ func TestNode_Remove(t *testing.T) { if err != nil { t.Fatal(err) } - root.Add(p.ID, path.Nodes) + root.Add(p.ID, path.Nodes, p.terminalType) } pCheck, err := parser.Parse(tc.toCheck) if err != nil { t.Fatal(err) } - gotConflictBefore, gotConflictBeforeErr := root.HasConflicts(pCheck.Nodes) + gotConflictBefore, gotConflictBeforeErr := root.HasConflicts(pCheck.Nodes, Unknown) if gotConflictBefore != tc.wantConflictBefore { t.Errorf("before remove got HasConflicts(%q) = %t, want %t", tc.toCheck, gotConflictBefore, tc.wantConflictBefore) @@ -338,10 +396,10 @@ func TestNode_Remove(t *testing.T) { if err != nil { t.Fatal(err) } - root.Remove(toRemove.ID, pRemove.Nodes) + root.Remove(toRemove.ID, pRemove.Nodes, toRemove.terminalType) } - gotConflictAfter, gotConflictAfterErr := root.HasConflicts(pCheck.Nodes) + gotConflictAfter, gotConflictAfterErr := root.HasConflicts(pCheck.Nodes, Unknown) if gotConflictAfter != tc.wantConflictAfter { t.Errorf("after remove got HasConflicts(%q) = %t, want %t", tc.toCheck, gotConflictAfter, tc.wantConflictAfter) @@ -360,10 +418,11 @@ func TestNode_Add_Internals(t *testing.T) { // desired. testCases := []struct { - name string - before []string - toAdd string - want node + name string + before []string + toAdd string + terminalType parser.NodeType + want node }{ { name: "just root", @@ -372,7 +431,7 @@ func TestNode_Add_Internals(t *testing.T) { ReferencedBy: ids("added"), Children: map[string]map[parser.NodeType]node{ "spec": { - unknown: node{ + Unknown: node{ ReferencedBy: ids("added"), }, }, @@ -389,7 +448,7 @@ func TestNode_Add_Internals(t *testing.T) { ReferencedBy: ids("0", "added"), Children: map[string]map[parser.NodeType]node{ "spec": { - unknown: node{ + Unknown: node{ ReferencedBy: ids("0", "added"), }, }, @@ -407,7 +466,7 @@ func TestNode_Add_Internals(t *testing.T) { ReferencedBy: ids("added"), Children: map[string]map[parser.NodeType]node{ "name": { - unknown: node{ + Unknown: node{ ReferencedBy: ids("added"), }, }, @@ -428,7 +487,29 @@ func TestNode_Add_Internals(t *testing.T) { ReferencedBy: ids("added"), Children: map[string]map[parser.NodeType]node{ "name": { - unknown: node{ + Unknown: node{ + ReferencedBy: ids("added"), + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "set node", + toAdd: "spec.containers", + terminalType: Set, + want: node{ + ReferencedBy: ids("added"), + Children: map[string]map[parser.NodeType]node{ + "spec": { + parser.ObjectNode: node{ + ReferencedBy: ids("added"), + Children: map[string]map[parser.NodeType]node{ + "containers": { + Set: node{ ReferencedBy: ids("added"), }, }, @@ -452,7 +533,7 @@ func TestNode_Add_Internals(t *testing.T) { ReferencedBy: ids("0"), Children: map[string]map[parser.NodeType]node{ "name": { - unknown: node{ + Unknown: node{ ReferencedBy: ids("0"), }, }, @@ -462,7 +543,7 @@ func TestNode_Add_Internals(t *testing.T) { ReferencedBy: ids("added"), Children: map[string]map[parser.NodeType]node{ "name": { - unknown: node{ + Unknown: node{ ReferencedBy: ids("added"), }, }, @@ -483,7 +564,7 @@ func TestNode_Add_Internals(t *testing.T) { if err != nil { t.Fatal(err) } - root.Add(id(fmt.Sprint(i)), p.Nodes) + root.Add(id(fmt.Sprint(i)), p.Nodes, Unknown) } rootBefore := *root.DeepCopy() @@ -492,13 +573,16 @@ func TestNode_Add_Internals(t *testing.T) { t.Fatal(err) } - root.Add(id("added"), p.Nodes) + if tc.terminalType == parser.NodeType("") { + tc.terminalType = Unknown + } + root.Add(id("added"), p.Nodes, tc.terminalType) if diff := cmp.Diff(tc.want, root, cmpopts.EquateEmpty()); diff != "" { t.Error(diff) } - root.Remove(id("added"), p.Nodes) + root.Remove(id("added"), p.Nodes, tc.terminalType) // We expect that adding and then removing the path causes no change. if diff := cmp.Diff(rootBefore, root, cmpopts.EquateEmpty()); diff != "" { diff --git a/pkg/mutation/schema/schema.go b/pkg/mutation/schema/schema.go index e3123823776..99be79d8608 100644 --- a/pkg/mutation/schema/schema.go +++ b/pkg/mutation/schema/schema.go @@ -4,6 +4,7 @@ import ( "fmt" "sync" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" "k8s.io/apimachinery/pkg/runtime/schema" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -16,6 +17,9 @@ type MutatorWithSchema interface { // SchemaBindings returns the set of GVKs this Mutator applies to. SchemaBindings() []schema.GroupVersionKind + + // TerminalType specifies the inferred type of the last node in a path + TerminalType() parser.NodeType } var log = logf.Log.WithName("mutation_schema") @@ -101,7 +105,7 @@ func (db *DB) upsert(mutator MutatorWithSchema) error { s = &node{} db.schemas[gvk] = s } - newConflicts := s.Add(id, path.Nodes) + newConflicts := s.Add(id, path.Nodes, mutator.TerminalType()) conflicts = merge(conflicts, newConflicts) } @@ -138,7 +142,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) + s.Remove(id, cachedMutator.Path().Nodes, cachedMutator.TerminalType()) db.schemas[gvk] = s if len(s.ReferencedBy) == 0 { @@ -161,7 +165,7 @@ func (db *DB) remove(id types.ID) { mutator := db.cachedMutators[conflictID] hasConflict := false for _, gvk := range mutator.SchemaBindings() { - if isConflict, _ := db.schemas[gvk].HasConflicts(mutator.Path().Nodes); isConflict { + if isConflict, _ := db.schemas[gvk].HasConflicts(mutator.Path().Nodes, mutator.TerminalType()); isConflict { hasConflict = true break } diff --git a/pkg/mutation/schema/schema_test.go b/pkg/mutation/schema/schema_test.go index ceaaf0acb93..5ae05e694e9 100644 --- a/pkg/mutation/schema/schema_test.go +++ b/pkg/mutation/schema/schema_test.go @@ -47,6 +47,10 @@ func (m *fakeMutator) Value() (interface{}, error) { panic("should not be called") } +func (m *fakeMutator) TerminalType() parser.NodeType { + return Unknown +} + func (m *fakeMutator) ID() types.ID { return m.id } func (m *fakeMutator) HasDiff(other types.Mutator) bool { diff --git a/pkg/mutation/schema/terminal_types.go b/pkg/mutation/schema/terminal_types.go new file mode 100644 index 00000000000..c535af4cf00 --- /dev/null +++ b/pkg/mutation/schema/terminal_types.go @@ -0,0 +1,10 @@ +package schema + +import "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" + +// Unknown represents a path element we do not know the type of. +// Elements of type unknown do not conflict with path elements of known types. +const Unknown = parser.NodeType("Unknown") + +// Set represents a list populated by unique values. +const Set = parser.NodeType("Set") diff --git a/pkg/mutation/system.go b/pkg/mutation/system.go index e3438b32d0f..7c1506c76c5 100644 --- a/pkg/mutation/system.go +++ b/pkg/mutation/system.go @@ -152,16 +152,6 @@ func (s *System) Mutate(obj *unstructured.Unstructured, ns *corev1.Namespace) (b convergence = SystemConvergenceTrue return false, nil } - if cmp.Equal(original, obj) { - if *MutationLoggingEnabled { - logAppliedMutations("Oscillating mutation.", mutationUUID, original, allAppliedMutations) - } - return false, fmt.Errorf("oscillating mutation for %s %s %s %s", - obj.GroupVersionKind().Group, - obj.GroupVersionKind().Kind, - obj.GetNamespace(), - obj.GetName()) - } if *MutationLoggingEnabled { logAppliedMutations("Mutation applied", mutationUUID, original, allAppliedMutations) } diff --git a/pkg/mutation/system_test.go b/pkg/mutation/system_test.go index 97a252f96c0..bec327b3c95 100644 --- a/pkg/mutation/system_test.go +++ b/pkg/mutation/system_test.go @@ -7,6 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/parser" + mschema "github.com/open-policy-agent/gatekeeper/pkg/mutation/schema" "github.com/open-policy-agent/gatekeeper/pkg/mutation/types" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -53,6 +54,10 @@ func (m *fakeMutator) Mutate(obj *unstructured.Unstructured) (bool, error) { return true, nil } +func (m *fakeMutator) TerminalType() parser.NodeType { + return mschema.Unknown +} + func (m *fakeMutator) ID() types.ID { return m.MID } diff --git a/pkg/readiness/ready_tracker.go b/pkg/readiness/ready_tracker.go index 8efb9cb1a2d..afb46d2cd0d 100644 --- a/pkg/readiness/ready_tracker.go +++ b/pkg/readiness/ready_tracker.go @@ -61,6 +61,7 @@ type Tracker struct { config *objectTracker assignMetadata *objectTracker assign *objectTracker + modifySet *objectTracker constraints *trackerMap data *trackerMap @@ -90,6 +91,7 @@ func newTracker(lister Lister, mutationEnabled bool, fn objDataFactory) *Tracker if mutationEnabled { tracker.assignMetadata = newObjTracker(mutationv1alpha.GroupVersion.WithKind("AssignMetadata"), fn) tracker.assign = newObjTracker(mutationv1alpha.GroupVersion.WithKind("Assign"), fn) + tracker.modifySet = newObjTracker(mutationv1alpha.GroupVersion.WithKind("ModifySet"), fn) } return &tracker } @@ -120,6 +122,11 @@ func (t *Tracker) For(gvk schema.GroupVersionKind) Expectations { return t.assign } return noopExpectations{} + case gvk.GroupVersion() == mutationv1alpha.GroupVersion && gvk.Kind == "ModifySet": + if t.mutationEnabled { + return t.modifySet + } + return noopExpectations{} } // Avoid new constraint trackers after templates have been populated. @@ -183,11 +190,12 @@ func (t *Tracker) Satisfied() bool { } if t.mutationEnabled { - if !t.assignMetadata.Satisfied() || !t.assign.Satisfied() { + if !t.assignMetadata.Satisfied() || !t.assign.Satisfied() || !t.modifySet.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") } if !t.templates.Satisfied() { @@ -235,6 +243,9 @@ func (t *Tracker) Run(ctx context.Context) error { grp.Go(func() error { return t.trackAssign(gctx) }) + grp.Go(func() error { + return t.trackModifySet(gctx) + }) } grp.Go(func() error { return t.trackConstraintTemplates(gctx) @@ -284,7 +295,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() + mutationPopulated = t.assignMetadata.Populated() && t.assign.Populated() && t.modifySet.Populated() } return t.templates.Populated() && t.config.Populated() && mutationPopulated && t.constraints.Populated() && t.data.Populated() } @@ -448,6 +459,31 @@ func (t *Tracker) trackAssign(ctx context.Context) error { return nil } +func (t *Tracker) trackModifySet(ctx context.Context) error { + defer func() { + t.modifySet.ExpectationsDone() + log.V(1).Info("ModifySet expectations populated") + _ = t.constraintTrackers.Wait() + }() + + if !t.mutationEnabled { + return nil + } + + modifySetList := &mutationv1alpha.ModifySetList{} + lister := retryLister(t.lister, retryAll) + if err := lister.List(ctx, modifySetList); err != nil { + return fmt.Errorf("listing ModifySet: %w", err) + } + log.V(1).Info("setting expectations for ModifySet", "ModifySet Count", len(modifySetList.Items)) + + for index := range modifySetList.Items { + log.V(1).Info("expecting ModifySet", "name", modifySetList.Items[index].GetName()) + t.modifySet.Expect(&modifySetList.Items[index]) + } + return nil +} + func (t *Tracker) trackConstraintTemplates(ctx context.Context) error { defer func() { t.templates.ExpectationsDone() @@ -704,6 +740,7 @@ func (t *Tracker) statsPrinter(ctx context.Context) { if t.mutationEnabled { logUnsatisfiedAssignMetadata(t) logUnsatisfiedAssign(t) + logUnsatisfiedModifySet(t) } } } @@ -720,6 +757,12 @@ func logUnsatisfiedAssign(t *Tracker) { } } +func logUnsatisfiedModifySet(t *Tracker) { + for _, amKey := range t.modifySet.unsatisfied() { + log.Info("unsatisfied ModifySet", "name", amKey.namespacedName) + } +} + // Returns the constraint GVK that would be generated by a template. func constraintGVK(ct *templates.ConstraintTemplate) schema.GroupVersionKind { return schema.GroupVersionKind{ diff --git a/pkg/readiness/ready_tracker_test.go b/pkg/readiness/ready_tracker_test.go index 4df697cf087..074eeb54c9f 100644 --- a/pkg/readiness/ready_tracker_test.go +++ b/pkg/readiness/ready_tracker_test.go @@ -176,6 +176,55 @@ func Test_AssignMetadata(t *testing.T) { } } +func Test_ModifySet(t *testing.T) { + g := gomega.NewWithT(t) + + defer func() { + mutationEnabled := false + mutation.MutationEnabled = &mutationEnabled + }() + + mutationEnabled := true + mutation.MutationEnabled = &mutationEnabled + + os.Setenv("POD_NAME", "no-pod") + podstatus.DisablePodOwnership() + + // Apply fixtures *before* the controllers are setup. + err := applyFixtures("testdata") + g.Expect(err).NotTo(gomega.HaveOccurred(), "applying fixtures") + + // Wire up the rest. + mgr, wm := setupManager(t) + opaClient := setupOpa(t) + + mutationSystem := mutation.NewSystem(mutation.SystemOpts{}) + + if err := setupController(mgr, wm, opaClient, mutationSystem); err != nil { + t.Fatalf("setupControllers: %v", err) + } + + ctx, cancelFunc := context.WithCancel(context.Background()) + mgrStopped := StartTestManager(ctx, mgr, g) + defer func() { + cancelFunc() + mgrStopped.Wait() + }() + + g.Eventually(func() (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + return probeIsReady(ctx) + }, 300*time.Second, 1*time.Second).Should(gomega.BeTrue()) + + // Verify that the ModifySet is present in the cache + for _, am := range testModifySet { + id := mutationtypes.MakeID(am) + exptectedMutator := mutationSystem.Get(id) + g.Expect(exptectedMutator).NotTo(gomega.BeNil(), "expected mutator was not found") + } +} + func Test_Assign(t *testing.T) { g := gomega.NewWithT(t) diff --git a/pkg/readiness/testdata/99-modifyset.yaml b/pkg/readiness/testdata/99-modifyset.yaml new file mode 100644 index 00000000000..68fa2061f7e --- /dev/null +++ b/pkg/readiness/testdata/99-modifyset.yaml @@ -0,0 +1,18 @@ +apiVersion: mutations.gatekeeper.sh/v1alpha1 +kind: ModifySet +metadata: + name: demo +spec: + applyTo: + - groups: [""] + versions: ["v1"] + kinds: ["Pod"] + match: + scope: Namespaced + kinds: + - apiGroups: ["*"] + kinds: ["Pod"] + location: "spec.some.set" + parameters: + values: + fromList: ["demo"] diff --git a/pkg/readiness/testdata_test.go b/pkg/readiness/testdata_test.go index 6f18f30dec5..d154bbe0c65 100644 --- a/pkg/readiness/testdata_test.go +++ b/pkg/readiness/testdata_test.go @@ -46,6 +46,10 @@ var testAssignMetadata = []*mutationsv1alpha1.AssignMetadata{ makeAssignMetadata("demo"), } +var testModifySet = []*mutationsv1alpha1.ModifySet{ + makeModifySet("demo"), +} + var testAssign = []*mutationsv1alpha1.Assign{ makeAssign("demo"), } @@ -81,6 +85,21 @@ func makeAssignMetadata(name string) *mutationsv1alpha1.AssignMetadata { } } +func makeModifySet(name string) *mutationsv1alpha1.ModifySet { + return &mutationsv1alpha1.ModifySet{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "mutations.gatekeeper.sh/v1alpha1", + Kind: "ModifySet", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: mutationsv1alpha1.ModifySetSpec{ + Location: "spec.some.set", + }, + } +} + func makeAssign(name string) *mutationsv1alpha1.Assign { return &mutationsv1alpha1.Assign{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/webhook/mutation_test.go b/pkg/webhook/mutation_test.go index ec2337d20de..5ae58750499 100644 --- a/pkg/webhook/mutation_test.go +++ b/pkg/webhook/mutation_test.go @@ -11,6 +11,8 @@ 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" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assignmeta" admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -31,7 +33,7 @@ func TestWebhookAssign(t *testing.T) { }, }, } - if err := mutators.IsValidAssign(v); err != nil { + if err := assign.IsValidAssign(v); err != nil { t.Fatal(err) } @@ -97,7 +99,7 @@ func TestWebhookAssignMetadata(t *testing.T) { }, }, } - if err := mutators.IsValidAssignMetadata(v); err != nil { + if err := assignmeta.IsValidAssignMetadata(v); err != nil { t.Fatal(err) } diff --git a/pkg/webhook/policy.go b/pkg/webhook/policy.go index cdfca22dae6..8770ce8fdf8 100644 --- a/pkg/webhook/policy.go +++ b/pkg/webhook/policy.go @@ -33,7 +33,9 @@ import ( "github.com/open-policy-agent/gatekeeper/pkg/keys" "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" + "github.com/open-policy-agent/gatekeeper/pkg/mutation/mutators/assign" + "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/target" "github.com/open-policy-agent/gatekeeper/pkg/util" admissionv1 "k8s.io/api/admission/v1" @@ -324,6 +326,8 @@ func (h *validationHandler) validateGatekeeperResources(ctx context.Context, req return h.validateAssignMetadata(req) case req.AdmissionRequest.Kind.Group == mutationsGroup && req.AdmissionRequest.Kind.Kind == "Assign": return h.validateAssign(req) + case req.AdmissionRequest.Kind.Group == mutationsGroup && req.AdmissionRequest.Kind.Kind == "ModifySet": + return h.validateModifySet(req) } return false, nil @@ -387,7 +391,7 @@ func (h *validationHandler) validateAssignMetadata(req *admission.Request) (bool if !ok { return false, fmt.Errorf("Deserialized object is not of type AssignMetadata") } - err = mutators.IsValidAssignMetadata(assignMetadata) + err = assignmeta.IsValidAssignMetadata(assignMetadata) if err != nil { return true, err } @@ -400,12 +404,30 @@ func (h *validationHandler) validateAssign(req *admission.Request) (bool, error) if err != nil { return false, err } - assign, ok := obj.(*mutationsv1alpha1.Assign) + a, ok := obj.(*mutationsv1alpha1.Assign) if !ok { return false, fmt.Errorf("Deserialized object is not of type Assign") } - err = mutators.IsValidAssign(assign) + err = assign.IsValidAssign(a) + 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, &mutationsv1alpha1.ModifySet{}) + if err != nil { + return false, err + } + m, ok := obj.(*mutationsv1alpha1.ModifySet) + if !ok { + return false, fmt.Errorf("Deserialized object is not of type ModifySet") + } + + err = modifyset.IsValidModifySet(m) if err != nil { return true, err }