From aa84028119251b93042a1fc24ca6ebf13299ad29 Mon Sep 17 00:00:00 2001 From: David Eads Date: Mon, 8 Jul 2019 09:50:18 -0400 Subject: [PATCH] add protection for reserved API groups --- .../src/k8s.io/apiextensions-apiserver/BUILD | 2 + .../pkg/apihelpers/BUILD | 31 ++ .../pkg/apihelpers/helpers.go | 71 +++++ .../pkg/apihelpers/helpers_test.go | 138 +++++++++ .../pkg/apis/apiextensions/types.go | 5 + .../pkg/apis/apiextensions/v1beta1/types.go | 10 + .../pkg/apiserver/BUILD | 1 + .../pkg/apiserver/apiserver.go | 3 + .../pkg/controller/apiapproval/BUILD | 48 ++++ .../apiapproval/apiapproval_controller.go | 264 ++++++++++++++++++ .../apiapproval_controller_test.go | 104 +++++++ .../registry/customresourcedefinition/BUILD | 7 + .../customresourcedefinition/strategy.go | 48 +++- .../customresourcedefinition/strategy_test.go | 159 +++++++++++ .../test/integration/BUILD | 1 + .../test/integration/apiapproval_test.go | 82 ++++++ vendor/modules.txt | 2 + 17 files changed, 974 insertions(+), 2 deletions(-) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/BUILD create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers_test.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/BUILD create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller_test.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/test/integration/apiapproval_test.go diff --git a/staging/src/k8s.io/apiextensions-apiserver/BUILD b/staging/src/k8s.io/apiextensions-apiserver/BUILD index 9d8e154f72b..1c7a503ca64 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/BUILD @@ -36,6 +36,7 @@ filegroup( srcs = [ ":package-srcs", "//staging/src/k8s.io/apiextensions-apiserver/examples/client-go:all-srcs", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:all-srcs", @@ -46,6 +47,7 @@ filegroup( "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/openapi:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server:all-srcs", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/nonstructuralschema:all-srcs", diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/BUILD new file mode 100644 index 00000000000..4934a87078d --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/BUILD @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["helpers.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/apihelpers", + importpath = "k8s.io/apiextensions-apiserver/pkg/apihelpers", + visibility = ["//visibility:public"], + deps = ["//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library"], +) + +go_test( + name = "go_default_test", + srcs = ["helpers_test.go"], + embed = [":go_default_library"], + deps = ["//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers.go new file mode 100644 index 00000000000..b874785a16d --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers.go @@ -0,0 +1,71 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apihelpers + +import ( + "fmt" + "net/url" + "strings" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" +) + +// IsProtectedCommunityGroup returns whether or not a group specified for a CRD is protected for the community and needs +// to have the v1beta1.KubeAPIApprovalAnnotation set. +func IsProtectedCommunityGroup(group string) bool { + switch { + case group == "k8s.io" || strings.HasSuffix(group, ".k8s.io"): + return true + case group == "kubernetes.io" || strings.HasSuffix(group, ".kubernetes.io"): + return true + default: + return false + } + +} + +// APIApprovalState covers the various options for API approval annotation states +type APIApprovalState int + +const ( + // APIApprovalInvalid means the annotation doesn't have an expected value + APIApprovalInvalid APIApprovalState = iota + // APIApproved if the annotation has a URL (this means the API is approved) + APIApproved + // APIApprovalBypassed if the annotation starts with "unapproved" indicating that for whatever reason the API isn't approved, but we should allow its creation + APIApprovalBypassed + // APIApprovalMissing means the annotation is empty + APIApprovalMissing +) + +// GetAPIApprovalState returns the state of the API approval and reason for that state +func GetAPIApprovalState(annotations map[string]string) (state APIApprovalState, reason string) { + annotation := annotations[v1beta1.KubeAPIApprovedAnnotation] + + // we use the result of this parsing in the switch/case below + url, annotationURLParseErr := url.ParseRequestURI(annotation) + switch { + case len(annotation) == 0: + return APIApprovalMissing, fmt.Sprintf("protected groups must have approval annotation %q, see https://github.com/kubernetes/enhancements/pull/1111", v1beta1.KubeAPIApprovedAnnotation) + case strings.HasPrefix(annotation, "unapproved"): + return APIApprovalBypassed, fmt.Sprintf("not approved: %q", annotation) + case annotationURLParseErr == nil && url != nil && len(url.Host) > 0 && len(url.Scheme) > 0: + return APIApproved, fmt.Sprintf("approved in %v", annotation) + default: + return APIApprovalInvalid, fmt.Sprintf("protected groups must have approval annotation %q with either a URL or a reason starting with \"unapproved\", see https://github.com/kubernetes/enhancements/pull/1111", v1beta1.KubeAPIApprovedAnnotation) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers_test.go new file mode 100644 index 00000000000..7e12f7ec572 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers/helpers_test.go @@ -0,0 +1,138 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apihelpers + +import ( + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" +) + +func TestIsProtectedCommunityGroup(t *testing.T) { + tests := []struct { + name string + + group string + expected bool + }{ + { + name: "bare k8s", + group: "k8s.io", + expected: true, + }, + { + name: "bare kube", + group: "kubernetes.io", + expected: true, + }, + { + name: "nested k8s", + group: "sigs.k8s.io", + expected: true, + }, + { + name: "nested kube", + group: "sigs.kubernetes.io", + expected: true, + }, + { + name: "alternative", + group: "different.io", + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := IsProtectedCommunityGroup(test.group) + + if actual != test.expected { + t.Fatalf("expected %v, got %v", test.expected, actual) + } + }) + } +} + +func TestGetAPIApprovalState(t *testing.T) { + tests := []struct { + name string + + annotations map[string]string + expected APIApprovalState + }{ + { + name: "bare unapproved", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "unapproved"}, + expected: APIApprovalBypassed, + }, + { + name: "unapproved with message", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "unapproved, experimental-only"}, + expected: APIApprovalBypassed, + }, + { + name: "mismatched case", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "Unapproved"}, + expected: APIApprovalInvalid, + }, + { + name: "empty", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: ""}, + expected: APIApprovalMissing, + }, + { + name: "missing", + annotations: map[string]string{}, + expected: APIApprovalMissing, + }, + { + name: "url", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "https://github.com/kubernetes/kubernetes/pull/78458"}, + expected: APIApproved, + }, + { + name: "url - no scheme", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "github.com/kubernetes/kubernetes/pull/78458"}, + expected: APIApprovalInvalid, + }, + { + name: "url - no host", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "http:///kubernetes/kubernetes/pull/78458"}, + expected: APIApprovalInvalid, + }, + { + name: "url - just path", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "/"}, + expected: APIApprovalInvalid, + }, + { + name: "missing scheme", + annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "github.com/kubernetes/kubernetes/pull/78458"}, + expected: APIApprovalInvalid, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual, _ := GetAPIApprovalState(test.annotations) + + if actual != test.expected { + t.Fatalf("expected %v, got %v", test.expected, actual) + } + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go index 9c2798f0307..8f502e8b15a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go @@ -289,6 +289,11 @@ const ( NonStructuralSchema CustomResourceDefinitionConditionType = "NonStructuralSchema" // Terminating means that the CustomResourceDefinition has been deleted and is cleaning up. Terminating CustomResourceDefinitionConditionType = "Terminating" + // KubernetesAPIApprovalPolicyConformant indicates that an API in *.k8s.io or *.kubernetes.io is or is not approved. For CRDs + // outside those groups, this condition will not be set. For CRDs inside those groups, the condition will + // be true if .metadata.annotations["api-approved.kubernetes.io"] is set to a URL, otherwise it will be false. + // See https://github.com/kubernetes/enhancements/pull/1111 for more details. + KubernetesAPIApprovalPolicyConformant CustomResourceDefinitionConditionType = "KubernetesAPIApprovalPolicyConformant" ) // CustomResourceDefinitionCondition contains details for the current condition of this pod. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go index 6d931ca95c1..154ea836750 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.go @@ -26,6 +26,11 @@ import ( type ConversionStrategyType string const ( + // KubeAPIApprovedAnnotation is an annotation that must be set to create a CRD for the k8s.io, *.k8s.io, kubernetes.io, or *.kubernetes.io namespaces. + // The value should be a link to a URL where the current spec was approved, so updates to the spec should also update the URL. + // If the API is unapproved, you may set the annotation to a string starting with `"unapproved"`. For instance, `"unapproved, temporarily squatting"` or `"unapproved, experimental-only"`. This is discouraged. + KubeAPIApprovedAnnotation = "api-approved.kubernetes.io" + // NoneConverter is a converter that only sets apiversion of the CR and leave everything else unchanged. NoneConverter ConversionStrategyType = "None" // WebhookConverter is a converter that calls to an external webhook to convert the CR. @@ -304,6 +309,11 @@ const ( NonStructuralSchema CustomResourceDefinitionConditionType = "NonStructuralSchema" // Terminating means that the CustomResourceDefinition has been deleted and is cleaning up. Terminating CustomResourceDefinitionConditionType = "Terminating" + // KubernetesAPIApprovalPolicyConformant indicates that an API in *.k8s.io or *.kubernetes.io is or is not approved. For CRDs + // outside those groups, this condition will not be set. For CRDs inside those groups, the condition will + // be true if .metadata.annotations["api-approved.kubernetes.io"] is set to a URL, otherwise it will be false. + // See https://github.com/kubernetes/enhancements/pull/1111 for more details. + KubernetesAPIApprovalPolicyConformant CustomResourceDefinitionConditionType = "KubernetesAPIApprovalPolicyConformant" ) // CustomResourceDefinitionCondition contains details for the current condition of this pod. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD index ea062f2128b..a46020a3ce5 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD @@ -34,6 +34,7 @@ go_library( "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion/apiextensions/internalversion:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/internalversion:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/nonstructuralschema:go_default_library", diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go index 992872df85b..ad07c039491 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go @@ -28,6 +28,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/client/clientset/internalclientset" _ "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions" internalinformers "k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion" + "k8s.io/apiextensions-apiserver/pkg/controller/apiapproval" "k8s.io/apiextensions-apiserver/pkg/controller/establish" "k8s.io/apiextensions-apiserver/pkg/controller/finalizer" "k8s.io/apiextensions-apiserver/pkg/controller/nonstructuralschema" @@ -199,6 +200,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) crdController := NewDiscoveryController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), versionDiscoveryHandler, groupDiscoveryHandler) namingController := status.NewNamingConditionController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient.Apiextensions()) nonStructuralSchemaController := nonstructuralschema.NewConditionController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient.Apiextensions()) + apiApprovalController := apiapproval.NewKubernetesAPIApprovalPolicyConformantConditionController(s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient.Apiextensions()) finalizingController := finalizer.NewCRDFinalizer( s.Informers.Apiextensions().InternalVersion().CustomResourceDefinitions(), crdClient.Apiextensions(), @@ -226,6 +228,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) go namingController.Run(context.StopCh) go establishingController.Run(context.StopCh) go nonStructuralSchemaController.Run(5, context.StopCh) + go apiApprovalController.Run(5, context.StopCh) go finalizingController.Run(5, context.StopCh) return nil }) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/BUILD new file mode 100644 index 00000000000..2aa7eb49dc7 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/BUILD @@ -0,0 +1,48 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["apiapproval_controller.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval", + importpath = "k8s.io/apiextensions-apiserver/pkg/controller/apiapproval", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/internalclientset/typed/apiextensions/internalversion:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion/apiextensions/internalversion:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/internalversion:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//staging/src/k8s.io/client-go/tools/cache:go_default_library", + "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", + "//vendor/k8s.io/klog:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["apiapproval_controller_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller.go new file mode 100644 index 00000000000..599f3d38f86 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller.go @@ -0,0 +1,264 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apiapproval + +import ( + "fmt" + "sync" + "time" + + "k8s.io/apiextensions-apiserver/pkg/apihelpers" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + client "k8s.io/apiextensions-apiserver/pkg/client/clientset/internalclientset/typed/apiextensions/internalversion" + informers "k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion/apiextensions/internalversion" + listers "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/internalversion" + apierrors "k8s.io/apimachinery/pkg/api/errors" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/workqueue" + "k8s.io/klog" +) + +// KubernetesAPIApprovalPolicyConformantConditionController is maintaining the KubernetesAPIApprovalPolicyConformant condition. +type KubernetesAPIApprovalPolicyConformantConditionController struct { + crdClient client.CustomResourceDefinitionsGetter + + crdLister listers.CustomResourceDefinitionLister + crdSynced cache.InformerSynced + + // To allow injection for testing. + syncFn func(key string) error + + queue workqueue.RateLimitingInterface + + // last protectedAnnotation value this controller updated the condition per CRD name (to avoid two + // different version of the apiextensions-apiservers in HA to fight for the right message) + lastSeenProtectedAnnotationLock sync.Mutex + lastSeenProtectedAnnotation map[string]string +} + +// NewKubernetesAPIApprovalPolicyConformantConditionController constructs a KubernetesAPIApprovalPolicyConformant schema condition controller. +func NewKubernetesAPIApprovalPolicyConformantConditionController( + crdInformer informers.CustomResourceDefinitionInformer, + crdClient client.CustomResourceDefinitionsGetter, +) *KubernetesAPIApprovalPolicyConformantConditionController { + c := &KubernetesAPIApprovalPolicyConformantConditionController{ + crdClient: crdClient, + crdLister: crdInformer.Lister(), + crdSynced: crdInformer.Informer().HasSynced, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "kubernetes_api_approval_conformant_condition_controller"), + lastSeenProtectedAnnotation: map[string]string{}, + } + + crdInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: c.addCustomResourceDefinition, + UpdateFunc: c.updateCustomResourceDefinition, + DeleteFunc: c.deleteCustomResourceDefinition, + }) + + c.syncFn = c.sync + + return c +} + +// calculateCondition determines the new KubernetesAPIApprovalPolicyConformant condition +func calculateCondition(crd *apiextensions.CustomResourceDefinition) *apiextensions.CustomResourceDefinitionCondition { + if !apihelpers.IsProtectedCommunityGroup(crd.Spec.Group) { + return nil + } + + approvalState, reason := apihelpers.GetAPIApprovalState(crd.Annotations) + switch approvalState { + case apihelpers.APIApprovalInvalid: + return &apiextensions.CustomResourceDefinitionCondition{ + Type: apiextensions.KubernetesAPIApprovalPolicyConformant, + Status: apiextensions.ConditionFalse, + Reason: "InvalidAnnotation", + Message: reason, + } + case apihelpers.APIApprovalMissing: + return &apiextensions.CustomResourceDefinitionCondition{ + Type: apiextensions.KubernetesAPIApprovalPolicyConformant, + Status: apiextensions.ConditionFalse, + Reason: "MissingAnnotation", + Message: reason, + } + case apihelpers.APIApproved: + return &apiextensions.CustomResourceDefinitionCondition{ + Type: apiextensions.KubernetesAPIApprovalPolicyConformant, + Status: apiextensions.ConditionTrue, + Reason: "ApprovedAnnotation", + Message: reason, + } + case apihelpers.APIApprovalBypassed: + return &apiextensions.CustomResourceDefinitionCondition{ + Type: apiextensions.KubernetesAPIApprovalPolicyConformant, + Status: apiextensions.ConditionFalse, + Reason: "UnapprovedAnnotation", + Message: reason, + } + default: + return &apiextensions.CustomResourceDefinitionCondition{ + Type: apiextensions.KubernetesAPIApprovalPolicyConformant, + Status: apiextensions.ConditionUnknown, + Reason: "UnknownAnnotation", + Message: reason, + } + } +} + +func (c *KubernetesAPIApprovalPolicyConformantConditionController) sync(key string) error { + inCustomResourceDefinition, err := c.crdLister.Get(key) + if apierrors.IsNotFound(err) { + return nil + } + if err != nil { + return err + } + + // avoid repeated calculation for the same annotation + protectionAnnotationValue := inCustomResourceDefinition.Annotations[v1beta1.KubeAPIApprovedAnnotation] + c.lastSeenProtectedAnnotationLock.Lock() + lastSeen, seenBefore := c.lastSeenProtectedAnnotation[inCustomResourceDefinition.Name] + c.lastSeenProtectedAnnotationLock.Unlock() + if seenBefore && protectionAnnotationValue == lastSeen { + return nil + } + + // check old condition + cond := calculateCondition(inCustomResourceDefinition) + if cond == nil { + // because group is immutable, if we have no condition now, we have no need to remove a condition. + return nil + } + old := apiextensions.FindCRDCondition(inCustomResourceDefinition, apiextensions.KubernetesAPIApprovalPolicyConformant) + + // don't attempt a write if all the condition details are the same + if old != nil && old.Status == cond.Status && old.Reason == cond.Reason && old.Message == cond.Message { + // no need to update annotation because we took no action. + return nil + } + + // update condition + crd := inCustomResourceDefinition.DeepCopy() + apiextensions.SetCRDCondition(crd, *cond) + + _, err = c.crdClient.CustomResourceDefinitions().UpdateStatus(crd) + if apierrors.IsNotFound(err) || apierrors.IsConflict(err) { + // deleted or changed in the meantime, we'll get called again + return nil + } + if err != nil { + return err + } + + // store annotation in order to avoid repeated updates for the same annotation (and potential + // fights of API server in HA environments). + c.lastSeenProtectedAnnotationLock.Lock() + defer c.lastSeenProtectedAnnotationLock.Unlock() + c.lastSeenProtectedAnnotation[crd.Name] = protectionAnnotationValue + + return nil +} + +// Run starts the controller. +func (c *KubernetesAPIApprovalPolicyConformantConditionController) Run(threadiness int, stopCh <-chan struct{}) { + defer utilruntime.HandleCrash() + defer c.queue.ShutDown() + + klog.Infof("Starting KubernetesAPIApprovalPolicyConformantConditionController") + defer klog.Infof("Shutting down KubernetesAPIApprovalPolicyConformantConditionController") + + if !cache.WaitForCacheSync(stopCh, c.crdSynced) { + return + } + + for i := 0; i < threadiness; i++ { + go wait.Until(c.runWorker, time.Second, stopCh) + } + + <-stopCh +} + +func (c *KubernetesAPIApprovalPolicyConformantConditionController) runWorker() { + for c.processNextWorkItem() { + } +} + +// processNextWorkItem deals with one key off the queue. It returns false when it's time to quit. +func (c *KubernetesAPIApprovalPolicyConformantConditionController) processNextWorkItem() bool { + key, quit := c.queue.Get() + if quit { + return false + } + defer c.queue.Done(key) + + err := c.syncFn(key.(string)) + if err == nil { + c.queue.Forget(key) + return true + } + + utilruntime.HandleError(fmt.Errorf("%v failed with: %v", key, err)) + c.queue.AddRateLimited(key) + + return true +} + +func (c *KubernetesAPIApprovalPolicyConformantConditionController) enqueue(obj *apiextensions.CustomResourceDefinition) { + key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj) + if err != nil { + utilruntime.HandleError(fmt.Errorf("Couldn't get key for object %#v: %v", obj, err)) + return + } + + c.queue.Add(key) +} + +func (c *KubernetesAPIApprovalPolicyConformantConditionController) addCustomResourceDefinition(obj interface{}) { + castObj := obj.(*apiextensions.CustomResourceDefinition) + klog.V(4).Infof("Adding %s", castObj.Name) + c.enqueue(castObj) +} + +func (c *KubernetesAPIApprovalPolicyConformantConditionController) updateCustomResourceDefinition(obj, _ interface{}) { + castObj := obj.(*apiextensions.CustomResourceDefinition) + klog.V(4).Infof("Updating %s", castObj.Name) + c.enqueue(castObj) +} + +func (c *KubernetesAPIApprovalPolicyConformantConditionController) deleteCustomResourceDefinition(obj interface{}) { + castObj, ok := obj.(*apiextensions.CustomResourceDefinition) + if !ok { + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + klog.Errorf("Couldn't get object from tombstone %#v", obj) + return + } + castObj, ok = tombstone.Obj.(*apiextensions.CustomResourceDefinition) + if !ok { + klog.Errorf("Tombstone contained object that is not expected %#v", obj) + return + } + } + + c.lastSeenProtectedAnnotationLock.Lock() + defer c.lastSeenProtectedAnnotationLock.Unlock() + delete(c.lastSeenProtectedAnnotation, castObj.Name) +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller_test.go new file mode 100644 index 00000000000..49fa7a9fc08 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller_test.go @@ -0,0 +1,104 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apiapproval + +import ( + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestCalculateCondition(t *testing.T) { + noConditionFn := func(t *testing.T, condition *apiextensions.CustomResourceDefinitionCondition) { + t.Helper() + if condition != nil { + t.Fatal(condition) + } + } + + verifyCondition := func(status apiextensions.ConditionStatus, message string) func(t *testing.T, condition *apiextensions.CustomResourceDefinitionCondition) { + return func(t *testing.T, condition *apiextensions.CustomResourceDefinitionCondition) { + t.Helper() + if condition == nil { + t.Fatal("missing condition") + } + if e, a := status, condition.Status; e != a { + t.Errorf("expected %v, got %v", e, a) + } + if e, a := message, condition.Message; e != a { + t.Errorf("expected %v, got %v", e, a) + } + } + } + + tests := []struct { + name string + + group string + annotationValue string + validateCondition func(t *testing.T, condition *apiextensions.CustomResourceDefinitionCondition) + }{ + { + name: "for other group", + group: "other.io", + annotationValue: "", + validateCondition: noConditionFn, + }, + { + name: "missing annotation", + group: "sigs.k8s.io", + annotationValue: "", + validateCondition: verifyCondition(apiextensions.ConditionFalse, `protected groups must have approval annotation "api-approved.kubernetes.io", see https://github.com/kubernetes/enhancements/pull/1111`), + }, + { + name: "invalid annotation", + group: "sigs.k8s.io", + annotationValue: "bad value", + validateCondition: verifyCondition(apiextensions.ConditionFalse, `protected groups must have approval annotation "api-approved.kubernetes.io" with either a URL or a reason starting with "unapproved", see https://github.com/kubernetes/enhancements/pull/1111`), + }, + { + name: "approved", + group: "sigs.k8s.io", + annotationValue: "https://github.com/kubernetes/kubernetes/pull/79724", + validateCondition: verifyCondition(apiextensions.ConditionTrue, `approved in https://github.com/kubernetes/kubernetes/pull/79724`), + }, + { + name: "unapproved", + group: "sigs.k8s.io", + annotationValue: "unapproved for reasons", + validateCondition: verifyCondition(apiextensions.ConditionFalse, `not approved: "unapproved for reasons"`), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + crd := &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: test.annotationValue}}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: test.group, + }, + } + + actual := calculateCondition(crd) + test.validateCondition(t, actual) + + }) + } + +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD index 5b8846b832b..36ae182ed74 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD @@ -11,7 +11,9 @@ go_library( importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition", importpath = "k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition", deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apihelpers:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", @@ -21,6 +23,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/generic/registry:go_default_library", "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", @@ -51,8 +54,12 @@ go_test( embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", ], diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go index 123bcc6cde9..557fe88225f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go @@ -20,7 +20,9 @@ import ( "context" "fmt" + "k8s.io/apiextensions-apiserver/pkg/apihelpers" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation" apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -28,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" @@ -98,7 +101,8 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { // Validate validates a new CustomResourceDefinition. func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - return validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition)) + fieldErrors := validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition)) + return append(fieldErrors, validateAPIApproval(ctx, obj.(*apiextensions.CustomResourceDefinition), nil)...) } // AllowCreateOnUpdate is false for CustomResourceDefinition; this means a POST is @@ -118,7 +122,47 @@ func (strategy) Canonicalize(obj runtime.Object) { // ValidateUpdate is the default update validation for an end user updating status. func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateCustomResourceDefinitionUpdate(obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition)) + fieldErrors := validation.ValidateCustomResourceDefinitionUpdate(obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition)) + + return append(fieldErrors, validateAPIApproval(ctx, obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition))...) +} + +// validateAPIApproval returns a list of errors if the API approval annotation isn't valid +func validateAPIApproval(ctx context.Context, newCRD, oldCRD *apiextensions.CustomResourceDefinition) field.ErrorList { + // check to see if we need confirm API approval for kube group. Do nothing for non-protected groups and do nothing in v1beta1. + if requestInfo, ok := request.RequestInfoFrom(ctx); !ok || requestInfo.APIVersion == "v1beta1" { + return field.ErrorList{} + } + if !apihelpers.IsProtectedCommunityGroup(newCRD.Spec.Group) { + return field.ErrorList{} + } + + // default to a state that allows missing values to continue to be missing + var oldApprovalState *apihelpers.APIApprovalState + if oldCRD != nil { + t, _ := apihelpers.GetAPIApprovalState(oldCRD.Annotations) + oldApprovalState = &t + } + newApprovalState, reason := apihelpers.GetAPIApprovalState(newCRD.Annotations) + + // if the approval state hasn't changed, never fail on approval validation + // this is allowed so that a v1 client that is simply updating spec and not mutating this value doesn't get rejected. Imagine a controller controlling a CRD spec. + if oldApprovalState != nil && *oldApprovalState == newApprovalState { + return field.ErrorList{} + } + + // in v1, we require valid approval strings + switch newApprovalState { + case apihelpers.APIApprovalInvalid: + return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[v1beta1.KubeAPIApprovedAnnotation], reason)} + case apihelpers.APIApprovalMissing: + return field.ErrorList{field.Required(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), reason)} + case apihelpers.APIApproved, apihelpers.APIApprovalBypassed: + // success + return field.ErrorList{} + default: + return field.ErrorList{field.Invalid(field.NewPath("metadata", "annotations").Key(v1beta1.KubeAPIApprovedAnnotation), newCRD.Annotations[v1beta1.KubeAPIApprovedAnnotation], reason)} + } } type statusStrategy struct { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go index 9d42e7172d5..12d454d8f51 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go @@ -17,13 +17,18 @@ limitations under the License. package customresourcedefinition import ( + "context" "fmt" "reflect" "testing" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/endpoints/request" utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" ) @@ -469,3 +474,157 @@ func TestDropDisableFieldsCustomResourceDefinition(t *testing.T) { } } + +func strPtr(in string) *string { + return &in +} + +func TestValidateAPIApproval(t *testing.T) { + okFn := func(t *testing.T, errors field.ErrorList) { + t.Helper() + if len(errors) > 0 { + t.Fatal(errors) + } + } + + tests := []struct { + name string + + version string + group string + annotationValue string + oldAnnotationValue *string + validateError func(t *testing.T, errors field.ErrorList) + }{ + { + name: "ignore v1beta1", + version: "v1beta1", + group: "sigs.k8s.io", + annotationValue: "invalid", + validateError: okFn, + }, + { + name: "ignore non-k8s group", + version: "v1", + group: "other.io", + annotationValue: "invalid", + validateError: okFn, + }, + { + name: "invalid annotation create", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "invalid", + validateError: func(t *testing.T, errors field.ErrorList) { + t.Helper() + if e, a := `metadata.annotations[api-approved.kubernetes.io]: Invalid value: "invalid": protected groups must have approval annotation "api-approved.kubernetes.io" with either a URL or a reason starting with "unapproved", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a { + t.Fatal(errors) + } + }, + }, + { + name: "invalid annotation update", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "invalid", + oldAnnotationValue: strPtr("invalid"), + validateError: okFn, + }, + { + name: "invalid annotation to missing", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "", + oldAnnotationValue: strPtr("invalid"), + validateError: func(t *testing.T, errors field.ErrorList) { + t.Helper() + if e, a := `metadata.annotations[api-approved.kubernetes.io]: Required value: protected groups must have approval annotation "api-approved.kubernetes.io", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a { + t.Fatal(errors) + } + }, + }, + { + name: "missing to invalid annotation", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "invalid", + oldAnnotationValue: strPtr(""), + validateError: func(t *testing.T, errors field.ErrorList) { + t.Helper() + if e, a := `metadata.annotations[api-approved.kubernetes.io]: Invalid value: "invalid": protected groups must have approval annotation "api-approved.kubernetes.io" with either a URL or a reason starting with "unapproved", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a { + t.Fatal(errors) + } + }, + }, + { + name: "missing annotation", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "", + validateError: func(t *testing.T, errors field.ErrorList) { + t.Helper() + if e, a := `metadata.annotations[api-approved.kubernetes.io]: Required value: protected groups must have approval annotation "api-approved.kubernetes.io", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a { + t.Fatal(errors) + } + }, + }, + { + name: "missing annotation update", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "", + oldAnnotationValue: strPtr(""), + validateError: okFn, + }, + { + name: "url", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "https://github.com/kubernetes/kubernetes/pull/79724", + validateError: okFn, + }, + { + name: "unapproved", + version: "v1", + group: "sigs.k8s.io", + annotationValue: "unapproved, other reason", + validateError: okFn, + }, + { + name: "next version validates", + version: "v2", + group: "sigs.k8s.io", + annotationValue: "invalid", + validateError: func(t *testing.T, errors field.ErrorList) { + t.Helper() + if e, a := `metadata.annotations[api-approved.kubernetes.io]: Invalid value: "invalid": protected groups must have approval annotation "api-approved.kubernetes.io" with either a URL or a reason starting with "unapproved", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a { + t.Fatal(errors) + } + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx := request.WithRequestInfo(context.TODO(), &request.RequestInfo{APIVersion: test.version}) + crd := &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: test.annotationValue}}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: test.group, + }, + } + var oldCRD *apiextensions.CustomResourceDefinition + if test.oldAnnotationValue != nil { + oldCRD = &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foo", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: *test.oldAnnotationValue}}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: test.group, + }, + } + } + + actual := validateAPIApproval(ctx, crd, oldCRD) + test.validateError(t, actual) + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD index 164c5bb42aa..1de0448597b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD @@ -9,6 +9,7 @@ load( go_test( name = "go_default_test", srcs = [ + "apiapproval_test.go", "apply_test.go", "basic_test.go", "change_test.go", diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/apiapproval_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/apiapproval_test.go new file mode 100644 index 00000000000..5b736864f36 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/apiapproval_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "testing" + "time" + + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" +) + +func TestAPIApproval(t *testing.T) { + tearDown, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + noxuDefinition := fixtures.NewNoxuCustomResourceDefinition(apiextensionsv1beta1.NamespaceScoped) + noxuDefinition, err = fixtures.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + if noxuAPIApproved := findCRDCondition(noxuDefinition, apiextensionsv1beta1.KubernetesAPIApprovalPolicyConformant); noxuAPIApproved != nil { + t.Fatal(noxuAPIApproved) + } + + newSigKubeAPIFn := func(resource, approvalAnnotation string) *apiextensionsv1beta1.CustomResourceDefinition { + return &apiextensionsv1beta1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: resource + ".sigs.k8s.io", Annotations: map[string]string{apiextensionsv1beta1.KubeAPIApprovedAnnotation: approvalAnnotation}}, + Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ + Group: "sigs.k8s.io", + Version: "v1beta1", + Names: apiextensionsv1beta1.CustomResourceDefinitionNames{ + Plural: resource, + Singular: resource + "singular", + Kind: resource + "Kind", + ListKind: resource + "List", + }, + Scope: apiextensionsv1beta1.NamespaceScoped, + }, + } + } + // the unit tests cover all variations. We just need to be sure that we see the code being called + approvedKubeAPI := newSigKubeAPIFn("approved", "https://github.com/kubernetes/kubernetes/pull/79724") + approvedKubeAPI, err = fixtures.CreateNewCustomResourceDefinition(approvedKubeAPI, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + err = wait.PollImmediate(100*time.Millisecond, 30*time.Second, func() (bool, error) { + approvedKubeAPI, err = apiExtensionClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(approvedKubeAPI.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + if approvedKubeAPIApproved := findCRDCondition(approvedKubeAPI, apiextensionsv1beta1.KubernetesAPIApprovalPolicyConformant); approvedKubeAPIApproved == nil || approvedKubeAPIApproved.Status != apiextensionsv1beta1.ConditionTrue { + t.Log(approvedKubeAPIApproved) + return false, nil + } + return true, nil + }) + if err != nil { + t.Fatal(err) + } + +} diff --git a/vendor/modules.txt b/vendor/modules.txt index b7225ed3178..56d9f473919 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1058,6 +1058,7 @@ k8s.io/api/storage/v1 k8s.io/api/storage/v1alpha1 k8s.io/api/storage/v1beta1 # k8s.io/apiextensions-apiserver v0.0.0 => ./staging/src/k8s.io/apiextensions-apiserver +k8s.io/apiextensions-apiserver/pkg/apihelpers k8s.io/apiextensions-apiserver/pkg/apis/apiextensions k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1 @@ -1087,6 +1088,7 @@ k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/internalversion k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1 k8s.io/apiextensions-apiserver/pkg/cmd/server/options k8s.io/apiextensions-apiserver/pkg/cmd/server/testing +k8s.io/apiextensions-apiserver/pkg/controller/apiapproval k8s.io/apiextensions-apiserver/pkg/controller/establish k8s.io/apiextensions-apiserver/pkg/controller/finalizer k8s.io/apiextensions-apiserver/pkg/controller/nonstructuralschema