Skip to content

Commit 3d1cff2

Browse files
committed
Handle CSV approvals with unconventional package names
ref: https://issues.redhat.com/browse/ACM-20500 When approving multiple CSVs in an install plan, lookup the required package for each CSV. Removed the original logic assuming the CSV name matched the format of <package name>.v<version> Signed-off-by: Janelle Law <[email protected]>
1 parent d99383c commit 3d1cff2

File tree

3 files changed

+113
-4
lines changed

3 files changed

+113
-4
lines changed

controllers/operatorpolicy_controller.go

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,12 +2252,12 @@ func getApprovedCSVs(
22522252
approvedCSVs := sets.Set[string]{}
22532253
approvedCSVs.Insert(subscriptionCSV)
22542254

2255+
packageToCSV := getPackageToCSVMapping(installPlan)
2256+
22552257
// Approve all CSVs that are dependencies of the subscription CSV.
22562258
for _, packageDependency := range packageDependencies.UnsortedList() {
2257-
for _, csvName := range installPlan.Spec.ClusterServiceVersionNames {
2258-
if strings.HasPrefix(csvName, packageDependency+".v") {
2259-
approvedCSVs.Insert(csvName)
2260-
}
2259+
if csvName, ok := packageToCSV[packageDependency]; ok {
2260+
approvedCSVs.Insert(csvName)
22612261
}
22622262
}
22632263

@@ -2337,6 +2337,50 @@ func getBundleDependencies(
23372337
return packageDependencies
23382338
}
23392339

2340+
// getPackageToCSVMapping creates a map from olm package names to their CSV identifiers using bundle lookups
2341+
func getPackageToCSVMapping(installPlan *operatorv1alpha1.InstallPlan) map[string]string {
2342+
packageToCSV := make(map[string]string, len(installPlan.Status.BundleLookups))
2343+
2344+
for _, bundle := range installPlan.Status.BundleLookups {
2345+
props := map[string]interface{}{}
2346+
2347+
err := json.Unmarshal([]byte(bundle.Properties), &props)
2348+
if err != nil {
2349+
continue
2350+
}
2351+
2352+
propsList, ok := props["properties"].([]interface{})
2353+
if !ok {
2354+
continue
2355+
}
2356+
2357+
for _, prop := range propsList {
2358+
propMap, ok := prop.(map[string]interface{})
2359+
if !ok {
2360+
continue
2361+
}
2362+
2363+
if propMap["type"] != "olm.package" {
2364+
continue
2365+
}
2366+
2367+
propValue, ok := propMap["value"].(map[string]interface{})
2368+
if !ok {
2369+
continue
2370+
}
2371+
2372+
packageName, ok := propValue["packageName"].(string)
2373+
if !ok || packageName == "" {
2374+
continue
2375+
}
2376+
2377+
packageToCSV[packageName] = bundle.Identifier
2378+
}
2379+
}
2380+
2381+
return packageToCSV
2382+
}
2383+
23402384
func (r *OperatorPolicyReconciler) mustnothaveInstallPlan(
23412385
policy *policyv1beta1.OperatorPolicy,
23422386
latestInstallPlan *unstructured.Unstructured,

controllers/operatorpolicy_controller_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,53 @@ func TestGetApprovedCSVs(t *testing.T) {
334334
}
335335
}
336336

337+
func TestGetApprovedCSVsBundleLookup(t *testing.T) {
338+
mtcOadpIPRaw, err := os.ReadFile("../test/resources/unit/mtc-oadp-installplan.yaml")
339+
if err != nil {
340+
t.Fatalf("Encountered an error when reading the mtc-oadp-installplan.yaml: %v", err)
341+
}
342+
343+
installPlan := &operatorv1alpha1.InstallPlan{}
344+
345+
err = yaml.Unmarshal(mtcOadpIPRaw, installPlan)
346+
if err != nil {
347+
t.Fatalf("Encountered an error when unmarshaling the mtc-oadp-installplan.yaml: %v", err)
348+
}
349+
350+
policy := &policyv1beta1.OperatorPolicy{
351+
ObjectMeta: metav1.ObjectMeta{
352+
Name: "mtc-operator",
353+
},
354+
Spec: policyv1beta1.OperatorPolicySpec{
355+
RemediationAction: "enforce",
356+
ComplianceType: "musthave",
357+
Severity: "medium",
358+
UpgradeApproval: "Automatic",
359+
Versions: []string{"mtc-operator.v1.8.9"},
360+
},
361+
}
362+
363+
subscription := &operatorv1alpha1.Subscription{
364+
Status: operatorv1alpha1.SubscriptionStatus{
365+
CurrentCSV: "mtc-operator.v1.8.9",
366+
},
367+
}
368+
369+
csvs := getApprovedCSVs(policy, subscription, installPlan)
370+
371+
expectedCSVs := sets.Set[string]{}
372+
expectedCSVs.Insert("mtc-operator.v1.8.9")
373+
expectedCSVs.Insert("oadp-operator.v1.5.0")
374+
375+
if !csvs.Equal(expectedCSVs) {
376+
t.Fatalf(
377+
"Expected both MTC and OADP CSVs to be approved. Got: %s, Expected: %s",
378+
strings.Join(csvs.UnsortedList(), ", "),
379+
strings.Join(expectedCSVs.UnsortedList(), ", "),
380+
)
381+
}
382+
}
383+
337384
func TestCanonicalizeVersions(t *testing.T) {
338385
policy := &policyv1beta1.OperatorPolicy{
339386
Spec: policyv1beta1.OperatorPolicySpec{
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
apiVersion: operators.coreos.com/v1alpha1
2+
kind: InstallPlan
3+
metadata:
4+
name: install-mtc-oadp
5+
namespace: openshift-adp
6+
spec:
7+
approval: Manual
8+
approved: false
9+
clusterServiceVersionNames:
10+
- mtc-operator.v1.8.9
11+
- oadp-operator.v1.5.0
12+
generation: 1
13+
status:
14+
bundleLookups:
15+
- identifier: oadp-operator.v1.5.0
16+
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"}}]}'
17+
- identifier: mtc-operator.v1.8.9
18+
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"}}]}'

0 commit comments

Comments
 (0)