diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 121d18a3..60e802ce 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -571,7 +571,8 @@ func (r *OperatorPolicyReconciler) checkSubOverlap( } // applySubscriptionDefaults will set the subscription channel, source, and sourceNamespace when they are unset by -// utilizing the PackageManifest API. +// utilizing the PackageManifest API. If the PackageManifest can not be found, and a subscription already exists for the +// operator, then information from the found subscription will be used. func (r *OperatorPolicyReconciler) applySubscriptionDefaults( ctx context.Context, policy *policyv1beta1.OperatorPolicy, subscription *operatorv1alpha1.Subscription, ) error { @@ -594,10 +595,14 @@ func (r *OperatorPolicyReconciler) applySubscriptionDefaults( ) if err != nil { if k8serrors.IsNotFound(err) { - return fmt.Errorf( - "%wthe subscription defaults could not be determined because the PackageManifest was not found", - ErrPackageManifest, - ) + if !r.usingExistingSubIfFound(policy, subscription) { + return fmt.Errorf( + "%wthe subscription defaults could not be determined because the PackageManifest was not found", + ErrPackageManifest, + ) + } + + return nil } log.Error(err, "Failed to get the PackageManifest", "name", subSpec.Package) @@ -679,6 +684,55 @@ func (r *OperatorPolicyReconciler) applySubscriptionDefaults( return nil } +// usingExistingSubIfFound attempts to find an existing subscription for the operator, and uses values +// from that object to help build the desired subscription. It is meant to be used when the PackageManifest +// for the operator is not found. It returns true when a subscription was found and used successfully. +func (r *OperatorPolicyReconciler) usingExistingSubIfFound( + policy *policyv1beta1.OperatorPolicy, subscription *operatorv1alpha1.Subscription, +) bool { + if subscription.Namespace == "" { + // check for an already-known subscription to "adopt" + subs := policy.Status.RelatedObjsOfKind("Subscription") + + if len(subs) == 1 { + // Note: RelatedObjsOfKind returns a map - in this case we just want the one object + for _, sub := range subs { + subscription.Namespace = sub.Object.Metadata.Namespace + } + } + } + + if subscription.Namespace != "" { + watcher := opPolIdentifier(policy.Namespace, policy.Name) + + gotSub, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, subscription.Namespace, subscription.Name) + if err != nil || gotSub == nil { + return false + } + + foundSub := new(operatorv1alpha1.Subscription) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(gotSub.Object, foundSub); err != nil { + return false + } + + if subscription.Spec.Channel == "" { + subscription.Spec.Channel = foundSub.Spec.Channel + } + + if subscription.Spec.CatalogSource == "" { + subscription.Spec.CatalogSource = foundSub.Spec.CatalogSource + } + + if subscription.Spec.CatalogSourceNamespace == "" { + subscription.Spec.CatalogSourceNamespace = foundSub.Spec.CatalogSourceNamespace + } + + return true + } + + return false +} + // buildSubscription bootstraps the subscription spec defined in the operator policy // with the apiversion and kind in preparation for resource creation. // If an error is returned, it will include details on why the policy spec if invalid and diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 586a6b58..87d7f476 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -246,6 +246,19 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, testNamespace, gvrPolicy, gvrOperatorPolicy) }) + AfterAll(func(ctx context.Context) { + By("Fixing the catalog source") + KubectlTarget("patch", "catalogsource", "operatorhubio-catalog", "--namespace=olm", "--type=json", "-p", + `[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:latest"}]`) + + By("Waiting for a packagemanifest to reappear") + Eventually(func() error { + _, err := targetK8sDynamic.Resource(gvrPackageManifest).Namespace("default").Get( + ctx, "airflow-helm-operator", metav1.GetOptions{}) + + return err + }, olmWaitTimeout*2, 3).Should(Succeed()) + }) It("Should create the Subscription with default values", func(ctx context.Context) { By("Verifying the policy is compliant") @@ -284,6 +297,34 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu sourceNamespace, _, _ := unstructured.NestedString(sub.Object, "spec", "sourceNamespace") Expect(sourceNamespace).To(Equal("olm")) }) + + It("Should adopt an existing subscription if removed from the catalog", func(ctx context.Context) { + By("Breaking the catalog source") + KubectlTarget("patch", "catalogsource", "operatorhubio-catalog", "--namespace=olm", "--type=json", "-p", + `[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:broken"}]`) + + By("ensuring the packagemanifest is gone") + Eventually(func() error { + _, err := targetK8sDynamic.Resource(gvrPackageManifest).Namespace("default").Get( + ctx, "airflow-helm-operator", metav1.GetOptions{}) + + return err + }, olmWaitTimeout, 3).ShouldNot(Succeed()) + + By("Checking the validation condition") + check( + opPolName, + false, + []policyv1.RelatedObject{}, + metav1.Condition{ + Type: "ValidPolicySpec", + Status: metav1.ConditionTrue, + Reason: "PolicyValidated", + Message: `the policy spec is valid`, + }, + `the policy spec is valid`, + ) + }) }) Describe("Testing an operator policy with invalid partial defaults", Ordered, func() { diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 434578b2..6c5b6d80 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -53,6 +53,7 @@ var ( gvrOperatorGroup schema.GroupVersionResource gvrInstallPlan schema.GroupVersionResource gvrClusterServiceVersion schema.GroupVersionResource + gvrPackageManifest schema.GroupVersionResource defaultImageRegistry string IsHosted bool targetK8sClient kubernetes.Interface @@ -122,6 +123,11 @@ var _ = BeforeSuite(func() { Version: "v1alpha1", Resource: "clusterserviceversions", } + gvrPackageManifest = schema.GroupVersionResource{ + Group: "packages.operators.coreos.com", + Version: "v1", + Resource: "packagemanifests", + } gvrAPIService = schema.GroupVersionResource{ Group: "apiregistration.k8s.io", Version: "v1",