Skip to content

Commit f1b7296

Browse files
everettraveneverettraven
authored
(bugfix): make k8s 1.25 validation logic check api group before issuing a warning (#274)
* fix a bug in k8s 1.25 validation logic to now check the apiGroup/resource to determine if an api is deprecated. Signed-off-by: Bryce Palmer <[email protected]> * update warning and error checks to use a map Signed-off-by: Bryce Palmer <[email protected]> Signed-off-by: Bryce Palmer <[email protected]>
1 parent 9fe16de commit f1b7296

7 files changed

+432
-95
lines changed

‎pkg/validation/internal/removed_apis.go

+32-45
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
1313
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1414
"k8s.io/apimachinery/pkg/runtime"
15+
"k8s.io/apimachinery/pkg/runtime/schema"
1516
)
1617

1718
// k8sVersionKey defines the key which can be used by its consumers
@@ -306,51 +307,35 @@ func getRemovedAPIsOn1_25From(bundle *manifests.Bundle) (map[string][]string, ma
306307
deprecatedAPIs := make(map[string][]string)
307308
warnDeprecatedAPIs := make(map[string][]string)
308309

310+
deprecatedGvk := map[schema.GroupVersionKind]struct{}{
311+
{Group: "batch", Version: "v1beta1", Kind: "CronJob"}: {},
312+
{Group: "discovery.k8s.io", Version: "v1beta1", Kind: "EndpointSlice"}: {},
313+
{Group: "events.k8s.io", Version: "v1beta1", Kind: "Event"}: {},
314+
{Group: "autoscaling", Version: "v2beta1", Kind: "HorizontalPodAutoscaler"}: {},
315+
{Group: "policy", Version: "v1beta1", Kind: "PodDisruptionBudget"}: {},
316+
{Group: "policy", Version: "v1beta1", Kind: "PodSecurityPolicy"}: {},
317+
{Group: "node.k8s.io", Version: "v1beta1", Kind: "RuntimeClass"}: {},
318+
}
319+
309320
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-
}
321+
if _, ok := deprecatedGvk[u.GetObjectKind().GroupVersionKind()]; ok {
322+
deprecatedAPIs[u.GetKind()] = append(deprecatedAPIs[u.GetKind()], u.GetName())
335323
}
336324
}
337325

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)
326+
deprecatedGroupResource := map[schema.GroupResource]struct{}{
327+
{Group: "batch", Resource: "cronjobs"}: {},
328+
{Group: "discovery.k8s.io", Resource: "endpointslices"}: {},
329+
{Group: "events.k8s.io", Resource: "events"}: {},
330+
{Group: "autoscaling", Resource: "horizontalpodautoscalers"}: {},
331+
{Group: "policy", Resource: "poddisruptionbudgets"}: {},
332+
{Group: "policy", Resource: "podsecuritypolicies"}: {},
333+
{Group: "node.k8s.io", Resource: "runtimeclasses"}: {},
334+
}
335+
336+
warnIfDeprecated := func(gr schema.GroupResource, msg string) {
337+
if _, ok := deprecatedGroupResource[gr]; ok {
338+
warnDeprecatedAPIs[gr.Resource] = append(warnDeprecatedAPIs[gr.Resource], msg)
354339
}
355340
}
356341

@@ -403,11 +388,13 @@ func getRemovedAPIsOn1_25From(bundle *manifests.Bundle) (map[string][]string, ma
403388
permCheck := func(permField string, perms []v1alpha1.StrategyDeploymentPermissions) {
404389
for i, perm := range perms {
405390
for j, rule := range perm.Rules {
406-
for _, res := range rule.Resources {
407-
if _, ok := resInCsvCrds[res]; ok {
408-
continue
391+
for _, apiGroup := range rule.APIGroups {
392+
for _, res := range rule.Resources {
393+
if _, ok := resInCsvCrds[res]; ok {
394+
continue
395+
}
396+
warnIfDeprecated(schema.GroupResource{Group: apiGroup, Resource: res}, fmt.Sprintf("ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.%s[%d].Rules[%d]", permField, i, j))
409397
}
410-
warnIfDeprecated(res, fmt.Sprintf("ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.%s[%d].Rules[%d]", permField, i, j))
411398
}
412399
}
413400
}

‎pkg/validation/internal/removed_apis_test.go

+20-44
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
7373

7474
warnMock := make(map[string][]string)
7575
warnMock["cronjobs"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]"}
76+
warnMock["endpointslices"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[3]"}
7677
warnMock["events"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]"}
78+
warnMock["horizontalpodautoscalers"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[4]"}
79+
warnMock["poddisruptionbudgets"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[5]"}
80+
warnMock["podsecuritypolicies"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[5]"}
81+
warnMock["runtimeclasses"] = []string{"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[6]"}
7782

7883
type args struct {
7984
bundleDir string
@@ -98,6 +103,14 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
98103
bundleDir: "./testdata/removed_api_1_25",
99104
},
100105
errWant: mock,
106+
warnWant: map[string][]string{},
107+
},
108+
{
109+
name: "should return warnings with all deprecated APIs in 1.25",
110+
args: args{
111+
bundleDir: "./testdata/deprecated_api_1_25",
112+
},
113+
errWant: mock,
101114
warnWant: warnMock,
102115
},
103116
}
@@ -187,11 +200,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
187200
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
188201
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
189202
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
190-
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
191-
"this bundle is using APIs which were deprecated and removed in v1.25. " +
192-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
193-
"Migrate the API(s) for events: " +
194-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
203+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
195204
},
196205
{
197206
name: "should return an error when the k8sVersion is >= 1.22 and has the deprecated API",
@@ -205,11 +214,6 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
205214
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
206215
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\"" +
207216
" \"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
208-
wantWarning: true,
209-
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
210-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
211-
"Migrate the API(s) for events: " +
212-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
213217
},
214218
{
215219
name: "should return an error when the k8sVersion is >= 1.25 and found removed APIs on 1.25",
@@ -223,12 +227,6 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
223227
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
224228
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
225229
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
226-
wantWarning: true,
227-
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
228-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
229-
"Migrate the API(s) for cronjobs: " +
230-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]\"])" +
231-
",events: ([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"]),"},
232230
},
233231
{
234232
name: "should return a warning if the k8sVersion is empty and found removed APIs on 1.25",
@@ -241,12 +239,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
241239
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
242240
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
243241
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
244-
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),",
245-
"this bundle is using APIs which were deprecated and removed in v1.25. " +
246-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
247-
"Migrate the API(s) for cronjobs: " +
248-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.ClusterPermissions[0].Rules[7]\"])" +
249-
",events: ([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"]),"},
242+
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
250243
},
251244
{
252245
name: "should return an error when the k8sVersion is >= 1.26 and found removed APIs on 1.26",
@@ -259,11 +252,6 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
259252
errStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
260253
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
261254
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
262-
wantWarning: true,
263-
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
264-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
265-
"Migrate the API(s) for events: " +
266-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"])"},
267255
},
268256
{
269257
name: "should return a warning when the k8sVersion is empty and found removed APIs on 1.26",
@@ -273,13 +261,9 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
273261
directory: "./testdata/removed_api_1_26",
274262
},
275263
wantWarning: true,
276-
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
277-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
278-
"Migrate the API(s) for events: " +
279-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[2]\"])",
280-
"this bundle is using APIs which were deprecated and removed in v1.26. " +
281-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
282-
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
264+
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
265+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
266+
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
283267
},
284268
{
285269
name: "should return an error when the k8sVersion informed is invalid",
@@ -294,11 +278,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
294278
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
295279
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
296280
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
297-
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
298-
"this bundle is using APIs which were deprecated and removed in v1.25. " +
299-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
300-
"Migrate the API(s) for events: " +
301-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
281+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
302282
},
303283
{
304284
name: "should return an error when the csv.spec.minKubeVersion informed is invalid",
@@ -313,11 +293,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
313293
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
314294
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
315295
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\" " +
316-
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])",
317-
"this bundle is using APIs which were deprecated and removed in v1.25. " +
318-
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
319-
"Migrate the API(s) for events: " +
320-
"([\"ClusterServiceVersion.Spec.InstallStrategy.StrategySpec.Permissions[0].Rules[1]\"])"},
296+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
321297
},
322298
}
323299
for _, tt := range tests {

‎pkg/validation/internal/testdata/deprecated_api_1_25/cache.example.com_memcacheds.yaml

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
apiVersion: apiextensions.k8s.io/v1
2+
kind: CustomResourceDefinition
3+
metadata:
4+
annotations:
5+
controller-gen.kubebuilder.io/version: v0.4.1
6+
creationTimestamp: null
7+
name: memcacheds.cache.example.com
8+
spec:
9+
group: cache.example.com
10+
names:
11+
kind: Memcached
12+
listKind: MemcachedList
13+
plural: memcacheds
14+
singular: memcached
15+
scope: Namespaced
16+
versions:
17+
- name: v1alpha1
18+
schema:
19+
openAPIV3Schema:
20+
description: Memcached is the Schema for the memcacheds API
21+
properties:
22+
apiVersion:
23+
description: 'APIVersion defines the versioned schema of this representation
24+
of an object. Servers should convert recognized schemas to the latest
25+
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
26+
type: string
27+
kind:
28+
description: 'Kind is a string value representing the REST resource this
29+
object represents. Servers may infer this from the endpoint the client
30+
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
31+
type: string
32+
metadata:
33+
type: object
34+
spec:
35+
description: MemcachedSpec defines the desired state of Memcached
36+
properties:
37+
foo:
38+
description: Foo is an example field of Memcached. Edit memcached_types.go
39+
to remove/update
40+
type: string
41+
size:
42+
description: Size defines the number of Memcached instances
43+
format: int32
44+
type: integer
45+
type: object
46+
status:
47+
description: MemcachedStatus defines the observed state of Memcached
48+
properties:
49+
nodes:
50+
description: Nodes store the name of the pods which are running Memcached
51+
instances
52+
items:
53+
type: string
54+
type: array
55+
type: object
56+
type: object
57+
served: true
58+
storage: true
59+
subresources:
60+
status: {}
61+
status:
62+
acceptedNames:
63+
kind: ""
64+
plural: ""
65+
conditions: []
66+
storedVersions: []

‎pkg/validation/internal/testdata/deprecated_api_1_25/horizontal-pod.yaml

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
apiVersion: autoscaling/v2beta1
2+
kind: HorizontalPodAutoscaler
3+
metadata:
4+
name: memcached-operator-hpa
5+
spec:
6+
scaleTargetRef:
7+
apiVersion: apps/v1
8+
kind: Deployment
9+
name: memcached-operator-controller-manager
10+
minReplicas: 1
11+
maxReplicas: 10
12+
metrics:
13+
- type: Resource
14+
resource:
15+
name: cpu
16+
target:
17+
type: Utilization
18+
averageUtilization: 50

0 commit comments

Comments
 (0)