diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 071c1987..cb6882cc 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -13,6 +13,7 @@ import ( operatorv1 "github.com/operator-framework/api/pkg/operators/v1" operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" depclient "github.com/stolostron/kubernetes-dependency-watches/client" + corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -28,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + policyv1 "open-cluster-management.io/config-policy-controller/api/v1" policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" ) @@ -38,6 +40,7 @@ const ( var ( subscriptionGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1alpha1", Kind: "Subscription"} operatorGroupGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1", Kind: "OperatorGroup"} + installPlanGVK = schema.GroupVersionKind{Group: "operators.coreos.com", Version: "v1alpha1", Kind: "InstallPlan"} ) // OperatorPolicyReconciler reconciles a OperatorPolicy object @@ -125,13 +128,19 @@ func (r *OperatorPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reque return reconcile.Result{}, err } - _, err = r.handleSubscription(ctx, policy) + subscription, err := r.handleSubscription(ctx, policy) if err != nil { OpLog.Error(err, "Error handling Subscription") return reconcile.Result{}, err } + if err := r.handleInstallPlan(ctx, policy, subscription); err != nil { + OpLog.Error(err, "Error handling InstallPlan") + + return reconcile.Result{}, err + } + // FUTURE: more resource checks return reconcile.Result{}, nil @@ -418,11 +427,31 @@ func (r *OperatorPolicyReconciler) handleSubscription( } if !updateNeeded { - // FUTURE: Check more details about the *status* of the Subscription - // For now, just mark it as compliant + subResFailed := mergedSub.Status.GetCondition(operatorv1alpha1.SubscriptionResolutionFailed) + + if subResFailed.Status == corev1.ConditionTrue { + cond := metav1.Condition{ + Type: subConditionType, + Status: metav1.ConditionFalse, + Reason: subResFailed.Reason, + Message: subResFailed.Message, + } + + if subResFailed.LastTransitionTime != nil { + cond.LastTransitionTime = *subResFailed.LastTransitionTime + } + + err := r.updateStatus(ctx, policy, cond, nonCompObj(foundSub, subResFailed.Reason)) + if err != nil { + return nil, fmt.Errorf("error updating the status for a Subscription in ResolutionFailed: %w", err) + } + + return mergedSub, nil + } + err := r.updateStatus(ctx, policy, matchesCond("Subscription"), matchedObj(foundSub)) if err != nil { - return nil, fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err) + return nil, fmt.Errorf("error updating the status for a Subscription that matches: %w", err) } return mergedSub, nil @@ -492,6 +521,12 @@ func buildSubscription( subscription.ObjectMeta.Namespace = ns subscription.Spec = spec + // If the policy is in `enforce` mode and the allowed CSVs are restricted, + // the InstallPlanApproval will be set to Manual so that upgrades can be controlled. + if policy.Spec.RemediationAction.IsEnforce() && len(policy.Spec.Versions) > 0 { + subscription.Spec.InstallPlanApproval = operatorv1alpha1.ApprovalManual + } + return subscription, nil } @@ -550,3 +585,228 @@ func (r *OperatorPolicyReconciler) mergeObjects( return updateNeeded, false, nil } + +func (r *OperatorPolicyReconciler) handleInstallPlan( + ctx context.Context, policy *policyv1beta1.OperatorPolicy, sub *operatorv1alpha1.Subscription, +) error { + if sub == nil { + return errors.New("unable to handle InstallPlan: the subscription is nil") + } + + watcher := opPolIdentifier(policy.Namespace, policy.Name) + + foundInstallPlans, err := r.DynamicWatcher.List( + watcher, installPlanGVK, sub.Namespace, labels.Everything()) + if err != nil { + return fmt.Errorf("error listing InstallPlans: %w", err) + } + + ownedInstallPlans := make([]unstructured.Unstructured, 0, len(foundInstallPlans)) + + for _, installPlan := range foundInstallPlans { + for _, owner := range installPlan.GetOwnerReferences() { + match := owner.Name == sub.Name && + owner.Kind == subscriptionGVK.Kind && + owner.APIVersion == subscriptionGVK.GroupVersion().String() + if match { + ownedInstallPlans = append(ownedInstallPlans, installPlan) + + break + } + } + } + + // InstallPlans are generally kept in order to provide a history of actions on the cluster, but + // they can be deleted without impacting the installed operator. So, not finding any should not + // be considered a reason for NonCompliance. + if len(ownedInstallPlans) == 0 { + err := r.updateStatus(ctx, policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)) + if err != nil { + return fmt.Errorf("error updating status when no relevant InstallPlans were found: %w", err) + } + + return nil + } + + OpLog := ctrl.LoggerFrom(ctx) + relatedInstallPlans := make([]policyv1.RelatedObject, len(ownedInstallPlans)) + ipsRequiringApproval := make([]unstructured.Unstructured, 0) + anyInstalling := false + + // Construct the relevant relatedObjects, and collect any that might be considered for approval + for i, installPlan := range ownedInstallPlans { + phase, ok, err := unstructured.NestedString(installPlan.Object, "status", "phase") + if !ok && err == nil { + err = errors.New("the phase of the InstallPlan was not found") + } + + if err != nil { + OpLog.Error(err, "Unable to determine the phase of the related InstallPlan", + "InstallPlan.Name", installPlan.GetName()) + + // The InstallPlan will be added as unknown + phase = "" + } + + relObj := policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(&ownedInstallPlans[i]), + Properties: &policyv1.ObjectProperties{ + UID: string(installPlan.GetUID()), + }, + } + + // A `reason` should be added to most InstallPlans based on the phase. + // Note that a `compliant` value will only be added in certain cases. + if phase != "" { + relObj.Reason = "The InstallPlan is " + phase + } + + // handle some special phases + switch phase { + case string(operatorv1alpha1.InstallPlanPhaseRequiresApproval): + ipsRequiringApproval = append(ipsRequiringApproval, installPlan) + + // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. + // For now, assume it is set to 'NonCompliant' + relObj.Compliant = string(policyv1.NonCompliant) + case string(operatorv1alpha1.InstallPlanPhaseInstalling): + anyInstalling = true + + // if it's still installing, then it shouldn't be considered compliant yet. + relObj.Compliant = string(policyv1.NonCompliant) + } + + relatedInstallPlans[i] = relObj + } + + if anyInstalling { + err := r.updateStatus(ctx, policy, installPlanInstallingCond, relatedInstallPlans...) + if err != nil { + return fmt.Errorf("error updating status when an installing InstallPlan was found: %w", err) + } + + return nil + } + + if len(ipsRequiringApproval) == 0 { + err := r.updateStatus(ctx, policy, installPlansFineCond, relatedInstallPlans...) + if err != nil { + return fmt.Errorf("error updating status when InstallPlans were fine: %w", err) + } + + return nil + } + + allUpgradeVersions := make([]string, len(ipsRequiringApproval)) + + for i, installPlan := range ipsRequiringApproval { + csvNames, ok, err := unstructured.NestedStringSlice(installPlan.Object, + "spec", "clusterServiceVersionNames") + if !ok && err != nil { + err = errors.New("the clusterServiceVersionNames field of the InstallPlan was not found") + } + + if err != nil { + OpLog.Error(err, "Unable to determine the csv names of the related InstallPlan", + "InstallPlan.Name", installPlan.GetName()) + + csvNames = []string{"???"} + } + + allUpgradeVersions[i] = fmt.Sprintf("%v", csvNames) + } + + // Only report this status in `inform` mode, because otherwise it could easily oscillate between this and + // another condition below when being enforced. + if policy.Spec.RemediationAction.IsInform() { + // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. + // For now this condition assumes it is set to 'NonCompliant' + err := r.updateStatus(ctx, policy, installPlanUpgradeCond(allUpgradeVersions), relatedInstallPlans...) + if err != nil { + return fmt.Errorf("error updating status when an InstallPlan requiring approval was found: %w", err) + } + + return nil + } + + approvedVersion := "" // this will only be accurate when there is only one approvable InstallPlan + approvableInstallPlans := make([]unstructured.Unstructured, 0) + + for _, installPlan := range ipsRequiringApproval { + ipCSVs, ok, err := unstructured.NestedStringSlice(installPlan.Object, + "spec", "clusterServiceVersionNames") + if !ok && err != nil { + err = errors.New("the clusterServiceVersionNames field of the InstallPlan was not found") + } + + if err != nil { + OpLog.Error(err, "Unable to determine the csv names of the related InstallPlan", + "InstallPlan.Name", installPlan.GetName()) + + continue + } + + if len(ipCSVs) != 1 { + continue // Don't automate approving any InstallPlans for multiple CSVs + } + + matchingCSV := len(policy.Spec.Versions) == 0 // true if `spec.versions` is not specified + + for _, acceptableCSV := range policy.Spec.Versions { + if string(acceptableCSV) == ipCSVs[0] { + matchingCSV = true + + break + } + } + + if matchingCSV { + approvedVersion = ipCSVs[0] + + approvableInstallPlans = append(approvableInstallPlans, installPlan) + } + } + + if len(approvableInstallPlans) == 0 { + // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. + // For now this condition assumes it is set to 'NonCompliant' + cond := installPlanUpgradeCond(allUpgradeVersions) + cond.Message += " but not allowed by the specified versions in the policy" + + err := r.updateStatus(ctx, policy, cond, relatedInstallPlans...) + if err != nil { + return fmt.Errorf("error updating status when no approvable InstallPlans were found: %w", err) + } + + return nil + } + + if len(approvableInstallPlans) > 1 { + // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. + // For now this condition assumes it is set to 'NonCompliant' + cond := installPlanUpgradeCond(allUpgradeVersions) + cond.Message += " but multiple of those match the versions specified in the policy" + + err := r.updateStatus(ctx, policy, cond, relatedInstallPlans...) + if err != nil { + return fmt.Errorf("error updating status when too many approvable InstallPlans were found: %w", err) + } + + return nil + } + + if err := unstructured.SetNestedField(approvableInstallPlans[0].Object, true, "spec", "approved"); err != nil { + return fmt.Errorf("error approving InstallPlan: %w", err) + } + + if err := r.Update(ctx, &approvableInstallPlans[0]); err != nil { + return fmt.Errorf("error updating approved InstallPlan: %w", err) + } + + err = r.updateStatus(ctx, policy, installPlanApprovedCond(approvedVersion), relatedInstallPlans...) + if err != nil { + return fmt.Errorf("error updating status after approving an InstallPlan: %w", err) + } + + return nil +} diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index d30bd707..463a0cac 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -199,6 +199,18 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C } } + idx, cond = policy.Status.GetCondition(installPlanConditionType) + if idx == -1 { + messages = append(messages, "the status of the InstallPlan is unknown") + foundNonCompliant = true + } else { + messages = append(messages, cond.Message) + + if cond.Status != metav1.ConditionTrue { + foundNonCompliant = true + } + } + // FUTURE: check additional conditions if foundNonCompliant { @@ -274,9 +286,10 @@ func (r *OperatorPolicyReconciler) emitComplianceEvent( } const ( - compliantConditionType = "Compliant" - opGroupConditionType = "OperatorGroupCompliant" - subConditionType = "SubscriptionCompliant" + compliantConditionType = "Compliant" + opGroupConditionType = "OperatorGroupCompliant" + subConditionType = "SubscriptionCompliant" + installPlanConditionType = "InstallPlanCompliant" ) func condType(kind string) string { @@ -356,6 +369,9 @@ func updatedCond(kind string) metav1.Condition { } } +// opGroupPreexistingCond is a Compliant condition with Reason 'PreexistingOperatorGroupFound', +// and Message 'the policy does not specify an OperatorGroup but one already exists in the +// namespace - assuming that OperatorGroup is correct' var opGroupPreexistingCond = metav1.Condition{ Type: opGroupConditionType, Status: metav1.ConditionTrue, @@ -364,8 +380,8 @@ var opGroupPreexistingCond = metav1.Condition{ "assuming that OperatorGroup is correct", } -// opGroupTooManyCond is a NonCompliant condition with a Reason like 'TooManyOperatorGroups', -// and a Message like 'there is more than one OperatorGroup in the namespace' +// opGroupTooManyCond is a NonCompliant condition with Reason 'TooManyOperatorGroups', +// and Message 'there is more than one OperatorGroup in the namespace' var opGroupTooManyCond = metav1.Condition{ Type: opGroupConditionType, Status: metav1.ConditionFalse, @@ -373,6 +389,63 @@ var opGroupTooManyCond = metav1.Condition{ Message: "there is more than one OperatorGroup in the namespace", } +// noInstallPlansCond is a Compliant condition with Reason 'NoInstallPlansFound', +// and Message 'there are no relevant InstallPlans in the namespace' +var noInstallPlansCond = metav1.Condition{ + Type: installPlanConditionType, + Status: metav1.ConditionTrue, + Reason: "NoInstallPlansFound", + Message: "there are no relevant InstallPlans in the namespace", +} + +// installPlanInstallingCond is a NonCompliant condition with Reason 'InstallPlansInstalling' +// and message 'a relevant InstallPlan is actively installing' +var installPlanInstallingCond = metav1.Condition{ + Type: installPlanConditionType, + Status: metav1.ConditionFalse, + Reason: "InstallPlansInstalling", + Message: "a relevant InstallPlan is actively installing", +} + +// installPlansFineCond is a Compliant condition with Reason 'InstallPlansFine' +// and message 'the relevant InstallPlans are fine' +var installPlansFineCond = metav1.Condition{ + Type: installPlanConditionType, + Status: metav1.ConditionTrue, + Reason: "InstallPlansFine", + Message: "the relevant InstallPlans are fine", +} + +// installPlanUpgradeCond is a NonCompliant condition with Reason 'InstallPlanRequiresApproval' +// and a message detailing which possible updates are available +func installPlanUpgradeCond(versions []string) metav1.Condition { + cond := metav1.Condition{ + Type: installPlanConditionType, + Status: metav1.ConditionFalse, + Reason: "InstallPlanRequiresApproval", + } + + if len(versions) == 1 { + cond.Message = fmt.Sprintf("an InstallPlan to update to %v is available for approval", versions[0]) + } else { + cond.Message = fmt.Sprintf("there are multiple InstallPlans available for approval (%v)", + strings.Join(versions, ", or ")) + } + + return cond +} + +// installPlanApprovedCond is a Compliant condition with Reason 'InstallPlanApproved' +// and a message like 'the InstallPlan for _____ was approved' +func installPlanApprovedCond(version string) metav1.Condition { + return metav1.Condition{ + Type: installPlanConditionType, + Status: metav1.ConditionTrue, + Reason: "InstallPlanApproved", + Message: fmt.Sprintf("the InstallPlan for %v was approved", version), + } +} + // missingWantedObj returns a NonCompliant RelatedObject with reason = 'Resource not found but should exist' func missingWantedObj(obj client.Object) policyv1.RelatedObject { return policyv1.RelatedObject{ @@ -451,3 +524,31 @@ func opGroupTooManyObjs(opGroups []unstructured.Unstructured) []policyv1.Related return objs } + +func nonCompObj(obj client.Object, reason string) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResourceFromObj(obj), + Compliant: string(policyv1.NonCompliant), + Reason: reason, + Properties: &policyv1.ObjectProperties{ + UID: string(obj.GetUID()), + }, + } +} + +// noInstallPlansObj returns a compliant RelatedObject with +// reason = 'There are no relevant InstallPlans in this namespace' +func noInstallPlansObj(namespace string) policyv1.RelatedObject { + return policyv1.RelatedObject{ + Object: policyv1.ObjectResource{ + Kind: installPlanGVK.Kind, + APIVersion: installPlanGVK.GroupVersion().String(), + Metadata: policyv1.ObjectMetadata{ + Name: "*", + Namespace: namespace, + }, + }, + Compliant: string(policyv1.Compliant), + Reason: "There are no relevant InstallPlans in this namespace", + } +} diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 55219c94..f4e49968 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1,11 +1,13 @@ package e2e import ( + "context" "encoding/json" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" policyv1 "open-cluster-management.io/config-policy-controller/api/v1" policyv1beta1 "open-cluster-management.io/config-policy-controller/api/v1beta1" @@ -69,7 +71,7 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun g.Expect(idx).NotTo(Equal(-1)) g.Expect(actualCondition.Status).To(Equal(expectedCondition.Status)) g.Expect(actualCondition.Reason).To(Equal(expectedCondition.Reason)) - g.Expect(actualCondition.Message).To(Equal(expectedCondition.Message)) + g.Expect(actualCondition.Message).To(MatchRegexp(".*" + expectedCondition.Message + ".*")) g.Expect(utils.GetMatchingEvents( clientManaged, opPolTestNS, parentPolicyName, "", expectedEventMsgSnippet, opPolTimeout, @@ -464,14 +466,15 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun Namespace: opPolTestNS, }, }, - Compliant: "Compliant", - Reason: "Resource found as expected", + Compliant: "NonCompliant", + Reason: "ConstraintsNotSatisfiable", }}, metav1.Condition{ - Type: "SubscriptionCompliant", - Status: metav1.ConditionTrue, - Reason: "SubscriptionMatches", - Message: "the Subscription matches what is required by the policy", + Type: "SubscriptionCompliant", + Status: metav1.ConditionFalse, + Reason: "ConstraintsNotSatisfiable", + Message: "constraints not satisfiable: no operators found from catalog operatorhubio-catalog in " + + "namespace fake referenced by subscription project-quay", }, "the Subscription was updated to match the policy", ) @@ -550,4 +553,230 @@ var _ = Describe("Test installing an operator from OperatorPolicy", Ordered, fun ) }) }) + Describe("Testing InstallPlan approval and status behavior", Ordered, func() { + const ( + opPolYAML = "../resources/case38_operator_install/operator-policy-manual-upgrades.yaml" + opPolName = "oppol-manual-upgrades" + subName = "strimzi-kafka-operator" + ) + + var ( + firstInstallPlanName string + secondInstallPlanName string + ) + + BeforeAll(func() { + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) + }) + + createObjWithParent(parentPolicyYAML, parentPolicyName, + opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) + }) + + It("Should initially report the ConstraintsNotSatisfiable Subscription", func() { + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "Subscription", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + Name: subName, + }, + }, + Compliant: "NonCompliant", + Reason: "ConstraintsNotSatisfiable", + }}, + metav1.Condition{ + Type: "SubscriptionCompliant", + Status: metav1.ConditionFalse, + Reason: "ConstraintsNotSatisfiable", + Message: "constraints not satisfiable: no operators found with name " + + "strimzi-cluster-operator.v0.0.0.1337 in channel strimzi-0.36.x of package " + + "strimzi-kafka-operator in the catalog referenced by subscription strimzi-kafka-operator, " + + "subscription strimzi-kafka-operator exists", + }, + "constraints not satisfiable", + ) + }) + It("Should initially report that no InstallPlans are found", func() { + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + Name: "*", + }, + }, + Compliant: "Compliant", + Reason: "There are no relevant InstallPlans in this namespace", + }}, + metav1.Condition{ + Type: "InstallPlanCompliant", + Status: metav1.ConditionTrue, + Reason: "NoInstallPlansFound", + Message: "there are no relevant InstallPlans in the namespace", + }, + "there are no relevant InstallPlans in the namespace", + ) + }) + It("Should report an available upgrade", func() { + goodVersion := "strimzi-cluster-operator.v0.36.0" + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/subscription/startingCSV", "value": "`+goodVersion+`"},`+ + `{"op": "replace", "path": "/spec/remediationAction", "value": "inform"}]`) + utils.Kubectl("patch", "subscription.operator", subName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/startingCSV", "value": "`+goodVersion+`"}]`) + Eventually(func() int { + ipList, _ := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + List(context.TODO(), metav1.ListOptions{}) + + return len(ipList.Items) + }, "15s", "5s").Should(Equal(1)) + check( + opPolName, + true, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + }, + }, + Compliant: "NonCompliant", + Reason: "The InstallPlan is RequiresApproval", + }}, + metav1.Condition{ + Type: "InstallPlanCompliant", + Status: metav1.ConditionFalse, + Reason: "InstallPlanRequiresApproval", + Message: "an InstallPlan to update to [strimzi-cluster-operator.v0.36.0] is available for approval", + }, + "an InstallPlan to update .* is available for approval", + ) + }) + It("Should do the upgrade when enforced, and stop at the next version", func() { + ipList, err := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + List(context.TODO(), metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ipList.Items).To(HaveLen(1)) + + firstInstallPlanName = ipList.Items[0].GetName() + + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) + + Eventually(func() int { + ipList, err = clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + List(context.TODO(), metav1.ListOptions{}) + + return len(ipList.Items) + }, "15s", "5s").Should(Equal(2)) + + secondInstallPlanName = ipList.Items[1].GetName() + if firstInstallPlanName == secondInstallPlanName { + secondInstallPlanName = ipList.Items[0].GetName() + } + + Eventually(func() string { + ip, _ := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + Get(context.TODO(), firstInstallPlanName, metav1.GetOptions{}) + phase, _, _ := unstructured.NestedString(ip.Object, "status", "phase") + + return phase + }, "30s", "5s").Should(Equal("Complete")) + + // This check covers several situations that occur quickly: the first InstallPlan eventually + // progresses to Complete after it is approved, and the next InstallPlan is created and + // recognized by the policy (but not yet approved). + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + Name: firstInstallPlanName, + }, + }, + Reason: "The InstallPlan is Complete", + }, { + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + Name: secondInstallPlanName, + }, + }, + Compliant: "NonCompliant", + Reason: "The InstallPlan is RequiresApproval", + }}, + metav1.Condition{ + Type: "InstallPlanCompliant", + Status: metav1.ConditionFalse, + Reason: "InstallPlanRequiresApproval", + Message: "an InstallPlan to update to [strimzi-cluster-operator.v0.36.1] is available for " + + "approval but not allowed by the specified versions in the policy", + }, + "the InstallPlan.*36.0.*was approved", + ) + }) + It("Should approve the next version when it's added to the spec", func() { + utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", + `[{"op": "add", "path": "/spec/versions/-", "value": "strimzi-cluster-operator.v0.36.1"}]`) + + Eventually(func() string { + ip, _ := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + Get(context.TODO(), secondInstallPlanName, metav1.GetOptions{}) + phase, _, _ := unstructured.NestedString(ip.Object, "status", "phase") + + return phase + }, "30s", "5s").Should(Equal("Complete")) + + check( + opPolName, + false, + []policyv1.RelatedObject{{ + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + Name: firstInstallPlanName, + }, + }, + Reason: "The InstallPlan is Complete", + }, { + Object: policyv1.ObjectResource{ + Kind: "InstallPlan", + APIVersion: "operators.coreos.com/v1alpha1", + Metadata: policyv1.ObjectMetadata{ + Namespace: opPolTestNS, + Name: secondInstallPlanName, + }, + }, + Reason: "The InstallPlan is Complete", + }}, + metav1.Condition{ + Type: "InstallPlanCompliant", + Status: metav1.ConditionTrue, + Reason: "InstallPlansFine", + Message: "the relevant InstallPlans are fine", + }, + "the InstallPlan.*36.1.*was approved", + ) + }) + }) }) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index a2ee84ec..bc5d4474 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -50,6 +50,7 @@ var ( gvrOperatorPolicy schema.GroupVersionResource gvrSubscription schema.GroupVersionResource gvrOperatorGroup schema.GroupVersionResource + gvrInstallPlan schema.GroupVersionResource defaultImageRegistry string ) @@ -93,6 +94,11 @@ var _ = BeforeSuite(func() { Version: "v1", Resource: "operatorgroups", } + gvrInstallPlan = schema.GroupVersionResource{ + Group: "operators.coreos.com", + Version: "v1alpha1", + Resource: "installplans", + } gvrAPIService = schema.GroupVersionResource{ Group: "apiregistration.k8s.io", Version: "v1", diff --git a/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml b/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml new file mode 100644 index 00000000..825be7b5 --- /dev/null +++ b/test/resources/case38_operator_install/operator-policy-manual-upgrades.yaml @@ -0,0 +1,23 @@ +apiVersion: policy.open-cluster-management.io/v1beta1 +kind: OperatorPolicy +metadata: + name: oppol-manual-upgrades + ownerReferences: + - apiVersion: policy.open-cluster-management.io/v1 + kind: Policy + name: parent-policy + uid: 12345678-90ab-cdef-1234-567890abcdef # must be replaced before creation +spec: + remediationAction: enforce + severity: medium + complianceType: musthave + subscription: + channel: strimzi-0.36.x + installPlanApproval: Manual + name: strimzi-kafka-operator + namespace: operator-policy-testns + source: operatorhubio-catalog + sourceNamespace: olm + startingCSV: strimzi-cluster-operator.v0.0.0.1337 # shouldn't match a real version + versions: + - strimzi-cluster-operator.v0.36.0