From 09fa21ab8791663d7e1665bd5702c50d9f509445 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 17 Aug 2023 09:32:57 -0400 Subject: [PATCH 1/2] Store validating admission policies and bindings as v1beta1 --- .../apis__admissionregistration.k8s.io__v1alpha1.json | 4 ++-- .../apis__admissionregistration.k8s.io__v1beta1.json | 4 ++-- pkg/kubeapiserver/default_storage_factory_builder.go | 4 ++-- test/integration/etcd/data.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/discovery/apis__admissionregistration.k8s.io__v1alpha1.json b/api/discovery/apis__admissionregistration.k8s.io__v1alpha1.json index d8730f0376b..5b3232fae3a 100644 --- a/api/discovery/apis__admissionregistration.k8s.io__v1alpha1.json +++ b/api/discovery/apis__admissionregistration.k8s.io__v1alpha1.json @@ -11,7 +11,7 @@ "name": "validatingadmissionpolicies", "namespaced": false, "singularName": "validatingadmissionpolicy", - "storageVersionHash": "Vd+hadMG3gs=", + "storageVersionHash": "P/h9c6yIbaY=", "verbs": [ "create", "delete", @@ -42,7 +42,7 @@ "name": "validatingadmissionpolicybindings", "namespaced": false, "singularName": "validatingadmissionpolicybinding", - "storageVersionHash": "Yc3M4GKADk4=", + "storageVersionHash": "XYju31JKYek=", "verbs": [ "create", "delete", diff --git a/api/discovery/apis__admissionregistration.k8s.io__v1beta1.json b/api/discovery/apis__admissionregistration.k8s.io__v1beta1.json index 228d2fbea29..05f2215c37a 100644 --- a/api/discovery/apis__admissionregistration.k8s.io__v1beta1.json +++ b/api/discovery/apis__admissionregistration.k8s.io__v1beta1.json @@ -11,7 +11,7 @@ "name": "validatingadmissionpolicies", "namespaced": false, "singularName": "validatingadmissionpolicy", - "storageVersionHash": "Vd+hadMG3gs=", + "storageVersionHash": "P/h9c6yIbaY=", "verbs": [ "create", "delete", @@ -42,7 +42,7 @@ "name": "validatingadmissionpolicybindings", "namespaced": false, "singularName": "validatingadmissionpolicybinding", - "storageVersionHash": "Yc3M4GKADk4=", + "storageVersionHash": "XYju31JKYek=", "verbs": [ "create", "delete", diff --git a/pkg/kubeapiserver/default_storage_factory_builder.go b/pkg/kubeapiserver/default_storage_factory_builder.go index f7700aacffb..f1946415d85 100644 --- a/pkg/kubeapiserver/default_storage_factory_builder.go +++ b/pkg/kubeapiserver/default_storage_factory_builder.go @@ -69,8 +69,8 @@ func NewStorageFactoryConfig() *StorageFactoryConfig { // // TODO (https://github.com/kubernetes/kubernetes/issues/108451): remove the override in 1.25. // apisstorage.Resource("csistoragecapacities").WithVersion("v1beta1"), - admissionregistration.Resource("validatingadmissionpolicies").WithVersion("v1alpha1"), - admissionregistration.Resource("validatingadmissionpolicybindings").WithVersion("v1alpha1"), + admissionregistration.Resource("validatingadmissionpolicies").WithVersion("v1beta1"), + admissionregistration.Resource("validatingadmissionpolicybindings").WithVersion("v1beta1"), networking.Resource("clustercidrs").WithVersion("v1alpha1"), networking.Resource("ipaddresses").WithVersion("v1alpha1"), certificates.Resource("clustertrustbundles").WithVersion("v1alpha1"), diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index 501a5ba961d..0e95c66de34 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -354,12 +354,10 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes gvr("admissionregistration.k8s.io", "v1beta1", "validatingadmissionpolicies"): { Stub: `{"metadata":{"name":"vap1b1","creationTimestamp":null},"spec":{"paramKind":{"apiVersion":"test.example.com/v1","kind":"Example"},"matchConstraints":{"resourceRules": [{"resourceNames": ["fakeName"], "apiGroups":["apps"],"apiVersions":["v1"],"operations":["CREATE", "UPDATE"], "resources":["deployments"]}]},"validations":[{"expression":"object.spec.replicas <= params.maxReplicas","message":"Too many replicas"}]}}`, ExpectedEtcdPath: "/registry/validatingadmissionpolicies/vap1b1", - ExpectedGVK: gvkP("admissionregistration.k8s.io", "v1alpha1", "ValidatingAdmissionPolicy"), }, gvr("admissionregistration.k8s.io", "v1beta1", "validatingadmissionpolicybindings"): { Stub: `{"metadata":{"name":"pb1b1","creationTimestamp":null},"spec":{"policyName":"replicalimit-policy.example.com","paramRef":{"name":"replica-limit-test.example.com","parameterNotFoundAction":"Deny"},"validationActions":["Deny"]}}`, ExpectedEtcdPath: "/registry/validatingadmissionpolicybindings/pb1b1", - ExpectedGVK: gvkP("admissionregistration.k8s.io", "v1alpha1", "ValidatingAdmissionPolicyBinding"), }, // -- @@ -367,10 +365,12 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes gvr("admissionregistration.k8s.io", "v1alpha1", "validatingadmissionpolicies"): { Stub: `{"metadata":{"name":"vap1","creationTimestamp":null},"spec":{"paramKind":{"apiVersion":"test.example.com/v1","kind":"Example"},"matchConstraints":{"resourceRules": [{"resourceNames": ["fakeName"], "apiGroups":["apps"],"apiVersions":["v1"],"operations":["CREATE", "UPDATE"], "resources":["deployments"]}]},"validations":[{"expression":"object.spec.replicas <= params.maxReplicas","message":"Too many replicas"}]}}`, ExpectedEtcdPath: "/registry/validatingadmissionpolicies/vap1", + ExpectedGVK: gvkP("admissionregistration.k8s.io", "v1beta1", "ValidatingAdmissionPolicy"), }, gvr("admissionregistration.k8s.io", "v1alpha1", "validatingadmissionpolicybindings"): { Stub: `{"metadata":{"name":"pb1","creationTimestamp":null},"spec":{"policyName":"replicalimit-policy.example.com","paramRef":{"name":"replica-limit-test.example.com"},"validationActions":["Deny"]}}`, ExpectedEtcdPath: "/registry/validatingadmissionpolicybindings/pb1", + ExpectedGVK: gvkP("admissionregistration.k8s.io", "v1beta1", "ValidatingAdmissionPolicyBinding"), }, // -- From af9bf7b41e08984cee83d5c4e3054156d8e4e27c Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 17 Aug 2023 09:33:22 -0400 Subject: [PATCH 2/2] Prefer non-alpha storage versions when available --- .../etcd/etcd_storage_path_test.go | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/test/integration/etcd/etcd_storage_path_test.go b/test/integration/etcd/etcd_storage_path_test.go index 53c24eb37c6..34858b03cab 100644 --- a/test/integration/etcd/etcd_storage_path_test.go +++ b/test/integration/etcd/etcd_storage_path_test.go @@ -169,28 +169,37 @@ func TestEtcdStoragePath(t *testing.T) { fixtureFilenameGroup = "core" } // find all versions of this group/kind in all versions of the serialization fixture testdata - previousReleaseGroupKindFiles, err := filepath.Glob("../../../staging/src/k8s.io/api/testdata/*/" + fixtureFilenameGroup + ".*." + expectedGVK.Kind + ".yaml") + releaseGroupKindFiles, err := filepath.Glob("../../../staging/src/k8s.io/api/testdata/*/" + fixtureFilenameGroup + ".*." + expectedGVK.Kind + ".yaml") if err != nil { t.Error(err) } - if len(previousReleaseGroupKindFiles) == 0 && !allowMissingTestdataFixtures[expectedGVK] { + if len(releaseGroupKindFiles) == 0 && !allowMissingTestdataFixtures[expectedGVK] { // We should at least find the HEAD fixtures t.Errorf("No testdata serialization files found for %#v, cannot determine if previous releases could read this group/kind. Add this group-version to k8s.io/api/roundtrip_test.go", expectedGVK) } - // find non-alpha versions of this group/kind understood by previous releases + + // find non-alpha versions of this group/kind understood by current and previous releases + currentNonAlphaVersions := sets.NewString() previousNonAlphaVersions := sets.NewString() - for _, previousReleaseGroupKindFile := range previousReleaseGroupKindFiles { - if serverVersion := filepath.Base(filepath.Dir(previousReleaseGroupKindFile)); serverVersion == "HEAD" { - continue - } + for _, previousReleaseGroupKindFile := range releaseGroupKindFiles { parts := strings.Split(filepath.Base(previousReleaseGroupKindFile), ".") version := parts[len(parts)-3] if !strings.Contains(version, "alpha") { - previousNonAlphaVersions.Insert(version) + if serverVersion := filepath.Base(filepath.Dir(previousReleaseGroupKindFile)); serverVersion == "HEAD" { + currentNonAlphaVersions.Insert(version) + } else { + previousNonAlphaVersions.Insert(version) + } } } - if len(previousNonAlphaVersions) > 0 && !previousNonAlphaVersions.Has(expectedGVK.Version) { - t.Errorf("Previous releases understand non-alpha versions %q, but do not understand the expected current storage version %q. "+ + if len(currentNonAlphaVersions) > 0 && strings.Contains(expectedGVK.Version, "alpha") { + t.Errorf("Non-alpha versions %q exist, but the expected storage version is %q. Prefer beta or GA storage versions over alpha.", + currentNonAlphaVersions.List(), + expectedGVK.Version, + ) + } + if !strings.Contains(expectedGVK.Version, "alpha") && len(previousNonAlphaVersions) > 0 && !previousNonAlphaVersions.Has(expectedGVK.Version) { + t.Errorf("Previous releases understand non-alpha versions %q, but do not understand the expected current non-alpha storage version %q. "+ "This means a current server will store data in etcd that is not understood by a previous version.", previousNonAlphaVersions.List(), expectedGVK.Version,