diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index ada6f6a8..e4ad7276 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -2247,49 +2247,36 @@ func getApprovedCSVs( return nil } - packageDependencies := getBundleDependencies(installPlan, subscriptionCSV, "") - - approvedCSVs := sets.Set[string]{} - approvedCSVs.Insert(subscriptionCSV) - - // Approve all CSVs that are dependencies of the subscription CSV. - for _, packageDependency := range packageDependencies.UnsortedList() { - for _, csvName := range installPlan.Spec.ClusterServiceVersionNames { - if strings.HasPrefix(csvName, packageDependency+".v") { - approvedCSVs.Insert(csvName) - } - } - } + approvedCSVs := getBundleDependencies(installPlan, subscriptionCSV) return approvedCSVs } -// getBundleDependencies recursively gets the dependencies from a target CSV or package name. If the package name is -// provided instead of the target CSV, then it assumes the CSVs are in the format of `.v`. The -// returned set is of package names and not the CSV (i.e. no version in it). +// getBundleDependencies reads the install plan for required package dependencies. +// The package names are mapped to their CSV identifiers before recursively evaluating the dependency chain. +// The returned set contains the target CSV and all of its dependency CSVs. func getBundleDependencies( - installPlan *operatorv1alpha1.InstallPlan, targetCSV string, packageName string, + installPlan *operatorv1alpha1.InstallPlan, targetCSV string, ) sets.Set[string] { - packageDependencies := sets.Set[string]{} + approvedCSVs := sets.Set[string]{} - if targetCSV == "" && packageName == "" { - return packageDependencies + if targetCSV == "" { + return approvedCSVs } - for i, bundle := range installPlan.Status.BundleLookups { - if targetCSV != "" && bundle.Identifier != targetCSV { - continue - } + approvedCSVs.Insert(targetCSV) - if packageName != "" && !strings.HasPrefix(bundle.Identifier, packageName+".v") { - continue - } + // stores the mapping of package names to unapproved CSV identifiers + packageToCSV := make(map[string]string) + // stores the required package names for each unapproved CSV + packageDependencies := make(map[string]sets.Set[string]) + for i, bundle := range installPlan.Status.BundleLookups { if bundle.Properties == "" { - break + continue } - props := map[string]interface{}{} + props := map[string]any{} err := json.Unmarshal([]byte(bundle.Properties), &props) if err != nil { @@ -2304,37 +2291,86 @@ func getBundleDependencies( break } - propsList, ok := props["properties"].([]interface{}) + propsList, ok := props["properties"].([]any) if !ok { - break + continue } for _, prop := range propsList { - propMap, ok := prop.(map[string]interface{}) + propMap, ok := prop.(map[string]any) + if !ok { + continue + } + + propType, ok := propMap["type"].(string) if !ok { continue } - if propMap["type"] != "olm.package.required" { + if propType != "olm.package" && propType != "olm.package.required" { continue } - propValue, ok := propMap["value"].(map[string]interface{}) + propValue, ok := propMap["value"].(map[string]any) if !ok { continue } - depPackageName, ok := propValue["packageName"].(string) - if !ok || depPackageName == "" { + pkgName, ok := propValue["packageName"].(string) + if !ok || pkgName == "" { continue } - packageDependencies.Insert(depPackageName) - packageDependencies = packageDependencies.Union(getBundleDependencies(installPlan, "", depPackageName)) + switch propType { + case "olm.package": + packageToCSV[pkgName] = bundle.Identifier + case "olm.package.required": + if packageDependencies[bundle.Identifier] == nil { + packageDependencies[bundle.Identifier] = sets.Set[string]{} + } + + packageDependencies[bundle.Identifier].Insert(pkgName) + } + } + } + + approvedCSVs = approvedCSVs.Union(getDependencyCSVs(targetCSV, packageToCSV, packageDependencies)) + + return approvedCSVs +} + +// getDependencyCSVs recursively converts the package dependencies to their CSV identifiers +func getDependencyCSVs( + startingCSV string, packageToCSV map[string]string, packageDependencies map[string]sets.Set[string], +) sets.Set[string] { + dependencyCSVs := sets.Set[string]{} + + if startingCSV == "" || packageToCSV == nil || packageDependencies == nil { + return dependencyCSVs + } + + packages, ok := packageDependencies[startingCSV] + if !ok || packages == nil { + return dependencyCSVs + } + + for pkg := range packages { + if csv, ok := packageToCSV[pkg]; ok { + dependencyCSVs.Insert(csv) } } - return packageDependencies + if len(dependencyCSVs) == 0 { + // The startingCSV's dependencies are already satisfied + // by other installed CSVs. No additional CSVs need approval. + return dependencyCSVs + } + + for csv := range dependencyCSVs { + dependencyCSVs = dependencyCSVs.Union(getDependencyCSVs(csv, packageToCSV, packageDependencies)) + } + + return dependencyCSVs } func (r *OperatorPolicyReconciler) mustnothaveInstallPlan( diff --git a/controllers/operatorpolicy_controller_test.go b/controllers/operatorpolicy_controller_test.go index 2c6ff9bb..b717d83f 100644 --- a/controllers/operatorpolicy_controller_test.go +++ b/controllers/operatorpolicy_controller_test.go @@ -334,6 +334,118 @@ func TestGetApprovedCSVs(t *testing.T) { } } +func TestGetApprovedCSVsWithPackageNameLookup(t *testing.T) { + mtcOadpIPRaw, err := os.ReadFile("../test/resources/unit/mtc-oadp-installplan.yaml") + if err != nil { + t.Fatalf("Encountered an error when reading the mtc-oadp-installplan.yaml: %v", err) + } + + installPlan := &operatorv1alpha1.InstallPlan{} + + err = yaml.Unmarshal(mtcOadpIPRaw, installPlan) + if err != nil { + t.Fatalf("Encountered an error when unmarshaling the mtc-oadp-installplan.yaml: %v", err) + } + + policy := &policyv1beta1.OperatorPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mtc-operator", + }, + Spec: policyv1beta1.OperatorPolicySpec{ + RemediationAction: "enforce", + ComplianceType: "musthave", + Severity: "medium", + UpgradeApproval: "Automatic", + Versions: []string{"mtc-operator.v1.8.9"}, + }, + } + + subscription := &operatorv1alpha1.Subscription{ + Status: operatorv1alpha1.SubscriptionStatus{ + CurrentCSV: "mtc-operator.v1.8.9", + }, + } + + csvs := getApprovedCSVs(policy, subscription, installPlan) + + expectedCSVs := sets.Set[string]{} + expectedCSVs.Insert("mtc-operator.v1.8.9") + expectedCSVs.Insert("oadp-operator.v1.5.0") + + if !csvs.Equal(expectedCSVs) { + t.Fatalf( + "Expected both MTC and OADP CSVs to be approved. Got: %s, Expected: %s", + strings.Join(csvs.UnsortedList(), ", "), + strings.Join(expectedCSVs.UnsortedList(), ", "), + ) + } +} + +func TestGetDependencyCSVs(t *testing.T) { + happyPackageToCSV := map[string]string{ + "starting-operator": "starting-operator.v7.8.9", + "first-dependency-operator": "primary-operator.v1.1.1", + "second-dependency-operator": "secondary-operator.v2.2.2", + } + + happyPackageDependencies := map[string]sets.Set[string]{ + "starting-operator.v7.8.9": func() sets.Set[string] { + s := sets.New[string]() + s.Insert("first-dependency-operator") + + return s + }(), + "primary-operator.v1.1.1": func() sets.Set[string] { + s := sets.New[string]() + s.Insert("second-dependency-operator") + + return s + }(), + "secondary-operator.v2.2.2": sets.New[string](), + } + + packageDependenciesAlreadyInstalled := map[string]sets.Set[string]{ + "starting-operator.v7.8.9": func() sets.Set[string] { + s := sets.New[string]() + s.Insert("first-dependency-operator") + + return s + }(), + } + dependencyCSVsAlreadyInstalled := map[string]string{ + "irrelevant-uninstalled-operator": "irrelevant-uninstalled-operator.v1.2.3", + } + + expectedCSVs := sets.Set[string]{} + + dependencyCSVs := getDependencyCSVs("starting-operator.v7.8.9", nil, nil) + if !dependencyCSVs.Equal(expectedCSVs) { + t.Fatalf("Expected no dependency CSVs to be approved. Got: %s, Expected: %s", + strings.Join(dependencyCSVs.UnsortedList(), ", "), + strings.Join(expectedCSVs.UnsortedList(), ", ")) + } + + dependencyCSVs = getDependencyCSVs( + "starting-operator.v7.8.9", dependencyCSVsAlreadyInstalled, packageDependenciesAlreadyInstalled) + if !dependencyCSVs.Equal(expectedCSVs) { + t.Fatalf("Expected no dependency CSVs to be approved. Got: %s, Expected: %s", + strings.Join(dependencyCSVs.UnsortedList(), ", "), + strings.Join(expectedCSVs.UnsortedList(), ", ")) + } + + dependencyCSVs = getDependencyCSVs("starting-operator.v7.8.9", happyPackageToCSV, happyPackageDependencies) + + expectedCSVs.Insert("primary-operator.v1.1.1") + expectedCSVs.Insert("secondary-operator.v2.2.2") + + if !dependencyCSVs.Equal(expectedCSVs) { + t.Fatalf( + "Expected dependency CSVs to be approved. Got: %s, Expected: %s", + strings.Join(dependencyCSVs.UnsortedList(), ", "), + strings.Join(expectedCSVs.UnsortedList(), ", ")) + } +} + func TestCanonicalizeVersions(t *testing.T) { policy := &policyv1beta1.OperatorPolicy{ Spec: policyv1beta1.OperatorPolicySpec{ diff --git a/test/resources/unit/mtc-oadp-installplan.yaml b/test/resources/unit/mtc-oadp-installplan.yaml new file mode 100644 index 00000000..144bcf86 --- /dev/null +++ b/test/resources/unit/mtc-oadp-installplan.yaml @@ -0,0 +1,18 @@ +apiVersion: operators.coreos.com/v1alpha1 +kind: InstallPlan +metadata: + name: install-mtc-oadp + namespace: openshift-adp +spec: + approval: Manual + approved: false + clusterServiceVersionNames: + - mtc-operator.v1.8.9 + - oadp-operator.v1.5.0 + generation: 1 +status: + bundleLookups: + - identifier: oadp-operator.v1.5.0 + properties: '{"properties":[{"type":"olm.gvk","value":{"group":"oadp.openshift.io","kind":"CloudStorage","version":"v1alpha1"}},{"type":"olm.package","value":{"packageName":"redhat-oadp-operator","version":"1.5.0"}}]}' + - identifier: mtc-operator.v1.8.9 + properties: '{"properties":[{"type":"olm.gvk","value":{"group":"migration.openshift.io","kind":"DirectImageMigration","version":"v1alpha1"}},{"type":"olm.package","value":{"packageName":"mtc-operator","version":"1.8.9"}},{"type":"olm.package.required","value":{"packageName":"redhat-oadp-operator","versionRange":">=1.2.3"}}]}'