Merge pull request #78 from MisterMX/fix/environment-correct-patches

fix(environment): Validate against correct patch types

Author: phisco

fn_test.go

@@ -658,9 +658,9 @@ func TestRunFunction(t *testing.T) {
 						Environment: &v1beta1.Environment{
 							Patches: []v1beta1.EnvironmentPatch{
 								{
-									Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
+									Type: v1beta1.PatchTypeToCompositeFieldPath,
 									Patch: v1beta1.Patch{
-										FromFieldPath: ptr.To[string]("data.widgets"),
+										FromFieldPath: ptr.To[string]("widgets"),
 										ToFieldPath:   ptr.To[string]("spec.watchers"),
 										Transforms: []v1beta1.Transform{
 											{
@@ -713,10 +713,10 @@ func TestRunFunction(t *testing.T) {
 						Environment: &v1beta1.Environment{
 							Patches: []v1beta1.EnvironmentPatch{
 								{
-									Type: v1beta1.PatchTypeToEnvironmentFieldPath,
+									Type: v1beta1.PatchTypeFromCompositeFieldPath,
 									Patch: v1beta1.Patch{
 										FromFieldPath: ptr.To[string]("spec.watchers"),
-										ToFieldPath:   ptr.To[string]("data.widgets"),
+										ToFieldPath:   ptr.To[string]("widgets"),
 										Transforms: []v1beta1.Transform{
 											{
 												Type: v1beta1.TransformTypeMath,
@@ -764,7 +764,7 @@ func TestRunFunction(t *testing.T) {
 							Patches: []v1beta1.ComposedPatch{{
 								Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
 								Patch: v1beta1.Patch{
-									FromFieldPath: ptr.To[string]("data.widgets"),
+									FromFieldPath: ptr.To[string]("widgets"),
 									ToFieldPath:   ptr.To[string]("spec.watchers"),
 									Transforms: []v1beta1.Transform{{
 										Type: v1beta1.TransformTypeConvert,
@@ -816,7 +816,7 @@ func TestRunFunction(t *testing.T) {
 							Patches: []v1beta1.ComposedPatch{{
 								Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
 								Patch: v1beta1.Patch{
-									FromFieldPath: ptr.To[string]("data.widgets"),
+									FromFieldPath: ptr.To[string]("widgets"),
 									ToFieldPath:   ptr.To[string]("spec.watchers"),
 									Transforms: []v1beta1.Transform{{
 										Type: v1beta1.TransformTypeConvert,
@@ -872,7 +872,7 @@ func TestRunFunction(t *testing.T) {
 							Patches: []v1beta1.ComposedPatch{{
 								Type: v1beta1.PatchTypeFromEnvironmentFieldPath,
 								Patch: v1beta1.Patch{
-									FromFieldPath: ptr.To[string]("data.widgets"),
+									FromFieldPath: ptr.To[string]("widgets"),
 									ToFieldPath:   ptr.To[string]("spec.watchers"),
 									Transforms: []v1beta1.Transform{{
 										Type: v1beta1.TransformTypeConvert,
@@ -937,18 +937,16 @@ func TestRunFunction(t *testing.T) {
 }
 
 // Crossplane sends as context a fake resource:
-// { "apiVersion": "internal.crossplane.io/v1alpha1", "kind": "Environment", "data": {... the actual environment content ...} }
+// { "apiVersion": "internal.crossplane.io/v1alpha1", "kind": "Environment", ... 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.
+// an object.
 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 := unstructured.Unstructured{Object: data}
 	u.SetGroupVersionKind(schema.GroupVersionKind{Group: "internal.crossplane.io", Version: "v1alpha1", Kind: "Environment"})
 	d, err := structpb.NewStruct(u.UnstructuredContent())
 	if err != nil {

render.go

@@ -66,15 +66,15 @@ func RenderEnvironmentPatches(env *unstructured.Unstructured, oxr, dxr *composit
 	for i, p := range ps {
 		p := p
 		switch p.GetType() {
-		case v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment:
-			if err := ApplyToObjects(&p, env, oxr); err != nil {
+		case v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite:
+			if err := ApplyToObjects(&p, oxr, env); err != nil {
 				return errors.Wrapf(err, errFmtPatch, p.GetType(), i)
 			}
-		case v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment:
-			if err := ApplyToObjects(&p, env, dxr); err != nil {
+		case v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite:
+			if err := ApplyToObjects(&p, dxr, env); err != nil {
 				return errors.Wrapf(err, errFmtPatch, p.GetType(), i)
 			}
-		case v1beta1.PatchTypePatchSet, v1beta1.PatchTypeFromCompositeFieldPath, v1beta1.PatchTypeCombineFromComposite, v1beta1.PatchTypeToCompositeFieldPath, v1beta1.PatchTypeCombineToComposite:
+		case v1beta1.PatchTypePatchSet, v1beta1.PatchTypeFromEnvironmentFieldPath, v1beta1.PatchTypeCombineFromEnvironment, v1beta1.PatchTypeToEnvironmentFieldPath, v1beta1.PatchTypeCombineToEnvironment:
 			// nothing to do
 		}
 	}

validate.go

@@ -98,11 +98,12 @@ func ValidateEnvironment(e *v1beta1.Environment) *field.Error {
 	}
 	for i, p := range e.Patches {
 		p := p
-		switch p.GetType() { //nolint:exhaustive // Intentionally targeting only environment patches.
-		case v1beta1.PatchTypeCombineToEnvironment,
-			v1beta1.PatchTypeCombineFromEnvironment,
-			v1beta1.PatchTypeFromEnvironmentFieldPath,
-			v1beta1.PatchTypeToEnvironmentFieldPath:
+		switch p.GetType() { //nolint:exhaustive // Only target valid patches according the API spec
+		case
+			v1beta1.PatchTypeFromCompositeFieldPath,
+			v1beta1.PatchTypeToCompositeFieldPath,
+			v1beta1.PatchTypeCombineFromComposite,
+			v1beta1.PatchTypeCombineToComposite:
 		default:
 			return field.Invalid(field.NewPath("patches").Index(i).Key("type"), p.GetType(), "invalid environment patch type")
 		}