Skip to content

Commit 8bb1991

Browse files
everettraventmshort
everettraven
authored and
tmshort
committed
(bugfix): make k8s 1.25 validation logic check api group before issuing a warning (openshift#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]> Upstream-repository: api Upstream-commit: f1b729684854a053f229464eb327527222188fd1
1 parent c15e987 commit 8bb1991

File tree

9 files changed

+466
-142
lines changed

9 files changed

+466
-142
lines changed

Diff for: ‎pkg/manifests/csv.yaml

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ metadata:
55
name: packageserver
66
namespace: openshift-operator-lifecycle-manager
77
labels:
8-
olm.version: 0.0.0-d7f3cde7050e452011a0aa095c81a6fc92bff114
8+
olm.version: 0.0.0-ca4246370d0c9e9f6fdcf042be05ec3c4c8b06f0
99
olm.clusteroperator.name: operator-lifecycle-manager-packageserver
1010
annotations:
1111
include.release.openshift.io/self-managed-high-availability: "true"
@@ -159,7 +159,7 @@ spec:
159159
- packageserver
160160
topologyKey: "kubernetes.io/hostname"
161161
maturity: alpha
162-
version: 0.0.0-d7f3cde7050e452011a0aa095c81a6fc92bff114
162+
version: 0.0.0-ca4246370d0c9e9f6fdcf042be05ec3c4c8b06f0
163163
apiservicedefinitions:
164164
owned:
165165
- group: packages.operators.coreos.com

Diff for: ‎staging/api/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
}

Diff for: ‎staging/api/pkg/validation/internal/removed_apis_test.go

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

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

7984
type args struct {
8085
bundleDir string
@@ -99,6 +104,14 @@ func Test_GetRemovedAPIsOn1_25From(t *testing.T) {
99104
bundleDir: "./testdata/removed_api_1_25",
100105
},
101106
errWant: mock,
107+
warnWant: map[string][]string{},
108+
},
109+
{
110+
name: "should return warnings with all deprecated APIs in 1.25",
111+
args: args{
112+
bundleDir: "./testdata/deprecated_api_1_25",
113+
},
114+
errWant: mock,
102115
warnWant: warnMock,
103116
},
104117
}
@@ -188,11 +201,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
188201
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
189202
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
190203
"Migrate the API(s) for CRD: ([\"etcdbackups.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]\"])"},
204+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
196205
},
197206
{
198207
name: "should return an error when the k8sVersion is >= 1.22 and has the deprecated API",
@@ -206,11 +215,6 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
206215
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
207216
"Migrate the API(s) for CRD: ([\"etcdbackups.etcd.database.coreos.com\"" +
208217
" \"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]\"])"},
214218
},
215219
{
216220
name: "should return an error when the k8sVersion is >= 1.25 and found removed APIs on 1.25",
@@ -224,12 +228,6 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
224228
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
225229
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
226230
"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]\"]),"},
233231
},
234232
{
235233
name: "should return a warning if the k8sVersion is empty and found removed APIs on 1.25",
@@ -242,12 +240,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
242240
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.25. " +
243241
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-25. " +
244242
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])," +
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]\"]),"},
243+
"PodDisruptionBudget: ([\"memcached-operator-policy-manager\"]),"},
251244
},
252245
{
253246
name: "should return an error when the k8sVersion is >= 1.26 and found removed APIs on 1.26",
@@ -260,11 +253,6 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
260253
errStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
261254
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
262255
"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]\"])"},
268256
},
269257
{
270258
name: "should return a warning when the k8sVersion is empty and found removed APIs on 1.26",
@@ -274,13 +262,9 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
274262
directory: "./testdata/removed_api_1_26",
275263
},
276264
wantWarning: true,
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\"])"},
265+
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.26. " +
266+
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-26. " +
267+
"Migrate the API(s) for HorizontalPodAutoscaler: ([\"memcached-operator-hpa\"])"},
284268
},
285269
{
286270
name: "should return an error when the k8sVersion informed is invalid",
@@ -295,11 +279,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
295279
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
296280
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
297281
"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]\"])"},
282+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
303283
},
304284
{
305285
name: "should return an error when the csv.spec.minKubeVersion informed is invalid",
@@ -314,11 +294,7 @@ func TestValidateDeprecatedAPIS(t *testing.T) {
314294
warnStrings: []string{"this bundle is using APIs which were deprecated and removed in v1.22. " +
315295
"More info: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. " +
316296
"Migrate the API(s) for CRD: ([\"etcdbackups.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]\"])"},
297+
"\"etcdclusters.etcd.database.coreos.com\" \"etcdrestores.etcd.database.coreos.com\"])"},
322298
},
323299
}
324300
for _, tt := range tests {

Diff for: ‎staging/api/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: []

Diff for: ‎staging/api/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)