Merge pull request crossplane#5051 from MisterMX/fix/composition-envi…

…ronment-patch-order fix(environment): Revert patch order back to v1.13
nokia · Nov 22, 2023 · a0c5868 · a0c5868
2 parents e7f998d + 6c97f9b
commit a0c5868
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 30 deletions.
diff --git a/internal/controller/apiextensions/composite/composition_pt.go b/internal/controller/apiextensions/composite/composition_pt.go
@@ -48,15 +48,14 @@ const (
 	errFetchDetails  = "cannot fetch connection details"
 	errInline        = "cannot inline Composition patch sets"
 
-	errFmtPatchEnvironment             = "cannot apply environment patch at index %d"
-	errFmtParseBase                    = "cannot parse base template of composed resource %q"
-	errFmtRenderFromCompositePatches   = "cannot render FromComposite patches for composed resource %q"
-	errFmtRenderToCompositePatches     = "cannot render ToComposite patches for composed resource %q"
-	errFmtRenderFromEnvironmentPatches = "cannot render FromEnvironment patches for composed resource %q"
-	errFmtRenderMetadata               = "cannot render metadata for composed resource %q"
-	errFmtGenerateName                 = "cannot generate a name for composed resource %q"
-	errFmtExtractDetails               = "cannot extract composite resource connection details from composed resource %q"
-	errFmtCheckReadiness               = "cannot check whether composed resource %q is ready"
+	errFmtPatchEnvironment           = "cannot apply environment patch at index %d"
+	errFmtParseBase                  = "cannot parse base template of composed resource %q"
+	errFmtRenderFromCompositePatches = "cannot render FromComposite or environment patches for composed resource %q"
+	errFmtRenderToCompositePatches   = "cannot render ToComposite patches for composed resource %q"
+	errFmtRenderMetadata             = "cannot render metadata for composed resource %q"
+	errFmtGenerateName               = "cannot generate a name for composed resource %q"
+	errFmtExtractDetails             = "cannot extract composite resource connection details from composed resource %q"
+	errFmtCheckReadiness             = "cannot check whether composed resource %q is ready"
 )
 
 // TODO(negz): Move P&T Composition logic into its own package?
@@ -217,16 +216,11 @@ func (c *PTComposer) Compose(ctx context.Context, xr *composite.Unstructured, re
 		// unblock it.
 
 		rendered := true
-		if err := RenderFromCompositePatches(r, xr, ta.Template.Patches); err != nil {
+		if err := RenderFromCompositeAndEnvironmentPatches(r, xr, req.Environment, ta.Template.Patches); err != nil {
 			events = append(events, event.Warning(reasonCompose, errors.Wrapf(err, errFmtRenderFromCompositePatches, name)))
 			rendered = false
 		}
 
-		if err = RenderToAndFromEnvironmentPatches(r, req.Environment, ta.Template.Patches); err != nil {
-			events = append(events, event.Warning(reasonCompose, errors.Wrapf(err, errFmtRenderFromEnvironmentPatches, name)))
-			rendered = false
-		}
-
 		if err := RenderComposedResourceMetadata(r, xr, ResourceName(ptr.Deref(ta.Template.Name, ""))); err != nil {
 			events = append(events, event.Warning(reasonCompose, errors.Wrapf(err, errFmtRenderMetadata, name)))
 			rendered = false

diff --git a/internal/controller/apiextensions/composite/composition_render.go b/internal/controller/apiextensions/composite/composition_render.go
@@ -80,26 +80,19 @@ 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 []v1.Patch) error {
+// RenderFromCompositeAndEnvironmentPatches renders the supplied composed
+// resource by applying all patches that are _from_ the supplied composite
+// resource or are from or to the supplied environment.
+func RenderFromCompositeAndEnvironmentPatches(cd resource.Composed, xr resource.Composite, e *Environment, p []v1.Patch) error {
 	for i := range p {
 		if err := Apply(p[i], xr, cd, patchTypesFromXR()...); err != nil {
 			return errors.Wrapf(err, errFmtPatch, p[i].Type, i)
 		}
-	}
-	return nil
-}
 
-// RenderToAndFromEnvironmentPatches renders the supplied composed resource by
-// applying all patches that are from or to the supplied environment.
-func RenderToAndFromEnvironmentPatches(cd resource.Composed, e *Environment, p []v1.Patch) error {
-	if e == nil {
-		return nil
-	}
-	for i := range p {
-		if err := ApplyToObjects(p[i], e, cd, patchTypesFromToEnvironment()...); err != nil {
-			return errors.Wrapf(err, errFmtPatch, p[i].Type, i)
+		if e != nil {
+			if err := ApplyToObjects(p[i], e, cd, patchTypesFromToEnvironment()...); err != nil {
+				return errors.Wrapf(err, errFmtPatch, p[i].Type, i)
+			}
 		}
 	}
 	return nil