diff --git a/pkg/controllers/installer/helpers_test.go b/pkg/controllers/installer/helpers_test.go index 53cac9265..b4df24f77 100644 --- a/pkg/controllers/installer/helpers_test.go +++ b/pkg/controllers/installer/helpers_test.go @@ -26,8 +26,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/types" + apiextensionsv1alpha1 "github.com/openshift/api/apiextensions/v1alpha1" configv1 "github.com/openshift/api/config/v1" operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" @@ -253,17 +255,48 @@ func latestRevision(revisions []operatorv1alpha1.ClusterAPIInstallerRevision) op func addRevision(ctx context.Context, providerNames ...string) operatorv1alpha1.ClusterAPIInstallerRevision { GinkgoHelper() - // Get current ClusterAPI to determine revision index. + return addRevisionWithOptions(ctx, providerNames, nil) +} + +func addRevisionAndWaitForSuccess(ctx context.Context, providerNames ...string) { + GinkgoHelper() + + By("Adding a revision with providers: "+strings.Join(providerNames, ", "), func() { + revision := addRevision(ctx, providerNames...) + waitForRevision(ctx, revision.Name) + }) +} + +// addRevisionWithUnmanagedCRDs appends a new revision with the given providers and unmanaged CRDs. +func addRevisionWithUnmanagedCRDs(ctx context.Context, providerNames []string, unmanagedCRDs []string) operatorv1alpha1.ClusterAPIInstallerRevision { + GinkgoHelper() + + return addRevisionWithOptions(ctx, providerNames, unmanagedCRDs) +} + +func addRevisionWithOptions(ctx context.Context, providerNames []string, unmanagedCRDs []string) operatorv1alpha1.ClusterAPIInstallerRevision { + GinkgoHelper() + clusterAPI := &operatorv1alpha1.ClusterAPI{} Expect(cl.Get(ctx, client.ObjectKey{Name: clusterAPIName}, clusterAPI)).To(Succeed()) var apiRev operatorv1alpha1.ClusterAPIInstallerRevision - By("Rendering new revision", func() { + byMsg := "Rendering new revision" + if len(unmanagedCRDs) > 0 { + byMsg += " with unmanaged CRDs: " + strings.Join(unmanagedCRDs, ", ") + } + + By(byMsg, func() { profiles := lookupProfiles(providerNames...) - // Render the revision to compute the correct content ID. rendered, err := revisiongenerator.NewRenderedRevision(profiles) + if len(unmanagedCRDs) > 0 { + rendered, err = revisiongenerator.NewRenderedRevision(profiles, + revisiongenerator.WithUnmanagedCRDs(unmanagedCRDs), + ) + } + Expect(err).NotTo(HaveOccurred()) var revisionIndex int64 @@ -289,13 +322,33 @@ func addRevision(ctx context.Context, providerNames ...string) operatorv1alpha1. return apiRev } -func addRevisionAndWaitForSuccess(ctx context.Context, providerNames ...string) { +// setCompatibilityRequirementConditions sets Admitted and Compatible conditions on a CompatibilityRequirement. +func setCompatibilityRequirementConditions(ctx context.Context, name string, admitted, compatible bool) { GinkgoHelper() - By("Adding a revision with providers: "+strings.Join(providerNames, ", "), func() { - revision := addRevision(ctx, providerNames...) - waitForRevision(ctx, revision.Name) - }) + conditionStatus := func(b bool) metav1.ConditionStatus { + if b { + return metav1.ConditionTrue + } + + return metav1.ConditionFalse + } + + cr := &apiextensionsv1alpha1.CompatibilityRequirement{} + cr.SetName(name) + + Eventually(kWithCtx(ctx).UpdateStatus(cr, func() { + meta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{ + Type: apiextensionsv1alpha1.CompatibilityRequirementAdmitted, + Status: conditionStatus(admitted), + Reason: "Test", + }) + meta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{ + Type: apiextensionsv1alpha1.CompatibilityRequirementCompatible, + Status: conditionStatus(compatible), + Reason: "Test", + }) + })).WithContext(ctx).WithTimeout(defaultEventuallyTimeout).Should(Succeed()) } // addEmptyRevision appends a revision with no components. diff --git a/pkg/controllers/installer/installer_controller_test.go b/pkg/controllers/installer/installer_controller_test.go index 78bcfd9ec..3ec848fa7 100644 --- a/pkg/controllers/installer/installer_controller_test.go +++ b/pkg/controllers/installer/installer_controller_test.go @@ -25,6 +25,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apiextensionsv1alpha1 "github.com/openshift/api/apiextensions/v1alpha1" configv1 "github.com/openshift/api/config/v1" operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" appsv1 "k8s.io/api/apps/v1" @@ -124,6 +125,34 @@ var _ = Describe("InstallerController", Serial, func() { Expect(checkConfigMap(ctx, infraCMName)).To(test.BeK8SNotFound()) }, defaultNodeTimeout) + It("tears down CompatibilityRequirements when a new revision removes unmanaged CRDs", func(ctx context.Context) { + unmanagedCRDs := []string{"testgadgets.test.example.com"} + crName := "ccapio-testgadgets.test.example.com" + + By("installing a revision with an unmanaged CRD") + + revision := addRevisionWithUnmanagedCRDs(ctx, + []string{providerMixed}, + unmanagedCRDs, + ) + setCompatibilityRequirementConditions(ctx, crName, true, true) + waitForRevision(ctx, revision.Name) + + By("verifying the CompatibilityRequirement and ConfigMap both exist") + + cr := &apiextensionsv1alpha1.CompatibilityRequirement{} + cr.SetName(crName) + Expect(cl.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + Expect(checkConfigMap(ctx, mixedCMName)).To(Succeed()) + + By("installing a revision without unmanaged CRDs") + addRevisionAndWaitForSuccess(ctx, providerMixed) + + By("verifying the CompatibilityRequirement is torn down but the ConfigMap remains") + Expect(cl.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(test.BeK8SNotFound()) + Expect(checkConfigMap(ctx, mixedCMName)).To(Succeed()) + }, defaultNodeTimeout) + It("tears down all objects when an empty revision is added", func(ctx context.Context) { addRevisionAndWaitForSuccess(ctx, providerCore, providerInfra) @@ -418,6 +447,47 @@ var _ = Describe("InstallerController", Serial, func() { WithContext(ctx). Should(HaveField("Data", HaveKeyWithValue("version", "v1"))) }, defaultNodeTimeout) + + It("continues reconciling previous revision when new revision is blocked on CompatibilityRequirement", func(ctx context.Context) { + By("installing a valid first revision") + addRevisionAndWaitForSuccess(ctx, providerCore) + + By("adding a second revision blocked on a CompatibilityRequirement") + addRevisionWithUnmanagedCRDs(ctx, + []string{providerCore, providerCRD}, + []string{"testwidgets.test.example.com"}, + ) + + waitForConditions(ctx, + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("waiting on phase compatibility-requirements")), + ) + + By("verifying the first revision's objects still exist") + + cm, err := getConfigMap(ctx, coreCMName) + Expect(err).NotTo(HaveOccurred()) + Expect(cm.Data).To(HaveKeyWithValue("version", "v1")) + + By("modifying the managed ConfigMap to verify drift correction") + Eventually(kWithCtx(ctx).Update(cm, func() { + cm.Data["version"] = "modified" + })). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(Succeed()) + + restored := &corev1.ConfigMap{} + restored.SetName(coreCMName) + restored.SetNamespace("default") + + Eventually(kWithCtx(ctx).Object(restored)). + WithTimeout(defaultEventuallyTimeout). + WithContext(ctx). + Should(HaveField("Data", HaveKeyWithValue("version", "v1"))) + }, defaultNodeTimeout) }) Context("Deployment Probes", func() { @@ -473,6 +543,184 @@ var _ = Describe("InstallerController", Serial, func() { }, defaultNodeTimeout) }) + Context("CompatibilityRequirement Probes", func() { + It("gates phase on Admitted and Compatible conditions", func(ctx context.Context) { + unmanagedCRDs := []string{"testgadgets.test.example.com"} + crName := "ccapio-testgadgets.test.example.com" + + revision := addRevisionWithUnmanagedCRDs(ctx, + []string{providerMixed}, + unmanagedCRDs, + ) + + By("waiting for the phase to block on compatibility-requirements") + waitForConditions(ctx, + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("waiting on phase compatibility-requirements")), + ) + + By("verifying the CompatibilityRequirement exists and the ConfigMap does not") + + cr := &apiextensionsv1alpha1.CompatibilityRequirement{} + cr.SetName(crName) + Expect(cl.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + Expect(checkConfigMap(ctx, mixedCMName)).To(test.BeK8SNotFound()) + + By("setting Admitted=True and Compatible=True") + setCompatibilityRequirementConditions(ctx, crName, true, true) + + By("waiting for the revision to complete") + waitForRevision(ctx, revision.Name) + + cm, err := getConfigMap(ctx, mixedCMName) + Expect(err).NotTo(HaveOccurred()) + Expect(cm.Data).To(HaveKeyWithValue("source", "mixed")) + }, defaultNodeTimeout) + + It("stays incomplete when compatibility checker has not reconciled", func(ctx context.Context) { + unmanagedCRDs := []string{"testwidgets.test.example.com"} + crName := "ccapio-testwidgets.test.example.com" + + addRevisionWithUnmanagedCRDs(ctx, + []string{providerCRD}, + unmanagedCRDs, + ) + + By("waiting for the phase to block on compatibility-requirements") + waitForConditions(ctx, + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("waiting on phase compatibility-requirements")), + ) + + By("verifying the CompatibilityRequirement exists") + + cr := &apiextensionsv1alpha1.CompatibilityRequirement{} + cr.SetName(crName) + Expect(cl.Get(ctx, client.ObjectKeyFromObject(cr), cr)).To(Succeed()) + + By("verifying the phase remains blocked with no conditions set") + + co := &configv1.ClusterOperator{} + co.SetName("cluster-api") + Consistently(kWithCtx(ctx).Object(co)). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(HaveField("Status.Conditions", + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue), + )) + }, defaultNodeTimeout) + + It("blocks when Admitted is false even if Compatible is true", func(ctx context.Context) { + unmanagedCRDs := []string{"testgadgets.test.example.com"} + crName := "ccapio-testgadgets.test.example.com" + + addRevisionWithUnmanagedCRDs(ctx, + []string{providerMixed}, + unmanagedCRDs, + ) + + By("waiting for the phase to block on compatibility-requirements") + waitForConditions(ctx, + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("waiting on phase compatibility-requirements")), + ) + + By("setting Admitted=False and Compatible=True") + setCompatibilityRequirementConditions(ctx, crName, false, true) + + By("verifying the phase remains blocked") + + co := &configv1.ClusterOperator{} + co.SetName("cluster-api") + Consistently(kWithCtx(ctx).Object(co)). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(HaveField("Status.Conditions", + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue), + )) + + Expect(checkConfigMap(ctx, mixedCMName)).To(test.BeK8SNotFound()) + }, defaultNodeTimeout) + + It("unblocks when transitioning from incompatible to compatible", func(ctx context.Context) { + unmanagedCRDs := []string{"testgadgets.test.example.com"} + crName := "ccapio-testgadgets.test.example.com" + + revision := addRevisionWithUnmanagedCRDs(ctx, + []string{providerMixed}, + unmanagedCRDs, + ) + + By("waiting for the phase to block on compatibility-requirements") + waitForConditions(ctx, + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing). + WithMessage(ContainSubstring("waiting on phase compatibility-requirements")), + ) + + By("setting Compatible=False to confirm blocking") + setCompatibilityRequirementConditions(ctx, crName, true, false) + + Expect(checkConfigMap(ctx, mixedCMName)).To(test.BeK8SNotFound()) + + By("transitioning to Compatible=True") + setCompatibilityRequirementConditions(ctx, crName, true, true) + + By("waiting for the revision to complete") + waitForRevision(ctx, revision.Name) + + cm, err := getConfigMap(ctx, mixedCMName) + Expect(err).NotTo(HaveOccurred()) + Expect(cm.Data).To(HaveKeyWithValue("source", "mixed")) + }, defaultNodeTimeout) + + It("blocks entire phase when one of multiple CRDs is incompatible", func(ctx context.Context) { + unmanagedCRDs := []string{ + "testwidgets.test.example.com", + "testgadgets.test.example.com", + } + + addRevisionWithUnmanagedCRDs(ctx, + []string{providerCRD, providerMixed}, + unmanagedCRDs, + ) + + By("waiting for the phase to block on compatibility-requirements") + waitForConditions(ctx, + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue). + WithReason(operatorstatus.ReasonProgressing), + ) + + By("setting one compatible and one incompatible") + setCompatibilityRequirementConditions(ctx, "ccapio-testwidgets.test.example.com", true, true) + setCompatibilityRequirementConditions(ctx, "ccapio-testgadgets.test.example.com", true, false) + + By("verifying the phase remains blocked") + + co := &configv1.ClusterOperator{} + co.SetName("cluster-api") + Consistently(kWithCtx(ctx).Object(co)). + WithContext(ctx). + WithTimeout(defaultEventuallyTimeout). + Should(HaveField("Status.Conditions", + test.HaveCondition(conditionTypeProgressing). + WithStatus(configv1.ConditionTrue), + )) + + Expect(checkConfigMap(ctx, mixedCMName)).To(test.BeK8SNotFound()) + }, defaultNodeTimeout) + }) + Context("RelatedObjects", func() { It("does not produce relatedObjects for namespaced-only objects", func(ctx context.Context) { addRevisionAndWaitForSuccess(ctx, providerCore) diff --git a/pkg/controllers/installer/probes.go b/pkg/controllers/installer/probes.go index 08039ffe2..55cb48fbf 100644 --- a/pkg/controllers/installer/probes.go +++ b/pkg/controllers/installer/probes.go @@ -31,6 +31,8 @@ func allProbes() []*probing.GroupKindSelector { return []*probing.GroupKindSelector{ crdEstablishedProbe(), deploymentAvailableProbe(), + compatibilityRequirementAdmittedProbe(), + compatibilityRequirementCompatibleProbe(), } } @@ -88,6 +90,26 @@ func probeSucceededPredicate(probes ...*probing.GroupKindSelector) predicate.Pre } } +// compatibilityRequirementAdmittedProbe checks that a CompatibilityRequirement +// has the Admitted condition set to True. This confirms the validating webhook +// is in place to guard against future incompatible CRD updates. +func compatibilityRequirementAdmittedProbe() *probing.GroupKindSelector { + return &probing.GroupKindSelector{ + GroupKind: schema.GroupKind{Group: "apiextensions.openshift.io", Kind: "CompatibilityRequirement"}, + Prober: &probing.ConditionProbe{Type: "Admitted", Status: "True"}, + } +} + +// compatibilityRequirementCompatibleProbe checks that a CompatibilityRequirement +// has the Compatible condition set to True. This confirms the current CRD +// satisfies the compatibility contract. +func compatibilityRequirementCompatibleProbe() *probing.GroupKindSelector { + return &probing.GroupKindSelector{ + GroupKind: schema.GroupKind{Group: "apiextensions.openshift.io", Kind: "CompatibilityRequirement"}, + Prober: &probing.ConditionProbe{Type: "Compatible", Status: "True"}, + } +} + // noGenerationPredicate returns a predicate that passes all update events for // objects that don't have generation tracking (e.g., ConfigMaps, Secrets). // Objects with generation tracking have it initialised to 1 on creation, so diff --git a/pkg/controllers/installer/revision_reconciler.go b/pkg/controllers/installer/revision_reconciler.go index 4cb4a6ff2..9e634482f 100644 --- a/pkg/controllers/installer/revision_reconciler.go +++ b/pkg/controllers/installer/revision_reconciler.go @@ -137,7 +137,10 @@ func (r *revisionReconciler) reconcile(ctx context.Context, revisions []operator // Convert all API revisions upfront so that collectObjects (and thus // relatedObjects) is fully populated before reconciliation begins. converted := util.SliceMap(revisions, func(apiRev operatorv1alpha1.ClusterAPIInstallerRevision) convertedRevision { - rev, err := revisiongenerator.NewInstallerRevisionFromAPI(apiRev, r.providerProfiles, revisiongenerator.WithObjectCollectors(r.collectObjects)) + rev, err := revisiongenerator.NewInstallerRevisionFromAPI(apiRev, r.providerProfiles, + revisiongenerator.WithObjectCollectors(r.collectObjects), + revisiongenerator.WithUnmanagedCRDs(apiRev.UnmanagedCustomResourceDefinitions), + ) if err != nil { err = fmt.Errorf("error creating installer revision from API revision %s: %w", apiRev.Name, reconcile.TerminalError(err)) } diff --git a/pkg/controllers/revision/revision_controller.go b/pkg/controllers/revision/revision_controller.go index d276ff8b9..95557a6c3 100644 --- a/pkg/controllers/revision/revision_controller.go +++ b/pkg/controllers/revision/revision_controller.go @@ -89,13 +89,7 @@ func (r *RevisionController) Reconcile(ctx context.Context, _ ctrl.Request) (ctr } func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) operatorstatus.ReconcileResult { - // Generate a desired revision from the current state - desiredRevision, result := r.generateDesiredRevision(ctx) - if result != nil { - return *result - } - - // Get ClusterAPI singleton + // Get ClusterAPI singleton first — generateDesiredRevision needs spec fields. clusterAPI := &operatorv1alpha1.ClusterAPI{} if err := r.Get(ctx, client.ObjectKey{Name: clusterAPIName}, clusterAPI); err != nil { if apierrors.IsNotFound(err) { @@ -105,6 +99,16 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope return opresult.Error(fmt.Errorf("fetching ClusterAPI: %w", err)) } + var unmanagedCRDs []string + if clusterAPI.Spec != nil { + unmanagedCRDs = clusterAPI.Spec.UnmanagedCustomResourceDefinitions + } + + desiredRevision, result := r.generateDesiredRevision(ctx, unmanagedCRDs) + if result != nil { + return *result + } + // Create a reverse sorted, merged list of revisions. It will prepend the // new revision if necessary. Note that the latest revision is always // first, and there is guaranteed to be at least one revision. @@ -134,7 +138,7 @@ func (r *RevisionController) reconcile(ctx context.Context, log logr.Logger) ope return opresult.Success() } -func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revisiongenerator.RenderedRevision, *operatorstatus.ReconcileResult) { +func (r *RevisionController) generateDesiredRevision(ctx context.Context, unmanagedCRDs []string) (revisiongenerator.RenderedRevision, *operatorstatus.ReconcileResult) { infra := &configv1.Infrastructure{} if err := r.Get(ctx, client.ObjectKey{Name: infrastructureName}, infra); err != nil { return nil, opresult.ErrorP(fmt.Errorf("fetching infrastructure: %w", err)) @@ -147,9 +151,12 @@ func (r *RevisionController) generateDesiredRevision(ctx context.Context) (revis // Build ordered component list from provider metadata providerComponents := r.buildComponentList(infra.Status.PlatformStatus.Type) - revision, err := revisiongenerator.NewRenderedRevision(providerComponents, revisiongenerator.WithManifestSubstitutions(r.manifestSubstitutions)) + revision, err := revisiongenerator.NewRenderedRevision(providerComponents, + revisiongenerator.WithManifestSubstitutions(r.manifestSubstitutions), + revisiongenerator.WithUnmanagedCRDs(unmanagedCRDs), + ) if err != nil { - return nil, opresult.ErrorP(fmt.Errorf("error creating rendered revision: %w", err)) + return nil, opresult.NonRetryableErrorP(fmt.Errorf("error creating rendered revision: %w", err)) } return revision, nil diff --git a/pkg/controllers/revision/revision_controller_test.go b/pkg/controllers/revision/revision_controller_test.go index c303e0e19..53e9557d6 100644 --- a/pkg/controllers/revision/revision_controller_test.go +++ b/pkg/controllers/revision/revision_controller_test.go @@ -379,6 +379,42 @@ var _ = Describe("RevisionController", Serial, func() { Expect(updatedClusterAPI.Status.Revisions).To(HaveLen(16)) }, defaultNodeTimeout) + Context("when unmanagedCustomResourceDefinitions is set on the spec", func() { + It("should include them in the revision status", func(ctx context.Context) { + By("setting unmanagedCustomResourceDefinitions on the ClusterAPI spec") + Eventually(kWithCtx(ctx).Update(clusterAPI, func() { + clusterAPI.Spec.UnmanagedCustomResourceDefinitions = []string{"widgets.example.com"} + })).WithContext(ctx).Should(Succeed()) + + Eventually(kWithCtx(ctx).Object(clusterAPI)). + WithContext(ctx). + Should(HaveField("Status.Revisions", ContainElement( + HaveField("UnmanagedCustomResourceDefinitions", Equal([]string{"widgets.example.com"})), + ))) + }, defaultNodeTimeout) + + It("should produce a different content ID", func(ctx context.Context) { + By("capturing the original content ID") + Eventually(kWithCtx(ctx).Object(clusterAPI)). + WithContext(ctx). + Should(HaveField("Status.Revisions", HaveLen(1))) + originalContentID := clusterAPI.Status.Revisions[0].ContentID + + By("setting unmanagedCustomResourceDefinitions on the ClusterAPI spec") + Eventually(kWithCtx(ctx).Update(clusterAPI, func() { + clusterAPI.Spec.UnmanagedCustomResourceDefinitions = []string{"widgets.example.com"} + })).WithContext(ctx).Should(Succeed()) + + By("waiting for a new revision") + Eventually(kWithCtx(ctx).Object(clusterAPI)). + WithContext(ctx). + Should(HaveField("Status.Revisions", HaveLen(2))) + + newRev := latestRevision(clusterAPI.Status.Revisions) + Expect(newRev.ContentID).NotTo(Equal(originalContentID)) + }, defaultNodeTimeout) + }) + It("sets Available=False with NonRetryableError when manifest has invalid adopt-existing annotation", func(ctx context.Context) { // Stop first manager (created by BeforeEach with valid providers) mgr.stop() diff --git a/pkg/controllers/revision/suite_test.go b/pkg/controllers/revision/suite_test.go index 2bc949368..248571dee 100644 --- a/pkg/controllers/revision/suite_test.go +++ b/pkg/controllers/revision/suite_test.go @@ -25,6 +25,7 @@ import ( . "github.com/onsi/gomega" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/cluster-capi-operator/pkg/providerimages" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -73,10 +74,12 @@ var _ = BeforeSuite(func() { func setupProviderFixtures() { tb := GinkgoTB() + widgetCRD := test.GenerateCRD(schema.GroupVersionKind{Group: "example.com", Version: "v1", Kind: "Widget"}) + defaultProviderImgs = []providerimages.ProviderImageManifests{ test.NewProviderImageManifests(tb, "core"). WithImageRef("registry.example.com/core@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"). - WithManifests(test.ConfigMapYAML("core-cm")). + WithManifests(test.ConfigMapYAML("core-cm"), test.CRDToYAML(widgetCRD)). Build(), test.NewProviderImageManifests(tb, "infra-aws"). WithInstallOrder(20). diff --git a/pkg/revisiongenerator/compatibility.go b/pkg/revisiongenerator/compatibility.go new file mode 100644 index 000000000..f70acc7bd --- /dev/null +++ b/pkg/revisiongenerator/compatibility.go @@ -0,0 +1,81 @@ +/* +Copyright 2026 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package revisiongenerator + +import ( + "fmt" + + apiextensionsv1alpha1 "github.com/openshift/api/apiextensions/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + k8syaml "sigs.k8s.io/yaml" +) + +const ( + compatibilityRequirementNamePrefix = "ccapio-" + capiNamespace = "openshift-cluster-api" +) + +// buildCompatibilityRequirement constructs a CompatibilityRequirement for the +// given CRD and returns it as an unstructured object for inclusion in a +// renderedComponent. +func buildCompatibilityRequirement(crd unstructured.Unstructured) (unstructured.Unstructured, error) { + crdYAML, err := k8syaml.Marshal(crd.Object) + if err != nil { + return unstructured.Unstructured{}, fmt.Errorf("marshalling CRD %s to YAML: %w", crd.GetName(), err) + } + + cr := &apiextensionsv1alpha1.CompatibilityRequirement{ + TypeMeta: metav1.TypeMeta{ + APIVersion: apiextensionsv1alpha1.GroupVersion.String(), + Kind: "CompatibilityRequirement", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: compatibilityRequirementNamePrefix + crd.GetName(), + }, + Spec: apiextensionsv1alpha1.CompatibilityRequirementSpec{ + CompatibilitySchema: apiextensionsv1alpha1.CompatibilitySchema{ + CustomResourceDefinition: apiextensionsv1alpha1.CRDData{ + Type: apiextensionsv1alpha1.CRDDataTypeYAML, + Data: string(crdYAML), + }, + RequiredVersions: apiextensionsv1alpha1.APIVersions{ + DefaultSelection: apiextensionsv1alpha1.APIVersionSetTypeStorageOnly, + }, + }, + CustomResourceDefinitionSchemaValidation: apiextensionsv1alpha1.CustomResourceDefinitionSchemaValidation{ + Action: apiextensionsv1alpha1.CRDAdmitActionDeny, + }, + ObjectSchemaValidation: apiextensionsv1alpha1.ObjectSchemaValidation{ + Action: apiextensionsv1alpha1.CRDAdmitActionDeny, + NamespaceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kubernetes.io/metadata.name": capiNamespace, + }, + }, + }, + }, + } + + data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(cr) + if err != nil { + return unstructured.Unstructured{}, fmt.Errorf("converting CompatibilityRequirement to unstructured: %w", err) + } + + return unstructured.Unstructured{Object: data}, nil +} diff --git a/pkg/revisiongenerator/revision.go b/pkg/revisiongenerator/revision.go index 70704811e..2e66590c8 100644 --- a/pkg/revisiongenerator/revision.go +++ b/pkg/revisiongenerator/revision.go @@ -30,6 +30,7 @@ import ( operatorv1alpha1ac "github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" k8syaml "sigs.k8s.io/yaml" @@ -46,6 +47,7 @@ const ( var ( errProviderProfileNotFound = errors.New("no provider profile found for component") errContentIDMismatch = errors.New("content ID mismatch") + errUnmanagedCRDsNotFound = errors.New("unmanaged CRDs not found in any component") ) // RenderedRevision represents a set of components whose manifests have been @@ -93,6 +95,7 @@ type renderedRevision struct { components []*renderedComponent contentID string substitutions []operatorv1alpha1.ClusterAPIInstallerRevisionManifestSubstitution + unmanagedCRDs []string } var _ RenderedRevision = &renderedRevision{} @@ -124,15 +127,79 @@ func newRenderedRevision(profiles []providerimages.ProviderImageManifests, opts rev := &renderedRevision{ components: components, substitutions: substitutionsFromMap(cfg.substitutions), + unmanagedCRDs: cfg.unmanagedCRDs, } if err := validateRenderedRevision(rev); err != nil { return nil, err } + if err := buildSyntheticCompatibilityComponent(rev, cfg); err != nil { + return nil, err + } + return rev, nil } +const compatibilityComponentName = "compatibility-requirements" + +func buildSyntheticCompatibilityComponent(rev *renderedRevision, cfg *revisionRenderConfig) error { + if len(rev.unmanagedCRDs) == 0 { + return nil + } + + unmanagedSet := sets.New(rev.unmanagedCRDs...) + foundCRDs := sets.New[string]() + + var compatObjects []unstructured.Unstructured + + for _, component := range rev.components { + var kept []unstructured.Unstructured + + for _, crd := range component.crds { + if unmanagedSet.Has(crd.GetName()) { + if !foundCRDs.Has(crd.GetName()) { + foundCRDs.Insert(crd.GetName()) + + cr, err := buildCompatibilityRequirement(crd) + if err != nil { + return err + } + + cr = transformObject(cr, compatibilityComponentName) + + for _, collector := range cfg.objectCollectors { + collector(cr) + } + + compatObjects = append(compatObjects, cr) + } + } else { + kept = append(kept, crd) + } + } + + component.crds = kept + } + + missing := unmanagedSet.Difference(foundCRDs) + if missing.Len() > 0 { + return fmt.Errorf("unmanaged CRDs not found in any component: %w: %v", errUnmanagedCRDsNotFound, sets.List(missing)) + } + + syntheticComponent := &renderedComponent{ + name: compatibilityComponentName, + synthetic: true, + objects: compatObjects, + } + + // Prepend: the synthetic component must be phase 0 so Boxcutter gates + // all other phases on compatibility. This ordering is load-bearing. + rev.components = append([]*renderedComponent{syntheticComponent}, rev.components...) + + return nil +} + // substitutionsFromMap converts a map to a sorted slice of API substitutions. func substitutionsFromMap(m map[string]string) []operatorv1alpha1.ClusterAPIInstallerRevisionManifestSubstitution { if len(m) == 0 { @@ -236,9 +303,14 @@ func (r *installerRevision) ForInstall(_ string, _ int64) (InstallerRevision, er // ToAPIRevision converts this revision to an API revision. func (r *installerRevision) ToAPIRevision() (operatorv1alpha1.ClusterAPIInstallerRevision, error) { - apiComponents := make([]operatorv1alpha1.ClusterAPIInstallerComponent, len(r.components)) - for i, component := range r.components { - apiComponents[i] = operatorv1alpha1.ClusterAPIInstallerComponent{ + var apiComponents []operatorv1alpha1.ClusterAPIInstallerComponent + + for _, component := range r.components { + if component.synthetic { + continue + } + + apiComponents = append(apiComponents, operatorv1alpha1.ClusterAPIInstallerComponent{ Name: component.name, ClusterAPIInstallerComponentSource: operatorv1alpha1.ClusterAPIInstallerComponentSource{ Type: operatorv1alpha1.InstallerComponentTypeImage, @@ -247,7 +319,7 @@ func (r *installerRevision) ToAPIRevision() (operatorv1alpha1.ClusterAPIInstalle Profile: component.profile, }, }, - } + }) } contentID, err := r.ContentID() @@ -256,11 +328,12 @@ func (r *installerRevision) ToAPIRevision() (operatorv1alpha1.ClusterAPIInstalle } return operatorv1alpha1.ClusterAPIInstallerRevision{ - Name: r.revisionName, - Revision: r.revisionIndex, - ContentID: contentID, - ManifestSubstitutions: slices.Clone(r.substitutions), - Components: apiComponents, + Name: r.revisionName, + Revision: r.revisionIndex, + ContentID: contentID, + ManifestSubstitutions: slices.Clone(r.substitutions), + Components: apiComponents, + UnmanagedCustomResourceDefinitions: slices.Clone(r.unmanagedCRDs), }, nil } @@ -289,6 +362,7 @@ func buildRevisionName(releaseVersion, contentID string, index int64) operatorv1 type revisionRenderConfig struct { objectCollectors []RevisionObjectCollector substitutions map[string]string + unmanagedCRDs []string } type revisionRenderOption func(*revisionRenderConfig) @@ -304,6 +378,15 @@ func WithObjectCollectors(collectors ...RevisionObjectCollector) revisionRenderO } } +// WithUnmanagedCRDs sets the list of CRD names that should not be installed +// by the installer. These CRDs will be used to build CompatibilityRequirement +// objects in a synthetic component and filtered from their normal phases. +func WithUnmanagedCRDs(crds []string) revisionRenderOption { + return func(opts *revisionRenderConfig) { + opts.unmanagedCRDs = crds + } +} + // WithManifestSubstitutions adds envsubst-style substitutions that will be // applied to manifests during rendering and recorded on the revision. When // called multiple times, later values merge with and override earlier ones. @@ -383,9 +466,10 @@ func NewInstallerRevisionFromAPI( } type renderedComponent struct { - name string - imageRef string - profile string + name string + imageRef string + profile string + synthetic bool crds []unstructured.Unstructured objects []unstructured.Unstructured diff --git a/pkg/revisiongenerator/revision_test.go b/pkg/revisiongenerator/revision_test.go index ef17dd5f9..8a6602fb7 100644 --- a/pkg/revisiongenerator/revision_test.go +++ b/pkg/revisiongenerator/revision_test.go @@ -26,6 +26,7 @@ import ( operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/cluster-capi-operator/pkg/providerimages" "github.com/openshift/cluster-capi-operator/pkg/test" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) // Reusable YAML manifest fixtures. @@ -418,6 +419,53 @@ func TestToAPIRevision(t *testing.T) { g.Expect(apiRev.ContentID).NotTo(BeEmpty()) }) + t.Run("unmanagedCRDs included in API revision", func(t *testing.T) { + g := NewWithT(t) + + unmanagedCRDs := []string{"widgets.example.com", "gadgets.example.com"} + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", multiDoc(configMapA, crdA, crdB))}, + WithUnmanagedCRDs(unmanagedCRDs), + ))(g) + + apiRev := must(forInstall(g, rev, "4.18.0", 1).ToAPIRevision())(g) + + g.Expect(apiRev.UnmanagedCustomResourceDefinitions).To(Equal(unmanagedCRDs)) + }) + + t.Run("nil unmanagedCRDs omitted from API revision", func(t *testing.T) { + g := NewWithT(t) + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", configMapA)}, + ))(g) + + apiRev := must(forInstall(g, rev, "4.18.0", 1).ToAPIRevision())(g) + + g.Expect(apiRev.UnmanagedCustomResourceDefinitions).To(BeNil()) + }) + + t.Run("synthetic components excluded from API revision", func(t *testing.T) { + g := NewWithT(t) + + rev := must(newRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", configMapA)}, + ))(g) + + rev.components = append(rev.components, &renderedComponent{ + name: "compatibility-requirements", + synthetic: true, + }) + rev.contentID = "" + + installer := forInstall(g, rev, "4.18.0", 1) + apiRev := must(installer.ToAPIRevision())(g) + + g.Expect(apiRev.Components).To(HaveLen(1), "synthetic component should be excluded") + g.Expect(apiRev.Components[0].Name).To(Equal("core")) + }) + t.Run("substitutions included in API revision", func(t *testing.T) { g := NewWithT(t) @@ -530,6 +578,142 @@ data: }) } +func TestSyntheticCompatibilityComponent(t *testing.T) { + t.Run("no synthetic component when unmanagedCRDs is empty", func(t *testing.T) { + g := NewWithT(t) + + rev := must(NewRenderedRevision([]providerimages.ProviderImageManifests{ + profile(t, "core", "img1", "default", multiDoc(crdA, configMapA)), + }))(g) + + components := rev.Components() + g.Expect(components).To(HaveLen(1)) + g.Expect(components[0].Name()).To(Equal("core")) + g.Expect(components[0].CRDs()).To(HaveLen(1)) + g.Expect(components[0].Objects()).To(HaveLen(1)) + }) + + t.Run("synthetic component created as first component", func(t *testing.T) { + g := NewWithT(t) + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", multiDoc(crdA, configMapA))}, + WithUnmanagedCRDs([]string{"widgets.example.com"}), + ))(g) + + components := rev.Components() + g.Expect(components).To(HaveLen(2)) + g.Expect(components[0].Name()).To(Equal(compatibilityComponentName)) + g.Expect(components[1].Name()).To(Equal("core")) + + compatObjects := components[0].Objects() + g.Expect(compatObjects).To(HaveLen(1)) + g.Expect(compatObjects[0].GetName()).To(Equal("ccapio-widgets.example.com")) + g.Expect(compatObjects[0].GetObjectKind().GroupVersionKind().Kind).To(Equal("CompatibilityRequirement")) + + g.Expect(components[0].CRDs()).To(BeEmpty(), "synthetic component should have no CRDs") + }) + + t.Run("unmanaged CRDs removed from original component", func(t *testing.T) { + g := NewWithT(t) + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", multiDoc(crdA, crdB, configMapA))}, + WithUnmanagedCRDs([]string{"widgets.example.com"}), + ))(g) + + components := rev.Components() + coreComponent := components[1] + g.Expect(coreComponent.Name()).To(Equal("core")) + + coreCRDs := coreComponent.CRDs() + g.Expect(coreCRDs).To(HaveLen(1), "only the non-unmanaged CRD should remain") + g.Expect(coreCRDs[0].GetName()).To(Equal("gadgets.example.com")) + + g.Expect(coreComponent.Objects()).To(HaveLen(1), "non-CRD objects should be unaffected") + }) + + t.Run("error when unmanaged CRD not found in any component", func(t *testing.T) { + _, err := NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", configMapA)}, + WithUnmanagedCRDs([]string{"nonexistent.example.com"}), + ) + + g := NewWithT(t) + g.Expect(err).To(MatchError(ContainSubstring("nonexistent.example.com"))) + }) + + t.Run("unmanaged CRDs change content ID", func(t *testing.T) { + g := NewWithT(t) + + profiles := []providerimages.ProviderImageManifests{ + profile(t, "core", "img1", "default", multiDoc(crdA, configMapA)), + } + + revWithout := must(NewRenderedRevision(profiles))(g) + revWith := must(NewRenderedRevision(profiles, WithUnmanagedCRDs([]string{"widgets.example.com"})))(g) + + idWithout := must(revWithout.ContentID())(g) + idWith := must(revWith.ContentID())(g) + + g.Expect(idWith).NotTo(Equal(idWithout)) + }) + + t.Run("managed label applied to CompatibilityRequirement objects", func(t *testing.T) { + g := NewWithT(t) + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", multiDoc(crdA, configMapA))}, + WithUnmanagedCRDs([]string{"widgets.example.com"}), + ))(g) + + compatObj := rev.Components()[0].Objects()[0] + g.Expect(compatObj.GetLabels()).To(HaveKeyWithValue(ManagedLabelKey, compatibilityComponentName)) + }) + + t.Run("multiple unmanaged CRDs across components", func(t *testing.T) { + g := NewWithT(t) + + rev := must(NewRenderedRevision( + []providerimages.ProviderImageManifests{ + profile(t, "core", "img1", "default", multiDoc(crdA, configMapA)), + profile(t, "infra", "img2", "aws", multiDoc(crdB, configMapB)), + }, + WithUnmanagedCRDs([]string{"widgets.example.com", "gadgets.example.com"}), + ))(g) + + components := rev.Components() + g.Expect(components).To(HaveLen(3)) + g.Expect(components[0].Name()).To(Equal(compatibilityComponentName)) + + compatObjects := components[0].Objects() + g.Expect(compatObjects).To(HaveLen(2)) + g.Expect(compatObjects[0].GetName()).To(Equal("ccapio-widgets.example.com")) + g.Expect(compatObjects[1].GetName()).To(Equal("ccapio-gadgets.example.com")) + + g.Expect(components[1].CRDs()).To(BeEmpty(), "core CRDs should be empty after removal") + g.Expect(components[2].CRDs()).To(BeEmpty(), "infra CRDs should be empty after removal") + }) + + t.Run("object collectors invoked for CompatibilityRequirement objects", func(t *testing.T) { + g := NewWithT(t) + + var collected []string + + collector := func(obj unstructured.Unstructured) { + collected = append(collected, obj.GetName()) + } + + must(NewRenderedRevision( + []providerimages.ProviderImageManifests{profile(t, "core", "img1", "default", multiDoc(crdA, configMapA))}, + WithUnmanagedCRDs([]string{"widgets.example.com"}), + WithObjectCollectors(collector), + ))(g) + + g.Expect(collected).To(ContainElement("ccapio-widgets.example.com")) + }) +} + func TestComponents(t *testing.T) { t.Run("returns correct component count and names", func(t *testing.T) { g := NewWithT(t)