From 272cda7e7d23d36152c72a0cdf0f0c11f6afabaf Mon Sep 17 00:00:00 2001 From: David Festal Date: Wed, 10 Mar 2021 15:54:45 +0100 Subject: [PATCH 1/3] HACK: Enable adding legacy scheme resources as CRDs... This may be useful in case when some legacy scheme resources are not registered, which is the case for example in KCP. There has to be special cases for `core` resources that have an empty group and are at a dedicated api prefix. This doesn't fully work with `kubectl` or kubernetes client, due to the fact that those client tools sustematically use strategic merge patch of legacy scheme resources, and CRDs don't support strategic merge patch. The fix for this limitation will be in a next commit. Signed-off-by: David Festal --- pkg/controlplane/server.go | 10 ++++ .../apiextensions/validation/validation.go | 15 ++++- .../pkg/apiserver/apiserver.go | 2 + .../customresource_discovery_controller.go | 60 +++++++++++++------ .../pkg/apiserver/customresource_handler.go | 16 ++++- .../pkg/controller/openapi/builder/builder.go | 40 ++++++++++--- .../controller/status/naming_controller.go | 4 ++ .../pkg/registry/customresource/validator.go | 7 ++- .../pkg/endpoints/discovery/version.go | 48 ++++++++++++++- 9 files changed, 170 insertions(+), 32 deletions(-) diff --git a/pkg/controlplane/server.go b/pkg/controlplane/server.go index d2870edfc9478..d2e19ea07f0d8 100644 --- a/pkg/controlplane/server.go +++ b/pkg/controlplane/server.go @@ -27,11 +27,13 @@ import ( "strings" "time" + "github.com/emicklei/go-restful" extensionsapiserver "k8s.io/apiextensions-apiserver/pkg/apiserver" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/authorization/union" + "k8s.io/apiserver/pkg/endpoints/discovery" openapinamer "k8s.io/apiserver/pkg/endpoints/openapi" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" genericapiserver "k8s.io/apiserver/pkg/server" @@ -112,6 +114,14 @@ func CreateServerChain(completedOptions completedServerRunOptions, stopCh <-chan return nil, err } + kubeAPIServer.GenericAPIServer.Handler.GoRestfulContainer.Filter(func(req *restful.Request, res *restful.Response, chain *restful.FilterChain){ + if discovery.IsAPIContributed(req.Request.URL.Path) { + apiExtensionsServer.GenericAPIServer.Handler.NonGoRestfulMux.ServeHTTP(res.ResponseWriter, req.Request) + } else { + chain.ProcessFilter(req, res) + } + }) + return aggregatorServer, nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 82e4e9d15dd43..f442734b89692 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -30,6 +30,7 @@ import ( utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/util/webhook" + "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -48,7 +49,11 @@ var ( func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList { nameValidationFn := func(name string, prefix bool) []string { ret := genericvalidation.NameIsDNSSubdomain(name, prefix) - requiredName := obj.Spec.Names.Plural + "." + obj.Spec.Group + group := obj.Spec.Group + if group == "" { + group = "core" + } + requiredName := obj.Spec.Names.Plural + "." + group if name != requiredName { ret = append(ret, fmt.Sprintf(`must be spec.names.plural+"."+spec.group`)) } @@ -171,8 +176,12 @@ func validateCustomResourceDefinitionVersion(version *apiextensions.CustomResour func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, opts validationOptions, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if len(spec.Group) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("group"), "")) + // HACK: Relax naming constraints when registering legacy schema resources through CRDs + // for the KCP scenario + if legacyscheme.Scheme.IsGroupRegistered(spec.Group) { + // No error: these are legacy schema kubernetes types + // that are not added in the controlplane schema + // and that we want to move up to the KCP as CRDs } else if errs := utilvalidation.IsDNS1123Subdomain(spec.Group); len(errs) > 0 { allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), spec.Group, strings.Join(errs, ","))) } else if len(strings.Split(spec.Group, ".")) < 2 { 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 390b5a4471b00..d32b292f9538b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go @@ -206,6 +206,8 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) } s.GenericAPIServer.Handler.NonGoRestfulMux.Handle("/apis", crdHandler) s.GenericAPIServer.Handler.NonGoRestfulMux.HandlePrefix("/apis/", crdHandler) + // HACK: Added to allow serving core resources registered through CRDs (for the KCP scenario) + s.GenericAPIServer.Handler.NonGoRestfulMux.HandlePrefix("/api/v1/", crdHandler) crdController := NewDiscoveryController(s.Informers.Apiextensions().V1().CustomResourceDefinitions(), versionDiscoveryHandler, groupDiscoveryHandler) namingController := status.NewNamingConditionController(s.Informers.Apiextensions().V1().CustomResourceDefinitions(), crdClient.ApiextensionsV1()) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller.go index f897abd9ac50a..4f23b51581e8a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller.go @@ -22,6 +22,7 @@ import ( "time" "k8s.io/klog" + "k8s.io/kubernetes/pkg/api/legacyscheme" autoscaling "k8s.io/api/autoscaling/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -104,11 +105,18 @@ func (c *DiscoveryController) sync(version schema.GroupVersion) error { // If there is any Served version, that means the group should show up in discovery foundGroup = true + // HACK: support the case when we add core resources through CRDs (KCP scenario) + groupVersion := crd.Spec.Group + "/" + v.Name + if crd.Spec.Group == "" { + groupVersion = v.Name + } + gv := metav1.GroupVersion{Group: crd.Spec.Group, Version: v.Name} + if !versionsForDiscoveryMap[gv] { versionsForDiscoveryMap[gv] = true apiVersionsForDiscovery = append(apiVersionsForDiscovery, metav1.GroupVersionForDiscovery{ - GroupVersion: crd.Spec.Group + "/" + v.Name, + GroupVersion: groupVersion, Version: v.Name, }) } @@ -167,30 +175,46 @@ func (c *DiscoveryController) sync(version schema.GroupVersion) error { } } - if !foundGroup { - c.groupHandler.unsetDiscovery(version.Group) - c.versionHandler.unsetDiscovery(version) - return nil - } - sortGroupDiscoveryByKubeAwareVersion(apiVersionsForDiscovery) - apiGroup := metav1.APIGroup{ - Name: version.Group, - Versions: apiVersionsForDiscovery, - // the preferred versions for a group is the first item in - // apiVersionsForDiscovery after it put in the right ordered - PreferredVersion: apiVersionsForDiscovery[0], + resourceListerFunc := discovery.APIResourceListerFunc(func() []metav1.APIResource { + return apiResourcesForDiscovery + }) + + // HACK: if we are adding resources in legacy scheme group through CRDs (KCP scenario) + // then do not expose the CRD `APIResource`s in their own CRD-related group`, + // But instead add them in the existing legacy schema group + if legacyscheme.Scheme.IsGroupRegistered(version.Group) { + if !foundGroup || !foundVersion{ + delete(discovery.ContributedResources, version) + } + + discovery.ContributedResources[version] = resourceListerFunc } - c.groupHandler.setDiscovery(version.Group, discovery.NewAPIGroupHandler(Codecs, apiGroup)) - if !foundVersion { + if !foundGroup { + c.groupHandler.unsetDiscovery(version.Group) c.versionHandler.unsetDiscovery(version) return nil } - c.versionHandler.setDiscovery(version, discovery.NewAPIVersionHandler(Codecs, version, discovery.APIResourceListerFunc(func() []metav1.APIResource { - return apiResourcesForDiscovery - }))) + + if version.Group != "" { + // If we don't add resources in the core API group + apiGroup := metav1.APIGroup{ + Name: version.Group, + Versions: apiVersionsForDiscovery, + // the preferred versions for a group is the first item in + // apiVersionsForDiscovery after it put in the right ordered + PreferredVersion: apiVersionsForDiscovery[0], + } + c.groupHandler.setDiscovery(version.Group, discovery.NewAPIGroupHandler(Codecs, apiGroup)) + + if !foundVersion { + c.versionHandler.unsetDiscovery(version) + return nil + } + c.versionHandler.setDiscovery(version, discovery.NewAPIVersionHandler(Codecs, version, resourceListerFunc)) + } return nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index fcaa954c9425c..3e568379323a3 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -46,6 +46,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/crdserverscheme" "k8s.io/apiextensions-apiserver/pkg/registry/customresource" "k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor" + "k8s.io/kubernetes/pkg/api/legacyscheme" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -257,6 +258,10 @@ func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } crdName := requestInfo.Resource + "." + requestInfo.APIGroup + // HACK: support the case when we add core resources through CRDs (KCP scenario) + if requestInfo.APIGroup == "" { + crdName = crdName + "core" + } crd, err := r.crdLister.Get(crdName) if apierrors.IsNotFound(err) { if !r.hasSynced() { @@ -331,6 +336,9 @@ func (r *crdHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { string(types.JSONPatchType), string(types.MergePatchType), } + if legacyscheme.Scheme.IsGroupRegistered(requestInfo.APIGroup) { + supportedTypes = append(supportedTypes, string(types.StrategicMergePatchType)) + } if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { supportedTypes = append(supportedTypes, string(types.ApplyPatchType)) } @@ -764,12 +772,16 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd table, ) + selfLinkPrefixPrefix := path.Join("apis", crd.Spec.Group, v.Name) + if crd.Spec.Group == "" { + selfLinkPrefixPrefix = path.Join("api", v.Name) + } selfLinkPrefix := "" switch crd.Spec.Scope { case apiextensionsv1.ClusterScoped: - selfLinkPrefix = "/" + path.Join("apis", crd.Spec.Group, v.Name) + "/" + crd.Status.AcceptedNames.Plural + "/" + selfLinkPrefix = "/" + selfLinkPrefixPrefix + "/" + crd.Status.AcceptedNames.Plural + "/" case apiextensionsv1.NamespaceScoped: - selfLinkPrefix = "/" + path.Join("apis", crd.Spec.Group, v.Name, "namespaces") + "/" + selfLinkPrefix = "/" + selfLinkPrefixPrefix + "/namespaces/" } clusterScoped := crd.Spec.Scope == apiextensionsv1.ClusterScoped diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go index 06dfc7f1add22..32d014eb08f73 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go @@ -45,6 +45,7 @@ import ( openapibuilder "k8s.io/kube-openapi/pkg/builder" "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/util" + "k8s.io/kubernetes/pkg/api/legacyscheme" ) const ( @@ -142,11 +143,17 @@ func BuildSwagger(crd *apiextensionsv1.CustomResourceDefinition, version string, scale := &v1.Scale{} routes := make([]*restful.RouteBuilder, 0) - root := fmt.Sprintf("/apis/%s/%s/%s", b.group, b.version, b.plural) + // HACK: support the case when we add core resources through CRDs (KCP scenario) + rootPrefix := fmt.Sprintf("/apis/%s/%s", b.group, b.version) + if b.group == "" { + rootPrefix = fmt.Sprintf("/api/%s", b.version) + } + + root := fmt.Sprintf("%s/%s", rootPrefix, b.plural) if b.namespaced { routes = append(routes, b.buildRoute(root, "", "GET", "list", "list", sampleList).Operation("list"+b.kind+"ForAllNamespaces")) - root = fmt.Sprintf("/apis/%s/%s/namespaces/{namespace}/%s", b.group, b.version, b.plural) + root = fmt.Sprintf("%s/namespaces/{namespace}/%s", rootPrefix, b.plural) } routes = append(routes, b.buildRoute(root, "", "GET", "list", "list", sampleList)) routes = append(routes, b.buildRoute(root, "", "POST", "post", "create", sample).Reads(sample)) @@ -195,9 +202,21 @@ type CRDCanonicalTypeNamer struct { kind string } +// HACK: support the case when we add core or other legacy scheme resources through CRDs (KCP scenario) +func packagePrefix(group string) string { + if !strings.Contains(group, ".") && + legacyscheme.Scheme.IsGroupRegistered(group) { + if group == "" { + group = "core" + } + return "k8s.io/api/" + group + } + return group +} + // OpenAPICanonicalTypeName returns canonical type name for given CRD func (c *CRDCanonicalTypeNamer) OpenAPICanonicalTypeName() string { - return fmt.Sprintf("%s/%s.%s", c.group, c.version, c.kind) + return fmt.Sprintf("%s/%s.%s", packagePrefix(c.group), c.version, c.kind) } // builder contains validation schema and basic naming information for a CRD in @@ -452,7 +471,7 @@ func addTypeMetaProperties(s *spec.Schema) { // buildListSchema builds the list kind schema for the CRD func (b *builder) buildListSchema() *spec.Schema { - name := definitionPrefix + util.ToRESTFriendlyName(fmt.Sprintf("%s/%s/%s", b.group, b.version, b.kind)) + name := definitionPrefix + util.ToRESTFriendlyName(fmt.Sprintf("%s/%s/%s", packagePrefix(b.group), b.version, b.kind)) doc := fmt.Sprintf("List of %s. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md", b.plural) s := new(spec.Schema).WithDescription(fmt.Sprintf("%s is a list of %s", b.listKind, b.kind)). WithRequired("items"). @@ -489,14 +508,19 @@ func (b *builder) getOpenAPIConfig() *common.Config { GetOperationIDAndTags: openapi.GetOperationIDAndTags, GetDefinitionName: func(name string) (string, spec.Extensions) { buildDefinitions.Do(buildDefinitionsFunc) + // HACK: support the case when we add core or other legacy scheme resources through CRDs (KCP scenario) + parts := strings.Split(name, "/") + if len(parts) == 2 { + name = packagePrefix(parts[0]) + } return namer.GetDefinitionName(name) }, GetDefinitions: func(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { def := generatedopenapi.GetOpenAPIDefinitions(ref) - def[fmt.Sprintf("%s/%s.%s", b.group, b.version, b.kind)] = common.OpenAPIDefinition{ + def[fmt.Sprintf("%s/%s.%s", packagePrefix(b.group), b.version, b.kind)] = common.OpenAPIDefinition{ Schema: *b.schema, } - def[fmt.Sprintf("%s/%s.%s", b.group, b.version, b.listKind)] = common.OpenAPIDefinition{ + def[fmt.Sprintf("%s/%s.%s", packagePrefix(b.group), b.version, b.listKind)] = common.OpenAPIDefinition{ Schema: *b.listSchema, } return def @@ -505,6 +529,8 @@ func (b *builder) getOpenAPIConfig() *common.Config { } func newBuilder(crd *apiextensionsv1.CustomResourceDefinition, version string, schema *structuralschema.Structural, v2 bool) *builder { + group := crd.Spec.Group + // HACK: support the case when we add core resources through CRDs (KCP scenario) b := &builder{ schema: &spec.Schema{ SchemaProps: spec.SchemaProps{Type: []string{"object"}}, @@ -512,7 +538,7 @@ func newBuilder(crd *apiextensionsv1.CustomResourceDefinition, version string, s listSchema: &spec.Schema{}, ws: &restful.WebService{}, - group: crd.Spec.Group, + group: group, version: version, kind: crd.Spec.Names.Kind, listKind: crd.Spec.Names.ListKind, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller.go index d621f8474888c..822c39874b0af 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller.go @@ -368,6 +368,10 @@ func (c *NamingConditionController) deleteCustomResourceDefinition(obj interface func (c *NamingConditionController) requeueAllOtherGroupCRDs(name string) error { pluralGroup := strings.SplitN(name, ".", 2) + // In case the group is empty because we're adding core resources as CRDs in KCP + if len(pluralGroup) == 1 { + pluralGroup = append(pluralGroup, "") + } list, err := c.crdLister.List(labels.Everything()) if err != nil { return err diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go index 6d89e1bbe24df..27458318e2a98 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go @@ -131,7 +131,12 @@ func (a customResourceValidator) ValidateTypeMeta(ctx context.Context, obj *unst if typeAccessor.GetKind() != a.kind.Kind { allErrs = append(allErrs, field.Invalid(field.NewPath("kind"), typeAccessor.GetKind(), fmt.Sprintf("must be %v", a.kind.Kind))) } - if typeAccessor.GetAPIVersion() != a.kind.Group+"/"+a.kind.Version { + // HACK: support the case when we add core resources through CRDs (KCP scenario) + expectedAPIVersion := a.kind.Group+"/"+a.kind.Version + if a.kind.Group == "" { + expectedAPIVersion = a.kind.Version + } + if typeAccessor.GetAPIVersion() != expectedAPIVersion { allErrs = append(allErrs, field.Invalid(field.NewPath("apiVersion"), typeAccessor.GetAPIVersion(), fmt.Sprintf("must be %v", a.kind.Group+"/"+a.kind.Version))) } return allErrs diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/version.go b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/version.go index 0976041bff0e0..544992ae45d4b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/version.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/discovery/version.go @@ -18,6 +18,9 @@ package discovery import ( "net/http" + "regexp" + "sort" + "strings" restful "github.com/emicklei/go-restful" @@ -47,6 +50,49 @@ type APIVersionHandler struct { apiResourceLister APIResourceLister } +// HACK: support the case when we can add core or other legacy scheme resources through CRDs (KCP scenario) +var ContributedResources map[schema.GroupVersion]APIResourceLister = map[schema.GroupVersion]APIResourceLister{} + +func withContributedResources(groupVersion schema.GroupVersion, apiResourceLister APIResourceLister) APIResourceLister { + return APIResourceListerFunc(func() []metav1.APIResource { + result := apiResourceLister.ListAPIResources() + if additionalResources := ContributedResources[groupVersion]; additionalResources != nil { + result = append(result, additionalResources.ListAPIResources()...) + } + sort.Slice(result, func(i, j int) bool { + return result[i].Name < result[j].Name + }) + + return result + }) +} + +func IsAPIContributed(path string) bool { + for gv, resourceLister := range ContributedResources { + prefix := gv.Group + if prefix != "" { + prefix = "/apis/" + prefix + "/" + gv.Version + "/" + } else { + prefix = "/api/" + gv.Version + "/" + } + if !strings.HasPrefix(path, prefix) { + return false + } + + for _, resource := range resourceLister.ListAPIResources() { + if strings.HasPrefix(path, prefix+resource.Name) { + return true + } + if resource.Namespaced { + if matched, _ := regexp.MatchString(prefix+"namespaces/[^/][^/]*/"+resource.Name+"(/[^/].*)?", path); matched { + return true + } + } + } + } + return false +} + func NewAPIVersionHandler(serializer runtime.NegotiatedSerializer, groupVersion schema.GroupVersion, apiResourceLister APIResourceLister) *APIVersionHandler { if keepUnversioned(groupVersion.Group) { // Because in release 1.1, /apis/extensions returns response with empty @@ -58,7 +104,7 @@ func NewAPIVersionHandler(serializer runtime.NegotiatedSerializer, groupVersion return &APIVersionHandler{ serializer: serializer, groupVersion: groupVersion, - apiResourceLister: apiResourceLister, + apiResourceLister: withContributedResources(groupVersion, apiResourceLister), } } From 8bd076f4a4d812524d8ffb3342dbbbdb638a74d1 Mon Sep 17 00:00:00 2001 From: David Festal Date: Tue, 30 Mar 2021 14:31:17 +0200 Subject: [PATCH 2/3] KUBEFIX: Add strategic merge patch support to CRDs ... ... based on CRD published openapi v2 definitions --- .../pkg/apiserver/customresource_handler.go | 7 ++ .../pkg/apiserver/schema/goopenapi.go | 10 +++ .../apiserver/pkg/endpoints/handlers/patch.go | 18 ++++- .../apiserver/pkg/endpoints/handlers/rest.go | 3 + .../pkg/endpoints/openapi/openapi.go | 75 +++++++++++++++++++ 5 files changed, 111 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 3e568379323a3..9b1bd2f9b13b7 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -70,6 +70,7 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" "k8s.io/apiserver/pkg/endpoints/metrics" + "k8s.io/apiserver/pkg/endpoints/openapi" apirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/generic" @@ -803,6 +804,10 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd standardSerializers = append(standardSerializers, s) } + modelsByGKV, err := openapi.GetModelsByGKV(openAPIModels) + if err != nil { + klog.V(2).Infof("The CRD cannot gather openapi models by GKV: %v", err) + } requestScopes[v.Name] = &handlers.RequestScope{ Namer: handlers.ContextBasedNaming{ SelfLinker: meta.NewAccessor(), @@ -834,6 +839,8 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd Authorizer: r.authorizer, MaxRequestBodyBytes: r.maxRequestBodyBytes, + + OpenapiModels: modelsByGKV, } if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { reqScope := *requestScopes[v.Name] diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go index 607489c718a51..7139282e2a335 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go @@ -80,12 +80,22 @@ func (x *Extensions) toGoOpenAPI(ret *spec.Schema) { } if len(x.XListMapKeys) > 0 { ret.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", x.XListMapKeys) + ret.VendorExtensible.AddExtension("x-kubernetes-patch-merge-key", x.XListMapKeys[0]) } if x.XListType != nil { ret.VendorExtensible.AddExtension("x-kubernetes-list-type", *x.XListType) + if *x.XListType == "map" || *x.XListType == "set" { + ret.VendorExtensible.AddExtension("x-kubernetes-patch-strategy", "merge") + } + if *x.XListType == "atomic" { + ret.VendorExtensible.AddExtension("x-kubernetes-patch-strategy", "replace") + } } if x.XMapType != nil { ret.VendorExtensible.AddExtension("x-kubernetes-map-type", *x.XMapType) + if *x.XMapType == "atomic" { + ret.VendorExtensible.AddExtension("x-kubernetes-patch-strategy", "replace") + } } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index c295d0aa65933..ae96403648b3f 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -48,6 +48,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kube-openapi/pkg/util/proto" utiltrace "k8s.io/utils/trace" "sigs.k8s.io/yaml" ) @@ -383,6 +384,7 @@ type smpPatcher struct { // Schema schemaReferenceObj runtime.Object fieldManager *fieldmanager.FieldManager + openapiModel proto.Schema } func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (runtime.Object, error) { @@ -396,7 +398,7 @@ func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (ru if err != nil { return nil, err } - if err := strategicPatchObject(p.defaulter, currentVersionedObject, p.patchBytes, versionedObjToUpdate, p.schemaReferenceObj); err != nil { + if err := strategicPatchObject(p.defaulter, currentVersionedObject, p.patchBytes, versionedObjToUpdate, p.schemaReferenceObj, p.openapiModel); err != nil { return nil, err } // Convert the object back to the hub version @@ -459,6 +461,7 @@ func strategicPatchObject( patchBytes []byte, objToUpdate runtime.Object, schemaReferenceObj runtime.Object, + openapiModel proto.Schema, ) error { originalObjMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(originalObject) if err != nil { @@ -470,7 +473,7 @@ func strategicPatchObject( return errors.NewBadRequest(err.Error()) } - if err := applyPatchToObject(defaulter, originalObjMap, patchMap, objToUpdate, schemaReferenceObj); err != nil { + if err := applyPatchToObject(defaulter, originalObjMap, patchMap, objToUpdate, schemaReferenceObj, openapiModel); err != nil { return err } return nil @@ -556,10 +559,17 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti if err != nil { return nil, false, err } + + var schema proto.Schema + modelsByGKV := scope.OpenapiModels + if modelsByGKV != nil { + schema = modelsByGKV[p.kind] + } p.mechanism = &smpPatcher{ patcher: p, schemaReferenceObj: schemaReferenceObj, fieldManager: scope.FieldManager, + openapiModel: schema, } // this case is unreachable if ServerSideApply is not enabled because we will have already rejected the content type case types.ApplyPatchType: @@ -612,8 +622,12 @@ func applyPatchToObject( patchMap map[string]interface{}, objToUpdate runtime.Object, schemaReferenceObj runtime.Object, + openapiModel proto.Schema, ) error { patchedObjMap, err := strategicpatch.StrategicMergeMapPatch(originalMap, patchMap, schemaReferenceObj) + if err == mergepatch.ErrUnsupportedStrategicMergePatchFormat && openapiModel !=nil { + patchedObjMap, err = strategicpatch.StrategicMergeMapPatchUsingLookupPatchMeta(originalMap, patchMap, strategicpatch.NewPatchMetaFromOpenAPI(openapiModel)) + } if err != nil { return interpretStrategicMergePatchError(err) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 97471637e56c8..26399f626a846 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -42,6 +42,7 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" "k8s.io/apiserver/pkg/endpoints/metrics" + "k8s.io/apiserver/pkg/endpoints/openapi" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" @@ -83,6 +84,8 @@ type RequestScope struct { HubGroupVersion schema.GroupVersion MaxRequestBodyBytes int64 + + OpenapiModels openapi.ModelsByGKV } func (scope *RequestScope) err(err error, w http.ResponseWriter, req *http.Request) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/openapi/openapi.go b/staging/src/k8s.io/apiserver/pkg/endpoints/openapi/openapi.go index e3bd028bbf918..56d0d50519054 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/openapi/openapi.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/openapi/openapi.go @@ -18,6 +18,7 @@ package openapi import ( "bytes" + "errors" "fmt" "reflect" "sort" @@ -31,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/kube-openapi/pkg/util" + "k8s.io/kube-openapi/pkg/util/proto" ) var verbs = util.NewTrie([]string{"get", "log", "read", "replace", "patch", "delete", "deletecollection", "watch", "connect", "proxy", "list", "create", "patch"}) @@ -189,3 +191,76 @@ func (d *DefinitionNamer) GetDefinitionName(name string) (string, spec.Extension } return friendlyName(name), nil } + +type ModelsByGKV map[schema.GroupVersionKind]proto.Schema + +// NewOpenAPIData creates a new `Resources` out of the openapi models +func GetModelsByGKV(models proto.Models) (ModelsByGKV, error) { + result := map[schema.GroupVersionKind]proto.Schema{} + for _, modelName := range models.ListModels() { + model := models.LookupModel(modelName) + if model == nil { + return map[schema.GroupVersionKind]proto.Schema{}, errors.New("ListModels returns a model that can't be looked-up.") + } + gvkList := parseGroupVersionKind(model) + for _, gvk := range gvkList { + if len(gvk.Kind) > 0 { + key := schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind} + if key.Group == "core" { + key.Group = "" + } + result[key] = model + } + } + } + + return result, nil +} + +// Get and parse GroupVersionKind from the extension. Returns empty if it doesn't have one. +func parseGroupVersionKind(s proto.Schema) []schema.GroupVersionKind { + extensions := s.GetExtensions() + + gvkListResult := []schema.GroupVersionKind{} + + // Get the extensions + gvkExtension, ok := extensions[extensionGVK] + if !ok { + return []schema.GroupVersionKind{} + } + + // gvk extension must be a list of at least 1 element. + gvkList, ok := gvkExtension.([]interface{}) + if !ok { + return []schema.GroupVersionKind{} + } + + for _, gvk := range gvkList { + // gvk extension list must be a map with group, version, and + // kind fields + gvkMap, ok := gvk.(map[interface{}]interface{}) + if !ok { + continue + } + group, ok := gvkMap["group"].(string) + if !ok { + continue + } + version, ok := gvkMap["version"].(string) + if !ok { + continue + } + kind, ok := gvkMap["kind"].(string) + if !ok { + continue + } + + gvkListResult = append(gvkListResult, schema.GroupVersionKind{ + Group: group, + Version: version, + Kind: kind, + }) + } + + return gvkListResult +} From c3b3ce0ac05d9d840523069fe4c5202d1efc3056 Mon Sep 17 00:00:00 2001 From: David Festal Date: Tue, 30 Mar 2021 14:16:59 +0200 Subject: [PATCH 3/3] WORKAROUND: Reintegrate minimal part of PR kubernetes/kubernetes#97172 ... to bypass issue kubernetes/kubernetes#85127 until we possibly rebase this branch to get the real fix. Signed-off-by: David Festal --- .../pkg/apiserver/customresource_handler.go | 2 +- .../pkg/controller/openapi/builder/builder.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 9b1bd2f9b13b7..4d91f6c22b1bd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -1266,7 +1266,7 @@ func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensi specs := []*spec.Swagger{} for _, v := range crd.Spec.Versions { - s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true, StripNullable: true, AllowNonStructural: true}) + s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true, StripNullable: true, AllowNonStructural: false}) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go index 32d014eb08f73..e023846e3cae0 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go @@ -105,6 +105,9 @@ func BuildSwagger(crd *apiextensionsv1.CustomResourceDefinition, version string, if opts.AllowNonStructural || len(structuralschema.ValidateStructural(nil, ss)) == 0 { schema = ss + // This adds ValueValidation fields (anyOf, allOf) which may be stripped below if opts.StripValueValidation is true + schema = schema.Unfold() + if opts.StripDefaults { schema = schema.StripDefaults() } @@ -114,8 +117,6 @@ func BuildSwagger(crd *apiextensionsv1.CustomResourceDefinition, version string, if opts.StripNullable { schema = schema.StripNullable() } - - schema = schema.Unfold() } } }