Skip to content

Commit

Permalink
update k8s 1.25 validation logic (#270)
Browse files Browse the repository at this point in the history 
* update k8s 1.25 validation logic

to look for deprecated k8s APIs in various CSV fields

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

* update validation logic

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

* remove debug statements

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
  • Loading branch information
@everettraven
everettraven committed Oct 28, 2022
1 parent b527a19 commit b611f6c
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 48 deletions.
155 changes: 129 additions & 26 deletions pkg/validation/internal/removed_apis.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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
}

Expand Down Expand Up @@ -288,40 +302,129 @@ 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))
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)

// 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 {
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
Expand Down
80 changes: 65 additions & 15 deletions pkg/validation/internal/removed_apis_test.go
View file
Expand Up @@ -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 {
Expand All @@ -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)
}
})
}
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions ...lidation/internal/testdata/removed_api_1_25/memcached-operator.clusterserviceversion.yaml
View file
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions ...alidation/internal/testdata/valid_bundle_v1/memcached-operator.clusterserviceversion.yaml
View file
Expand Up @@ -176,13 +176,6 @@ spec:
- update
- patch
- delete
- apiGroups:
- ""
resources:
- events
verbs:
- create
- patch
serviceAccountName: memcached-operator-controller-manager
strategy: deployment
installModes:
Expand Down

0 comments on commit b611f6c

Please sign in to comment.