From 2b220b13fa095cb0c9e8836af66160d8bb1401fb Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Fri, 9 Feb 2024 15:48:40 +0000 Subject: [PATCH 1/5] fix: ignore invalid resources when composing Signed-off-by: Philippe Scorsolini (cherry picked from commit f816713039df09a0fe53b6085b4f47e73c20cccd) Signed-off-by: Philippe Scorsolini --- .../apiextensions/composite/composed.go | 8 ++- .../composite/composition_functions.go | 66 ++++++++++++------- .../composite/composition_functions_test.go | 4 +- .../apiextensions/composite/composition_pt.go | 19 +++++- .../composite/composition_pt_test.go | 3 + .../apiextensions/composite/reconciler.go | 58 +++++++++++----- .../composite/reconciler_test.go | 6 ++ 7 files changed, 121 insertions(+), 43 deletions(-) diff --git a/internal/controller/apiextensions/composite/composed.go b/internal/controller/apiextensions/composite/composed.go index 14b293a5b..8d89a5c75 100644 --- a/internal/controller/apiextensions/composite/composed.go +++ b/internal/controller/apiextensions/composite/composed.go @@ -35,8 +35,14 @@ type ComposedResource struct { ResourceName ResourceName // Ready indicates whether this composed resource is ready - i.e. whether - // all of its readiness checks passed. + // all of its readiness checks passed. Setting it to false will cause the + // XR to be marked as not ready. Ready bool + + // Synced indicates whether the composition process was able to sync the + // composed resource with its desired state. Setting it to false will cause + // the XR to be marked as not synced. + Synced bool } // ComposedResourceState represents a composed resource (either desired or diff --git a/internal/controller/apiextensions/composite/composition_functions.go b/internal/controller/apiextensions/composite/composition_functions.go index 747100b48..0b7ff59ae 100644 --- a/internal/controller/apiextensions/composite/composition_functions.go +++ b/internal/controller/apiextensions/composite/composition_functions.go @@ -373,6 +373,46 @@ func (c *FunctionComposer) Compose(ctx context.Context, xr *composite.Unstructur return CompositionResult{}, errors.Wrap(err, errApplyXRRefs) } + // Produce our array of resources to return to the Reconciler. The + // Reconciler uses this array to determine whether the XR is ready. + resources := make([]ComposedResource, 0, len(desired)) + + // We apply all of our desired resources before we observe them in the loop + // below. This ensures that issues observing and processing one composed + // resource won't block the application of another. + for name, cd := range desired { + // We don't need any crossplane-runtime resource.Applicator style apply + // options here because server-side apply takes care of everything. + // Specifically it will merge rather than replace owner references (e.g. + // for Usages), and will fail if we try to add a controller reference to + // a resource that already has a different one. + // NOTE(phisco): We need to set a field owner unique for each XR here, + // this prevents multiple XRs composing the same resource to be + // continuously alternated as controllers. + if err := c.client.Patch(ctx, cd.Resource, client.Apply, client.ForceOwnership, client.FieldOwner(ComposedFieldOwnerName(xr))); err != nil { + if kerrors.IsInvalid(err) { + // We tried applying an invalid resource, we can't tell whether + // this means the resource will never be valid or it will if we + // run again the composition after some other resource is + // created or updated successfully. So, we emit a warning event + // and move on. + // We mark the resource as not synced, so that once we get to + // decide the XR's Synced condition, we can set it to false if + // any of the resources didn't sync successfully. + events = append(events, event.Warning(reasonCompose, errors.Wrapf(err, errFmtApplyCD, name))) + // NOTE(phisco): here we behave differently w.r.t. the native + // p&t composer, as we respect the readiness reported by + // functions, while there we defaulted to also set ready false + // in case of apply errors. + resources = append(resources, ComposedResource{ResourceName: name, Ready: cd.Ready, Synced: false}) + continue + } + return CompositionResult{}, errors.Wrapf(err, errFmtApplyCD, name) + } + + resources = append(resources, ComposedResource{ResourceName: name, Ready: cd.Ready, Synced: true}) + } + // Our goal here is to patch our XR's status using server-side apply. We // want the resulting, patched object loaded into uxr. We need to pass in // only our "fully specified intent" - i.e. only the fields that we actually @@ -394,32 +434,12 @@ func (c *FunctionComposer) Compose(ctx context.Context, xr *composite.Unstructur // NOTE(phisco): Here we are fine using a hardcoded field owner as there is // no risk of conflict between different XRs. if err := c.client.Status().Patch(ctx, xr, client.Apply, client.ForceOwnership, client.FieldOwner(FieldOwnerXR)); err != nil { + // Note(phisco): here we are fine with this error being terminal, as + // there is no other resource to apply that might eventually resolve + // this issue. return CompositionResult{}, errors.Wrap(err, errApplyXRStatus) } - // Produce our array of resources to return to the Reconciler. The - // Reconciler uses this array to determine whether the XR is ready. - resources := make([]ComposedResource, 0, len(desired)) - - // We apply all of our desired resources before we observe them in the loop - // below. This ensures that issues observing and processing one composed - // resource won't block the application of another. - for name, cd := range desired { - // We don't need any crossplane-runtime resource.Applicator style apply - // options here because server-side apply takes care of everything. - // Specifically it will merge rather than replace owner references (e.g. - // for Usages), and will fail if we try to add a controller reference to - // a resource that already has a different one. - // NOTE(phisco): We need to set a field owner unique for each XR here, - // this prevents multiple XRs composing the same resource to be - // continuously alternated as controllers. - if err := c.client.Patch(ctx, cd.Resource, client.Apply, client.ForceOwnership, client.FieldOwner(ComposedFieldOwnerName(xr))); err != nil { - return CompositionResult{}, errors.Wrapf(err, errFmtApplyCD, name) - } - - resources = append(resources, ComposedResource{ResourceName: name, Ready: cd.Ready}) - } - return CompositionResult{ConnectionDetails: d.GetComposite().GetConnectionDetails(), Composed: resources, Events: events}, nil } diff --git a/internal/controller/apiextensions/composite/composition_functions_test.go b/internal/controller/apiextensions/composite/composition_functions_test.go index bc0d81305..0894303c9 100644 --- a/internal/controller/apiextensions/composite/composition_functions_test.go +++ b/internal/controller/apiextensions/composite/composition_functions_test.go @@ -614,8 +614,8 @@ func TestFunctionCompose(t *testing.T) { want: want{ res: CompositionResult{ Composed: []ComposedResource{ - {ResourceName: "desired-resource-a"}, - {ResourceName: "observed-resource-a", Ready: true}, + {ResourceName: "desired-resource-a", Synced: true}, + {ResourceName: "observed-resource-a", Ready: true, Synced: true}, }, ConnectionDetails: managed.ConnectionDetails{ "from": []byte("function-pipeline"), diff --git a/internal/controller/apiextensions/composite/composition_pt.go b/internal/controller/apiextensions/composite/composition_pt.go index efb82ede5..682422cf5 100644 --- a/internal/controller/apiextensions/composite/composition_pt.go +++ b/internal/controller/apiextensions/composite/composition_pt.go @@ -272,6 +272,21 @@ func (c *PTComposer) Compose(ctx context.Context, xr *composite.Unstructured, re o := []resource.ApplyOption{resource.MustBeControllableBy(xr.GetUID()), usage.RespectOwnerRefs()} o = append(o, mergeOptions(filterPatches(t.Patches, patchTypesFromXR()...))...) if err := c.client.Apply(ctx, cd, o...); err != nil { + if kerrors.IsInvalid(err) { + // We tried applying an invalid resource, we can't tell whether + // this means the resource will never be valid or it will if we + // run again the composition after some other resource is + // created or updated successfully. So, we emit a warning event + // and move on. + events = append(events, event.Warning(reasonCompose, errors.Wrap(err, errApplyComposed))) + // We unset the cd here so that we don't try to observe it + // later. This will also mean we report it as not ready and not + // synced. Resulting in the XR being reported as not ready nor + // synced too. + cds[i] = nil + continue + } + // TODO(negz): Include the template name (if any) in this error. // Including the rendered resource's kind may help too (e.g. if the // template is anonymous). @@ -297,7 +312,7 @@ func (c *PTComposer) Compose(ctx context.Context, xr *composite.Unstructured, re // to observe it. We still want to return it to the Reconciler so that // it knows that this desired composed resource is not ready. if cd == nil { - resources[i] = ComposedResource{ResourceName: name, Ready: false} + resources[i] = ComposedResource{ResourceName: name, Synced: false, Ready: false} continue } @@ -327,7 +342,7 @@ func (c *PTComposer) Compose(ctx context.Context, xr *composite.Unstructured, re return CompositionResult{}, errors.Wrapf(err, errFmtCheckReadiness, name) } - resources[i] = ComposedResource{ResourceName: name, Ready: ready} + resources[i] = ComposedResource{ResourceName: name, Ready: ready, Synced: true} } // Call Apply so that we do not just replace fields on existing XR but diff --git a/internal/controller/apiextensions/composite/composition_pt_test.go b/internal/controller/apiextensions/composite/composition_pt_test.go index 1b9bacc94..31f01de0c 100644 --- a/internal/controller/apiextensions/composite/composition_pt_test.go +++ b/internal/controller/apiextensions/composite/composition_pt_test.go @@ -391,6 +391,7 @@ func TestPTCompose(t *testing.T) { Composed: []ComposedResource{{ ResourceName: "cool-resource", Ready: true, + Synced: true, }}, ConnectionDetails: details, }, @@ -456,10 +457,12 @@ func TestPTCompose(t *testing.T) { { ResourceName: "cool-resource", Ready: true, + Synced: true, }, { ResourceName: "uncool-resource", Ready: false, + Synced: false, }, }, ConnectionDetails: details, diff --git a/internal/controller/apiextensions/composite/reconciler.go b/internal/controller/apiextensions/composite/reconciler.go index 964c8e5ad..04153cd98 100644 --- a/internal/controller/apiextensions/composite/reconciler.go +++ b/internal/controller/apiextensions/composite/reconciler.go @@ -73,6 +73,7 @@ const ( errCompose = "cannot compose resources" errInvalidResources = "some resources were invalid, check events" errRenderCD = "cannot render composed resource" + errSyncResources = "cannot sync composed resources" reconcilePausedMsg = "Reconciliation (including deletion) is paused via the pause annotation" ) @@ -682,6 +683,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco } var unready []ComposedResource + var unsynced []ComposedResource for i, cd := range res.Composed { // Specifying a name for P&T templates is optional but encouraged. // If there was no name, fall back to using the index. @@ -690,6 +692,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco id = strconv.Itoa(i) } + if !cd.Synced { + log.Debug("Composed resource is not yet synced", "id", id) + unsynced = append(unsynced, cd) + r.record.Event(xr, event.Normal(reasonCompose, fmt.Sprintf("Composed resource %q is not yet synced", id))) + continue + } + if !cd.Ready { log.Debug("Composed resource is not yet ready", "id", id) unready = append(unready, cd) @@ -698,30 +707,49 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco } } - xr.SetConditions(xpv1.ReconcileSuccess()) - - // TODO(muvaf): If a resource becomes Unavailable at some point, should we - // still report it as Creating? - if len(unready) > 0 { - // We want to requeue to wait for our composed resources to - // become ready, since we can't watch them. - names := make([]string, len(unready)) - for i, cd := range unready { - names[i] = string(cd.ResourceName) - } - // sort for stable condition messages. With functions, we don't have a - // stable order otherwise. - xr.SetConditions(xpv1.Creating().WithMessage(fmt.Sprintf("Unready resources: %s", resource.StableNAndSomeMore(resource.DefaultFirstN, names)))) + if updateXRConditions(xr, unsynced, unready) { + // This requeue is subject to rate limiting. Requeues will exponentially + // backoff from 1 to 30 seconds. See the 'definition' (XRD) reconciler + // that sets up the ratelimiter. return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, xr), errUpdateStatus) } // We requeue after our poll interval because we can't watch composed // resources - we can't know what type of resources we might compose // when this controller is started. - xr.SetConditions(xpv1.Available()) return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, xr), errUpdateStatus) } +// updateXRConditions updates the conditions of the supplied composite resource +// based on the supplied composed resources. It returns true if the XR should be +// requeued immediately. +func updateXRConditions(xr *composite.Unstructured, unsynced, unready []ComposedResource) (requeueImmediately bool) { + readyCond := xpv1.Available() + syncedCond := xpv1.ReconcileSuccess() + if len(unsynced) > 0 { + // We want to requeue to wait for our composed resources to + // become ready, since we can't watch them. + syncedCond = xpv1.ReconcileError(errors.New(errSyncResources)).WithMessage(fmt.Sprintf("Unsynced resources: %s", resource.StableNAndSomeMore(resource.DefaultFirstN, getComposerResourcesNames(unsynced)))) + requeueImmediately = true + } + if len(unready) > 0 { + // We want to requeue to wait for our composed resources to + // become ready, since we can't watch them. + readyCond = xpv1.Creating().WithMessage(fmt.Sprintf("Unready resources: %s", resource.StableNAndSomeMore(resource.DefaultFirstN, getComposerResourcesNames(unready)))) + requeueImmediately = true + } + xr.SetConditions(syncedCond, readyCond) + return requeueImmediately +} + +func getComposerResourcesNames(cds []ComposedResource) []string { + names := make([]string, len(cds)) + for i, cd := range cds { + names[i] = string(cd.ResourceName) + } + return names +} + // EnqueueForCompositionRevisionFunc returns a function that enqueues (the // related) XRs when a new CompositionRevision is created. This speeds up // reconciliation of XRs on changes to the Composition by not having to wait for diff --git a/internal/controller/apiextensions/composite/reconciler_test.go b/internal/controller/apiextensions/composite/reconciler_test.go index a364a79a4..fd4a0169c 100644 --- a/internal/controller/apiextensions/composite/reconciler_test.go +++ b/internal/controller/apiextensions/composite/reconciler_test.go @@ -547,21 +547,27 @@ func TestReconcile(t *testing.T) { Composed: []ComposedResource{{ ResourceName: "elephant", Ready: false, + Synced: true, }, { ResourceName: "cow", Ready: false, + Synced: true, }, { ResourceName: "pig", Ready: true, + Synced: true, }, { ResourceName: "cat", Ready: false, + Synced: true, }, { ResourceName: "dog", Ready: true, + Synced: true, }, { ResourceName: "snake", Ready: false, + Synced: true, }}, }, nil })), From 76c8db6e40093df4cd73d4820e852953c6098353 Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Thu, 29 Feb 2024 10:17:13 +0000 Subject: [PATCH 2/5] chore: reword event and debug log Signed-off-by: Philippe Scorsolini (cherry picked from commit 320a2bec274ad96e3ce4e54b572902eca4a750e7) Signed-off-by: Philippe Scorsolini --- internal/controller/apiextensions/composite/reconciler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/apiextensions/composite/reconciler.go b/internal/controller/apiextensions/composite/reconciler.go index 04153cd98..027b8f450 100644 --- a/internal/controller/apiextensions/composite/reconciler.go +++ b/internal/controller/apiextensions/composite/reconciler.go @@ -693,9 +693,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco } if !cd.Synced { - log.Debug("Composed resource is not yet synced", "id", id) + log.Debug("Composed resource is not yet valid", "id", id) unsynced = append(unsynced, cd) - r.record.Event(xr, event.Normal(reasonCompose, fmt.Sprintf("Composed resource %q is not yet synced", id))) + r.record.Event(xr, event.Normal(reasonCompose, fmt.Sprintf("Composed resource %q is not yet valid", id))) continue } From 6bdaa9ccfed8d34d4f4bd7e57c6c0880622a2d34 Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Thu, 29 Feb 2024 16:23:57 +0000 Subject: [PATCH 3/5] fix: consider both ready and sync Signed-off-by: Philippe Scorsolini (cherry picked from commit 4b641f8fb83554273c268d3dd22af1e3a0d3c1ed) Signed-off-by: Philippe Scorsolini --- internal/controller/apiextensions/composite/reconciler.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/controller/apiextensions/composite/reconciler.go b/internal/controller/apiextensions/composite/reconciler.go index 027b8f450..f4862dd61 100644 --- a/internal/controller/apiextensions/composite/reconciler.go +++ b/internal/controller/apiextensions/composite/reconciler.go @@ -696,14 +696,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco log.Debug("Composed resource is not yet valid", "id", id) unsynced = append(unsynced, cd) r.record.Event(xr, event.Normal(reasonCompose, fmt.Sprintf("Composed resource %q is not yet valid", id))) - continue } if !cd.Ready { log.Debug("Composed resource is not yet ready", "id", id) unready = append(unready, cd) r.record.Event(xr, event.Normal(reasonCompose, fmt.Sprintf("Composed resource %q is not yet ready", id))) - continue } } From 55256856350dd97a2f1825fba7bb55e944060e22 Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Thu, 29 Feb 2024 20:26:48 +0000 Subject: [PATCH 4/5] tests(e2e): add TestCompositionInvalidComposed Signed-off-by: Philippe Scorsolini (cherry picked from commit 4cfc571d8bf6163954997999b2aadacdca1c583e) Signed-off-by: Philippe Scorsolini --- test/e2e/apiextensions_test.go | 44 +++++++++++ .../invalid-composed/setup/composition.yaml | 75 +++++++++++++++++++ .../invalid-composed/setup/definition.yaml | 51 +++++++++++++ .../invalid-composed/setup/provider.yaml | 7 ++ .../composition/invalid-composed/xr.yaml | 11 +++ 5 files changed, 188 insertions(+) create mode 100644 test/e2e/manifests/apiextensions/composition/invalid-composed/setup/composition.yaml create mode 100644 test/e2e/manifests/apiextensions/composition/invalid-composed/setup/definition.yaml create mode 100644 test/e2e/manifests/apiextensions/composition/invalid-composed/setup/provider.yaml create mode 100644 test/e2e/manifests/apiextensions/composition/invalid-composed/xr.yaml diff --git a/test/e2e/apiextensions_test.go b/test/e2e/apiextensions_test.go index a3284d3b9..10f4fe90e 100644 --- a/test/e2e/apiextensions_test.go +++ b/test/e2e/apiextensions_test.go @@ -87,6 +87,50 @@ func TestCompositionMinimal(t *testing.T) { ) } +// TestCompositionInvalidComposed tests Crossplane's Composition functionality, +// checking that although a composed resource is invalid, i.e. it didn't apply +// successfully. +func TestCompositionInvalidComposed(t *testing.T) { + manifests := "test/e2e/manifests/apiextensions/composition/invalid-composed" + + xrList := composed.NewList(composed.FromReferenceToList(corev1.ObjectReference{ + APIVersion: "example.org/v1alpha1", + Kind: "XParent", + }), composed.FromReferenceToList(corev1.ObjectReference{ + APIVersion: "example.org/v1alpha1", + Kind: "XChild", + })) + + environment.Test(t, + features.New(t.Name()). + WithLabel(LabelArea, LabelAreaAPIExtensions). + WithLabel(LabelSize, LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithSetup("PrerequisitesAreCreated", funcs.AllOf( + funcs.ApplyResources(FieldManager, manifests, "setup/*.yaml"), + funcs.ResourcesCreatedWithin(30*time.Second, manifests, "setup/*.yaml"), + funcs.ResourcesHaveConditionWithin(1*time.Minute, manifests, "setup/definition.yaml", apiextensionsv1.WatchingComposite()), + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "setup/provider.yaml", pkgv1.Healthy(), pkgv1.Active()), + )). + Assess("CreateXR", funcs.AllOf( + funcs.ApplyResources(FieldManager, manifests, "xr.yaml"), + funcs.InBackground(funcs.LogResources(xrList)), + funcs.InBackground(funcs.LogResources(nopList)), + funcs.ResourcesCreatedWithin(30*time.Second, manifests, "xr.yaml"), + )). + Assess("XRStillAnnotated", funcs.AllOf( + // Check the XR it has metadata.annotations set + funcs.ResourcesHaveFieldValueWithin(1*time.Minute, manifests, "xr.yaml", "metadata.annotations[exampleVal]", "foo"), + )). + WithTeardown("DeleteXR", funcs.AllOf( + funcs.DeleteResources(manifests, "xr.yaml"), + funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "xr.yaml"), + )). + WithTeardown("DeletePrerequisites", funcs.ResourcesDeletedAfterListedAreGone(3*time.Minute, manifests, "setup/*.yaml", nopList)). + Feature(), + ) +} + // TestCompositionPatchAndTransform tests Crossplane's Composition functionality, // checking that a claim using patch-and-transform Composition will become // available when its composed resources do, and have a field derived from diff --git a/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/composition.yaml b/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/composition.yaml new file mode 100644 index 000000000..6c9c38716 --- /dev/null +++ b/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/composition.yaml @@ -0,0 +1,75 @@ +--- +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: parent +spec: + compositeTypeRef: + apiVersion: example.org/v1alpha1 + kind: XParent + resources: + - name: child + base: + apiVersion: example.org/v1alpha1 + kind: XChild + spec: {} + patches: + - type: FromCompositeFieldPath + # this is going to be 1 + fromFieldPath: spec.someField + # this will fail because it's supposed to be > 1 + toFieldPath: spec.someField + - name: nop-resource-1 + base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: NopResource + metadata: + annotations: + exampleVal: "foo" + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "False" + time: 0s + - conditionType: Ready + conditionStatus: "True" + time: 1s + patches: + - type: FromCompositeFieldPath + fromFieldPath: metadata.name + # we should still see this in the child + toFieldPath: metadata.annotations[something] + - type: ToCompositeFieldPath + fromFieldPath: metadata.annotations[exampleVal] + # we should still see this in the composite + toFieldPath: metadata.annotations[exampleVal] +--- +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: child +spec: + compositeTypeRef: + apiVersion: example.org/v1alpha1 + kind: XChild + resources: + # we don't really care about what happens here, it's not going to work + # because the composite resource will be invalid + - name: nop-resource-1 + base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: NopResource + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "False" + time: 0s + - conditionType: Ready + conditionStatus: "True" + time: 1s + patches: + - type: FromCompositeFieldPath + fromFieldPath: metadata.name + toFieldPath: metadata.annotations[something] diff --git a/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/definition.yaml b/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/definition.yaml new file mode 100644 index 000000000..192c76708 --- /dev/null +++ b/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/definition.yaml @@ -0,0 +1,51 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xparents.example.org +spec: + defaultCompositionRef: + name: parent + group: example.org + names: + kind: XParent + plural: xparents + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + someField: + # no limits on its value + type: integer +--- +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xchildren.example.org +spec: + defaultCompositionRef: + name: child + group: example.org + names: + kind: XChild + plural: xchildren + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + someField: + minimum: 2 + type: integer diff --git a/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/provider.yaml b/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/provider.yaml new file mode 100644 index 000000000..b82f2c560 --- /dev/null +++ b/test/e2e/manifests/apiextensions/composition/invalid-composed/setup/provider.yaml @@ -0,0 +1,7 @@ +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-nop +spec: + package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.2.1 + ignoreCrossplaneConstraints: true diff --git a/test/e2e/manifests/apiextensions/composition/invalid-composed/xr.yaml b/test/e2e/manifests/apiextensions/composition/invalid-composed/xr.yaml new file mode 100644 index 000000000..a233b2916 --- /dev/null +++ b/test/e2e/manifests/apiextensions/composition/invalid-composed/xr.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: example.org/v1alpha1 +kind: XParent +metadata: + name: test +# Expected: +# annotations: +# exampleVal: "foo" +spec: + # this should be > 1 in the XChild composed resource, so it will fail applying it + someField: 1 From 6b49247bb4e8b271d056922875aa380622ff6fcd Mon Sep 17 00:00:00 2001 From: Philippe Scorsolini Date: Wed, 6 Mar 2024 09:14:36 +0000 Subject: [PATCH 5/5] chore: reword sync error condition message Signed-off-by: Philippe Scorsolini (cherry picked from commit 2aaf7cff6ee3cd19701c60ab5935bf85e712821d) Signed-off-by: Philippe Scorsolini --- internal/controller/apiextensions/composite/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/apiextensions/composite/reconciler.go b/internal/controller/apiextensions/composite/reconciler.go index f4862dd61..52afd7092 100644 --- a/internal/controller/apiextensions/composite/reconciler.go +++ b/internal/controller/apiextensions/composite/reconciler.go @@ -727,7 +727,7 @@ func updateXRConditions(xr *composite.Unstructured, unsynced, unready []Composed if len(unsynced) > 0 { // We want to requeue to wait for our composed resources to // become ready, since we can't watch them. - syncedCond = xpv1.ReconcileError(errors.New(errSyncResources)).WithMessage(fmt.Sprintf("Unsynced resources: %s", resource.StableNAndSomeMore(resource.DefaultFirstN, getComposerResourcesNames(unsynced)))) + syncedCond = xpv1.ReconcileError(errors.New(errSyncResources)).WithMessage(fmt.Sprintf("Invalid resources: %s", resource.StableNAndSomeMore(resource.DefaultFirstN, getComposerResourcesNames(unsynced)))) requeueImmediately = true } if len(unready) > 0 {