Skip to content

Commit 75d261c

Browse files
everettraventmshort
everettraven
authored and
tmshort
committed
update k8s 1.25 validation logic (openshift#270)
* update k8s 1.25 validation logic to look for deprecated k8s APIs in various CSV fields Signed-off-by: Bryce Palmer <[email protected]> * update validation logic Signed-off-by: Bryce Palmer <[email protected]> * remove debug statements Signed-off-by: Bryce Palmer <[email protected]> Signed-off-by: Bryce Palmer <[email protected]> Upstream-repository: api Upstream-commit: b611f6cef49cb8c6d621145c4e31d8ddfd4c59f4
1 parent bdb279e commit 75d261c

File tree

5 files changed

+329
-74
lines changed

5 files changed

+329
-74
lines changed

‎staging/api/pkg/validation/internal/removed_apis.go

Lines changed: 129 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@ package internal
33
import (
44
"fmt"
55
"sort"
6+
"strings"
67

78
"github.com/blang/semver/v4"
89
"github.com/operator-framework/api/pkg/manifests"
10+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
911
"github.com/operator-framework/api/pkg/validation/errors"
1012
interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
1113
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
14+
"k8s.io/apimachinery/pkg/runtime"
1215
)
1316

1417
// k8sVersionKey defines the key which can be used by its consumers
@@ -148,11 +151,12 @@ func checkRemovedAPIsForVersion(
148151
errs []error, warns []error) ([]error, []error) {
149152

150153
found := map[string][]string{}
154+
warnsFound := map[string][]string{}
151155
switch k8sVersionToCheck.String() {
152156
case "1.22.0":
153157
found = getRemovedAPIsOn1_22From(bundle)
154158
case "1.25.0":
155-
found = getRemovedAPIsOn1_25From(bundle)
159+
found, warnsFound = getRemovedAPIsOn1_25From(bundle)
156160
case "1.26.0":
157161
found = getRemovedAPIsOn1_26From(bundle)
158162
default:
@@ -173,6 +177,16 @@ func checkRemovedAPIsForVersion(
173177
warns = append(warns, msg)
174178
}
175179
}
180+
181+
if len(warnsFound) > 0 {
182+
deprecatedAPIsMessage := generateMessageWithDeprecatedAPIs(warnsFound)
183+
msg := fmt.Errorf(DeprecateMessage,
184+
k8sVersionToCheck.Major, k8sVersionToCheck.Minor,
185+
k8sVersionToCheck.Major, k8sVersionToCheck.Minor,
186+
deprecatedAPIsMessage)
187+
warns = append(warns, msg)
188+
}
189+
176190
return errs, warns
177191
}
178192

@@ -288,40 +302,129 @@ func getRemovedAPIsOn1_22From(bundle *manifests.Bundle) map[string][]string {
288302
// add manifests on the bundle using these APIs. On top of that some Kinds such as the CronJob
289303
// are not currently a valid/supported by OLM and never would to be added to bundle.
290304
// See: https://github.com/operator-framework/operator-registry/blob/v1.19.5/pkg/lib/bundle/supported_resources.go#L3-L23
291-
func getRemovedAPIsOn1_25From(bundle *manifests.Bundle) map[string][]string {
305+
func getRemovedAPIsOn1_25From(bundle *manifests.Bundle) (map[string][]string, map[string][]string) {
292306
deprecatedAPIs := make(map[string][]string)
307+
warnDeprecatedAPIs := make(map[string][]string)
308+
309+
addIfDeprecated := func(u *unstructured.Unstructured) {
310+
switch u.GetAPIVersion() {
311+
case "batch/v1beta1":
312+
if u.GetKind() == "CronJob" {
313+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
314+
}
315+
case "discovery.k8s.io/v1beta1":
316+
if u.GetKind() == "EndpointSlice" {
317+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
318+
}
319+
case "events.k8s.io/v1beta1":
320+
if u.GetKind() == "Event" {
321+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
322+
}
323+
case "autoscaling/v2beta1":
324+
if u.GetKind() == "HorizontalPodAutoscaler" {
325+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
326+
}
327+
case "policy/v1beta1":
328+
if u.GetKind() == "PodDisruptionBudget" || u.GetKind() == "PodSecurityPolicy" {
329+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
330+
}
331+
case "node.k8s.io/v1beta1":
332+
if u.GetKind() == "RuntimeClass" {
333+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
334+
}
335+
}
336+
}
337+
338+
warnIfDeprecated := func(res string, msg string) {
339+
switch res {
340+
case "cronjobs":
341+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
342+
case "endpointslices":
343+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
344+
case "events":
345+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
346+
case "horizontalpodautoscalers":
347+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
348+
case "poddisruptionbudgets":
349+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
350+
case "podsecuritypolicies":
351+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
352+
case "runtimeclasses":
353+
warnDeprecatedAPIs[res] = append(warnDeprecatedAPIs[res], msg)
354+
}
355+
}
356+
293357
for _, obj := range bundle.Objects {
294358
switch u := obj.GetObjectKind().(type) {
295359
case *unstructured.Unstructured:
296360
switch u.GetAPIVersion() {
297-
case "batch/v1beta1":
298-
if u.GetKind() == "CronJob" {
299-
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
300-
}
301-
case "discovery.k8s.io/v1beta1":
302-
if u.GetKind() == "EndpointSlice" {
303-
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
304-
}
305-
case "events.k8s.io/v1beta1":
306-
if u.GetKind() == "Event" {
307-
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
308-
}
309-
case "autoscaling/v2beta1":
310-
if u.GetKind() == "HorizontalPodAutoscaler" {
311-
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
312-
}
313-
case "policy/v1beta1":
314-
if u.GetKind() == "PodDisruptionBudget" || u.GetKind() == "PodSecurityPolicy" {
315-
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
316-
}
317-
case "node.k8s.io/v1beta1":
318-
if u.GetKind() == "RuntimeClass" {
319-
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], obj.GetName())
361+
case "operators.coreos.com/v1alpha1":
362+
// Check a couple CSV fields for references to deprecated APIs
363+
if u.GetKind() == "ClusterServiceVersion" {
364+
resInCsvCrds := make(map[string]struct{})
365+
csv := &v1alpha1.ClusterServiceVersion{}
366+
err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, csv)
367+
if err != nil {
368+
fmt.Println("failed to convert unstructured.Unstructed to v1alpha1.ClusterServiceVersion:", err)
369+
}
370+
371+
// Loop through all the CRDDescriptions to see
372+
// if there is any with an API Version & Kind that is deprecated
373+
crdCheck := func(crdsField string, crdDescriptions []v1alpha1.CRDDescription) {
374+
for i, desc := range crdDescriptions {
375+
for j, res := range desc.Resources {
376+
resFromKind := fmt.Sprintf("%ss", strings.ToLower(res.Kind))
377+
resInCsvCrds[resFromKind] = struct{}{}
378+
unstruct := &unstructured.Unstructured{
379+
Object: map[string]interface{}{
380+
"apiVersion": res.Version,
381+
"kind": res.Kind,
382+
"metadata": map[string]interface{}{
383+
"name": fmt.Sprintf("ClusterServiceVersion.Spec.CustomResourceDefinitions.%s[%d].Resource[%d]", crdsField, i, j),
384+
},
385+
},
386+
}
387+
addIfDeprecated(unstruct)
388+
}
389+
}
390+
}
391+
392+
// Check the Owned Resources
393+
crdCheck("Owned", csv.Spec.CustomResourceDefinitions.Owned)
394+
395+
// Check the Required Resources
396+
crdCheck("Required", csv.Spec.CustomResourceDefinitions.Required)
397+
398+
// Loop through all the StrategyDeploymentPermissions to see
399+
// if the rbacv1.PolicyRule that is defined specifies a resource that
400+
// *may* have a deprecated API then add it to the warnings.
401+
// Only present a warning if the resource was NOT found as a resource
402+
// in the ClusterServiceVersion.Spec.CustomResourceDefinitions fields
403+
permCheck := func(permField string, perms []v1alpha1.StrategyDeploymentPermissions) {
404+
for i, perm := range perms {
405+
for j, rule := range perm.Rules {
406+
for _, res := range rule.Resources {
407+
if _, ok := resInCsvCrds[res]; ok {
408+
continue
409+
}
410+
warnIfDeprecated(res, fmt.Sprintf("ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.%s[%d].Rules[%d]", permField, i, j))
411+
}
412+
}
413+
}
414+
}
415+
416+
// Check the ClusterPermissions
417+
permCheck("ClusterPermissions", csv.Spec.InstallStrategy.StrategySpec.ClusterPermissions)
418+
419+
// Check the Permissions
420+
permCheck("Permissions", csv.Spec.InstallStrategy.StrategySpec.Permissions)
320421
}
422+
default:
423+
addIfDeprecated(u)
321424
}
322425
}
323426
}
324-
return deprecatedAPIs
427+
return deprecatedAPIs, warnDeprecatedAPIs
325428
}
326429

327430
// getRemovedAPIsOn1_26From return the list of resources which were deprecated

‎staging/api/pkg/validation/internal/removed_apis_test.go

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,27 +72,34 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
7272
mock["HorizontalPodAutoscaler"] = []string{"memcached-operator-hpa"}
7373
mock["PodDisruptionBudget"] = []string{"memcached-operator-policy-manager"}
7474

75+
warnMock := make(map[string][]string)
76+
warnMock["cronjobs"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]"}
77+
warnMock["events"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]"}
78+
7579
type args struct {
7680
bundleDir string
7781
}
7882
tests := []struct {
79-
name string
80-
args args
81-
want map[string][]string
83+
name string
84+
args args
85+
errWant map[string][]string
86+
warnWant map[string][]string
8287
}{
8388
{
8489
name: "should return an empty map when no deprecated apis are found",
8590
args: args{
8691
bundleDir: "./testdata/valid_bundle_v1",
8792
},
88-
want: map[string][]string{},
93+
errWant: map[string][]string{},
94+
warnWant: map[string][]string{},
8995
},
9096
{
9197
name: "should fail return the removed APIs in 1.25",
9298
args: args{
9399
bundleDir: "./testdata/removed_api_1_25",
94100
},
95-
want: mock,
101+
errWant: mock,
102+
warnWant: warnMock,
96103
},
97104
}
98105
for _, tt := range tests {
@@ -102,8 +109,14 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
102109
bundle, err := manifests.GetBundleFromDir(tt.args.bundleDir)
103110
require.NoError(t, err)
104111

105-
if got := getRemovedAPIsOn1_25From(bundle); !reflect.DeepEqual(got, tt.want) {
106-
t.Errorf("getRemovedAPIsOn1_25From() = %v, want %v", got, tt.want)
112+
errGot, warnGot := getRemovedAPIsOn1_25From(bundle)
113+
114+
if !reflect.DeepEqual(errGot, tt.errWant) {
115+
t.Errorf("getRemovedAPIsOn1_25From() = %v, want %v", errGot, tt.errWant)
116+
}
117+
118+
if !reflect.DeepEqual(warnGot, tt.warnWant) {
119+
t.Errorf("getRemovedAPIsOn1_25From() = %v, want %v", warnGot, tt.warnWant)
107120
}
108121
})
109122
}
@@ -175,7 +188,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
175188
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
176189
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
177190
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
178-
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
191+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
192+
"this bundle is using APIs which were deprecated and removed in v1.25. " +
193+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
194+
"Migrate the API(s) for events: " +
195+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
179196
},
180197
{
181198
name: "should return an error when the k8sVersion is >= 1.22 and has the deprecated API",
@@ -189,6 +206,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
189206
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
190207
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\"" +
191208
" \"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
209+
wantWarning: true,
210+
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
211+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
212+
"Migrate the API(s) for events: " +
213+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
192214
},
193215
{
194216
name: "should return an error when the k8sVersion is >= 1.25 and found removed APIs on 1.25",
@@ -202,6 +224,12 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
202224
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
203225
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
204226
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
227+
wantWarning: true,
228+
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
229+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
230+
"Migrate the API(s) for cronjobs: " +
231+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]\"])" +
232+
",events: ([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"]),"},
205233
},
206234
{
207235
name: "should return a warning if the k8sVersion is empty and found removed APIs on 1.25",
@@ -214,7 +242,12 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
214242
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
215243
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
216244
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
217-
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
245+
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),",
246+
"this bundle is using APIs which were deprecated and removed in v1.25. " +
247+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
248+
"Migrate the API(s) for cronjobs: " +
249+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]\"])" +
250+
",events: ([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"]),"},
218251
},
219252
{
220253
name: "should return an error when the k8sVersion is >= 1.26 and found removed APIs on 1.26",
@@ -227,6 +260,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
227260
errStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
228261
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
229262
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
263+
wantWarning: true,
264+
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
265+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
266+
"Migrate the API(s) for events: " +
267+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"])"},
230268
},
231269
{
232270
name: "should return a warning when the k8sVersion is empty and found removed APIs on 1.26",
@@ -236,9 +274,13 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
236274
directory: "./testdata/removed_api_1_26",
237275
},
238276
wantWarning: true,
239-
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
240-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
241-
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
277+
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
278+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
279+
"Migrate the API(s) for events: " +
280+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"])",
281+
"this bundle is using APIs which were deprecated and removed in v1.26. " +
282+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
283+
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
242284
},
243285
{
244286
name: "should return an error when the k8sVersion informed is invalid",
@@ -252,8 +294,12 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
252294
wantWarning: true,
253295
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
254296
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
255-
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\"" +
256-
" \"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
297+
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
298+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
299+
"this bundle is using APIs which were deprecated and removed in v1.25. " +
300+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
301+
"Migrate the API(s) for events: " +
302+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
257303
},
258304
{
259305
name: "should return an error when the csv.spec.minKubeVersion informed is invalid",
@@ -268,7 +314,11 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
268314
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
269315
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
270316
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
271-
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
317+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
318+
"this bundle is using APIs which were deprecated and removed in v1.25. " +
319+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
320+
"Migrate the API(s) for events: " +
321+
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
272322
},
273323
}
274324
for _, tt := range tests {

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ spec:
9494
- subjectaccessreviews
9595
verbs:
9696
- create
97+
- apiGroups:
98+
- batch
99+
resources:
100+
- cronjobs
101+
verbs:
102+
- get
97103
serviceAccountName: memcached-operator-controller-manager
98104
deployments:
99105
- name: memcached-operator-controller-manager

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,6 @@ spec:
176176
- update
177177
- patch
178178
- delete
179-
- apiGroups:
180-
- ""
181-
resources:
182-
- events
183-
verbs:
184-
- create
185-
- patch
186179
serviceAccountName: memcached-operator-controller-manager
187180
strategy: deployment
188181
installModes:

0 commit comments

Comments
 (0)