From 81e00f0b7bcdfdb64936020f6b182f44edf89946 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Wed, 28 Aug 2019 13:33:49 -0700 Subject: [PATCH] apiextensions: merge openapi spec ignore path conflict --- .../pkg/controller/openapi/builder/merge.go | 19 ++++++++++++++----- .../pkg/controller/openapi/controller.go | 6 +++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/merge.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/merge.go index f2cc8135b51..65324947d5b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/merge.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/merge.go @@ -18,12 +18,13 @@ package builder import ( "github.com/go-openapi/spec" + "k8s.io/kube-openapi/pkg/aggregator" ) // MergeSpecs aggregates all OpenAPI specs, reusing the metadata of the first, static spec as the basis. -// Later paths and definitions override earlier ones. None of the input is mutated, but input -// and output share data structures. -func MergeSpecs(staticSpec *spec.Swagger, crdSpecs ...*spec.Swagger) *spec.Swagger { +// The static spec has the highest priority, and its paths and definitions won't get overlapped by +// user-defined CRDs. None of the input is mutated, but input and output share data structures. +func MergeSpecs(staticSpec *spec.Swagger, crdSpecs ...*spec.Swagger) (*spec.Swagger, error) { // create shallow copy of staticSpec, but replace paths and definitions because we modify them. specToReturn := *staticSpec if staticSpec.Definitions != nil { @@ -41,11 +42,19 @@ func MergeSpecs(staticSpec *spec.Swagger, crdSpecs ...*spec.Swagger) *spec.Swagg } } + crdSpec := &spec.Swagger{} for _, s := range crdSpecs { - mergeSpec(&specToReturn, s) + // merge specs without checking conflicts, since the naming controller prevents + // conflicts between user-defined CRDs + mergeSpec(crdSpec, s) } - return &specToReturn + // The static spec has the highest priority. Resolve conflicts to prevent user-defined + // CRDs potentially overlapping the built-in apiextensions API + if err := aggregator.MergeSpecsIgnorePathConflict(&specToReturn, crdSpec); err != nil { + return nil, err + } + return &specToReturn, nil } // mergeSpec copies paths and definitions from source to dest, mutating dest, but not source. 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 4d54b411f3d..a192add33a3 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 @@ -218,7 +218,11 @@ func (c *Controller) updateSpecLocked() error { crdSpecs = append(crdSpecs, s) } } - return c.openAPIService.UpdateSpec(builder.MergeSpecs(c.staticSpec, crdSpecs...)) + mergedSpec, err := builder.MergeSpecs(c.staticSpec, crdSpecs...) + if err != nil { + return fmt.Errorf("failed to merge specs: %v", err) + } + return c.openAPIService.UpdateSpec(mergedSpec) } func (c *Controller) addCustomResourceDefinition(obj interface{}) {