diff --git a/README.md b/README.md index 6c0a44d..775cc79 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ metadata: annotations: render.crossplane.io/runtime: Development spec: - package: xpkg.upbound.io/borrelli-org/function-conditional-patch-and-transform:v0.3.0 + package: xpkg.upbound.io/borrelli-org/function-conditional-patch-and-transform:v0.4.0 ``` ## What this function does diff --git a/examples/conditional-rendering/functions.yaml b/examples/conditional-rendering/functions.yaml index 20ff604..b197224 100644 --- a/examples/conditional-rendering/functions.yaml +++ b/examples/conditional-rendering/functions.yaml @@ -3,5 +3,5 @@ kind: Function metadata: name: function-conditional-patch-and-transform spec: - package: xpkg.upbound.io/borrelli-org/function-conditional-patch-and-transform:v0.3.0 + package: xpkg.upbound.io/borrelli-org/function-conditional-patch-and-transform:v0.4.0 packagePullPolicy: Always diff --git a/examples/conditional-resources/composition.yaml b/examples/conditional-resources/composition.yaml index 2d3b277..7fc75ee 100644 --- a/examples/conditional-resources/composition.yaml +++ b/examples/conditional-resources/composition.yaml @@ -15,6 +15,7 @@ spec: input: apiVersion: conditional-pt.fn.crossplane.io/v1beta1 kind: Resources + condition: observed.composite.resource.spec.render == true resources: - name: blue-resource condition: observed.composite.resource.spec.deployment.blue == true diff --git a/examples/conditional-resources/functions.yaml b/examples/conditional-resources/functions.yaml index 20ff604..b197224 100644 --- a/examples/conditional-resources/functions.yaml +++ b/examples/conditional-resources/functions.yaml @@ -3,5 +3,5 @@ kind: Function metadata: name: function-conditional-patch-and-transform spec: - package: xpkg.upbound.io/borrelli-org/function-conditional-patch-and-transform:v0.3.0 + package: xpkg.upbound.io/borrelli-org/function-conditional-patch-and-transform:v0.4.0 packagePullPolicy: Always diff --git a/fn.go b/fn.go index 5f9e8a5..df01b37 100644 --- a/fn.go +++ b/fn.go @@ -131,17 +131,11 @@ func (f *Function) RunFunction(ctx context.Context, req *fnv1beta1.RunFunctionRe } if input.Environment != nil { - // Run all patches that are from the (observed) XR to the environment. - if err := RenderToEnvironmentPatches(env, oxr.Resource, input.Environment.Patches); err != nil { + // Run all patches that are from the (observed) XR to the environment or from the environment to the (desired) XR. + if err := RenderEnvironmentPatches(env, oxr.Resource, dxr.Resource, input.Environment.Patches); err != nil { response.Fatal(rsp, errors.Wrapf(err, "cannot render ToEnvironment patches from the composite resource")) return rsp, nil } - - // Run all patches that are from the environment to the (desired) XR. - if err := RenderFromEnvironmentPatches(dxr.Resource, env, input.Environment.Patches); err != nil { - response.Fatal(rsp, errors.Wrapf(err, "cannot render FromEnvironment patches to the composite resource")) - return rsp, nil - } } // Increment this if you emit a warning result. @@ -228,58 +222,19 @@ func (f *Function) RunFunction(ctx context.Context, req *fnv1beta1.RunFunctionRe log.Debug("Found corresponding observed resource", "ready", ready, "name", ocd.Resource.GetName()) - - // TODO(negz): Should failures to patch the XR be terminal? It could - // indicate a required patch failed. A required patch means roughly - // "this patch has to succeed before you mutate the resource". This - // is useful to make sure we never create a composed resource in the - // wrong state. It's less clear how useful it is for the XR, given - // we'll only ever be updating it, not creating it. - - // We want to patch the XR from observed composed resources, not - // from desired state. This is because folks will typically be - // patching from a field that is set once the observed resource is - // applied such as its status. - if err := RenderToCompositePatches(dxr.Resource, ocd.Resource, t.Patches); err != nil { - response.Warning(rsp, errors.Wrapf(err, "cannot render ToComposite patches for composed resource %q", t.Name)) - log.Info("Cannot render ToComposite patches for composed resource", "warning", err) - warnings++ - } - - // TODO(negz): Same as above, but for the Environment. What does it - // mean for a required patch to the environment to fail? Should it - // be terminal? - - // Run all patches that are from the (observed) composed resource to - // the environment. - if err := RenderToEnvironmentPatches(env, ocd.Resource, t.Patches); err != nil { - response.Warning(rsp, errors.Wrapf(err, "cannot render ToEnvironment patches for composed resource %q", t.Name)) - log.Info("Cannot render ToEnvironment patches for composed resource", "warning", err) - warnings++ - } } - // If either of the below renderings return an error, most likely a - // required FromComposite or FromEnvironment patch failed. A required - // patch means roughly "this patch has to succeed before you mutate the - // resource." This is useful to make sure we never create a composed - // resource in the wrong state. To that end, we don't want to add this - // resource to our accumulated desired state. - if err := RenderFromCompositePatches(dcd.Resource, oxr.Resource, t.Patches); err != nil { - response.Warning(rsp, errors.Wrapf(err, "cannot render FromComposite patches for composed resource %q", t.Name)) - log.Info("Cannot render FromComposite patches for composed resource", "warning", err) + errs, store := RenderComposedPatches(ocd.Resource, dcd.Resource, oxr.Resource, dxr.Resource, env, t.Patches) + for _, err := range errs { + response.Warning(rsp, errors.Wrapf(err, "cannot render patches for composed resource %q", t.Name)) + log.Info("Cannot render patches for composed resource", "warning", err) warnings++ - continue - } - if err := RenderFromEnvironmentPatches(dcd.Resource, env, t.Patches); err != nil { - response.Warning(rsp, errors.Wrapf(err, "cannot render FromEnvironment patches for composed resource %q", t.Name)) - log.Info("Cannot render FromEnvironment patches for composed resource", "warning", err) - warnings++ - continue } - // Add or replace our desired resource. - desired[resource.Name(t.Name)] = dcd + if store { + // Add or replace our desired resource. + desired[resource.Name(t.Name)] = dcd + } } if err := response.SetDesiredCompositeResource(rsp, dxr); err != nil { diff --git a/fn_test.go b/fn_test.go index 112229a..6cac9cd 100644 --- a/fn_test.go +++ b/fn_test.go @@ -10,7 +10,9 @@ import ( "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/structpb" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/utils/ptr" "github.com/crossplane/crossplane-runtime/pkg/logging" @@ -156,23 +158,25 @@ func TestRunFunction(t *testing.T) { { Name: "cool-resource", Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.widgets"), - ToFieldPath: ptr.To[string]("spec.watchers"), - Transforms: []v1beta1.Transform{ - { - Type: v1beta1.TransformTypeConvert, - Convert: &v1beta1.ConvertTransform{ - ToType: v1beta1.TransformIOTypeInt64, + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{ + { + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, }, - }, - { - Type: v1beta1.TransformTypeMath, - Math: &v1beta1.MathTransform{ - Type: v1beta1.MathTransformTypeMultiply, - Multiply: ptr.To[int64](3), + { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, }, }, }, @@ -221,23 +225,25 @@ func TestRunFunction(t *testing.T) { // patch the resource named "cool-resource" in // the desired resources array. Name: "cool-resource", - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.widgets"), - ToFieldPath: ptr.To[string]("spec.watchers"), - Transforms: []v1beta1.Transform{ - { - Type: v1beta1.TransformTypeConvert, - Convert: &v1beta1.ConvertTransform{ - ToType: v1beta1.TransformIOTypeInt64, + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{ + { + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, }, - }, - { - Type: v1beta1.TransformTypeMath, - Math: &v1beta1.MathTransform{ - Type: v1beta1.MathTransformTypeMultiply, - Multiply: ptr.To[int64](3), + { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, }, }, }, @@ -291,11 +297,13 @@ func TestRunFunction(t *testing.T) { // patch the resource named "cool-resource" in // the desired resources array. Name: "cool-resource", - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.widgets"), - ToFieldPath: ptr.To[string]("spec.watchers"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + }, }, }, }, @@ -387,25 +395,29 @@ func TestRunFunction(t *testing.T) { // patch the resource named "cool-resource" in // the desired resources array. Name: "cool-resource", - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { // This patch should work. - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.widgets"), - ToFieldPath: ptr.To[string]("spec.watchers"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + }, }, { // This patch should return an error, // because the required path does not // exist. - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.doesNotExist"), - ToFieldPath: ptr.To[string]("spec.explode"), - Policy: &v1beta1.PatchPolicy{ - FromFieldPath: func() *v1beta1.FromFieldPathPolicy { - r := v1beta1.FromFieldPathPolicyRequired - return &r - }(), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.doesNotExist"), + ToFieldPath: ptr.To[string]("spec.explode"), + Policy: &v1beta1.PatchPolicy{ + FromFieldPath: func() *v1beta1.FromFieldPathPolicy { + r := v1beta1.FromFieldPathPolicyRequired + return &r + }(), + }, }, }, }, @@ -447,7 +459,7 @@ func TestRunFunction(t *testing.T) { Results: []*fnv1beta1.Result{ { Severity: fnv1beta1.Severity_SEVERITY_WARNING, - Message: fmt.Sprintf("cannot render FromComposite patches for composed resource %q: cannot apply the %q patch at index 1: spec.doesNotExist: no such field", "cool-resource", "FromCompositeFieldPath"), + Message: fmt.Sprintf("cannot render patches for composed resource %q: cannot apply the %q patch at index 1: spec.doesNotExist: no such field", "cool-resource", "FromCompositeFieldPath"), }, }, Context: &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(nil)}}, @@ -572,23 +584,25 @@ func TestRunFunction(t *testing.T) { { Name: "cool-resource", Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeToCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.widgets"), - ToFieldPath: ptr.To[string]("spec.watchers"), - Transforms: []v1beta1.Transform{ - { - Type: v1beta1.TransformTypeConvert, - Convert: &v1beta1.ConvertTransform{ - ToType: v1beta1.TransformIOTypeInt64, + Type: v1beta1.PatchTypeToCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{ + { + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, }, - }, - { - Type: v1beta1.TransformTypeMath, - Math: &v1beta1.MathTransform{ - Type: v1beta1.MathTransformTypeMultiply, - Multiply: ptr.To[int64](3), + { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, }, }, }, @@ -631,6 +645,279 @@ func TestRunFunction(t *testing.T) { }, }, }, + "PatchToCompositeWithEnvironmentPatches": { + reason: "A basic ToCompositeFieldPath patch should work with environment.patches.", + args: args{ + req: &fnv1beta1.RunFunctionRequest{ + Input: resource.MustStructObject(&v1beta1.Resources{ + Resources: []v1beta1.ComposedTemplate{ + { + Name: "cool-resource", + Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, + }}, + Environment: &v1beta1.Environment{ + Patches: []v1beta1.EnvironmentPatch{ + { + Type: v1beta1.PatchTypeFromEnvironmentFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("data.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{ + { + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, + }, + { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }}}}}}}}), + Observed: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{}}`), + }, + Resources: map[string]*fnv1beta1.Resource{}, + }, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}, + }, + want: want{ + rsp: &fnv1beta1.RunFunctionResponse{ + Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)}, + Desired: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + // spec.watchers = 10 * 3 = 30 + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{"watchers":30}}`), + }, + Resources: map[string]*fnv1beta1.Resource{ + "cool-resource": { + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD"}`), + }}}, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}}}, + "EnvironmentPatchToEnvironment": { + reason: "A basic ToEnvironment patch should work with environment.patches.", + args: args{ + req: &fnv1beta1.RunFunctionRequest{ + Input: resource.MustStructObject(&v1beta1.Resources{ + Resources: []v1beta1.ComposedTemplate{ + { + Name: "cool-resource", + Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, + }}, + Environment: &v1beta1.Environment{ + Patches: []v1beta1.EnvironmentPatch{ + { + Type: v1beta1.PatchTypeToEnvironmentFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.watchers"), + ToFieldPath: ptr.To[string]("data.widgets"), + Transforms: []v1beta1.Transform{ + { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, + }, + { + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeString, + }, + }}}}}}}), + Observed: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{"watchers":10}}`), + }, + Resources: map[string]*fnv1beta1.Resource{}, + }, + Context: contextWithEnvironment(nil)}, + }, + want: want{ + rsp: &fnv1beta1.RunFunctionResponse{ + Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)}, + Desired: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD"}`), + }, + Resources: map[string]*fnv1beta1.Resource{ + "cool-resource": { + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD"}`), + }}}, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "30", + })}}}, + "PatchComposedResourceFromEnvironment": { + reason: "A basic FromEnvironmentPatch should work if defined at spec.resources[*].patches.", + args: args{ + req: &fnv1beta1.RunFunctionRequest{ + Input: resource.MustStructObject(&v1beta1.Resources{ + Resources: []v1beta1.ComposedTemplate{{ + Name: "cool-resource", + Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, + Patches: []v1beta1.ComposedPatch{{ + Type: v1beta1.PatchTypeFromEnvironmentFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("data.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{{ + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, + }, { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, + }}}}}, + }}}), + Observed: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{}}`), + }, + Resources: map[string]*fnv1beta1.Resource{}, + }, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}, + }, + want: want{ + rsp: &fnv1beta1.RunFunctionResponse{ + Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)}, + Desired: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD"}`), + }, + Resources: map[string]*fnv1beta1.Resource{ + "cool-resource": { + // spec.watchers = 10 * 3 = 30 + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{"watchers":30}}`), + }}}, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}}}, + + "PatchComposedResourceFromEnvironmentShadowedNotSet": { + reason: "A basic FromEnvironmentPatch should work if defined at spec.resources[*].patches, even if a successive patch shadows it and its source is not set.", + args: args{ + req: &fnv1beta1.RunFunctionRequest{ + Input: resource.MustStructObject(&v1beta1.Resources{ + Resources: []v1beta1.ComposedTemplate{{ + Name: "cool-resource", + Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, + Patches: []v1beta1.ComposedPatch{{ + Type: v1beta1.PatchTypeFromEnvironmentFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("data.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{{ + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, + }, { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, + }}}}, + { + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.watchers"), + ToFieldPath: ptr.To[string]("spec.watchers"), + }}}}}}), + Observed: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{}}`), + }, + Resources: map[string]*fnv1beta1.Resource{}, + }, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}, + }, + want: want{ + rsp: &fnv1beta1.RunFunctionResponse{ + Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)}, + Desired: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD"}`), + }, + Resources: map[string]*fnv1beta1.Resource{ + "cool-resource": { + // spec.watchers = 10 * 3 = 30 + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{"watchers":30}}`), + }}}, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}}}, + "PatchComposedResourceFromEnvironmentShadowedSet": { + reason: "A basic FromEnvironmentPatch should work if defined at spec.resources[*].patches, even if a successive patch shadows it and its source is set.", + args: args{ + req: &fnv1beta1.RunFunctionRequest{ + Input: resource.MustStructObject(&v1beta1.Resources{ + Resources: []v1beta1.ComposedTemplate{{ + Name: "cool-resource", + Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, + Patches: []v1beta1.ComposedPatch{{ + Type: v1beta1.PatchTypeFromEnvironmentFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("data.widgets"), + ToFieldPath: ptr.To[string]("spec.watchers"), + Transforms: []v1beta1.Transform{{ + Type: v1beta1.TransformTypeConvert, + Convert: &v1beta1.ConvertTransform{ + ToType: v1beta1.TransformIOTypeInt64, + }, + }, { + Type: v1beta1.TransformTypeMath, + Math: &v1beta1.MathTransform{ + Type: v1beta1.MathTransformTypeMultiply, + Multiply: ptr.To[int64](3), + }, + }}}}, + { + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.watchers"), + ToFieldPath: ptr.To[string]("spec.watchers"), + }}}}}}), + Observed: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + // I want this in the environment, 42 + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{"watchers":42}}`), + }, + Resources: map[string]*fnv1beta1.Resource{}, + }, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}, + }, + want: want{ + rsp: &fnv1beta1.RunFunctionResponse{ + Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)}, + Desired: &fnv1beta1.State{ + Composite: &fnv1beta1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD"}`), + }, + Resources: map[string]*fnv1beta1.Resource{ + "cool-resource": { + // spec.watchers comes from the composite resource, 42 + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","spec":{"watchers":42}}`), + }}}, + Context: contextWithEnvironment(map[string]interface{}{ + "widgets": "10", + })}}}, } for name, tc := range cases { @@ -648,3 +935,24 @@ func TestRunFunction(t *testing.T) { }) } } + +// Crossplane sends as context a fake resource: +// { "apiVersion": "internal.crossplane.io/v1alpha1", "kind": "Environment", "data": {... the actual environment content ...} } +// See: https://github.com/crossplane/crossplane/blob/806f0d20d146f6f4f1735c5ec6a7dc78923814b3/internal/controller/apiextensions/composite/environment_fetcher.go#L85C1-L85C1 +// That's because the patching code expects a resource to be able to use +// runtime.DefaultUnstructuredConverter.FromUnstructured to convert it back to +// an object. This is also why all patches need to specify the full path from data. +func contextWithEnvironment(data map[string]interface{}) *structpb.Struct { + if data == nil { + data = map[string]interface{}{} + } + u := unstructured.Unstructured{Object: map[string]interface{}{ + "data": data, + }} + u.SetGroupVersionKind(schema.GroupVersionKind{Group: "internal.crossplane.io", Version: "v1alpha1", Kind: "Environment"}) + d, err := structpb.NewStruct(u.UnstructuredContent()) + if err != nil { + panic(err) + } + return &structpb.Struct{Fields: map[string]*structpb.Value{fncontext.KeyEnvironment: structpb.NewStructValue(d)}} +} diff --git a/go.mod b/go.mod index b172f45..40102c9 100644 --- a/go.mod +++ b/go.mod @@ -6,16 +6,16 @@ toolchain go1.21.3 require ( github.com/alecthomas/kong v0.8.1 - github.com/crossplane/crossplane-runtime v1.14.1 + github.com/crossplane/crossplane-runtime v1.14.2 github.com/crossplane/function-sdk-go v0.1.0 github.com/google/cel-go v0.16.1 github.com/google/go-cmp v0.6.0 github.com/pkg/errors v0.9.1 google.golang.org/protobuf v1.31.0 - k8s.io/api v0.28.3 - k8s.io/apiextensions-apiserver v0.28.3 - k8s.io/apimachinery v0.28.3 - k8s.io/utils v0.0.0-20230726121419-3b25d923346b + k8s.io/api v0.28.4 + k8s.io/apiextensions-apiserver v0.28.4 + k8s.io/apimachinery v0.28.4 + k8s.io/utils v0.0.0-20231121161247-cf03d44ff3cf sigs.k8s.io/controller-tools v0.13.0 ) @@ -79,8 +79,8 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/client-go v0.28.3 // indirect - k8s.io/component-base v0.28.3 // indirect + k8s.io/client-go v0.28.4 // indirect + k8s.io/component-base v0.28.4 // indirect k8s.io/klog/v2 v2.100.1 // indirect k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9 // indirect sigs.k8s.io/controller-runtime v0.16.3 // indirect diff --git a/go.sum b/go.sum index 00ef314..1217ae5 100644 --- a/go.sum +++ b/go.sum @@ -43,7 +43,6 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo= github.com/agext/levenshtein v1.2.3/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= github.com/alecthomas/assert/v2 v2.1.0 h1:tbredtNcQnoSd3QBhQWI7QZ3XHOVkw1Moklp2ojoH/0= -github.com/alecthomas/assert/v2 v2.1.0/go.mod h1:b/+1DI2Q6NckYi+3mXyH3wFb8qG37K/DuK80n7WefXA= github.com/alecthomas/kong v0.8.1 h1:acZdn3m4lLRobeh3Zi2S2EpnXTd1mOL6U7xVml+vfkY= github.com/alecthomas/kong v0.8.1/go.mod h1:n1iCIO2xS46oE8ZfYCNDqdR0b0wZNrXAIAqro/2132U= github.com/alecthomas/repr v0.1.0 h1:ENn2e1+J3k09gyj2shc0dHr/yjaWSHRlrJ4DPMevDqE= @@ -71,10 +70,10 @@ github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnht github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/stevendborrelli/function-conditional-patch-and-transform v0.2.1 h1:8CNU6BGXWO+lqQw7t+calDuM9JEjQ61YYVA9fM9yv+8= -github.com/stevendborrelli/function-conditional-patch-and-transform v0.2.1/go.mod h1:B9hSlytx1BSH/zezwsRkLhcnEG1NHDQJE7Q5P4zxJGI= github.com/crossplane/crossplane-runtime v1.14.1 h1:TCa7R1N4bDGHjsLhiRxR/mUhwmistlMACHm0kiiYKck= github.com/crossplane/crossplane-runtime v1.14.1/go.mod h1:aOP+5W2wKpvthVs3pFNbVOe1jwrKYbJho0ThGNCVz9o= +github.com/crossplane/crossplane-runtime v1.14.2 h1:pV5JMzyzi/kcbeVBVPCat5MHH8zS94MBUapAyGx/Ry0= +github.com/crossplane/crossplane-runtime v1.14.2/go.mod h1:aOP+5W2wKpvthVs3pFNbVOe1jwrKYbJho0ThGNCVz9o= github.com/crossplane/function-sdk-go v0.1.0 h1:WN3CUSaj6wgDqQPZZP1whMVIkTAZtN3HVCS67pTMzd8= github.com/crossplane/function-sdk-go v0.1.0/go.mod h1:oRqHZ6RufpfZJ1N8aT4EfkP+5KpkhOKpWz7SeUDwwcw= github.com/crossplane/upjet v0.11.0-rc.0.0.20231012093706-c4a76d2a7505 h1:eCmYgfRopVn6r8RM1Ra4XQAPwVsjTGfktBj2Dk7yy+Y= @@ -310,6 +309,8 @@ github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stevendborrelli/function-conditional-patch-and-transform v0.2.1 h1:8CNU6BGXWO+lqQw7t+calDuM9JEjQ61YYVA9fM9yv+8= +github.com/stevendborrelli/function-conditional-patch-and-transform v0.2.1/go.mod h1:B9hSlytx1BSH/zezwsRkLhcnEG1NHDQJE7Q5P4zxJGI= github.com/stoewer/go-strcase v1.2.0 h1:Z2iHWqGXH00XYgqDmNgQbIBxf3wrNq0F3feEy0ainaU= github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -702,22 +703,22 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -k8s.io/api v0.28.3 h1:Gj1HtbSdB4P08C8rs9AR94MfSGpRhJgsS+GF9V26xMM= -k8s.io/api v0.28.3/go.mod h1:MRCV/jr1dW87/qJnZ57U5Pak65LGmQVkKTzf3AtKFHc= -k8s.io/apiextensions-apiserver v0.28.3 h1:Od7DEnhXHnHPZG+W9I97/fSQkVpVPQx2diy+2EtmY08= -k8s.io/apiextensions-apiserver v0.28.3/go.mod h1:NE1XJZ4On0hS11aWWJUTNkmVB03j9LM7gJSisbRt8Lc= -k8s.io/apimachinery v0.28.3 h1:B1wYx8txOaCQG0HmYF6nbpU8dg6HvA06x5tEffvOe7A= -k8s.io/apimachinery v0.28.3/go.mod h1:uQTKmIqs+rAYaq+DFaoD2X7pcjLOqbQX2AOiO0nIpb8= -k8s.io/client-go v0.28.3 h1:2OqNb72ZuTZPKCl+4gTKvqao0AMOl9f3o2ijbAj3LI4= -k8s.io/client-go v0.28.3/go.mod h1:LTykbBp9gsA7SwqirlCXBWtK0guzfhpoW4qSm7i9dxo= -k8s.io/component-base v0.28.3 h1:rDy68eHKxq/80RiMb2Ld/tbH8uAE75JdCqJyi6lXMzI= -k8s.io/component-base v0.28.3/go.mod h1:fDJ6vpVNSk6cRo5wmDa6eKIG7UlIQkaFmZN2fYgIUD8= +k8s.io/api v0.28.4 h1:8ZBrLjwosLl/NYgv1P7EQLqoO8MGQApnbgH8tu3BMzY= +k8s.io/api v0.28.4/go.mod h1:axWTGrY88s/5YE+JSt4uUi6NMM+gur1en2REMR7IRj0= +k8s.io/apiextensions-apiserver v0.28.4 h1:AZpKY/7wQ8n+ZYDtNHbAJBb+N4AXXJvyZx6ww6yAJvU= +k8s.io/apiextensions-apiserver v0.28.4/go.mod h1:pgQIZ1U8eJSMQcENew/0ShUTlePcSGFq6dxSxf2mwPM= +k8s.io/apimachinery v0.28.4 h1:zOSJe1mc+GxuMnFzD4Z/U1wst50X28ZNsn5bhgIIao8= +k8s.io/apimachinery v0.28.4/go.mod h1:wI37ncBvfAoswfq626yPTe6Bz1c22L7uaJ8dho83mgg= +k8s.io/client-go v0.28.4 h1:Np5ocjlZcTrkyRJ3+T3PkXDpe4UpatQxj85+xjaD2wY= +k8s.io/client-go v0.28.4/go.mod h1:0VDZFpgoZfelyP5Wqu0/r/TRYcLYuJ2U1KEeoaPa1N4= +k8s.io/component-base v0.28.4 h1:c/iQLWPdUgI90O+T9TeECg8o7N3YJTiuz2sKxILYcYo= +k8s.io/component-base v0.28.4/go.mod h1:m9hR0uvqXDybiGL2nf/3Lf0MerAfQXzkfWhUY58JUbU= k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg= k8s.io/klog/v2 v2.100.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9 h1:LyMgNKD2P8Wn1iAwQU5OhxCKlKJy0sHc+PcDwFB24dQ= k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9/go.mod h1:wZK2AVp1uHCp4VamDVgBP2COHZjqD1T68Rf0CM3YjSM= -k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI= -k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20231121161247-cf03d44ff3cf h1:iTzha1p7Fi83476ypNSz8nV9iR9932jIIs26F7gNLsU= +k8s.io/utils v0.0.0-20231121161247-cf03d44ff3cf/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/input/v1beta1/resources_common.go b/input/v1beta1/resources_common.go index fd9f9e7..d5d4b5e 100644 --- a/input/v1beta1/resources_common.go +++ b/input/v1beta1/resources_common.go @@ -28,7 +28,19 @@ type PatchSet struct { Name string `json:"name"` // Patches will be applied as an overlay to the base resource. - Patches []Patch `json:"patches"` + Patches []PatchSetPatch `json:"patches"` +} + +// GetComposedPatches returns the composed patches from the patch set. +func (ps *PatchSet) GetComposedPatches() []ComposedPatch { + out := make([]ComposedPatch, len(ps.Patches)) + for i, p := range ps.Patches { + out[i] = ComposedPatch{ + Type: p.GetType(), + Patch: p.Patch, + } + } + return out } // ComposedTemplate is used to provide information about how the composed @@ -52,7 +64,7 @@ type ComposedTemplate struct { // Patches to and from the composed resource. // +optional - Patches []Patch `json:"patches,omitempty"` + Patches []ComposedPatch `json:"patches,omitempty"` // ConnectionDetails lists the propagation secret keys from this composed // resource to the composition instance connection secret. diff --git a/input/v1beta1/resources_patches.go b/input/v1beta1/resources_patches.go index 8924362..9478a79 100644 --- a/input/v1beta1/resources_patches.go +++ b/input/v1beta1/resources_patches.go @@ -57,21 +57,91 @@ type Environment struct { // composition's resources are composed. These patches are between the XR // and the Environment. Either from the Environment to the XR, or vice // versa. - Patches []Patch `json:"patches,omitempty"` + Patches []EnvironmentPatch `json:"patches,omitempty"` } -// Patch objects are applied between composite and composed resources. Their -// behaviour depends on the Type selected. The default Type, -// FromCompositeFieldPath, copies a value from the composite resource to -// the composed resource, applying any defined transformers. -type Patch struct { +// EnvironmentPatch objects are applied between the composite resource and +// the environment. Their behaviour depends on the Type selected. The default +// Type, FromCompositeFieldPath, copies a value from the composite resource +// to the environment, applying any defined transformers. +type EnvironmentPatch struct { // Type sets the patching behaviour to be used. Each patch type may require // its own fields to be set on the Patch object. // +optional - // +kubebuilder:validation:Enum=FromCompositeFieldPath;PatchSet;ToCompositeFieldPath;CombineFromComposite;CombineToComposite + // +kubebuilder:validation:Enum=FromCompositeFieldPath;ToCompositeFieldPath;CombineFromComposite;CombineToComposite // +kubebuilder:default=FromCompositeFieldPath Type PatchType `json:"type,omitempty"` + Patch `json:",inline"` +} + +// GetType returns the patch type. If the type is not set, it returns the default type. +func (ep *EnvironmentPatch) GetType() PatchType { + if ep.Type == "" { + return PatchTypeFromCompositeFieldPath + } + return ep.Type +} + +// ComposedPatch objects are applied between composite and composed resources. +// Their behaviour depends on the Type selected. The default Type, +// FromCompositeFieldPath, copies a value from the composite resource to the +// composed resource, applying any defined transformers. +type ComposedPatch struct { + // Type sets the patching behaviour to be used. Each patch type may require + // its own fields to be set on the ComposedPatch object. + // +optional + // +kubebuilder:validation:Enum=FromCompositeFieldPath;PatchSet;ToCompositeFieldPath;CombineFromComposite;CombineToComposite;FromEnvironmentFieldPath;ToEnvironmentFieldPath;CombineFromEnvironment;CombineToEnvironment + // +kubebuilder:default=FromCompositeFieldPath + Type PatchType `json:"type,omitempty"` + + // PatchSetName to include patches from. Required when type is PatchSet. + // +optional + PatchSetName *string `json:"patchSetName,omitempty"` + + Patch `json:",inline"` +} + +// GetType returns the patch type. If the type is not set, it returns the default type. +func (p *ComposedPatch) GetType() PatchType { + if p.Type == "" { + return PatchTypeFromCompositeFieldPath + } + return p.Type +} + +// GetPatchSetName returns the PatchSetName for this ComposedPatch, or an empty +// string if it is nil. +func (p *ComposedPatch) GetPatchSetName() string { + if p.PatchSetName == nil { + return "" + } + return *p.PatchSetName +} + +// PatchSetPatch defines a set of Patches that can be referenced by name by +// other patches of type PatchSet. +type PatchSetPatch struct { + // Type sets the patching behaviour to be used. Each patch type may require + // its own fields to be set on the ComposedPatch object. + // +optional + // +kubebuilder:validation:Enum=FromCompositeFieldPath;ToCompositeFieldPath;CombineFromComposite;CombineToComposite;FromEnvironmentFieldPath;ToEnvironmentFieldPath;CombineFromEnvironment;CombineToEnvironment + // +kubebuilder:default=FromCompositeFieldPath + Type PatchType `json:"type,omitempty"` + + Patch `json:",inline"` +} + +// GetType returns the patch type. If the type is not set, it returns the default type. +func (psp *PatchSetPatch) GetType() PatchType { + if psp.Type == "" { + return PatchTypeFromCompositeFieldPath + } + return psp.Type +} + +// Patch defines a patch between a source and destination. +type Patch struct { // FromFieldPath is the path of the field on the resource whose value is // to be used as input. Required when type is FromCompositeFieldPath or // ToCompositeFieldPath. @@ -89,10 +159,6 @@ type Patch struct { // +optional ToFieldPath *string `json:"toFieldPath,omitempty"` - // PatchSetName to include patches from. Required when type is PatchSet. - // +optional - PatchSetName *string `json:"patchSetName,omitempty"` - // Transforms are the list of functions that are used as a FIFO pipe for the // input to be transformed. // +optional @@ -114,17 +180,25 @@ func (p *Patch) GetFromFieldPath() string { // GetToFieldPath returns the ToFieldPath for this Patch, or an empty string if it is nil. func (p *Patch) GetToFieldPath() string { if p.ToFieldPath == nil { - return "" + // Default to patching the same field on the composed resource. + return p.GetFromFieldPath() } return *p.ToFieldPath } -// GetType returns the patch type. If the type is not set, it returns the default type. -func (p *Patch) GetType() PatchType { - if p.Type == "" { - return PatchTypeFromCompositeFieldPath - } - return p.Type +// GetCombine returns the Combine for this ComposedPatch, or nil if it is nil. +func (p *Patch) GetCombine() *Combine { + return p.Combine +} + +// GetTransforms returns the Transforms for this ComposedPatch, or nil if it is nil. +func (p *Patch) GetTransforms() []Transform { + return p.Transforms +} + +// GetPolicy returns the PatchPolicy for this ComposedPatch, or nil if it is nil. +func (p *Patch) GetPolicy() *PatchPolicy { + return p.Policy } // A CombineVariable defines the source of a value that is combined with diff --git a/input/v1beta1/zz_generated.deepcopy.go b/input/v1beta1/zz_generated.deepcopy.go index 1bcbf6b..fcc56b2 100644 --- a/input/v1beta1/zz_generated.deepcopy.go +++ b/input/v1beta1/zz_generated.deepcopy.go @@ -49,6 +49,27 @@ func (in *CombineVariable) DeepCopy() *CombineVariable { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComposedPatch) DeepCopyInto(out *ComposedPatch) { + *out = *in + if in.PatchSetName != nil { + in, out := &in.PatchSetName, &out.PatchSetName + *out = new(string) + **out = **in + } + in.Patch.DeepCopyInto(&out.Patch) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComposedPatch. +func (in *ComposedPatch) DeepCopy() *ComposedPatch { + if in == nil { + return nil + } + out := new(ComposedPatch) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ComposedTemplate) DeepCopyInto(out *ComposedTemplate) { *out = *in @@ -64,7 +85,7 @@ func (in *ComposedTemplate) DeepCopyInto(out *ComposedTemplate) { } if in.Patches != nil { in, out := &in.Patches, &out.Patches - *out = make([]Patch, len(*in)) + *out = make([]ComposedPatch, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -150,7 +171,7 @@ func (in *Environment) DeepCopyInto(out *Environment) { *out = *in if in.Patches != nil { in, out := &in.Patches, &out.Patches - *out = make([]Patch, len(*in)) + *out = make([]EnvironmentPatch, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -167,6 +188,22 @@ func (in *Environment) DeepCopy() *Environment { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EnvironmentPatch) DeepCopyInto(out *EnvironmentPatch) { + *out = *in + in.Patch.DeepCopyInto(&out.Patch) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvironmentPatch. +func (in *EnvironmentPatch) DeepCopy() *EnvironmentPatch { + if in == nil { + return nil + } + out := new(EnvironmentPatch) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MapTransform) DeepCopyInto(out *MapTransform) { *out = *in @@ -301,11 +338,6 @@ func (in *Patch) DeepCopyInto(out *Patch) { *out = new(string) **out = **in } - if in.PatchSetName != nil { - in, out := &in.PatchSetName, &out.PatchSetName - *out = new(string) - **out = **in - } if in.Transforms != nil { in, out := &in.Transforms, &out.Transforms *out = make([]Transform, len(*in)) @@ -355,7 +387,7 @@ func (in *PatchSet) DeepCopyInto(out *PatchSet) { *out = *in if in.Patches != nil { in, out := &in.Patches, &out.Patches - *out = make([]Patch, len(*in)) + *out = make([]PatchSetPatch, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -372,6 +404,22 @@ func (in *PatchSet) DeepCopy() *PatchSet { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PatchSetPatch) DeepCopyInto(out *PatchSetPatch) { + *out = *in + in.Patch.DeepCopyInto(&out.Patch) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PatchSetPatch. +func (in *PatchSetPatch) DeepCopy() *PatchSetPatch { + if in == nil { + return nil + } + out := new(PatchSetPatch) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ReadinessCheck) DeepCopyInto(out *ReadinessCheck) { *out = *in diff --git a/package/input/conditional-pt.fn.crossplane.io_resources.yaml b/package/input/conditional-pt.fn.crossplane.io_resources.yaml index 7c3ec29..a805c0c 100644 --- a/package/input/conditional-pt.fn.crossplane.io_resources.yaml +++ b/package/input/conditional-pt.fn.crossplane.io_resources.yaml @@ -40,10 +40,11 @@ spec: between the XR and the Environment. Either from the Environment to the XR, or vice versa. items: - description: Patch objects are applied between composite and composed - resources. Their behaviour depends on the Type selected. The default - Type, FromCompositeFieldPath, copies a value from the composite - resource to the composed resource, applying any defined transformers. + description: EnvironmentPatch objects are applied between the composite + resource and the environment. Their behaviour depends on the Type + selected. The default Type, FromCompositeFieldPath, copies a value + from the composite resource to the environment, applying any defined + transformers. properties: combine: description: Combine is the patch configuration for a CombineFromComposite, @@ -94,10 +95,6 @@ spec: whose value is to be used as input. Required when type is FromCompositeFieldPath or ToCompositeFieldPath. type: string - patchSetName: - description: PatchSetName to include patches from. Required - when type is PatchSet. - type: string policy: description: Policy configures the specifics of patching behaviour. properties: @@ -325,7 +322,6 @@ spec: object. enum: - FromCompositeFieldPath - - PatchSet - ToCompositeFieldPath - CombineFromComposite - CombineToComposite @@ -353,11 +349,8 @@ spec: patches: description: Patches will be applied as an overlay to the base resource. items: - description: Patch objects are applied between composite and composed - resources. Their behaviour depends on the Type selected. The - default Type, FromCompositeFieldPath, copies a value from the - composite resource to the composed resource, applying any defined - transformers. + description: PatchSetPatch defines a set of Patches that can be + referenced by name by other patches of type PatchSet. properties: combine: description: Combine is the patch configuration for a CombineFromComposite, @@ -409,10 +402,6 @@ spec: resource whose value is to be used as input. Required when type is FromCompositeFieldPath or ToCompositeFieldPath. type: string - patchSetName: - description: PatchSetName to include patches from. Required - when type is PatchSet. - type: string policy: description: Policy configures the specifics of patching behaviour. properties: @@ -641,13 +630,16 @@ spec: default: FromCompositeFieldPath description: Type sets the patching behaviour to be used. Each patch type may require its own fields to be set on - the Patch object. + the ComposedPatch object. enum: - FromCompositeFieldPath - - PatchSet - ToCompositeFieldPath - CombineFromComposite - CombineToComposite + - FromEnvironmentFieldPath + - ToEnvironmentFieldPath + - CombineFromEnvironment + - CombineToEnvironment type: string type: object type: array @@ -727,11 +719,11 @@ spec: patches: description: Patches to and from the composed resource. items: - description: Patch objects are applied between composite and composed - resources. Their behaviour depends on the Type selected. The - default Type, FromCompositeFieldPath, copies a value from the - composite resource to the composed resource, applying any defined - transformers. + description: ComposedPatch objects are applied between composite + and composed resources. Their behaviour depends on the Type + selected. The default Type, FromCompositeFieldPath, copies a + value from the composite resource to the composed resource, + applying any defined transformers. properties: combine: description: Combine is the patch configuration for a CombineFromComposite, @@ -1015,13 +1007,17 @@ spec: default: FromCompositeFieldPath description: Type sets the patching behaviour to be used. Each patch type may require its own fields to be set on - the Patch object. + the ComposedPatch object. enum: - FromCompositeFieldPath - PatchSet - ToCompositeFieldPath - CombineFromComposite - CombineToComposite + - FromEnvironmentFieldPath + - ToEnvironmentFieldPath + - CombineFromEnvironment + - CombineToEnvironment type: string type: object type: array diff --git a/patches.go b/patches.go index 565369d..57fecdf 100644 --- a/patches.go +++ b/patches.go @@ -25,16 +25,32 @@ const ( errFmtExpandingArrayFieldPaths = "cannot expand ToFieldPath %s" ) +// A PatchInterface is a patch that can be applied between resources. +type PatchInterface interface { + GetType() v1beta1.PatchType + GetFromFieldPath() string + GetToFieldPath() string + GetCombine() *v1beta1.Combine + GetTransforms() []v1beta1.Transform + GetPolicy() *v1beta1.PatchPolicy +} + +// PatchWithPatchSetName is a PatchInterface that has a PatchSetName field. +type PatchWithPatchSetName interface { + PatchInterface + GetPatchSetName() string +} + // Apply executes a patching operation between the from and to resources. // Applies all patch types unless an 'only' filter is supplied. -func Apply(p v1beta1.Patch, xr resource.Composite, cd resource.Composed, only ...v1beta1.PatchType) error { +func Apply(p PatchInterface, xr resource.Composite, cd resource.Composed, only ...v1beta1.PatchType) error { return ApplyToObjects(p, xr, cd, only...) } // ApplyToObjects works like Apply but accepts any kind of runtime.Object. It // might be vulnerable to conversion panics (see // https://github.com/crossplane/crossplane/pull/3394 for details). -func ApplyToObjects(p v1beta1.Patch, a, b runtime.Object, only ...v1beta1.PatchType) error { +func ApplyToObjects(p PatchInterface, a, b runtime.Object, only ...v1beta1.PatchType) error { if filterPatch(p, only...) { return nil } @@ -51,11 +67,11 @@ func ApplyToObjects(p v1beta1.Patch, a, b runtime.Object, only ...v1beta1.PatchT case v1beta1.PatchTypePatchSet: // Already resolved - nothing to do. } - return errors.Errorf(errFmtInvalidPatchType, p.Type) + return errors.Errorf(errFmtInvalidPatchType, p.GetType()) } // filterPatch returns true if patch should be filtered (not applied) -func filterPatch(p v1beta1.Patch, only ...v1beta1.PatchType) bool { +func filterPatch(p PatchInterface, only ...v1beta1.PatchType) bool { // filter does not apply if not set if len(only) == 0 { return false @@ -70,9 +86,9 @@ func filterPatch(p v1beta1.Patch, only ...v1beta1.PatchType) bool { } // ResolveTransforms applies a list of transforms to a patch value. -func ResolveTransforms(c v1beta1.Patch, input any) (any, error) { +func ResolveTransforms(ts []v1beta1.Transform, input any) (any, error) { var err error - for i, t := range c.Transforms { + for i, t := range ts { if input, err = Resolve(t, input); err != nil { // TODO(negz): Including the type might help find the offending transform faster. return nil, errors.Wrapf(err, errFmtTransformAtIndex, i) @@ -84,14 +100,9 @@ func ResolveTransforms(c v1beta1.Patch, input any) (any, error) { // ApplyFromFieldPathPatch patches the "to" resource, using a source field // on the "from" resource. Values may be transformed if any are defined on // the patch. -func ApplyFromFieldPathPatch(p v1beta1.Patch, from, to runtime.Object) error { - if p.FromFieldPath == nil { - return errors.Errorf(errFmtRequiredField, "FromFieldPath", p.Type) - } - - // Default to patching the same field on the composed resource. - if p.ToFieldPath == nil { - p.ToFieldPath = p.FromFieldPath +func ApplyFromFieldPathPatch(p PatchInterface, from, to runtime.Object) error { + if p.GetFromFieldPath() == "" { + return errors.Errorf(errFmtRequiredField, "FromFieldPath", p.GetType()) } fromMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(from) @@ -99,8 +110,8 @@ func ApplyFromFieldPathPatch(p v1beta1.Patch, from, to runtime.Object) error { return err } - in, err := fieldpath.Pave(fromMap).GetValue(*p.FromFieldPath) - if IsOptionalFieldPathNotFound(err, p.Policy) { + in, err := fieldpath.Pave(fromMap).GetValue(p.GetFromFieldPath()) + if IsOptionalFieldPathNotFound(err, p.GetPolicy()) { return nil } if err != nil { @@ -108,35 +119,36 @@ func ApplyFromFieldPathPatch(p v1beta1.Patch, from, to runtime.Object) error { } // Apply transform pipeline - out, err := ResolveTransforms(p, in) + out, err := ResolveTransforms(p.GetTransforms(), in) if err != nil { return err } - // Patch all expanded fields if the ToFieldPath contains wildcards - if strings.Contains(*p.ToFieldPath, "[*]") { - return patchFieldValueToMultiple(*p.ToFieldPath, out, to) + // ComposedPatch all expanded fields if the ToFieldPath contains wildcards + if strings.Contains(p.GetToFieldPath(), "[*]") { + return patchFieldValueToMultiple(p.GetToFieldPath(), out, to) } - return errors.Wrap(patchFieldValueToObject(*p.ToFieldPath, out, to), "cannot patch to object") + return errors.Wrap(patchFieldValueToObject(p.GetToFieldPath(), out, to), "cannot patch to object") } // ApplyCombineFromVariablesPatch patches the "to" resource, taking a list of // input variables and combining them into a single output value. // The single output value may then be further transformed if they are defined // on the patch. -func ApplyCombineFromVariablesPatch(p v1beta1.Patch, from, to runtime.Object) error { +func ApplyCombineFromVariablesPatch(p PatchInterface, from, to runtime.Object) error { // Combine patch requires configuration - if p.Combine == nil { - return errors.Errorf(errFmtRequiredField, "Combine", p.Type) + if p.GetCombine() == nil { + return errors.Errorf(errFmtRequiredField, "Combine", p.GetType()) } // Destination field path is required since we can't default to multiple // fields. - if p.ToFieldPath == nil { - return errors.Errorf(errFmtRequiredField, "ToFieldPath", p.Type) + if p.GetToFieldPath() == "" { + return errors.Errorf(errFmtRequiredField, "ToFieldPath", p.GetType()) } - vl := len(p.Combine.Variables) + combine := p.GetCombine() + vl := len(combine.Variables) if vl < 1 { return errors.New(errCombineRequiresVariables) @@ -153,7 +165,7 @@ func ApplyCombineFromVariablesPatch(p v1beta1.Patch, from, to runtime.Object) er // NOTE: This currently assumes all variables define a 'fromFieldPath' // value. If we add new variable types, this may not be the case and // this code may be better served split out into a dedicated function. - for i, sp := range p.Combine.Variables { + for i, sp := range combine.Variables { iv, err := fieldpath.Pave(fromMap).GetValue(sp.FromFieldPath) // If any source field is not found, we will not @@ -162,7 +174,7 @@ func ApplyCombineFromVariablesPatch(p v1beta1.Patch, from, to runtime.Object) er // number of inputs (e.g. a string format // expecting 3 fields '%s-%s-%s' but only // receiving 2 values). - if IsOptionalFieldPathNotFound(err, p.Policy) { + if IsOptionalFieldPathNotFound(err, p.GetPolicy()) { return nil } if err != nil { @@ -172,18 +184,18 @@ func ApplyCombineFromVariablesPatch(p v1beta1.Patch, from, to runtime.Object) er } // Combine input values - cb, err := Combine(*p.Combine, in) + cb, err := Combine(*p.GetCombine(), in) if err != nil { return err } // Apply transform pipeline - out, err := ResolveTransforms(p, cb) + out, err := ResolveTransforms(p.GetTransforms(), cb) if err != nil { return err } - return errors.Wrap(patchFieldValueToObject(*p.ToFieldPath, out, to), "cannot patch to object") + return errors.Wrap(patchFieldValueToObject(p.GetToFieldPath(), out, to), "cannot patch to object") } // IsOptionalFieldPathNotFound returns true if the supplied error indicates a @@ -231,19 +243,19 @@ func CombineString(format string, vars []any) string { // ComposedTemplates returns the supplied composed resource templates with any // supplied patchsets dereferenced. func ComposedTemplates(pss []v1beta1.PatchSet, cts []v1beta1.ComposedTemplate) ([]v1beta1.ComposedTemplate, error) { - pn := make(map[string][]v1beta1.Patch) + pn := make(map[string][]v1beta1.ComposedPatch) for _, s := range pss { for _, p := range s.Patches { if p.Type == v1beta1.PatchTypePatchSet { return nil, errors.New(errPatchSetType) } } - pn[s.Name] = s.Patches + pn[s.Name] = s.GetComposedPatches() } ct := make([]v1beta1.ComposedTemplate, len(cts)) for i, r := range cts { - var po []v1beta1.Patch + var po []v1beta1.ComposedPatch for _, p := range r.Patches { if p.Type != v1beta1.PatchTypePatchSet { po = append(po, p) diff --git a/patches_test.go b/patches_test.go index 05f77d0..335a19c 100644 --- a/patches_test.go +++ b/patches_test.go @@ -28,7 +28,7 @@ func TestPatchApply(t *testing.T) { } type args struct { - patch v1beta1.Patch + patch v1beta1.ComposedPatch xr *composite.Unstructured cd *composed.Unstructured only []v1beta1.PatchType @@ -47,7 +47,7 @@ func TestPatchApply(t *testing.T) { "InvalidCompositeFieldPathPatch": { reason: "Should return error when required fields not passed to applyFromFieldPathPatch", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeFromCompositeFieldPath, // This is missing fields. }, @@ -61,7 +61,7 @@ func TestPatchApply(t *testing.T) { "Invalidv1.PatchType": { reason: "Should return an error if an invalid patch type is specified", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: "invalid-patchtype", }, xr: &composite.Unstructured{}, @@ -74,10 +74,12 @@ func TestPatchApply(t *testing.T) { "ValidCompositeFieldPathPatch": { reason: "Should correctly apply a CompositeFieldPathPatch with valid settings", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.labels"), - ToFieldPath: ptr.To[string]("metadata.labels"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.labels"), + ToFieldPath: ptr.To[string]("metadata.labels"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -119,10 +121,12 @@ func TestPatchApply(t *testing.T) { "ValidCompositeFieldPathPatchWithWildcards": { reason: "When passed a wildcarded path, adds a field to each element of an array", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), - ToFieldPath: ptr.To[string]("metadata.ownerReferences[*].name"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + ToFieldPath: ptr.To[string]("metadata.ownerReferences[*].name"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -172,10 +176,12 @@ func TestPatchApply(t *testing.T) { "InvalidCompositeFieldPathPatchWithWildcards": { reason: "When passed a wildcarded path, throws an error if ToFieldPath cannot be expanded", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), - ToFieldPath: ptr.To[string]("metadata.ownerReferences[*].badField"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + ToFieldPath: ptr.To[string]("metadata.ownerReferences[*].badField"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -210,10 +216,12 @@ func TestPatchApply(t *testing.T) { "MissingOptionalFieldPath": { reason: "A FromFieldPath patch should be a no-op when an optional fromFieldPath doesn't exist", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.labels"), - ToFieldPath: ptr.To[string]("metadata.labels"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.labels"), + ToFieldPath: ptr.To[string]("metadata.labels"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -250,16 +258,18 @@ func TestPatchApply(t *testing.T) { "MissingRequiredFieldPath": { reason: "A FromFieldPath patch should return an error when a required fromFieldPath doesn't exist", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("wat"), - Policy: &v1beta1.PatchPolicy{ - FromFieldPath: func() *v1beta1.FromFieldPathPolicy { - s := v1beta1.FromFieldPathPolicyRequired - return &s - }(), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("wat"), + Policy: &v1beta1.PatchPolicy{ + FromFieldPath: func() *v1beta1.FromFieldPathPolicy { + s := v1beta1.FromFieldPathPolicyRequired + return &s + }(), + }, + ToFieldPath: ptr.To[string]("wat"), }, - ToFieldPath: ptr.To[string]("wat"), }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -296,10 +306,12 @@ func TestPatchApply(t *testing.T) { "FilterExcludeCompositeFieldPathPatch": { reason: "Should not apply the patch as the v1.PatchType is not present in filter.", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.labels"), - ToFieldPath: ptr.To[string]("metadata.labels"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.labels"), + ToFieldPath: ptr.To[string]("metadata.labels"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -323,10 +335,12 @@ func TestPatchApply(t *testing.T) { "FilterIncludeCompositeFieldPathPatch": { reason: "Should apply the patch as the v1.PatchType is present in filter.", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.labels"), - ToFieldPath: ptr.To[string]("metadata.labels"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.labels"), + ToFieldPath: ptr.To[string]("metadata.labels"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -365,9 +379,11 @@ func TestPatchApply(t *testing.T) { "DefaultToFieldCompositeFieldPathPatch": { reason: "Should correctly default the ToFieldPath value if not specified.", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.labels"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.labels"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -405,10 +421,12 @@ func TestPatchApply(t *testing.T) { "ValidToCompositeFieldPathPatch": { reason: "Should correctly apply a ToCompositeFieldPath patch with valid settings", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeToCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.labels"), - ToFieldPath: ptr.To[string]("metadata.labels"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeToCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.labels"), + ToFieldPath: ptr.To[string]("metadata.labels"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -446,10 +464,12 @@ func TestPatchApply(t *testing.T) { "ValidToCompositeFieldPathPatchWithWildcards": { reason: "When passed a wildcarded path, adds a field to each element of an array", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeToCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), - ToFieldPath: ptr.To[string]("metadata.ownerReferences[*].name"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeToCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + ToFieldPath: ptr.To[string]("metadata.ownerReferences[*].name"), + }, }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -499,10 +519,13 @@ func TestPatchApply(t *testing.T) { "MissingCombineFromCompositeConfig": { reason: "Should return an error if Combine config is not passed", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeCombineFromComposite, - ToFieldPath: ptr.To[string]("metadata.labels.destination"), - // Missing a Combine field + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeCombineFromComposite, + Patch: v1beta1.Patch{ + ToFieldPath: ptr.To[string]("metadata.labels.destination"), + // Missing a Combine field + Combine: nil, + }, }, xr: &composite.Unstructured{}, cd: &composed.Unstructured{}, @@ -516,17 +539,19 @@ func TestPatchApply(t *testing.T) { "MissingCombineStrategyFromCompositeConfig": { reason: "Should return an error if Combine strategy config is not passed", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineFromComposite, - Combine: &v1beta1.Combine{ - Variables: []v1beta1.CombineVariable{ - {FromFieldPath: "metadata.labels.source1"}, - {FromFieldPath: "metadata.labels.source2"}, + Patch: v1beta1.Patch{ + Combine: &v1beta1.Combine{ + Variables: []v1beta1.CombineVariable{ + {FromFieldPath: "metadata.labels.source1"}, + {FromFieldPath: "metadata.labels.source2"}, + }, + Strategy: v1beta1.CombineStrategyString, + // Missing a String combine config. }, - Strategy: v1beta1.CombineStrategyString, - // Missing a String combine config. + ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, - ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -548,15 +573,17 @@ func TestPatchApply(t *testing.T) { "MissingCombineVariablesFromCompositeConfig": { reason: "Should return an error if no variables have been passed", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineFromComposite, - Combine: &v1beta1.Combine{ - // This is empty. - Variables: []v1beta1.CombineVariable{}, - Strategy: v1beta1.CombineStrategyString, - String: &v1beta1.StringCombine{Format: "%s-%s"}, + Patch: v1beta1.Patch{ + Combine: &v1beta1.Combine{ + // This is empty. + Variables: []v1beta1.CombineVariable{}, + Strategy: v1beta1.CombineStrategyString, + String: &v1beta1.StringCombine{Format: "%s-%s"}, + }, + ToFieldPath: ptr.To[string]("objectMeta.labels.destination"), }, - ToFieldPath: ptr.To[string]("objectMeta.labels.destination"), }, }, want: want{ @@ -569,18 +596,20 @@ func TestPatchApply(t *testing.T) { // not available. reason: "Should return no error and not apply patch if an optional variable is missing", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineFromComposite, - Combine: &v1beta1.Combine{ - Variables: []v1beta1.CombineVariable{ - {FromFieldPath: "metadata.labels.source1"}, - {FromFieldPath: "metadata.labels.source2"}, - {FromFieldPath: "metadata.labels.source3"}, + Patch: v1beta1.Patch{ + Combine: &v1beta1.Combine{ + Variables: []v1beta1.CombineVariable{ + {FromFieldPath: "metadata.labels.source1"}, + {FromFieldPath: "metadata.labels.source2"}, + {FromFieldPath: "metadata.labels.source3"}, + }, + Strategy: v1beta1.CombineStrategyString, + String: &v1beta1.StringCombine{Format: "%s-%s"}, }, - Strategy: v1beta1.CombineStrategyString, - String: &v1beta1.StringCombine{Format: "%s-%s"}, + ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, - ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -602,17 +631,19 @@ func TestPatchApply(t *testing.T) { "ValidCombineFromComposite": { reason: "Should correctly apply a CombineFromComposite patch with valid settings", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineFromComposite, - Combine: &v1beta1.Combine{ - Variables: []v1beta1.CombineVariable{ - {FromFieldPath: "metadata.labels.source1"}, - {FromFieldPath: "metadata.labels.source2"}, + Patch: v1beta1.Patch{ + Combine: &v1beta1.Combine{ + Variables: []v1beta1.CombineVariable{ + {FromFieldPath: "metadata.labels.source1"}, + {FromFieldPath: "metadata.labels.source2"}, + }, + Strategy: v1beta1.CombineStrategyString, + String: &v1beta1.StringCombine{Format: "%s-%s"}, }, - Strategy: v1beta1.CombineStrategyString, - String: &v1beta1.StringCombine{Format: "%s-%s"}, + ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, - ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -657,17 +688,19 @@ func TestPatchApply(t *testing.T) { "ValidCombineToComposite": { reason: "Should correctly apply a CombineToComposite patch with valid settings", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineToComposite, - Combine: &v1beta1.Combine{ - Variables: []v1beta1.CombineVariable{ - {FromFieldPath: "metadata.labels.source1"}, - {FromFieldPath: "metadata.labels.source2"}, + Patch: v1beta1.Patch{ + Combine: &v1beta1.Combine{ + Variables: []v1beta1.CombineVariable{ + {FromFieldPath: "metadata.labels.source1"}, + {FromFieldPath: "metadata.labels.source2"}, + }, + Strategy: v1beta1.CombineStrategyString, + String: &v1beta1.StringCombine{Format: "%s-%s"}, }, - Strategy: v1beta1.CombineStrategyString, - String: &v1beta1.StringCombine{Format: "%s-%s"}, + ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, - ToFieldPath: ptr.To[string]("metadata.labels.destination"), }, xr: &composite.Unstructured{ Unstructured: unstructured.Unstructured{Object: MustObject(`{ @@ -713,7 +746,7 @@ func TestPatchApply(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { ncp := tc.args.xr.DeepCopyObject().(resource.Composite) - err := Apply(tc.args.patch, ncp, tc.args.cd, tc.args.only...) + err := Apply(&tc.args.patch, ncp, tc.args.cd, tc.args.only...) if tc.want.xr != nil { if diff := cmp.Diff(tc.want.xr, ncp); diff != "" { @@ -851,14 +884,18 @@ func TestComposedTemplates(t *testing.T) { args: args{ cts: []v1beta1.ComposedTemplate{ { - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.namespace"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.namespace"), + }, }, }, }, @@ -867,14 +904,18 @@ func TestComposedTemplates(t *testing.T) { want: want{ ct: []v1beta1.ComposedTemplate{ { - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.namespace"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.namespace"), + }, }, }, }, @@ -885,7 +926,7 @@ func TestComposedTemplates(t *testing.T) { reason: "Should return error and not modify the patches field when referring to an undefined PatchSet", args: args{ cts: []v1beta1.ComposedTemplate{{ - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { Type: v1beta1.PatchTypePatchSet, PatchSetName: ptr.To[string]("patch-set-1"), @@ -905,50 +946,60 @@ func TestComposedTemplates(t *testing.T) { pss: []v1beta1.PatchSet{ { Name: "patch-set-1", - Patches: []v1beta1.Patch{ + Patches: []v1beta1.PatchSetPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.namespace"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.namespace"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.parameters.test"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.parameters.test"), + }, }, }, }, { Name: "patch-set-2", - Patches: []v1beta1.Patch{ + Patches: []v1beta1.PatchSetPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-1"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-1"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-2"), - Transforms: []v1beta1.Transform{{ - Type: v1beta1.TransformTypeMap, - Map: &v1beta1.MapTransform{ - Pairs: map[string]extv1.JSON{ - "k-1": asJSON("v-1"), - "k-2": asJSON("v-2"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-2"), + Transforms: []v1beta1.Transform{{ + Type: v1beta1.TransformTypeMap, + Map: &v1beta1.MapTransform{ + Pairs: map[string]extv1.JSON{ + "k-1": asJSON("v-1"), + "k-2": asJSON("v-2"), + }, }, - }, - }}, + }}, + }, }, }, }, }, cts: []v1beta1.ComposedTemplate{ { - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { Type: v1beta1.PatchTypePatchSet, PatchSetName: ptr.To[string]("patch-set-2"), }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + }, }, { Type: v1beta1.PatchTypePatchSet, @@ -957,7 +1008,7 @@ func TestComposedTemplates(t *testing.T) { }, }, { - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { Type: v1beta1.PatchTypePatchSet, PatchSetName: ptr.To[string]("patch-set-1"), @@ -970,47 +1021,61 @@ func TestComposedTemplates(t *testing.T) { err: nil, ct: []v1beta1.ComposedTemplate{ { - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-1"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-1"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-2"), - Transforms: []v1beta1.Transform{{ - Type: v1beta1.TransformTypeMap, - Map: &v1beta1.MapTransform{ - Pairs: map[string]extv1.JSON{ - "k-1": asJSON("v-1"), - "k-2": asJSON("v-2"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.annotations.patch-test-2"), + Transforms: []v1beta1.Transform{{ + Type: v1beta1.TransformTypeMap, + Map: &v1beta1.MapTransform{ + Pairs: map[string]extv1.JSON{ + "k-1": asJSON("v-1"), + "k-2": asJSON("v-2"), + }, }, - }, - }}, + }}, + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.name"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.name"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.namespace"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.namespace"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.parameters.test"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.parameters.test"), + }, }, }, }, { - Patches: []v1beta1.Patch{ + Patches: []v1beta1.ComposedPatch{ { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("metadata.namespace"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("metadata.namespace"), + }, }, { - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.parameters.test"), + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.parameters.test"), + }, }, }, }, @@ -1114,7 +1179,7 @@ func TestResolveTransforms(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ResolveTransforms(v1beta1.Patch{Transforms: tt.args.ts}, tt.args.input) + got, err := ResolveTransforms(tt.args.ts, tt.args.input) if diff := cmp.Diff(tt.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("ResolveTransforms(...): -want error, +got error:\n%s", diff) } diff --git a/render.go b/render.go index a2dd8cd..20398ea 100644 --- a/render.go +++ b/render.go @@ -2,13 +2,15 @@ package main import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/json" "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/crossplane/function-sdk-go/resource/composed" + "github.com/crossplane/function-sdk-go/resource/composite" + "github.com/stevendborrelli/function-conditional-patch-and-transform/input/v1beta1" ) @@ -58,47 +60,92 @@ func RenderFromJSON(o resource.Object, data []byte) error { return nil } -// RenderFromCompositePatches renders the supplied composed resource by applying -// all patches that are _from_ the supplied composite resource. -func RenderFromCompositePatches(cd resource.Composed, xr resource.Composite, p []v1beta1.Patch) error { - for i := range p { - if err := Apply(p[i], xr, cd, v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite); err != nil { - return errors.Wrapf(err, errFmtPatch, p[i].GetType(), i) - } - } - return nil -} - -// RenderToCompositePatches renders the supplied composite resource by applying -// all patches that are _from_ the supplied composed resource. -func RenderToCompositePatches(xr resource.Composite, cd resource.Composed, p []v1beta1.Patch) error { - for i := range p { - if err := Apply(p[i], xr, cd, v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite); err != nil { - return errors.Wrapf(err, errFmtPatch, p[i].GetType(), i) - } - } - return nil -} - -// RenderFromEnvironmentPatches renders the supplied object (an XR or composed -// resource) by applying all patches that are from the supplied environment. -func RenderFromEnvironmentPatches(o runtime.Object, env *unstructured.Unstructured, p []v1beta1.Patch) error { - for i := range p { - if err := ApplyToObjects(p[i], env, o, v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment); err != nil { - return errors.Wrapf(err, errFmtPatch, p[i].Type, i) +// RenderEnvironmentPatches renders the supplied environment by applying all +// patches that are to the environment, from the supplied XR. +func RenderEnvironmentPatches(env *unstructured.Unstructured, oxr, dxr *composite.Unstructured, ps []v1beta1.EnvironmentPatch) error { + for i, p := range ps { + p := p + switch p.Type { + case v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment: + if err := ApplyToObjects(&p, env, oxr); err != nil { + return errors.Wrapf(err, errFmtPatch, p.Type, i) + } + case v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment: + if err := ApplyToObjects(&p, env, dxr); err != nil { + return errors.Wrapf(err, errFmtPatch, p.Type, i) + } + case v1beta1.PatchTypePatchSet, v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite, v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite: + // nothing to do } } return nil } -// RenderToEnvironmentPatches renders the supplied environment by applying all -// patches that are to the environment, from the supplied object (an XR or -// composed resource). -func RenderToEnvironmentPatches(env *unstructured.Unstructured, o runtime.Object, p []v1beta1.Patch) error { - for i := range p { - if err := ApplyToObjects(p[i], env, o, v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment); err != nil { - return errors.Wrapf(err, errFmtPatch, p[i].Type, i) +// RenderComposedPatches renders the supplied composed resource by applying all +// patches that are to or from the supplied composite resource and environment +// in the order they were defined. Properly selecting the right source or +// destination between observed and desired resources. +func RenderComposedPatches( //nolint:gocyclo // just a switch + ocd *composed.Unstructured, + dcd *composed.Unstructured, + oxr *composite.Unstructured, + dxr *composite.Unstructured, + env *unstructured.Unstructured, + ps []v1beta1.ComposedPatch, +) (errs []error, store bool) { + for i, p := range ps { + p := p + switch t := p.Type; t { + case v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite: + // TODO(negz): Should failures to patch the XR be terminal? It could + // indicate a required patch failed. A required patch means roughly + // "this patch has to succeed before you mutate the resource". This + // is useful to make sure we never create a composed resource in the + // wrong state. It's less clear how useful it is for the XR, given + // we'll only ever be updating it, not creating it. + + // We want to patch the XR from observed composed resources, not + // from desired state. This is because folks will typically be + // patching from a field that is set once the observed resource is + // applied such as its status. + if ocd == nil { + continue + } + if err := ApplyToObjects(&p, dxr, ocd); err != nil { + errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i)) + } + case v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment: + // TODO(negz): Same as above, but for the Environment. What does it + // mean for a required patch to the environment to fail? Should it + // be terminal? + + // Run all patches that are from the (observed) composed resource to + // the environment. + if ocd == nil { + continue + } + if err := ApplyToObjects(&p, env, ocd); err != nil { + errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i)) + } + // If either of the below renderings return an error, most likely a + // required FromComposite or FromEnvironment patch failed. A required + // patch means roughly "this patch has to succeed before you mutate the + // resource." This is useful to make sure we never create a composed + // resource in the wrong state. To that end, we don't want to add this + // resource to our accumulated desired state. + case v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite: + if err := ApplyToObjects(&p, oxr, dcd); err != nil { + errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i)) + return errs, false + } + case v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment: + if err := ApplyToObjects(&p, env, dcd); err != nil { + errs = append(errs, errors.Wrapf(err, errFmtPatch, t, i)) + return errs, false + } + case v1beta1.PatchTypePatchSet: + // Already resolved - nothing to do. } } - return nil + return errs, true } diff --git a/validate.go b/validate.go index 9a1d451..bcaf695 100644 --- a/validate.go +++ b/validate.go @@ -59,7 +59,8 @@ func ValidateComposedTemplate(t v1beta1.ComposedTemplate) *field.Error { return field.Required(field.NewPath("name"), "name is required") } for i, p := range t.Patches { - if err := ValidatePatch(p); err != nil { + p := p + if err := ValidatePatch(&p); err != nil { return WrapFieldError(err, field.NewPath("patches").Index(i)) } } @@ -82,7 +83,8 @@ func ValidatePatchSet(ps v1beta1.PatchSet) *field.Error { return field.Required(field.NewPath("name"), "name is required") } for i, p := range ps.Patches { - if err := ValidatePatch(p); err != nil { + p := p + if err := ValidatePatch(&p); err != nil { return WrapFieldError(err, field.NewPath("patches").Index(i)) } } @@ -95,6 +97,7 @@ func ValidateEnvironment(e *v1beta1.Environment) *field.Error { return nil } for i, p := range e.Patches { + p := p switch p.GetType() { //nolint:exhaustive // Intentionally targeting only environment patches. case v1beta1.PatchTypeCombineToEnvironment, v1beta1.PatchTypeCombineFromEnvironment, @@ -104,7 +107,7 @@ func ValidateEnvironment(e *v1beta1.Environment) *field.Error { return field.Invalid(field.NewPath("patches").Index(i).Key("type"), p.Type, "invalid environment patch type") } - if err := ValidatePatch(p); err != nil { + if err := ValidatePatch(&p); err != nil { return WrapFieldError(err, field.NewPath("patches").Index(i)) } } @@ -157,35 +160,39 @@ func ValidateMatchConditionReadinessCheck(m *v1beta1.MatchConditionReadinessChec return nil } -// ValidatePatch validates a Patch. -func ValidatePatch(p v1beta1.Patch) *field.Error { +// ValidatePatch validates a ComposedPatch. +func ValidatePatch(p PatchInterface) *field.Error { //nolint: gocyclo // This is a long but simple/same-y switch. switch p.GetType() { case v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeToEnvironmentFieldPath: - if p.FromFieldPath == nil { - return field.Required(field.NewPath("fromFieldPath"), fmt.Sprintf("fromFieldPath must be set for patch type %s", p.Type)) + if p.GetFromFieldPath() == "" { + return field.Required(field.NewPath("fromFieldPath"), fmt.Sprintf("fromFieldPath must be set for patch type %s", p.GetType())) } case v1beta1.PatchTypePatchSet: - if p.PatchSetName == nil { - return field.Required(field.NewPath("patchSetName"), fmt.Sprintf("patchSetName must be set for patch type %s", p.Type)) + ps, ok := p.(PatchWithPatchSetName) + if !ok { + return field.Invalid(field.NewPath("type"), p.GetType(), fmt.Sprintf("patch type %T does not support patch of type %s", p, p.GetType())) + } + if ps.GetPatchSetName() == "" { + return field.Required(field.NewPath("patchSetName"), fmt.Sprintf("patchSetName must be set for patch type %s", p.GetType())) } case v1beta1.PatchTypeCombineFromComposite, v1beta1.PatchTypeCombineToComposite, v1beta1.PatchTypeCombineFromEnvironment, v1beta1.PatchTypeCombineToEnvironment: - if p.Combine == nil { - return field.Required(field.NewPath("combine"), fmt.Sprintf("combine must be set for patch type %s", p.Type)) + if p.GetCombine() == nil { + return field.Required(field.NewPath("combine"), fmt.Sprintf("combine must be set for patch type %s", p.GetType())) } - if p.ToFieldPath == nil { - return field.Required(field.NewPath("toFieldPath"), fmt.Sprintf("toFieldPath must be set for patch type %s", p.Type)) + if p.GetToFieldPath() == "" { + return field.Required(field.NewPath("toFieldPath"), fmt.Sprintf("toFieldPath must be set for patch type %s", p.GetType())) } default: // Should never happen - return field.Invalid(field.NewPath("type"), p.Type, "unknown patch type") + return field.Invalid(field.NewPath("type"), p.GetType(), "unknown patch type") } - for i, t := range p.Transforms { + for i, t := range p.GetTransforms() { if err := ValidateTransform(t); err != nil { return WrapFieldError(err, field.NewPath("transforms").Index(i)) } diff --git a/validate_test.go b/validate_test.go index 23af3be..479aefe 100644 --- a/validate_test.go +++ b/validate_test.go @@ -237,7 +237,7 @@ func TestValidateConnectionDetail(t *testing.T) { func TestValidatePatch(t *testing.T) { type args struct { - patch v1beta1.Patch + patch v1beta1.ComposedPatch } type want struct { @@ -252,22 +252,26 @@ func TestValidatePatch(t *testing.T) { "ValidFromCompositeFieldPath": { reason: "FromCompositeFieldPath patch with FromFieldPath set should be valid", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.forProvider.foo"), + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.forProvider.foo"), + }, }, }, }, "FromCompositeFieldPathWithInvalidTransforms": { reason: "FromCompositeFieldPath with invalid transforms should return error", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: ptr.To[string]("spec.forProvider.foo"), - Transforms: []v1beta1.Transform{ - { - Type: v1beta1.TransformTypeMath, - Math: nil, + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: ptr.To[string]("spec.forProvider.foo"), + Transforms: []v1beta1.Transform{ + { + Type: v1beta1.TransformTypeMath, + Math: nil, + }, }, }, }, @@ -282,9 +286,11 @@ func TestValidatePatch(t *testing.T) { "InvalidFromCompositeFieldPathMissingFromFieldPath": { reason: "Invalid FromCompositeFieldPath missing FromFieldPath should return error", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeFromCompositeFieldPath, - FromFieldPath: nil, + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeFromCompositeFieldPath, + Patch: v1beta1.Patch{ + FromFieldPath: nil, + }, }, }, want: want{ @@ -297,9 +303,11 @@ func TestValidatePatch(t *testing.T) { "InvalidFromCompositeFieldPathMissingToFieldPath": { reason: "Invalid ToCompositeFieldPath missing ToFieldPath should return error", args: args{ - patch: v1beta1.Patch{ - Type: v1beta1.PatchTypeToCompositeFieldPath, - ToFieldPath: nil, + patch: v1beta1.ComposedPatch{ + Type: v1beta1.PatchTypeToCompositeFieldPath, + Patch: v1beta1.Patch{ + ToFieldPath: nil, + }, }, }, want: want{ @@ -312,7 +320,7 @@ func TestValidatePatch(t *testing.T) { "Invalidv1beta1.PatchSetMissingv1beta1.PatchSetName": { reason: "Invalid v1beta1.PatchSet missing v1beta1.PatchSetName should return error", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypePatchSet, }, }, @@ -326,7 +334,7 @@ func TestValidatePatch(t *testing.T) { "InvalidCombineMissingCombine": { reason: "Invalid Combine missing Combine should return error", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineToComposite, }, }, @@ -340,16 +348,18 @@ func TestValidatePatch(t *testing.T) { "InvalidCombineMissingToFieldPath": { reason: "Invalid Combine missing ToFieldPath should return error", args: args{ - patch: v1beta1.Patch{ + patch: v1beta1.ComposedPatch{ Type: v1beta1.PatchTypeCombineToComposite, - Combine: &v1beta1.Combine{ - Variables: []v1beta1.CombineVariable{ - { - FromFieldPath: "spec.forProvider.foo", + Patch: v1beta1.Patch{ + Combine: &v1beta1.Combine{ + Variables: []v1beta1.CombineVariable{ + { + FromFieldPath: "spec.forProvider.foo", + }, }, }, + ToFieldPath: nil, }, - ToFieldPath: nil, }, }, want: want{ @@ -362,7 +372,7 @@ func TestValidatePatch(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidatePatch(tc.args.patch) + err := ValidatePatch(&tc.args.patch) if diff := cmp.Diff(tc.want.err, err, cmpopts.IgnoreFields(field.Error{}, "Detail", "BadValue")); diff != "" { t.Errorf("%s\nValidatePatch(...): -want, +got:\n%s", tc.reason, diff) }