diff --git a/pkg/mutation/mutators/assign_mutator.go b/pkg/mutation/mutators/assign_mutator.go index bf343a8d0b7..faaaf2072e1 100644 --- a/pkg/mutation/mutators/assign_mutator.go +++ b/pkg/mutation/mutators/assign_mutator.go @@ -18,6 +18,7 @@ import ( "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" @@ -28,9 +29,11 @@ var log = logf.Log.WithName("mutation").WithValues(logging.Process, "mutation") // AssignMutator is a mutator object built out of a // Assign instance. type AssignMutator struct { - id types.ID - assign *mutationsv1alpha1.Assign - path parser.Path + id types.ID + assign *mutationsv1alpha1.Assign + assignValue interface{} + + path parser.Path // bindings are the set of GVKs this Mutator applies to. bindings []runtimeschema.GroupVersionKind @@ -98,7 +101,7 @@ func (m *AssignMutator) SchemaBindings() []runtimeschema.GroupVersionKind { } func (m *AssignMutator) Value() (interface{}, error) { - return types.UnmarshalValue(m.assign.Spec.Parameters.Assign.Raw) + return runtime.DeepCopyJSONValue(m.assignValue), nil } func (m *AssignMutator) HasDiff(mutator types.Mutator) bool { @@ -131,13 +134,15 @@ func (m *AssignMutator) Path() parser.Path { func (m *AssignMutator) DeepCopy() types.Mutator { res := &AssignMutator{ - id: m.id, - assign: m.assign.DeepCopy(), + id: m.id, + assign: m.assign.DeepCopy(), + assignValue: runtime.DeepCopyJSONValue(m.assignValue), 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() @@ -208,12 +213,13 @@ func MutatorForAssign(assign *mutationsv1alpha1.Assign) (*AssignMutator, error) } return &AssignMutator{ - id: id, - assign: assign.DeepCopy(), - bindings: gvks, - path: path, - tester: tester, - valueTest: &valueTests, + id: id, + assign: assign.DeepCopy(), + assignValue: value, + bindings: gvks, + path: path, + tester: tester, + valueTest: &valueTests, }, nil } diff --git a/pkg/mutation/mutators/assignmeta_mutator.go b/pkg/mutation/mutators/assignmeta_mutator.go index acc2d773f7e..7e6fb352836 100644 --- a/pkg/mutation/mutators/assignmeta_mutator.go +++ b/pkg/mutation/mutators/assignmeta_mutator.go @@ -42,9 +42,12 @@ var ( // AssignMeta instance. type AssignMetadataMutator struct { id types.ID - tester *tester.Tester assignMetadata *mutationsv1alpha1.AssignMetadata - path parser.Path + assignValue string + + path parser.Path + + tester *tester.Tester } // assignMetadataMutator implements mutator. @@ -60,6 +63,10 @@ func (m *AssignMetadataMutator) Matches(obj client.Object, ns *corev1.Namespace) } func (m *AssignMetadataMutator) 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) } @@ -91,24 +98,16 @@ func (m *AssignMetadataMutator) HasDiff(mutator types.Mutator) bool { func (m *AssignMetadataMutator) DeepCopy() types.Mutator { res := &AssignMetadataMutator{ id: m.id, - tester: m.tester.DeepCopy(), assignMetadata: m.assignMetadata.DeepCopy(), + assignValue: m.assignValue, path: m.path.DeepCopy(), + tester: m.tester.DeepCopy(), } return res } func (m *AssignMetadataMutator) Value() (interface{}, error) { - value, err := types.UnmarshalValue(m.assignMetadata.Spec.Parameters.Assign.Raw) - if err != nil { - return nil, err - } - switch t := value.(type) { - case string: - return value, nil - default: - return nil, fmt.Errorf("incorrect value for AssignMetadataMutator. Value must be a string. Value: %s Type: %s", value, t) - } + return m.assignValue, nil } func (m *AssignMetadataMutator) String() string { @@ -134,7 +133,8 @@ func MutatorForAssignMetadata(assignMeta *mutationsv1alpha1.AssignMetadata) (*As if !ok { return nil, errors.New("spec.parameters.assign must have a string value field for AssignMetadata " + assignMeta.GetName()) } - if _, ok := value.(string); !ok { + valueString, isString := value.(string) + if !isString { return nil, errors.New("spec.parameters.assign.value field must be a string for AssignMetadata " + assignMeta.GetName()) } @@ -147,9 +147,10 @@ func MutatorForAssignMetadata(assignMeta *mutationsv1alpha1.AssignMetadata) (*As return &AssignMetadataMutator{ id: types.MakeID(assignMeta), - tester: t, assignMetadata: assignMeta.DeepCopy(), + assignValue: valueString, path: path, + tester: t, }, nil }