update k8s 1.25 validation logic

pkg/validation/internal/removed_apis.go

@@ -3,12 +3,15 @@ package internal
 import (
 	"fmt"
 	"sort"
+	"strings"
 
 	"github.com/blang/semver/v4"
 	"github.com/operator-framework/api/pkg/manifests"
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/api/pkg/validation/errors"
 	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 // k8sVersionKey defines the key which can be used by its consumers
@@ -148,11 +151,12 @@ func checkRemovedAPIsForVersion(
 	errs []error, warns []error) ([]error, []error) {
 
 	found := map[string][]string{}
+	warnsFound := map[string][]string{}
 	switch k8sVersionToCheck.String() {
 	case "1.22.0":
 		found = getRemovedAPIsOn1_22From(bundle)
 	case "1.25.0":
-		found = getRemovedAPIsOn1_25From(bundle)
+		found, warnsFound = getRemovedAPIsOn1_25From(bundle)
 	case "1.26.0":
 		found = getRemovedAPIsOn1_26From(bundle)
 	default:
@@ -173,6 +177,16 @@ func checkRemovedAPIsForVersion(
 			warns = append(warns, msg)
 		}
 	}
+
+	if len(warnsFound) > 0 {
+		deprecatedAPIsMessage := generateMessageWithDeprecatedAPIs(warnsFound)
+		msg := fmt.Errorf(DeprecateMessage,
+			k8sVersionToCheck.Major, k8sVersionToCheck.Minor,
+			k8sVersionToCheck.Major, k8sVersionToCheck.Minor,
+			deprecatedAPIsMessage)
+		warns = append(warns, msg)
+	}
+
 	return errs, warns
 }
 
@@ -288,40 +302,133 @@ func getRemovedAPIsOn1_22From(bundle *manifests.Bundle) map[string][]string {
 // add manifests on the bundle using these APIs. On top of that some Kinds such as the CronJob
 // are not currently a valid/supported by OLM and never would to be added to bundle.
 // See: https://github.com/operator-framework/operator-registry/blob/v1.19.5/pkg/lib/bundle/supported_resources.go#L3-L23
-func getRemovedAPIsOn1_25From(bundle *manifests.Bundle) map[string][]string {
+func getRemovedAPIsOn1_25From(bundle *manifests.Bundle) (map[string][]string, map[string][]string) {
 	deprecatedAPIs := make(map[string][]string)
+	warnDeprecatedAPIs := make(map[string][]string)
+
+	addIfDeprecated := func(u *unstructured.Unstructured) {
+		switch u.GetAPIVersion() {
+		case "batch/v1beta1":
+			if u.GetKind() == "CronJob" {
+				deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
+			}
+		case "discovery.k8s.io/v1beta1":
+			if u.GetKind() == "EndpointSlice" {
+				deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
+			}
+		case "events.k8s.io/v1beta1":
+			if u.GetKind() == "Event" {
+				deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
+			}
+		case "autoscaling/v2beta1":
+			if u.GetKind() == "HorizontalPodAutoscaler" {
+				deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
+			}
+		case "policy/v1beta1":
+			if u.GetKind() == "PodDisruptionBudget" || u.GetKind() == "PodSecurityPolicy" {
+				deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
+			}
+		case "node.k8s.io/v1beta1":
+			if u.GetKind() == "RuntimeClass" {
+				deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
+			}
+		}
+	}
+
+	warnIfDeprecated := func(res string, msg string) {
+		switch res {
+		case "cronjobs":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		case "endpointslices":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		case "events":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		case "horizontalpodautoscalers":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		case "poddisruptionbudgets":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		case "podsecuritypolicies":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		case "runtimeclasses":
+			warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
+		}
+	}
+
 	for _, obj := range bundle.Objects {
 		switch u := obj.GetObjectKind().(type) {
 		case *unstructured.Unstructured:
 			switch u.GetAPIVersion() {
-			case "batch/v1beta1":
-				if u.GetKind() == "CronJob" {
-					deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
-				}
-			case "discovery.k8s.io/v1beta1":
-				if u.GetKind() == "EndpointSlice" {
-					deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
-				}
-			case "events.k8s.io/v1beta1":
-				if u.GetKind() == "Event" {
-					deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
-				}
-			case "autoscaling/v2beta1":
-				if u.GetKind() == "HorizontalPodAutoscaler" {
-					deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
-				}
-			case "policy/v1beta1":
-				if u.GetKind() == "PodDisruptionBudget" || u.GetKind() == "PodSecurityPolicy" {
-					deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
-				}
-			case "node.k8s.io/v1beta1":
-				if u.GetKind() == "RuntimeClass" {
-					deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
+			case "operators.coreos.com/v1alpha1":
+				// Check a couple CSV fields for references to deprecated APIs
+				if u.GetKind() == "ClusterServiceVersion" {
+					resInCsvCrds := make(map[string]struct{})
+					csv := &v1alpha1.ClusterServiceVersion{}
+					err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, csv)
+					if err != nil {
+						fmt.Println("failed to convert unstructured.Unstructed to v1alpha1.ClusterServiceVersion:", err)
+					}
+
+					// Loop through all the CRDDescriptions to see
+					// if there is any with an API Version & Kind that is deprecated
+					crdCheck := func(crdsField string, crdDescriptions []v1alpha1.CRDDescription) {
+						for i, desc := range crdDescriptions {
+							for j, res := range desc.Resources {
+								resFromKind := fmt.Sprintf("%ss", strings.ToLower(res.Kind))
+								fmt.Println("XXX resFromKind:", resFromKind)
+								resInCsvCrds[resFromKind] = struct{}{}
+								unstruct := &unstructured.Unstructured{
+									Object: map[string]interface{}{
+										"apiVersion": res.Version,
+										"kind":       res.Kind,
+										"metadata": map[string]interface{}{
+											"name": fmt.Sprintf("ClusterServiceVersion.Spec.CustomResourceDefinitions.%s[%d].Resource[%d]", crdsField, i, j),
+										},
+									},
+								}
+								addIfDeprecated(unstruct)
+							}
+						}
+					}
+
+					// Check the Owned Resources
+					crdCheck("Owned", csv.Spec.CustomResourceDefinitions.Owned)
+
+					// Check the Required Resources
+					crdCheck("Required", csv.Spec.CustomResourceDefinitions.Required)
+
+					fmt.Println("XXX resInCsvCrds:", resInCsvCrds)
+
+					// Loop through all the StrategyDeploymentPermissions to see
+					// if the rbacv1.PolicyRule that is defined specifies a resource that
+					// *may* have a deprecated API then add it to the warnings.
+					// Only present a warning if the resource was NOT found as a resource
+					// in the ClusterServiceVersion.Spec.CustomResourceDefinitions fields
+					permCheck := func(permField string, perms []v1alpha1.StrategyDeploymentPermissions) {
+						for i, perm := range perms {
+							for j, rule := range perm.Rules {
+								for _, res := range rule.Resources {
+									if _, ok := resInCsvCrds[res]; ok {
+										fmt.Println("XXX resource: ", res, "is in resInCsvCrds map!")
+										continue
+									}
+									warnIfDeprecated(res, fmt.Sprintf("ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.%s[%d].Rules[%d]", permField, i, j))
+								}
+							}
+						}
+					}
+
+					// Check the ClusterPermissions
+					permCheck("ClusterPermissions", csv.Spec.InstallStrategy.StrategySpec.ClusterPermissions)
+
+					// Check the Permissions
+					permCheck("Permissions", csv.Spec.InstallStrategy.StrategySpec.Permissions)
 				}
+			default:
+				addIfDeprecated(u)
 			}
 		}
 	}
-	return deprecatedAPIs
+	return deprecatedAPIs, warnDeprecatedAPIs
 }
 
 // getRemovedAPIsOn1_26From return the list of resources which were deprecated

pkg/validation/internal/removed_apis_test.go

@@ -71,27 +71,34 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
 	mock["HorizontalPodAutoscaler"] = []string{"memcached-operator-hpa"}
 	mock["PodDisruptionBudget"] = []string{"memcached-operator-policy-manager"}
 
+	warnMock := make(map[string][]string)
+	warnMock["cronjobs"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]"}
+	warnMock["events"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]"}
+
 	type args struct {
 		bundleDir string
 	}
 	tests := []struct {
-		name string
-		args args
-		want map[string][]string
+		name     string
+		args     args
+		errWant  map[string][]string
+		warnWant map[string][]string
 	}{
 		{
 			name: "should return an empty map when no deprecated apis are found",
 			args: args{
 				bundleDir: "./testdata/valid_bundle_v1",
 			},
-			want: map[string][]string{},
+			errWant:  map[string][]string{},
+			warnWant: map[string][]string{},
 		},
 		{
 			name: "should fail return the removed APIs in 1.25",
 			args: args{
 				bundleDir: "./testdata/removed_api_1_25",
 			},
-			want: mock,
+			errWant:  mock,
+			warnWant: warnMock,
 		},
 	}
 	for _, tt := range tests {
@@ -101,8 +108,14 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
 			bundle, err := manifests.GetBundleFromDir(tt.args.bundleDir)
 			require.NoError(t, err)
 
-			if got := getRemovedAPIsOn1_25From(bundle); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("getRemovedAPIsOn1_25From() = %v, want %v", got, tt.want)
+			errGot, warnGot := getRemovedAPIsOn1_25From(bundle)
+
+			if !reflect.DeepEqual(errGot, tt.errWant) {
+				t.Errorf("getRemovedAPIsOn1_25From() = %v, want %v", errGot, tt.errWant)
+			}
+
+			if !reflect.DeepEqual(warnGot, tt.warnWant) {
+				t.Errorf("getRemovedAPIsOn1_25From() = %v, want %v", warnGot, tt.warnWant)
 			}
 		})
 	}
@@ -174,7 +187,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
 				"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
-				"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
+				"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
+				"this bundle is using APIs which were deprecated and removed in v1.25. " +
+					"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+					"Migrate the API(s) for events: " +
+					"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
 		},
 		{
 			name: "should return an error when the k8sVersion is >= 1.22 and has the deprecated API",
@@ -188,6 +205,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
 				"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\"" +
 				" \"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
+			wantWarning: true,
+			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
+				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+				"Migrate the API(s) for events: " +
+				"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
 		},
 		{
 			name: "should return an error when the k8sVersion is >= 1.25 and found removed APIs on 1.25",
@@ -201,6 +223,12 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
 				"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
 				"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
+			wantWarning: true,
+			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
+				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+				"Migrate the API(s) for cronjobs: " +
+				"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]\"])" +
+				",events: ([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"]),"},
 		},
 		{
 			name: "should return a warning if the k8sVersion is empty and found removed APIs on 1.25",
@@ -213,7 +241,12 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
 				"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
-				"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
+				"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),",
+				"this bundle is using APIs which were deprecated and removed in v1.25. " +
+					"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+					"Migrate the API(s) for cronjobs: " +
+					"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]\"])" +
+					",events: ([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"]),"},
 		},
 		{
 			name: "should return an error when the k8sVersion is >= 1.26 and found removed APIs on 1.26",
@@ -226,6 +259,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 			errStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
 				"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
+			wantWarning: true,
+			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
+				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+				"Migrate the API(s) for events: " +
+				"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"])"},
 		},
 		{
 			name: "should return a warning when the k8sVersion is empty and found removed APIs on 1.26",
@@ -235,9 +273,13 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 				directory:      "./testdata/removed_api_1_26",
 			},
 			wantWarning: true,
-			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
-				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
-				"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
+			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
+				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+				"Migrate the API(s) for events: " +
+				"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"])",
+				"this bundle is using APIs which were deprecated and removed in v1.26. " +
+					"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
+					"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
 		},
 		{
 			name: "should return an error when the k8sVersion informed is invalid",
@@ -251,8 +293,12 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 			wantWarning: true,
 			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
-				"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\"" +
-				" \"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
+				"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
+				"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
+				"this bundle is using APIs which were deprecated and removed in v1.25. " +
+					"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+					"Migrate the API(s) for events: " +
+					"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
 		},
 		{
 			name: "should return an error when the csv.spec.minKubeVersion informed is invalid",
@@ -267,7 +313,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
 			warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
 				"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
 				"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
-				"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
+				"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
+				"this bundle is using APIs which were deprecated and removed in v1.25. " +
+					"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
+					"Migrate the API(s) for events: " +
+					"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
 		},
 	}
 	for _, tt := range tests {

pkg/validation/internal/testdata/removed_api_1_25/memcached-operator.clusterserviceversion.yaml

@@ -94,6 +94,12 @@ spec:
           - subjectaccessreviews
           verbs:
           - create
+        - apiGroups:
+          - batch
+          resources:
+          - cronjobs
+          verbs:
+          - get
         serviceAccountName: memcached-operator-controller-manager
       deployments:
       - name: memcached-operator-controller-manager

pkg/validation/internal/testdata/valid_bundle_v1/memcached-operator.clusterserviceversion.yaml

@@ -176,13 +176,6 @@ spec:
           - update
           - patch
           - delete
-        - apiGroups:
-          - ""
-          resources:
-          - events
-          verbs:
-          - create
-          - patch
         serviceAccountName: memcached-operator-controller-manager
     strategy: deployment
   installModes: