diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller.go index 877720539fa25..ee9c0afb0fafa 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller.go @@ -18,10 +18,10 @@ package openapi import ( "fmt" - "reflect" "sync" "time" + "github.com/google/uuid" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -29,6 +29,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" + "k8s.io/kube-openapi/pkg/cached" "k8s.io/kube-openapi/pkg/handler" "k8s.io/kube-openapi/pkg/validation/spec" @@ -49,21 +50,69 @@ type Controller struct { queue workqueue.RateLimitingInterface - staticSpec *spec.Swagger + staticSpec *spec.Swagger + openAPIService *handler.OpenAPIService - // specs per version and per CRD name - lock sync.Mutex - crdSpecs map[string]map[string]*spec.Swagger + // specs by name. The specs are lazily constructed on request. + // The lock is for the map only. + lock sync.Mutex + specsByName map[string]*specCache +} + +// specCache holds the merged version spec for a CRD as well as the CRD object. +// The spec is created lazily from the CRD object on request. +// The mergedVersionSpec is only created on instantiation and is never +// changed. crdCache is a cached.Replaceable and updates are thread +// safe. Thus, no lock is needed to protect this struct. +type specCache struct { + crdCache cached.Replaceable[*apiextensionsv1.CustomResourceDefinition] + mergedVersionSpec cached.Data[*spec.Swagger] +} + +func (s *specCache) update(crd *apiextensionsv1.CustomResourceDefinition) { + s.crdCache.Replace(cached.NewResultOK(crd, generateCRDHash(crd))) +} + +func createSpecCache(crd *apiextensionsv1.CustomResourceDefinition) *specCache { + s := specCache{} + s.update(crd) + + s.mergedVersionSpec = cached.NewTransformer[*apiextensionsv1.CustomResourceDefinition](func(result cached.Result[*apiextensionsv1.CustomResourceDefinition]) cached.Result[*spec.Swagger] { + if result.Err != nil { + // This should never happen, but return the err if it does. + return cached.NewResultErr[*spec.Swagger](result.Err) + } + crd := result.Data + mergeSpec := &spec.Swagger{} + for _, v := range crd.Spec.Versions { + if !v.Served { + continue + } + s, err := builder.BuildOpenAPIV2(crd, v.Name, builder.Options{V2: true}) + // Defaults must be pruned here for CRDs to cleanly merge with the static + // spec that already has defaults pruned + if err != nil { + return cached.NewResultErr[*spec.Swagger](err) + } + s.Definitions = handler.PruneDefaults(s.Definitions) + mergeSpec, err = builder.MergeSpecs(mergeSpec, s) + if err != nil { + return cached.NewResultErr[*spec.Swagger](err) + } + } + return cached.NewResultOK(mergeSpec, generateCRDHash(crd)) + }, &s.crdCache) + return &s } // NewController creates a new Controller with input CustomResourceDefinition informer func NewController(crdInformer informers.CustomResourceDefinitionInformer) *Controller { c := &Controller{ - crdLister: crdInformer.Lister(), - crdsSynced: crdInformer.Informer().HasSynced, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "crd_openapi_controller"), - crdSpecs: map[string]map[string]*spec.Swagger{}, + crdLister: crdInformer.Lister(), + crdsSynced: crdInformer.Informer().HasSynced, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "crd_openapi_controller"), + specsByName: map[string]*specCache{}, } crdInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -102,18 +151,9 @@ func (c *Controller) Run(staticSpec *spec.Swagger, openAPIService *handler.OpenA if !apiextensionshelpers.IsCRDConditionTrue(crd, apiextensionsv1.Established) { continue } - newSpecs, changed, err := buildVersionSpecs(crd, nil) - if err != nil { - utilruntime.HandleError(fmt.Errorf("failed to build OpenAPI spec of CRD %s: %v", crd.Name, err)) - } else if !changed { - continue - } - c.crdSpecs[crd.Name] = newSpecs - } - if err := c.updateSpecLocked(); err != nil { - utilruntime.HandleError(fmt.Errorf("failed to initially create OpenAPI spec for CRDs: %v", err)) - return + c.specsByName[crd.Name] = createSpecCache(crd) } + c.updateSpecLocked() // only start one worker thread since its a slow moving API go wait.Until(c.runWorker, time.Second, stopCh) @@ -164,76 +204,59 @@ func (c *Controller) sync(name string) error { // do we have to remove all specs of this CRD? if errors.IsNotFound(err) || !apiextensionshelpers.IsCRDConditionTrue(crd, apiextensionsv1.Established) { - if _, found := c.crdSpecs[name]; !found { + if _, found := c.specsByName[name]; !found { return nil } - delete(c.crdSpecs, name) + delete(c.specsByName, name) klog.V(2).Infof("Updating CRD OpenAPI spec because %s was removed", name) regenerationCounter.With(map[string]string{"crd": name, "reason": "remove"}) - return c.updateSpecLocked() + c.updateSpecLocked() + return nil } - // compute CRD spec and see whether it changed - oldSpecs, updated := c.crdSpecs[crd.Name] - newSpecs, changed, err := buildVersionSpecs(crd, oldSpecs) - if err != nil { - return err - } - if !changed { + // If CRD spec already exists, update the CRD. + // specCache.update() includes the ETag so an update on a spec + // resulting in the same ETag will be a noop. + s, exists := c.specsByName[crd.Name] + if exists { + s.update(crd) + klog.V(2).Infof("Updating CRD OpenAPI spec because %s changed", name) + regenerationCounter.With(map[string]string{"crd": name, "reason": "update"}) return nil } - // update specs of this CRD - c.crdSpecs[crd.Name] = newSpecs + c.specsByName[crd.Name] = createSpecCache(crd) klog.V(2).Infof("Updating CRD OpenAPI spec because %s changed", name) - reason := "add" - if updated { - reason = "update" - } - regenerationCounter.With(map[string]string{"crd": name, "reason": reason}) - return c.updateSpecLocked() + regenerationCounter.With(map[string]string{"crd": name, "reason": "add"}) + c.updateSpecLocked() + return nil } -func buildVersionSpecs(crd *apiextensionsv1.CustomResourceDefinition, oldSpecs map[string]*spec.Swagger) (map[string]*spec.Swagger, bool, error) { - newSpecs := map[string]*spec.Swagger{} - anyChanged := false - for _, v := range crd.Spec.Versions { - if !v.Served { - continue - } - spec, err := builder.BuildOpenAPIV2(crd, v.Name, builder.Options{V2: true}) - // Defaults must be pruned here for CRDs to cleanly merge with the static - // spec that already has defaults pruned - spec.Definitions = handler.PruneDefaults(spec.Definitions) - if err != nil { - return nil, false, err - } - newSpecs[v.Name] = spec - if oldSpecs[v.Name] == nil || !reflect.DeepEqual(oldSpecs[v.Name], spec) { - anyChanged = true - } - } - if !anyChanged && len(oldSpecs) == len(newSpecs) { - return newSpecs, false, nil +// updateSpecLocked updates the cached spec graph. +func (c *Controller) updateSpecLocked() { + specList := make([]cached.Data[*spec.Swagger], 0, len(c.specsByName)) + for crd := range c.specsByName { + specList = append(specList, c.specsByName[crd].mergedVersionSpec) } - return newSpecs, true, nil -} - -// updateSpecLocked aggregates all OpenAPI specs and updates openAPIService. -// It is not thread-safe. The caller is responsible to hold proper lock (Controller.lock). -func (c *Controller) updateSpecLocked() error { - crdSpecs := []*spec.Swagger{} - for _, versionSpecs := range c.crdSpecs { - for _, s := range versionSpecs { - crdSpecs = append(crdSpecs, s) + cache := cached.NewListMerger(func(results []cached.Result[*spec.Swagger]) cached.Result[*spec.Swagger] { + localCRDSpec := make([]*spec.Swagger, 0, len(results)) + for k := range results { + if results[k].Err == nil { + localCRDSpec = append(localCRDSpec, results[k].Data) + } } - } - mergedSpec, err := builder.MergeSpecs(c.staticSpec, crdSpecs...) - if err != nil { - return fmt.Errorf("failed to merge specs: %v", err) - } - return c.openAPIService.UpdateSpec(mergedSpec) + mergedSpec, err := builder.MergeSpecs(c.staticSpec, localCRDSpec...) + if err != nil { + return cached.NewResultErr[*spec.Swagger](fmt.Errorf("failed to merge specs: %v", err)) + } + // A UUID is returned for the etag because we will only + // create a new merger when a CRD has changed. A hash based + // etag is more expensive because the CRDs are not + // premarshalled. + return cached.NewResultOK(mergedSpec, uuid.New().String()) + }, specList) + c.openAPIService.UpdateSpecLazy(cache) } func (c *Controller) addCustomResourceDefinition(obj interface{}) { @@ -269,3 +292,7 @@ func (c *Controller) deleteCustomResourceDefinition(obj interface{}) { func (c *Controller) enqueue(obj *apiextensionsv1.CustomResourceDefinition) { c.queue.Add(obj.Name) } + +func generateCRDHash(crd *apiextensionsv1.CustomResourceDefinition) string { + return fmt.Sprintf("%s,%d", crd.UID, crd.Generation) +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller_test.go new file mode 100644 index 0000000000000..468d4cfb8a585 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller_test.go @@ -0,0 +1,425 @@ +/* +Copyright 2023 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 openapi + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake" + "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions" + "k8s.io/kube-openapi/pkg/handler" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + "k8s.io/kube-openapi/pkg/validation/spec" +) + +func TestBasicAddRemove(t *testing.T) { + env, ctx := setup(t) + env.runFunc() + defer env.cleanFunc() + + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolFooCRD, metav1.CreateOptions{}) + env.pollForPathExists("/apis/stable.example.com/v1/coolfoos") + s := env.fetchOpenAPIOrDie() + env.expectPath(s, "/apis/stable.example.com/v1/coolfoos") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") + + t.Logf("Removing CRD %s", coolFooCRD.Name) + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Delete(ctx, coolFooCRD.Name, metav1.DeleteOptions{}) + env.pollForPathNotExists("/apis/stable.example.com/v1/coolfoos") + s = env.fetchOpenAPIOrDie() + env.expectNoPath(s, "/apis/stable.example.com/v1/coolfoos") +} + +func TestTwoCRDsSameGroup(t *testing.T) { + env, ctx := setup(t) + env.runFunc() + defer env.cleanFunc() + + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolFooCRD, metav1.CreateOptions{}) + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolBarCRD, metav1.CreateOptions{}) + env.pollForPathExists("/apis/stable.example.com/v1/coolfoos") + env.pollForPathExists("/apis/stable.example.com/v1/coolbars") + s := env.fetchOpenAPIOrDie() + env.expectPath(s, "/apis/stable.example.com/v1/coolfoos") + env.expectPath(s, "/apis/stable.example.com/v1/coolbars") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") +} + +func TestCRDMultiVersion(t *testing.T) { + env, ctx := setup(t) + env.runFunc() + defer env.cleanFunc() + + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolMultiVersion, metav1.CreateOptions{}) + env.pollForPathExists("/apis/stable.example.com/v1/coolbars") + env.pollForPathExists("/apis/stable.example.com/v1beta1/coolbars") + s := env.fetchOpenAPIOrDie() + env.expectPath(s, "/apis/stable.example.com/v1/coolbars") + env.expectPath(s, "/apis/stable.example.com/v1beta1/coolbars") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") +} + +func TestCRDMultiVersionUpdate(t *testing.T) { + env, ctx := setup(t) + env.runFunc() + defer env.cleanFunc() + + crd, _ := env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolMultiVersion, metav1.CreateOptions{}) + env.pollForPathExists("/apis/stable.example.com/v1/coolbars") + env.pollForPathExists("/apis/stable.example.com/v1beta1/coolbars") + s := env.fetchOpenAPIOrDie() + env.expectPath(s, "/apis/stable.example.com/v1/coolbars") + env.expectPath(s, "/apis/stable.example.com/v1beta1/coolbars") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") + + t.Log("Removing version v1beta1") + crd.Spec.Versions = crd.Spec.Versions[1:] + crd.Generation += 1 + // Generation is updated before storage to etcd. Since we don't have that in the fake client, manually increase it. + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Update(ctx, crd, metav1.UpdateOptions{}) + env.pollForPathNotExists("/apis/stable.example.com/v1beta1/coolbars") + s = env.fetchOpenAPIOrDie() + env.expectPath(s, "/apis/stable.example.com/v1/coolbars") + env.expectNoPath(s, "/apis/stable.example.com/v1beta1/coolbars") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") +} + +func TestExistingCRDBeforeAPIServerStart(t *testing.T) { + env, ctx := setup(t) + defer env.cleanFunc() + + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolFooCRD, metav1.CreateOptions{}) + env.runFunc() + env.pollForPathExists("/apis/stable.example.com/v1/coolfoos") + s := env.fetchOpenAPIOrDie() + + env.expectPath(s, "/apis/stable.example.com/v1/coolfoos") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") +} + +func TestUpdate(t *testing.T) { + env, ctx := setup(t) + env.runFunc() + defer env.cleanFunc() + + crd, _ := env.Interface.ApiextensionsV1().CustomResourceDefinitions().Create(ctx, coolFooCRD, metav1.CreateOptions{}) + env.pollForPathExists("/apis/stable.example.com/v1/coolfoos") + s := env.fetchOpenAPIOrDie() + env.expectPath(s, "/apis/stable.example.com/v1/coolfoos") + env.expectPath(s, "/apis/apiextensions.k8s.io/v1") + + t.Log("Updating CRD CoolFoo") + crd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["num"] = v1.JSONSchemaProps{Type: "integer", Description: "updated description"} + crd.Generation += 1 + // Generation is updated before storage to etcd. Since we don't have that in the fake client, manually increase it. + + env.Interface.ApiextensionsV1().CustomResourceDefinitions().Update(ctx, crd, metav1.UpdateOptions{}) + env.pollForCondition(func(s *spec.Swagger) bool { + return s.Definitions["com.example.stable.v1.CoolFoo"].Properties["num"].Description == crd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["num"].Description + }) + s = env.fetchOpenAPIOrDie() + + // Ensure that description is updated + if s.Definitions["com.example.stable.v1.CoolFoo"].Properties["num"].Description != crd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["num"].Description { + t.Error("Error: Description not updated") + } + env.expectPath(s, "/apis/stable.example.com/v1/coolfoos") +} + +var coolFooCRD = &v1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "coolfoo.stable.example.com", + }, + Spec: v1.CustomResourceDefinitionSpec{ + Group: "stable.example.com", + Names: v1.CustomResourceDefinitionNames{ + Plural: "coolfoos", + Singular: "coolfoo", + ShortNames: []string{"foo"}, + Kind: "CoolFoo", + ListKind: "CoolFooList", + }, + Scope: v1.ClusterScoped, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + Storage: true, + Deprecated: false, + Subresources: &v1.CustomResourceSubresources{ + // This CRD has a /status subresource + Status: &v1.CustomResourceSubresourceStatus{}, + }, + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Type: "object", + Properties: map[string]v1.JSONSchemaProps{"num": {Type: "integer", Description: "description"}}, + }, + }, + }, + }, + Conversion: &v1.CustomResourceConversion{}, + }, + Status: v1.CustomResourceDefinitionStatus{ + Conditions: []v1.CustomResourceDefinitionCondition{ + { + Type: v1.Established, + Status: v1.ConditionTrue, + }, + }, + }, +} + +var coolBarCRD = &v1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "coolbar.stable.example.com", + }, + Spec: v1.CustomResourceDefinitionSpec{ + Group: "stable.example.com", + Names: v1.CustomResourceDefinitionNames{ + Plural: "coolbars", + Singular: "coolbar", + ShortNames: []string{"bar"}, + Kind: "CoolBar", + ListKind: "CoolBarList", + }, + Scope: v1.ClusterScoped, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + Storage: true, + Deprecated: false, + Subresources: &v1.CustomResourceSubresources{ + // This CRD has a /status subresource + Status: &v1.CustomResourceSubresourceStatus{}, + }, + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Type: "object", + Properties: map[string]v1.JSONSchemaProps{"num": {Type: "integer", Description: "description"}}, + }, + }, + }, + }, + Conversion: &v1.CustomResourceConversion{}, + }, + Status: v1.CustomResourceDefinitionStatus{ + Conditions: []v1.CustomResourceDefinitionCondition{ + { + Type: v1.Established, + Status: v1.ConditionTrue, + }, + }, + }, +} + +var coolMultiVersion = &v1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "coolbar.stable.example.com", + }, + Spec: v1.CustomResourceDefinitionSpec{ + Group: "stable.example.com", + Names: v1.CustomResourceDefinitionNames{ + Plural: "coolbars", + Singular: "coolbar", + ShortNames: []string{"bar"}, + Kind: "CoolBar", + ListKind: "CoolBarList", + }, + Scope: v1.ClusterScoped, + Versions: []v1.CustomResourceDefinitionVersion{ + { + Name: "v1beta1", + Served: true, + Storage: true, + Deprecated: false, + Subresources: &v1.CustomResourceSubresources{ + // This CRD has a /status subresource + Status: &v1.CustomResourceSubresourceStatus{}, + }, + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Type: "object", + Properties: map[string]v1.JSONSchemaProps{"num": {Type: "integer", Description: "description"}}, + }, + }, + }, + + { + Name: "v1", + Served: true, + Storage: true, + Deprecated: false, + Subresources: &v1.CustomResourceSubresources{ + // This CRD has a /status subresource + Status: &v1.CustomResourceSubresourceStatus{}, + }, + Schema: &v1.CustomResourceValidation{ + OpenAPIV3Schema: &v1.JSONSchemaProps{ + Type: "object", + Properties: map[string]v1.JSONSchemaProps{"test": {Type: "integer", Description: "foo"}}, + }, + }, + }, + }, + Conversion: &v1.CustomResourceConversion{}, + }, + Status: v1.CustomResourceDefinitionStatus{ + Conditions: []v1.CustomResourceDefinitionCondition{ + { + Type: v1.Established, + Status: v1.ConditionTrue, + }, + }, + }, +} + +type testEnv struct { + t *testing.T + clientset.Interface + mux *http.ServeMux + cleanFunc func() + runFunc func() +} + +func setup(t *testing.T) (*testEnv, context.Context) { + env := &testEnv{ + Interface: fake.NewSimpleClientset(), + t: t, + } + + factory := externalversions.NewSharedInformerFactoryWithOptions( + env.Interface, 30*time.Second) + + c := NewController(factory.Apiextensions().V1().CustomResourceDefinitions()) + ctx, cancel := context.WithCancel(context.Background()) + + factory.Start(ctx.Done()) + + env.mux = http.NewServeMux() + h := handler.NewOpenAPIService(&spec.Swagger{}) + h.RegisterOpenAPIVersionedService("/openapi/v2", env.mux) + + stopCh := make(chan struct{}) + + env.runFunc = func() { + go c.Run(&spec.Swagger{ + SwaggerProps: spec.SwaggerProps{ + Paths: &spec.Paths{ + Paths: map[string]spec.PathItem{ + "/apis/apiextensions.k8s.io/v1": {}, + }, + }, + }, + }, h, stopCh) + } + + env.cleanFunc = func() { + cancel() + close(stopCh) + } + return env, ctx +} + +func (t *testEnv) pollForCondition(conditionFunc func(*spec.Swagger) bool) { + wait.Poll(time.Second*1, wait.ForeverTestTimeout, func() (bool, error) { + openapi := t.fetchOpenAPIOrDie() + if conditionFunc(openapi) { + return true, nil + } + return false, nil + }) +} + +func (t *testEnv) pollForPathExists(path string) { + wait.Poll(time.Second*1, wait.ForeverTestTimeout, func() (bool, error) { + openapi := t.fetchOpenAPIOrDie() + if _, ok := openapi.Paths.Paths[path]; !ok { + return false, nil + } + return true, nil + }) +} + +func (t *testEnv) pollForPathNotExists(path string) { + wait.Poll(time.Second*1, wait.ForeverTestTimeout, func() (bool, error) { + openapi := t.fetchOpenAPIOrDie() + if _, ok := openapi.Paths.Paths[path]; ok { + return false, nil + } + return true, nil + }) +} + +func (t *testEnv) fetchOpenAPIOrDie() *spec.Swagger { + server := httptest.NewServer(t.mux) + defer server.Close() + client := server.Client() + + req, err := http.NewRequest("GET", server.URL+"/openapi/v2", nil) + if err != nil { + t.t.Error(err) + } + resp, err := client.Do(req) + if err != nil { + t.t.Error(err) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.t.Error(err) + } + swagger := &spec.Swagger{} + if err := swagger.UnmarshalJSON(body); err != nil { + t.t.Error(err) + } + return swagger +} + +func (t *testEnv) expectPath(swagger *spec.Swagger, path string) { + if _, ok := swagger.Paths.Paths[path]; !ok { + t.t.Errorf("Expected path %s to exist in OpenAPI", path) + } +} + +func (t *testEnv) expectNoPath(swagger *spec.Swagger, path string) { + if _, ok := swagger.Paths.Paths[path]; ok { + t.t.Errorf("Expected path %s to not exist in OpenAPI", path) + } +} diff --git a/test/integration/apiserver/openapi/openapi_crd_test.go b/test/integration/apiserver/openapi/openapi_crd_test.go new file mode 100644 index 0000000000000..7ebfa0103acab --- /dev/null +++ b/test/integration/apiserver/openapi/openapi_crd_test.go @@ -0,0 +1,112 @@ +/* +Copyright 2023 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 openapi + +import ( + "context" + "testing" + "time" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/dynamic" + kubernetes "k8s.io/client-go/kubernetes" + apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/test/integration/framework" + + "k8s.io/kube-openapi/pkg/validation/spec" +) + +func TestOpenAPICRDGenerationNumber(t *testing.T) { + server, err := apiservertesting.StartTestServer(t, apiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd()) + if err != nil { + t.Fatal(err) + } + defer server.TearDownFn() + config := server.ClientConfig + + apiExtensionClient, err := clientset.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + // Create a new CRD with group mygroup.example.com + crd := fixtures.NewRandomNameV1CustomResourceDefinition(apiextensionsv1.NamespaceScoped) + _, err = fixtures.CreateNewV1CustomResourceDefinition(crd, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + + defer func() { + apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Delete(context.TODO(), crd.Name, metav1.DeleteOptions{}) + }() + + crd, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), crd.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + // Update OpenAPI schema and ensure it's reflected in the publishing + crd.Spec.Versions[0].Schema = &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{"num": {Type: "integer", Description: "description"}}, + }, + } + + updatedCRD, err := apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{}) + if err != nil { + t.Fatal(err) + } + if updatedCRD.Generation <= crd.Generation { + t.Fatalf("Expected updated CRD to increment Generation counter. got preupdate: %d, postupdate %d", crd.Generation, updatedCRD.Generation) + } + + err = wait.Poll(time.Second*1, wait.ForeverTestTimeout, func() (bool, error) { + body, err := clientset.RESTClient().Get().AbsPath("/openapi/v2").Do(context.TODO()).Raw() + if err != nil { + t.Fatal(err) + } + swagger := &spec.Swagger{} + if err := swagger.UnmarshalJSON(body); err != nil { + t.Error(err) + } + + // Ensure that OpenAPI schema updated is reflected + if description := swagger.Definitions["com.example.mygroup.v1beta1."+crd.Spec.Names.Kind].Properties["num"].Description; description == "description" { + return true, nil + } + return false, nil + }) + + if err != nil { + t.Errorf("Expected description to be updated, err: %s", err) + } + +}