From 2285d3569ccbf443c9d087fe7565c8c789c11f45 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 15 May 2025 18:05:41 +0200 Subject: [PATCH 01/26] prepare to publish more than 1 version of a given GVK in a PublishedResource On-behalf-of: @SAP christoph.mewes@sap.com --- .../syncagent/v1alpha1/published_resource.go | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index 613924c..9c7b870 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -255,16 +255,19 @@ type TemplateExpression struct { Template string `json:"template,omitempty"` } -// SourceResourceDescriptor and ResourceProjection are very similar, but as we do not -// want to burden service clusters with validation webhooks, it's easier to split them -// into 2 structs here and rely on the schema for validation. - // SourceResourceDescriptor uniquely describes a resource type in the cluster. type SourceResourceDescriptor struct { // The API group of a resource, for example "storage.initroid.com". APIGroup string `json:"apiGroup"` - // The API version, for example "v1beta1". - Version string `json:"version"` + // The API version, for example "v1beta1". Setting this field will only publish + // the given version, otherwise all versions for the group/kind will be + // published. + // + // Deprecated: Use .versions instead. + Version string `json:"version,omitempty"` + // Versions allows to select a subset of versions to publish. Leave empty + // to publish all available versions. + Versions []string `json:"versions,omitempty"` // The resource Kind, for example "Database". Kind string `json:"kind"` } @@ -282,10 +285,17 @@ const ( // ResourceProjection describes how the source GVK should be modified before it's published in kcp. type ResourceProjection struct { - // The API group, for example "myservice.example.com". + // The API group, for example "myservice.example.com". Leave empty to not modify the API group. Group string `json:"group,omitempty"` - // The API version, for example "v1beta1". + // The API version, for example "v1beta1". Leave empty to not modify the version. + // + // This field must not be set when multiple versions have been selected. + // + // Deprecated: Use .versions instead. Version string `json:"version,omitempty"` + // Versions allows to map API versions onto new values in kcp. Leave empty to not modify the + // versions. + Versions []VersionProjection `json:"versions,omitempty"` // Whether or not the resource is namespaced. // +kubebuilder:validation:Enum=Cluster;Namespaced Scope ResourceScope `json:"scope,omitempty"` @@ -309,6 +319,11 @@ type ResourceProjection struct { Categories []string `json:"categories"` // not omitempty because we need to distinguish between [] and nil } +type VersionProjection struct { + From string `json:"from"` + To string `json:"to"` +} + // ResourceFilter can be used to limit what resources should be included in an operation. type ResourceFilter struct { // When given, the namespace filter will be applied to a resource's namespace. From 1650e9d2037459b14735be1fc166a8d4545ef781 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 15 May 2025 18:07:31 +0200 Subject: [PATCH 02/26] codegen On-behalf-of: @SAP christoph.mewes@sap.com --- .../syncagent.kcp.io_publishedresources.yaml | 39 +++++++++++++-- .../v1alpha1/zz_generated.deepcopy.go | 27 ++++++++++- .../syncagent/v1alpha1/resourceprojection.go | 32 +++++++++---- .../v1alpha1/sourceresourcedescriptor.go | 17 +++++-- .../syncagent/v1alpha1/versionprojection.go | 48 +++++++++++++++++++ sdk/applyconfiguration/utils.go | 2 + 6 files changed, 148 insertions(+), 17 deletions(-) create mode 100644 sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index 41b3ec6..427314b 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -286,7 +286,7 @@ spec: type: string type: array group: - description: The API group, for example "myservice.example.com". + description: The API group, for example "myservice.example.com". Leave empty to not modify the API group. type: string kind: description: |- @@ -316,8 +316,28 @@ spec: type: string type: array version: - description: The API version, for example "v1beta1". + description: |- + The API version, for example "v1beta1". Leave empty to not modify the version. + + This field must not be set when multiple versions have been selected. + + Deprecated: Use .versions instead. type: string + versions: + description: |- + Versions allows to map API versions onto new values in kcp. Leave empty to not modify the + versions. + items: + properties: + from: + type: string + to: + type: string + required: + - from + - to + type: object + type: array type: object related: items: @@ -674,12 +694,23 @@ spec: description: The resource Kind, for example "Database". type: string version: - description: The API version, for example "v1beta1". + description: |- + The API version, for example "v1beta1". Setting this field will only publish + the given version, otherwise all versions for the group/kind will be + published. + + Deprecated: Use .versions instead. type: string + versions: + description: |- + Versions allows to select a subset of versions to publish. Leave empty + to publish all available versions. + items: + type: string + type: array required: - apiGroup - kind - - version type: object required: - resource diff --git a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go index 4f0738b..3ab7c1d 100644 --- a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go +++ b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go @@ -87,7 +87,7 @@ func (in *PublishedResourceList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PublishedResourceSpec) DeepCopyInto(out *PublishedResourceSpec) { *out = *in - out.Resource = in.Resource + in.Resource.DeepCopyInto(&out.Resource) if in.Filter != nil { in, out := &in.Filter, &out.Filter *out = new(ResourceFilter) @@ -408,6 +408,11 @@ func (in *ResourceNaming) DeepCopy() *ResourceNaming { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResourceProjection) DeepCopyInto(out *ResourceProjection) { *out = *in + if in.Versions != nil { + in, out := &in.Versions, &out.Versions + *out = make([]VersionProjection, len(*in)) + copy(*out, *in) + } if in.ShortNames != nil { in, out := &in.ShortNames, &out.ShortNames *out = make([]string, len(*in)) @@ -463,6 +468,11 @@ func (in *ResourceTemplateMutation) DeepCopy() *ResourceTemplateMutation { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SourceResourceDescriptor) DeepCopyInto(out *SourceResourceDescriptor) { *out = *in + if in.Versions != nil { + in, out := &in.Versions, &out.Versions + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SourceResourceDescriptor. @@ -489,3 +499,18 @@ func (in *TemplateExpression) DeepCopy() *TemplateExpression { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VersionProjection) DeepCopyInto(out *VersionProjection) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VersionProjection. +func (in *VersionProjection) DeepCopy() *VersionProjection { + if in == nil { + return nil + } + out := new(VersionProjection) + in.DeepCopyInto(out) + return out +} diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go b/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go index 77c6b3f..5d96291 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go @@ -19,19 +19,20 @@ limitations under the License. package v1alpha1 import ( - v1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" ) // ResourceProjectionApplyConfiguration represents a declarative configuration of the ResourceProjection type for use // with apply. type ResourceProjectionApplyConfiguration struct { - Group *string `json:"group,omitempty"` - Version *string `json:"version,omitempty"` - Scope *v1alpha1.ResourceScope `json:"scope,omitempty"` - Kind *string `json:"kind,omitempty"` - Plural *string `json:"plural,omitempty"` - ShortNames []string `json:"shortNames,omitempty"` - Categories []string `json:"categories,omitempty"` + Group *string `json:"group,omitempty"` + Version *string `json:"version,omitempty"` + Versions []VersionProjectionApplyConfiguration `json:"versions,omitempty"` + Scope *syncagentv1alpha1.ResourceScope `json:"scope,omitempty"` + Kind *string `json:"kind,omitempty"` + Plural *string `json:"plural,omitempty"` + ShortNames []string `json:"shortNames,omitempty"` + Categories []string `json:"categories,omitempty"` } // ResourceProjectionApplyConfiguration constructs a declarative configuration of the ResourceProjection type for use with @@ -56,10 +57,23 @@ func (b *ResourceProjectionApplyConfiguration) WithVersion(value string) *Resour return b } +// WithVersions adds the given value to the Versions field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the Versions field. +func (b *ResourceProjectionApplyConfiguration) WithVersions(values ...*VersionProjectionApplyConfiguration) *ResourceProjectionApplyConfiguration { + for i := range values { + if values[i] == nil { + panic("nil value passed to WithVersions") + } + b.Versions = append(b.Versions, *values[i]) + } + return b +} + // WithScope sets the Scope field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Scope field is set to the value of the last call. -func (b *ResourceProjectionApplyConfiguration) WithScope(value v1alpha1.ResourceScope) *ResourceProjectionApplyConfiguration { +func (b *ResourceProjectionApplyConfiguration) WithScope(value syncagentv1alpha1.ResourceScope) *ResourceProjectionApplyConfiguration { b.Scope = &value return b } diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/sourceresourcedescriptor.go b/sdk/applyconfiguration/syncagent/v1alpha1/sourceresourcedescriptor.go index 376d384..21d07c0 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/sourceresourcedescriptor.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/sourceresourcedescriptor.go @@ -21,9 +21,10 @@ package v1alpha1 // SourceResourceDescriptorApplyConfiguration represents a declarative configuration of the SourceResourceDescriptor type for use // with apply. type SourceResourceDescriptorApplyConfiguration struct { - APIGroup *string `json:"apiGroup,omitempty"` - Version *string `json:"version,omitempty"` - Kind *string `json:"kind,omitempty"` + APIGroup *string `json:"apiGroup,omitempty"` + Version *string `json:"version,omitempty"` + Versions []string `json:"versions,omitempty"` + Kind *string `json:"kind,omitempty"` } // SourceResourceDescriptorApplyConfiguration constructs a declarative configuration of the SourceResourceDescriptor type for use with @@ -48,6 +49,16 @@ func (b *SourceResourceDescriptorApplyConfiguration) WithVersion(value string) * return b } +// WithVersions adds the given value to the Versions field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the Versions field. +func (b *SourceResourceDescriptorApplyConfiguration) WithVersions(values ...string) *SourceResourceDescriptorApplyConfiguration { + for i := range values { + b.Versions = append(b.Versions, values[i]) + } + return b +} + // WithKind sets the Kind field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Kind field is set to the value of the last call. diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go b/sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go new file mode 100644 index 0000000..d91482a --- /dev/null +++ b/sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go @@ -0,0 +1,48 @@ +/* +Copyright The KCP 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. +*/ + +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1alpha1 + +// VersionProjectionApplyConfiguration represents a declarative configuration of the VersionProjection type for use +// with apply. +type VersionProjectionApplyConfiguration struct { + From *string `json:"from,omitempty"` + To *string `json:"to,omitempty"` +} + +// VersionProjectionApplyConfiguration constructs a declarative configuration of the VersionProjection type for use with +// apply. +func VersionProjection() *VersionProjectionApplyConfiguration { + return &VersionProjectionApplyConfiguration{} +} + +// WithFrom sets the From field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the From field is set to the value of the last call. +func (b *VersionProjectionApplyConfiguration) WithFrom(value string) *VersionProjectionApplyConfiguration { + b.From = &value + return b +} + +// WithTo sets the To field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the To field is set to the value of the last call. +func (b *VersionProjectionApplyConfiguration) WithTo(value string) *VersionProjectionApplyConfiguration { + b.To = &value + return b +} diff --git a/sdk/applyconfiguration/utils.go b/sdk/applyconfiguration/utils.go index a8d3999..3ad1e16 100644 --- a/sdk/applyconfiguration/utils.go +++ b/sdk/applyconfiguration/utils.go @@ -73,6 +73,8 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &syncagentv1alpha1.SourceResourceDescriptorApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("TemplateExpression"): return &syncagentv1alpha1.TemplateExpressionApplyConfiguration{} + case v1alpha1.SchemeGroupVersion.WithKind("VersionProjection"): + return &syncagentv1alpha1.VersionProjectionApplyConfiguration{} } return nil From ce775046bd1b1622525f8173648eeb53aa81febb Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 16:29:27 +0200 Subject: [PATCH 03/26] refactor crd-puller to pull all versions (i.e. work on GK basis not GVK anymore) On-behalf-of: @SAP christoph.mewes@sap.com --- cmd/crd-puller/.gitignore | 2 + cmd/crd-puller/README.md | 9 +- cmd/crd-puller/main.go | 9 +- internal/discovery/client.go | 202 +++++++++++++++++++++++------------ 4 files changed, 141 insertions(+), 81 deletions(-) create mode 100644 cmd/crd-puller/.gitignore diff --git a/cmd/crd-puller/.gitignore b/cmd/crd-puller/.gitignore new file mode 100644 index 0000000..d2cedd7 --- /dev/null +++ b/cmd/crd-puller/.gitignore @@ -0,0 +1,2 @@ +/crd-puller +*.yaml diff --git a/cmd/crd-puller/README.md b/cmd/crd-puller/README.md index 07a4b7d..ef13c76 100644 --- a/cmd/crd-puller/README.md +++ b/cmd/crd-puller/README.md @@ -1,17 +1,18 @@ # CRD Puller The `crd-puller` can be used for testing and development in order to export a -CustomResourceDefinition for any Group/Version/Kind (GVK) in a Kubernetes cluster. +CustomResourceDefinition for any Group/Kind (GK) in a Kubernetes cluster. The main difference between this and kcp's own `crd-puller` is that this one -works based on GVKs and not resources (i.e. on `apps/v1 Deployment` instead of +works based on GKs and not resources (i.e. on `apps/Deployment` instead of `apps.deployments`). This is more useful since a PublishedResource publishes a -specific Kind and version. +specific Kind and version. Also, this puller pulls all available versions, not +just the preferred version. ## Usage ```shell export KUBECONFIG=/path/to/kubeconfig -./crd-puller Deployment.v1.apps.k8s.io +./crd-puller Deployment.apps.k8s.io ``` diff --git a/cmd/crd-puller/main.go b/cmd/crd-puller/main.go index 14b5789..2455849 100644 --- a/cmd/crd-puller/main.go +++ b/cmd/crd-puller/main.go @@ -41,13 +41,10 @@ func main() { pflag.Parse() if pflag.NArg() == 0 { - log.Fatal("No argument given. Please specify a GVK in the form 'Kind.version.apigroup.com' to pull.") + log.Fatal("No argument given. Please specify a GroupKind in the form 'Kind.apigroup.com' (case-sensitive) to pull.") } - gvk, _ := schema.ParseKindArg(pflag.Arg(0)) - if gvk == nil { - log.Fatal("Invalid GVK, please use the format 'Kind.version.apigroup.com'.") - } + gk := schema.ParseGroupKind(pflag.Arg(0)) loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() loadingRules.ExplicitPath = kubeconfigPath @@ -67,7 +64,7 @@ func main() { log.Fatalf("Failed to create discovery client: %v.", err) } - crd, err := discoveryClient.RetrieveCRD(ctx, *gvk) + crd, err := discoveryClient.RetrieveCRD(ctx, gk) if err != nil { log.Fatalf("Failed to pull CRD: %v.", err) } diff --git a/internal/discovery/client.go b/internal/discovery/client.go index 631ef5a..f8d9996 100644 --- a/internal/discovery/client.go +++ b/internal/discovery/client.go @@ -18,6 +18,7 @@ package discovery import ( "context" + "errors" "fmt" "slices" "strings" @@ -32,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/version" "k8s.io/apiserver/pkg/endpoints/openapi" "k8s.io/client-go/discovery" "k8s.io/client-go/rest" @@ -61,12 +63,9 @@ func NewClient(config *rest.Config) (*Client, error) { }, nil } -func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) (*apiextensionsv1.CustomResourceDefinition, error) { - // Most of this code follows the logic in kcp's crd-puller, but is slimmed down - // to extract a specific version, not necessarily the preferred version. - +func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiextensionsv1.CustomResourceDefinition, error) { //////////////////////////////////// - // Resolve GVK into GVR, because we need the resource name to construct + // Resolve GK into GR, because we need the resource name to construct // the full CRD name. _, resourceLists, err := c.discoveryClient.ServerGroupsAndResources() @@ -74,34 +73,57 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( return nil, err } + // resource is the resource described by gk in any of the found versions var resource *metav1.APIResource - allResourceNames := sets.New[string]() + + availableVersions := sets.New[string]() + subresourcesPerVersion := map[string]sets.Set[string]{} + for _, resList := range resourceLists { for _, res := range resList.APIResources { - allResourceNames.Insert(res.Name) + // ignore other groups + if res.Group != gk.Group || res.Kind != gk.Kind { + continue + } - // find the requested resource based on the Kind, but ensure that subresources - // are not misinterpreted as the main resource by checking for "/" - if resList.GroupVersion == gvk.GroupVersion().String() && res.Kind == gvk.Kind && !strings.Contains(res.Name, "/") { + // res could describe the main resource or one of its subresources. + name := res.Name + subresource := "" + if strings.Contains(name, "/") { + parts := strings.SplitN(name, "/", 2) + name = parts[0] + subresource = parts[1] + } + + if subresource == "" { resource = &res + } else { + list, ok := subresourcesPerVersion[res.Version] + if !ok { + list = sets.New[string]() + } + list.Insert(subresource) + subresourcesPerVersion[res.Version] = list } + + availableVersions.Insert(res.Version) } } if resource == nil { - return nil, fmt.Errorf("could not find %v in APIs", gvk) + return nil, fmt.Errorf("could not find %v in APIs", gk) } //////////////////////////////////// - // If possible, retrieve the GVK as its original CRD, which is always preferred + // If possible, retrieve the GK as its original CRD, which is always preferred // because it's much more precise than what we can retrieve from the OpenAPI. // If no CRD can be found, fallback to the OpenAPI schema. crdName := resource.Name - if gvk.Group == "" { + if gk.Group == "" { crdName += ".core" } else { - crdName += "." + gvk.Group + crdName += "." + gk.Group } crd, err := c.crdClient.CustomResourceDefinitions().Get(ctx, crdName, metav1.GetOptions{}) @@ -110,25 +132,20 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( // of re-creating it later on based on the openapi schema, we take the original // CRD and just strip it down to what we need. if err == nil { - // remove all but the requested version - crd.Spec.Versions = slices.DeleteFunc(crd.Spec.Versions, func(ver apiextensionsv1.CustomResourceDefinitionVersion) bool { - return ver.Name != gvk.Version - }) - - if len(crd.Spec.Versions) == 0 { - return nil, fmt.Errorf("CRD %s does not contain version %s", crdName, gvk.Version) - } - - crd.Spec.Versions[0].Served = true - crd.Spec.Versions[0].Storage = true - if apihelpers.IsCRDConditionTrue(crd, apiextensionsv1.NonStructuralSchema) { - crd.Spec.Versions[0].Schema = &apiextensionsv1.CustomResourceValidation{ + emptySchema := &apiextensionsv1.CustomResourceValidation{ OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ Type: "object", XPreserveUnknownFields: ptr.To(true), }, } + + for i, version := range crd.Spec.Versions { + if version.Schema == nil || version.Schema.OpenAPIV3Schema == nil { + version.Schema = emptySchema + crd.Spec.Versions[i] = version + } + } } crd.APIVersion = apiextensionsv1.SchemeGroupVersion.Identifier() @@ -140,6 +157,7 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( Name: oldMeta.Name, Annotations: filterAnnotations(oldMeta.Annotations), } + crd.Status.Conditions = []apiextensionsv1.CustomResourceDefinitionCondition{} // There is only ever one version, so conversion rules do not make sense // (and even if they did, the conversion webhook from the service cluster @@ -156,49 +174,34 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( return nil, err } + //////////////////////////////////// // CRD not found, so fall back to using the OpenAPI schema + openapiSchema, err := c.discoveryClient.OpenAPISchema() if err != nil { return nil, err } - models, err := proto.NewOpenAPIData(openapiSchema) - if err != nil { - return nil, err - } - modelsByGKV, err := openapi.GetModelsByGKV(models) + preferredVersion, err := c.getPreferredVersion(resource) if err != nil { return nil, err } - protoSchema := modelsByGKV[gvk] - if protoSchema == nil { - return nil, fmt.Errorf("no models for %v", gvk) - } - - var schemaProps apiextensionsv1.JSONSchemaProps - errs := crdpuller.Convert(protoSchema, &schemaProps) - if len(errs) > 0 { - return nil, utilerrors.NewAggregate(errs) + if preferredVersion == "" { + return nil, errors.New("cannot determine storage version because no preferred version exists in the schema") } - hasSubResource := func(subResource string) bool { - return allResourceNames.Has(resource.Name + "/" + subResource) - } - - var statusSubResource *apiextensionsv1.CustomResourceSubresourceStatus - if hasSubResource("status") { - statusSubResource = &apiextensionsv1.CustomResourceSubresourceStatus{} + models, err := proto.NewOpenAPIData(openapiSchema) + if err != nil { + return nil, err } - var scaleSubResource *apiextensionsv1.CustomResourceSubresourceScale - if hasSubResource("scale") { - scaleSubResource = &apiextensionsv1.CustomResourceSubresourceScale{ - SpecReplicasPath: ".spec.replicas", - StatusReplicasPath: ".status.replicas", - } + modelsByGKV, err := openapi.GetModelsByGKV(models) + if err != nil { + return nil, err } + // prepare an empty CRD scope := apiextensionsv1.ClusterScoped if resource.Namespaced { scope = apiextensionsv1.NamespaceScoped @@ -213,22 +216,9 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( Name: crdName, }, Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Group: gvk.Group, - Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ - { - Name: gvk.Version, - Schema: &apiextensionsv1.CustomResourceValidation{ - OpenAPIV3Schema: &schemaProps, - }, - Subresources: &apiextensionsv1.CustomResourceSubresources{ - Status: statusSubResource, - Scale: scaleSubResource, - }, - Served: true, - Storage: true, - }, - }, - Scope: scope, + Group: gk.Group, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{}, + Scope: scope, Names: apiextensionsv1.CustomResourceDefinitionNames{ Plural: resource.Name, Kind: resource.Kind, @@ -239,9 +229,62 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( }, } + // fill-in the schema for each version, making sure that versions are sorted + // according to Kubernetes rules. + sortedVersions := availableVersions.UnsortedList() + slices.SortFunc(sortedVersions, func(a, b string) int { + return version.CompareKubeAwareVersionStrings(a, b) + }) + + for _, version := range sortedVersions { + subresources := subresourcesPerVersion[version] + gvk := schema.GroupVersionKind{ + Group: gk.Group, + Version: version, + Kind: gk.Kind, + } + + protoSchema := modelsByGKV[gvk] + if protoSchema == nil { + return nil, fmt.Errorf("no models for %v", gk) + } + + var schemaProps apiextensionsv1.JSONSchemaProps + errs := crdpuller.Convert(protoSchema, &schemaProps) + if len(errs) > 0 { + return nil, utilerrors.NewAggregate(errs) + } + + var statusSubResource *apiextensionsv1.CustomResourceSubresourceStatus + if subresources.Has("status") { + statusSubResource = &apiextensionsv1.CustomResourceSubresourceStatus{} + } + + var scaleSubResource *apiextensionsv1.CustomResourceSubresourceScale + if subresources.Has("scale") { + scaleSubResource = &apiextensionsv1.CustomResourceSubresourceScale{ + SpecReplicasPath: ".spec.replicas", + StatusReplicasPath: ".status.replicas", + } + } + + out.Spec.Versions = append(out.Spec.Versions, apiextensionsv1.CustomResourceDefinitionVersion{ + Name: version, + Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &schemaProps, + }, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: statusSubResource, + Scale: scaleSubResource, + }, + Served: true, + Storage: version == preferredVersion, + }) + } + apiextensionsv1.SetDefaults_CustomResourceDefinition(out) - if apihelpers.IsProtectedCommunityGroup(gvk.Group) { + if apihelpers.IsProtectedCommunityGroup(gk.Group) { out.Annotations = map[string]string{ apiextensionsv1.KubeAPIApprovedAnnotation: "https://github.com/kcp-dev/kubernetes/pull/4", } @@ -250,6 +293,23 @@ func (c *Client) RetrieveCRD(ctx context.Context, gvk schema.GroupVersionKind) ( return out, nil } +func (c *Client) getPreferredVersion(resource *metav1.APIResource) (string, error) { + result, err := c.discoveryClient.ServerPreferredResources() + if err != nil { + return "", err + } + + for _, resList := range result { + for _, res := range resList.APIResources { + if res.Name == resource.Name && res.Group == resource.Group { + return res.Version, nil + } + } + } + + return "", nil +} + func filterAnnotations(ann map[string]string) map[string]string { allowlist := []string{ apiextensionsv1.KubeAPIApprovedAnnotation, From 3c7647a959ae2cb6c555a32cb1a71919a5a33f4a Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 19:00:42 +0200 Subject: [PATCH 04/26] create new APIResourceSchemas when CRDs or PRs change, improve mutation logic accessibility On-behalf-of: @SAP christoph.mewes@sap.com --- .../apiresourceschema/controller.go | 75 ++------ internal/discovery/client.go | 5 +- internal/projection/projection.go | 177 +++++++++++++++++- internal/projection/projection_test.go | 14 +- 4 files changed, 191 insertions(+), 80 deletions(-) diff --git a/internal/controller/apiresourceschema/controller.go b/internal/controller/apiresourceschema/controller.go index b7e4002..931b277 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "reflect" - "strings" "github.com/kcp-dev/logicalcluster/v3" "go.uber.org/zap" @@ -122,31 +121,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubResource *syncagentv1alpha1.PublishedResource) (*reconcile.Result, error) { // find the resource that the PublishedResource is referring to - localGVK := projection.PublishedResourceSourceGVK(pubResource) + localGK := projection.PublishedResourceSourceGK(pubResource) client, err := discovery.NewClient(r.restConfig) if err != nil { return nil, fmt.Errorf("failed to create discovery client: %w", err) } - crd, err := client.RetrieveCRD(ctx, localGVK) + // fetch the original, full CRD from the cluster + crd, err := client.RetrieveCRD(ctx, localGK) if err != nil { return nil, fmt.Errorf("failed to discover resource defined in PublishedResource: %w", err) } - // project the CRD - projectedCRD, err := r.applyProjection(crd, pubResource) + // project the CRD (i.e. strip unwanted versions, rename values etc.) + projectedCRD, err := projection.ApplyProjection(crd, pubResource) if err != nil { return nil, fmt.Errorf("failed to apply projection rules: %w", err) } - // to prevent changing the source GVK e.g. from "apps/v1 Daemonset" to "core/v1 Pod", - // we include the source GVK in hashed form in the final APIResourceSchema name. + // generate a unique name for this exact state of the CRD arsName := r.getAPIResourceSchemaName(projectedCRD) - // ARS'es cannot be updated, their entire spec is immutable. For now we do not care about - // CRDs being updated on the service cluster, but in the future (TODO) we must allow - // service owners to somehow publish updated CRDs without changing their API version. + // ensure ARS exists (don't try to reconcile it, it's basically entirely immutable) wsCtx := kontext.WithCluster(ctx, r.lcName) ars := &kcpdevv1alpha1.APIResourceSchema{} err = r.kcpClient.Get(wsCtx, types.NamespacedName{Name: arsName}, ars, &ctrlruntimeclient.GetOptions{}) @@ -159,7 +156,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR return nil, fmt.Errorf("failed to check for APIResourceSchema: %w", err) } - // Update Status with ARS name + // update Status with ARS name if pubResource.Status.ResourceSchemaName != arsName { original := pubResource.DeepCopy() pubResource.Status.ResourceSchemaName = arsName @@ -176,7 +173,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR } func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.SugaredLogger, projectedCRD *apiextensionsv1.CustomResourceDefinition, arsName string) error { - // prefix is irrelevant as the reconciling framework will use arsName anyway + // prefix is irrelevant as the name is overridden later converted, err := kcpdevv1alpha1.CRDToAPIResourceSchema(projectedCRD, "irrelevant") if err != nil { return fmt.Errorf("failed to convert CRD: %w", err) @@ -198,59 +195,13 @@ func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.Sugar return r.kcpClient.Create(ctx, ars) } -func (r *Reconciler) applyProjection(crd *apiextensionsv1.CustomResourceDefinition, pr *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { - result := crd.DeepCopy() - - // Currently CRDs generated by our discovery mechanism already set these to true, but that's just - // because it doesn't care to set them correctly; we keep this code here because from here on, - // in kcp, we definitely want them to be true. - result.Spec.Versions[0].Served = true - result.Spec.Versions[0].Storage = true - - projection := pr.Spec.Projection - if projection == nil { - return result, nil - } - - if projection.Group != "" { - result.Spec.Group = projection.Group - } - - if projection.Version != "" { - result.Spec.Versions[0].Name = projection.Version - } - - if projection.Kind != "" { - result.Spec.Names.Kind = projection.Kind - result.Spec.Names.ListKind = projection.Kind + "List" - - result.Spec.Names.Singular = strings.ToLower(result.Spec.Names.Kind) - result.Spec.Names.Plural = result.Spec.Names.Singular + "s" - } - - if projection.Plural != "" { - result.Spec.Names.Plural = projection.Plural - } - - if projection.Scope != "" { - result.Spec.Scope = apiextensionsv1.ResourceScope(projection.Scope) - } - - if projection.Categories != nil { - result.Spec.Names.Categories = projection.Categories - } - - if projection.ShortNames != nil { - result.Spec.Names.ShortNames = projection.ShortNames - } - - return result, nil -} - // getAPIResourceSchemaName generates the name for the ARS in kcp. Note that // kcp requires, just like CRDs, that ARS are named following a specific pattern. func (r *Reconciler) getAPIResourceSchemaName(crd *apiextensionsv1.CustomResourceDefinition) string { - checksum := crypto.Hash(crd.Spec.Names) + crd = crd.DeepCopy() + crd.Spec.Conversion = nil + + checksum := crypto.Hash(crd.Spec) // include a leading "v" to prevent SHA-1 hashes with digits to break the name return fmt.Sprintf("v%s.%s.%s", checksum[:8], crd.Spec.Names.Plural, crd.Spec.Group) diff --git a/internal/discovery/client.go b/internal/discovery/client.go index f8d9996..d335ba7 100644 --- a/internal/discovery/client.go +++ b/internal/discovery/client.go @@ -156,6 +156,7 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte crd.ObjectMeta = metav1.ObjectMeta{ Name: oldMeta.Name, Annotations: filterAnnotations(oldMeta.Annotations), + Generation: oldMeta.Generation, // is stored as an annotation for convenience on the ARS } crd.Status.Conditions = []apiextensionsv1.CustomResourceDefinitionCondition{} @@ -232,9 +233,7 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte // fill-in the schema for each version, making sure that versions are sorted // according to Kubernetes rules. sortedVersions := availableVersions.UnsortedList() - slices.SortFunc(sortedVersions, func(a, b string) int { - return version.CompareKubeAwareVersionStrings(a, b) - }) + slices.SortFunc(sortedVersions, version.CompareKubeAwareVersionStrings) for _, version := range sortedVersions { subresources := subresourcesPerVersion[version] diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 5586f7e..38d87b6 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -17,18 +17,25 @@ limitations under the License. package projection import ( + "errors" + "fmt" + "slices" + "strings" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/version" ) -// PublishedResourceSourceGVK returns the source GVK of the local resources +// PublishedResourceSourceGK returns the source GK of the local resources // that are supposed to be published. -func PublishedResourceSourceGVK(pubRes *syncagentv1alpha1.PublishedResource) schema.GroupVersionKind { - return schema.GroupVersionKind{ - Group: pubRes.Spec.Resource.APIGroup, - Version: pubRes.Spec.Resource.Version, - Kind: pubRes.Spec.Resource.Kind, +func PublishedResourceSourceGK(pubRes *syncagentv1alpha1.PublishedResource) schema.GroupKind { + return schema.GroupKind{ + Group: pubRes.Spec.Resource.APIGroup, + Kind: pubRes.Spec.Resource.Kind, } } @@ -59,3 +66,161 @@ func PublishedResourceProjectedGVK(pubRes *syncagentv1alpha1.PublishedResource) Kind: kind, } } + +func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { + result := crd.DeepCopy() + + // reduce the CRD down to the selected versions + result, err := stripUnwantedVersions(result, pubRes) + if err != nil { + return nil, err + } + + // if there is no storage version left, we use the latest served version + result, err = adjustStorageVersion(result) + if err != nil { + return nil, err + } + + // now we get to actually project something, if desired + result, err = projectCRD(result, pubRes) + if err != nil { + return nil, err + } + + return result, nil +} + +func stripUnwantedVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { + src := pubRes.Spec.Resource + + if src.Version != "" && len(src.Versions) > 0 { + return nil, errors.New("cannot configure both .version and .versions in as the source of a PublishedResource") + } + + crd.Spec.Versions = slices.DeleteFunc(crd.Spec.Versions, func(ver apiextensionsv1.CustomResourceDefinitionVersion) bool { + switch { + case src.Version != "": + return ver.Name != src.Version + case len(src.Versions) > 0: + return !slices.Contains(src.Versions, ver.Name) + default: + return false // i.e. keep all versions by default + } + }) + + if len(crd.Spec.Versions) == 0 { + switch { + case src.Version != "": + return nil, fmt.Errorf("CRD does not contain version %s", src.Version) + case len(src.Versions) > 0: + return nil, fmt.Errorf("CRD does not contain any of versions %v", src.Versions) + default: + return nil, errors.New("CRD contains no versions") + } + } + + return crd, nil +} + +func adjustStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (*apiextensionsv1.CustomResourceDefinition, error) { + var hasStorage bool + latestServed := -1 + for i, v := range crd.Spec.Versions { + if v.Storage { + hasStorage = true + } + if v.Served { + latestServed = i + } + } + + if latestServed < 0 { + return nil, errors.New("no CRD version selected that is marked as served") + } + + if !hasStorage { + crd.Spec.Versions[latestServed].Storage = true + } + + return crd, nil +} + +func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { + projection := pubRes.Spec.Projection + if projection == nil { + return crd, nil + } + + if projection.Group != "" { + crd.Spec.Group = projection.Group + } + + indexOfVersion := func(v string) int { + for i, version := range crd.Spec.Versions { + if version.Name == v { + return i + } + } + + return -1 + } + + // We already validated that Version and Versions can be set at the same time. + + if projection.Version != "" { + if size := len(crd.Spec.Versions); size != 1 { + return nil, fmt.Errorf("cannot project CRD version to a single version %q because it contains %d versions", projection.Version, size) + } + + crd.Spec.Versions[0].Name = projection.Version + } else if len(projection.Versions) > 0 { + for _, mut := range projection.Versions { + fromIdx := indexOfVersion(mut.From) + if fromIdx < 0 { + return nil, fmt.Errorf("cannot project CRD version %s to %s because there is no %s version", mut.From, mut.To, mut.From) + } + + crd.Spec.Versions[fromIdx].Name = mut.To + } + + // ensure we ended up with a unique set of versions + knownVersions := sets.New[string]() + for _, version := range crd.Spec.Versions { + if knownVersions.Has(version.Name) { + return nil, fmt.Errorf("CRD contains multiple entries for %s after applying mutation rules", version.Name) + } + } + + // ensure proper Kubernetes-style version order + slices.SortFunc(crd.Spec.Versions, func(a, b apiextensionsv1.CustomResourceDefinitionVersion) int { + return version.CompareKubeAwareVersionStrings(a.Name, b.Name) + }) + } + + if projection.Kind != "" { + crd.Spec.Names.Kind = projection.Kind + crd.Spec.Names.ListKind = projection.Kind + "List" + + crd.Spec.Names.Singular = strings.ToLower(crd.Spec.Names.Kind) + crd.Spec.Names.Plural = crd.Spec.Names.Singular + "s" + } + + if projection.Plural != "" { + crd.Spec.Names.Plural = projection.Plural + } + + if projection.Scope != "" { + crd.Spec.Scope = apiextensionsv1.ResourceScope(projection.Scope) + } + + if projection.Categories != nil { + crd.Spec.Names.Categories = projection.Categories + } + + if projection.ShortNames != nil { + crd.Spec.Names.ShortNames = projection.ShortNames + } + + return crd, nil +} diff --git a/internal/projection/projection_test.go b/internal/projection/projection_test.go index 5b9b1f5..0275079 100644 --- a/internal/projection/projection_test.go +++ b/internal/projection/projection_test.go @@ -41,18 +41,14 @@ func TestPublishedResourceSourceGVK(t *testing.T) { }, } - gvk := PublishedResourceSourceGVK(&pubRes) + gk := PublishedResourceSourceGK(&pubRes) - if gvk.Group != apiGroup { - t.Errorf("Expected API group to be %q, but got %q.", apiGroup, gvk.Group) + if gk.Group != apiGroup { + t.Errorf("Expected API group to be %q, but got %q.", apiGroup, gk.Group) } - if gvk.Version != version { - t.Errorf("Expected version to be %q, but got %q.", version, gvk.Version) - } - - if gvk.Kind != kind { - t.Errorf("Expected kind to be %q, but got %q.", kind, gvk.Kind) + if gk.Kind != kind { + t.Errorf("Expected kind to be %q, but got %q.", kind, gk.Kind) } } From a8bb1e8de2bdf1616702c790c72b158770079bb3 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 19:02:02 +0200 Subject: [PATCH 05/26] correctly merge and update existing ResourceSchemas with new ones when reconciling APIExports On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/apiexport/controller.go | 10 ++--- internal/controller/apiexport/reconciler.go | 48 +++++++++++++++++---- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/internal/controller/apiexport/controller.go b/internal/controller/apiexport/controller.go index ecd5453..7e8e302 100644 --- a/internal/controller/apiexport/controller.go +++ b/internal/controller/apiexport/controller.go @@ -19,6 +19,7 @@ package apiexport import ( "context" "fmt" + "slices" "github.com/kcp-dev/logicalcluster/v3" "go.uber.org/zap" @@ -121,12 +122,9 @@ func (r *Reconciler) reconcile(ctx context.Context) error { } // filter out those PRs that have not yet been processed into an ARS - filteredPubResources := []syncagentv1alpha1.PublishedResource{} - for i, pubResource := range pubResources.Items { - if pubResource.Status.ResourceSchemaName != "" { - filteredPubResources = append(filteredPubResources, pubResources.Items[i]) - } - } + filteredPubResources := slices.DeleteFunc(pubResources.Items, func(pr syncagentv1alpha1.PublishedResource) bool { + return pr.Status.ResourceSchemaName == "" + }) // for each PR, we note down the created ARS and also the GVKs of related resources arsList := sets.New[string]() diff --git a/internal/controller/apiexport/reconciler.go b/internal/controller/apiexport/reconciler.go index 53d3069..bcd3ab5 100644 --- a/internal/controller/apiexport/reconciler.go +++ b/internal/controller/apiexport/reconciler.go @@ -17,8 +17,8 @@ limitations under the License. package apiexport import ( - "cmp" "slices" + "strings" "github.com/kcp-dev/api-syncagent/internal/resources/reconciling" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" @@ -34,16 +34,13 @@ import ( func (r *Reconciler) createAPIExportReconciler(availableResourceSchemas sets.Set[string], claimedResourceKinds sets.Set[string], agentName string, apiExportName string) reconciling.NamedAPIExportReconcilerFactory { return func() (string, reconciling.APIExportReconciler) { return apiExportName, func(existing *kcpdevv1alpha1.APIExport) (*kcpdevv1alpha1.APIExport, error) { - known := sets.New(existing.Spec.LatestResourceSchemas...) - if existing.Annotations == nil { existing.Annotations = map[string]string{} } existing.Annotations[syncagentv1alpha1.AgentNameAnnotation] = agentName - // we only ever add new schemas - result := known.Union(availableResourceSchemas) - existing.Spec.LatestResourceSchemas = sets.List(result) + // combine existing schemas with new ones + existing.Spec.LatestResourceSchemas = mergeResourceSchemas(existing.Spec.LatestResourceSchemas, availableResourceSchemas) // To allow admins to configure additional permission claims, sometimes // useful for debugging, we do not override the permission claims, but @@ -73,11 +70,11 @@ func (r *Reconciler) createAPIExportReconciler(availableResourceSchemas sets.Set // prevent reconcile loops by ensuring a stable order slices.SortFunc(existing.Spec.PermissionClaims, func(a, b kcpdevv1alpha1.PermissionClaim) int { if a.Group != b.Group { - return cmp.Compare(a.Group, b.Group) + return strings.Compare(a.Group, b.Group) } if a.Resource != b.Resource { - return cmp.Compare(a.Resource, b.Resource) + return strings.Compare(a.Resource, b.Resource) } return 0 @@ -87,3 +84,38 @@ func (r *Reconciler) createAPIExportReconciler(availableResourceSchemas sets.Set } } } + +func mergeResourceSchemas(existing []string, configured sets.Set[string]) []string { + var result []string + + // first we copy all ARS that are coming from the PublishedResources + knownResources := sets.New[string]() + for _, schema := range configured.UnsortedList() { + result = append(result, schema) + knownResources.Insert(parseResourceGroup(schema)) + } + + // Now we include all other existing ARS that use unknown resources; + // this both allows an APIExport to contain "unmanaged" ARS, and also + // will purposefully leave behind ARS for deleted PublishedResources, + // allowing cleanup to take place outside of the agent's control. + for _, schema := range existing { + if !knownResources.Has(parseResourceGroup(schema)) { + result = append(result, schema) + } + } + + // for stability and beauty, sort the schemas + slices.SortFunc(result, func(a, b string) int { + return strings.Compare(parseResourceGroup(a), parseResourceGroup(b)) + }) + + return result +} + +func parseResourceGroup(schema string) string { + // .. + parts := strings.SplitN(schema, ".", 2) + + return parts[1] +} From 382725f3aedfe8709bfbcab7dd19b08886484920 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 19:32:43 +0200 Subject: [PATCH 06/26] update remaining controllers to perform the new routine to determine local/remote GVKs On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/sync/controller.go | 24 +++++++---- internal/projection/projection.go | 57 +++++++++++++++++--------- internal/projection/projection_test.go | 22 +++++++++- internal/sync/syncer.go | 22 +++++----- 4 files changed, 85 insertions(+), 40 deletions(-) diff --git a/internal/controller/sync/controller.go b/internal/controller/sync/controller.go index 43168e4..314caff 100644 --- a/internal/controller/sync/controller.go +++ b/internal/controller/sync/controller.go @@ -78,22 +78,30 @@ func Create( ) (controller.Controller, error) { log = log.Named(ControllerName) + // find the local CRD so we know the actual local object scope + localCRD, err := discoveryClient.RetrieveCRD(ctx, projection.PublishedResourceSourceGK(pubRes)) + if err != nil { + return nil, fmt.Errorf("failed to find local CRD: %w", err) + } + // create a dummy that represents the type used on the local service cluster - localGVK := projection.PublishedResourceSourceGVK(pubRes) + localGVK, err := projection.PublishedResourceSourceGVK(localCRD, pubRes) + if err != nil { + return nil, err + } + localDummy := &unstructured.Unstructured{} localDummy.SetGroupVersionKind(localGVK) // create a dummy unstructured object with the projected GVK inside the workspace - remoteGVK := projection.PublishedResourceProjectedGVK(pubRes) - remoteDummy := &unstructured.Unstructured{} - remoteDummy.SetGroupVersionKind(remoteGVK) - - // find the local CRD so we know the actual local object scope - localCRD, err := discoveryClient.RetrieveCRD(ctx, localGVK) + remoteGVK, err := projection.PublishedResourceProjectedGVK(localCRD, pubRes) if err != nil { - return nil, fmt.Errorf("failed to find local CRD: %w", err) + return nil, err } + remoteDummy := &unstructured.Unstructured{} + remoteDummy.SetGroupVersionKind(remoteGVK) + // create the syncer that holds the meat&potatoes of the synchronization logic mutator := mutation.NewMutator(pubRes.Spec.Mutation) syncer, err := sync.NewResourceSyncer(log, localManager.GetClient(), virtualWorkspaceCluster.GetClient(), pubRes, localCRD, mutator, stateNamespace, agentName) diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 38d87b6..076cf49 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -39,32 +39,49 @@ func PublishedResourceSourceGK(pubRes *syncagentv1alpha1.PublishedResource) sche } } -// PublishedResourceProjectedGVK returns the effective GVK after the projection -// rules have been applied according to the PublishedResource. -func PublishedResourceProjectedGVK(pubRes *syncagentv1alpha1.PublishedResource) schema.GroupVersionKind { - apiGroup := pubRes.Spec.Resource.APIGroup - apiVersion := pubRes.Spec.Resource.Version - kind := pubRes.Spec.Resource.Kind - - if projection := pubRes.Spec.Projection; projection != nil { - if v := projection.Group; v != "" { - apiGroup = v - } +func PublishedResourceSourceGVK(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (schema.GroupVersionKind, error) { + storageVersion := getStorageVersion(crd) + if storageVersion == "" { + return schema.GroupVersionKind{}, errors.New("CRD does not contain a storage version") + } - if v := projection.Version; v != "" { - apiVersion = v - } + sourceGV := PublishedResourceSourceGK(pubRes) - if k := projection.Kind; k != "" { - kind = k + return schema.GroupVersionKind{ + Group: sourceGV.Group, + Version: storageVersion, + Kind: sourceGV.Kind, + }, nil +} + +func getStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) string { + for _, version := range crd.Spec.Versions { + if version.Storage { + return version.Name } } - return schema.GroupVersionKind{ - Group: apiGroup, - Version: apiVersion, - Kind: kind, + return "" +} + +// PublishedResourceProjectedGVK returns the effective GVK after the projection +// rules have been applied according to the PublishedResource. +func PublishedResourceProjectedGVK(originalCRD *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (schema.GroupVersionKind, error) { + projectedCRD, err := ApplyProjection(originalCRD, pubRes) + if err != nil { + return schema.GroupVersionKind{}, fmt.Errorf("failed to project CRD: %w", err) } + + storageVersion := getStorageVersion(projectedCRD) + if storageVersion == "" { + return schema.GroupVersionKind{}, errors.New("projected CRD does not contain a storage version") + } + + return schema.GroupVersionKind{ + Group: projectedCRD.Spec.Group, + Version: storageVersion, + Kind: projectedCRD.Spec.Names.Kind, + }, nil } func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { diff --git a/internal/projection/projection_test.go b/internal/projection/projection_test.go index 0275079..1edfccb 100644 --- a/internal/projection/projection_test.go +++ b/internal/projection/projection_test.go @@ -20,6 +20,7 @@ import ( "testing" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -69,6 +70,22 @@ func TestPublishedResourceProjectedGVK(t *testing.T) { }, } + crd := &apiextensionsv1.CustomResourceDefinition{ + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: apiGroup, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Kind: kind, + }, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + { + Name: version, + Served: true, + Storage: true, + }, + }, + }, + } + testcases := []struct { name string projection *syncagentv1alpha1.ResourceProjection @@ -106,7 +123,10 @@ func TestPublishedResourceProjectedGVK(t *testing.T) { pr := pubRes.DeepCopy() pr.Spec.Projection = testcase.projection - gvk := PublishedResourceProjectedGVK(pr) + gvk, err := PublishedResourceProjectedGVK(crd, pr) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } if gvk.Group != testcase.expected.Group { t.Errorf("Expected API group to be %q, but got %q.", testcase.expected.Group, gvk.Group) diff --git a/internal/sync/syncer.go b/internal/sync/syncer.go index ddbea11..80628b6 100644 --- a/internal/sync/syncer.go +++ b/internal/sync/syncer.go @@ -63,21 +63,25 @@ func NewResourceSyncer( agentName string, ) (*ResourceSyncer, error) { // create a dummy that represents the type used on the local service cluster - localGVK := projection.PublishedResourceSourceGVK(pubRes) + localGVK, err := projection.PublishedResourceSourceGVK(localCRD, pubRes) + if err != nil { + return nil, err + } + + // create a dummy that represents the type used on the local service cluster localDummy := &unstructured.Unstructured{} localDummy.SetGroupVersionKind(localGVK) // create a dummy unstructured object with the projected GVK inside the workspace - remoteGVK := projection.PublishedResourceProjectedGVK(pubRes) + remoteGVK, err := projection.PublishedResourceProjectedGVK(localCRD, pubRes) + if err != nil { + return nil, err + } // determine whether the CRD has a status subresource in the relevant version subresources := []string{} - versionFound := false - for _, version := range localCRD.Spec.Versions { - if version.Name == pubRes.Spec.Resource.Version { - versionFound = true - + if version.Name == localGVK.Version { if sr := version.Subresources; sr != nil { if sr.Scale != nil { subresources = append(subresources, "scale") @@ -89,10 +93,6 @@ func NewResourceSyncer( } } - if !versionFound { - return nil, fmt.Errorf("CRD %s does not define version %s requested by PublishedResource", pubRes.Spec.Resource.APIGroup, pubRes.Spec.Resource.Version) - } - return &ResourceSyncer{ log: log.With("local-gvk", localGVK, "remote-gvk", remoteGVK), localClient: localClient, From 4eb8c44e7377632fcbfb1debb55758523d29ef2b Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 19:50:02 +0200 Subject: [PATCH 07/26] fix discovery of built-in resources (oh Kubernetes, why u like this sometimes?) On-behalf-of: @SAP christoph.mewes@sap.com --- internal/discovery/client.go | 38 ++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/internal/discovery/client.go b/internal/discovery/client.go index d335ba7..4b74e57 100644 --- a/internal/discovery/client.go +++ b/internal/discovery/client.go @@ -80,9 +80,19 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte subresourcesPerVersion := map[string]sets.Set[string]{} for _, resList := range resourceLists { + // .Group on an APIResource is empty for built-in resources, so we must + // parse and check GroupVersion of the entire list. + gv, err := schema.ParseGroupVersion(resList.GroupVersion) + if err != nil { + return nil, fmt.Errorf("Kubernetes reported invalid API group version %q: %w", resList.GroupVersion, err) + } + + if gv.Group != gk.Group { + continue + } + for _, res := range resList.APIResources { - // ignore other groups - if res.Group != gk.Group || res.Kind != gk.Kind { + if res.Kind != gk.Kind { continue } @@ -106,7 +116,8 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte subresourcesPerVersion[res.Version] = list } - availableVersions.Insert(res.Version) + // res.Version is also empty for built-in resources + availableVersions.Insert(gv.Version) } } @@ -114,6 +125,9 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte return nil, fmt.Errorf("could not find %v in APIs", gk) } + // fill-in the missing Group for built-in resources + resource.Group = gk.Group + //////////////////////////////////// // If possible, retrieve the GK as its original CRD, which is always preferred // because it's much more precise than what we can retrieve from the OpenAPI. @@ -245,7 +259,7 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte protoSchema := modelsByGKV[gvk] if protoSchema == nil { - return nil, fmt.Errorf("no models for %v", gk) + return nil, fmt.Errorf("no models for %v", gvk) } var schemaProps apiextensionsv1.JSONSchemaProps @@ -299,9 +313,21 @@ func (c *Client) getPreferredVersion(resource *metav1.APIResource) (string, erro } for _, resList := range result { + // .Group on an APIResource is empty for built-in resources, so we must + // parse and check GroupVersion of the entire list. + gv, err := schema.ParseGroupVersion(resList.GroupVersion) + if err != nil { + return "", fmt.Errorf("Kubernetes reported invalid API group version %q: %w", resList.GroupVersion, err) + } + + if gv.Group != resource.Group { + continue + } + for _, res := range resList.APIResources { - if res.Name == resource.Name && res.Group == resource.Group { - return res.Version, nil + if res.Name == resource.Name { + // res.Version is empty for built-in resources + return gv.Version, nil } } } From 4fd3f75fc3362728e0f7d54cba4d178edcca673b Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 19:58:12 +0200 Subject: [PATCH 08/26] lint On-behalf-of: @SAP christoph.mewes@sap.com --- internal/discovery/client.go | 8 +++----- internal/projection/projection.go | 7 +++++++ internal/projection/projection_test.go | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/internal/discovery/client.go b/internal/discovery/client.go index 4b74e57..6cb7bc0 100644 --- a/internal/discovery/client.go +++ b/internal/discovery/client.go @@ -97,11 +97,9 @@ func (c *Client) RetrieveCRD(ctx context.Context, gk schema.GroupKind) (*apiexte } // res could describe the main resource or one of its subresources. - name := res.Name - subresource := "" - if strings.Contains(name, "/") { - parts := strings.SplitN(name, "/", 2) - name = parts[0] + var subresource string + if strings.Contains(res.Name, "/") { + parts := strings.SplitN(res.Name, "/", 2) subresource = parts[1] } diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 076cf49..8f371cd 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -111,13 +111,16 @@ func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *sync func stripUnwantedVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { src := pubRes.Spec.Resource + //nolint:staticcheck if src.Version != "" && len(src.Versions) > 0 { return nil, errors.New("cannot configure both .version and .versions in as the source of a PublishedResource") } crd.Spec.Versions = slices.DeleteFunc(crd.Spec.Versions, func(ver apiextensionsv1.CustomResourceDefinitionVersion) bool { switch { + //nolint:staticcheck case src.Version != "": + //nolint:staticcheck return ver.Name != src.Version case len(src.Versions) > 0: return !slices.Contains(src.Versions, ver.Name) @@ -128,7 +131,9 @@ func stripUnwantedVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes if len(crd.Spec.Versions) == 0 { switch { + //nolint:staticcheck case src.Version != "": + //nolint:staticcheck return nil, fmt.Errorf("CRD does not contain version %s", src.Version) case len(src.Versions) > 0: return nil, fmt.Errorf("CRD does not contain any of versions %v", src.Versions) @@ -185,11 +190,13 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent // We already validated that Version and Versions can be set at the same time. + //nolint:staticcheck if projection.Version != "" { if size := len(crd.Spec.Versions); size != 1 { return nil, fmt.Errorf("cannot project CRD version to a single version %q because it contains %d versions", projection.Version, size) } + //nolint:staticcheck crd.Spec.Versions[0].Name = projection.Version } else if len(projection.Versions) > 0 { for _, mut := range projection.Versions { diff --git a/internal/projection/projection_test.go b/internal/projection/projection_test.go index 1edfccb..964b58a 100644 --- a/internal/projection/projection_test.go +++ b/internal/projection/projection_test.go @@ -20,8 +20,8 @@ import ( "testing" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) From 440144663270c3e0ed44026d7f62e34c9b3343c8 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 21:21:52 +0200 Subject: [PATCH 09/26] simplify projecting versions Since a CRD has a _list_ of versions, there is no risk of running into map collisions when renaming versions, so we can slim down our API to just a map, no individual steps. On-behalf-of: @SAP christoph.mewes@sap.com --- .../syncagent.kcp.io_publishedresources.yaml | 14 ++---- internal/projection/projection.go | 21 ++------ .../syncagent/v1alpha1/published_resource.go | 7 +-- .../v1alpha1/zz_generated.deepcopy.go | 6 ++- .../syncagent/v1alpha1/resourceprojection.go | 37 +++++++------- .../syncagent/v1alpha1/versionprojection.go | 48 ------------------- sdk/applyconfiguration/utils.go | 2 - 7 files changed, 32 insertions(+), 103 deletions(-) delete mode 100644 sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index 427314b..75641c1 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -324,20 +324,12 @@ spec: Deprecated: Use .versions instead. type: string versions: + additionalProperties: + type: string description: |- Versions allows to map API versions onto new values in kcp. Leave empty to not modify the versions. - items: - properties: - from: - type: string - to: - type: string - required: - - from - - to - type: object - type: array + type: object type: object related: items: diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 8f371cd..71fed1f 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -178,16 +178,6 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent crd.Spec.Group = projection.Group } - indexOfVersion := func(v string) int { - for i, version := range crd.Spec.Versions { - if version.Name == v { - return i - } - } - - return -1 - } - // We already validated that Version and Versions can be set at the same time. //nolint:staticcheck @@ -199,13 +189,12 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent //nolint:staticcheck crd.Spec.Versions[0].Name = projection.Version } else if len(projection.Versions) > 0 { - for _, mut := range projection.Versions { - fromIdx := indexOfVersion(mut.From) - if fromIdx < 0 { - return nil, fmt.Errorf("cannot project CRD version %s to %s because there is no %s version", mut.From, mut.To, mut.From) - } + for idx, version := range crd.Spec.Versions { + oldVersion := version.Name - crd.Spec.Versions[fromIdx].Name = mut.To + if newVersion := projection.Versions[oldVersion]; newVersion != "" { + crd.Spec.Versions[idx].Name = newVersion + } } // ensure we ended up with a unique set of versions diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index 9c7b870..4074500 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -295,7 +295,7 @@ type ResourceProjection struct { Version string `json:"version,omitempty"` // Versions allows to map API versions onto new values in kcp. Leave empty to not modify the // versions. - Versions []VersionProjection `json:"versions,omitempty"` + Versions map[string]string `json:"versions,omitempty"` // Whether or not the resource is namespaced. // +kubebuilder:validation:Enum=Cluster;Namespaced Scope ResourceScope `json:"scope,omitempty"` @@ -319,11 +319,6 @@ type ResourceProjection struct { Categories []string `json:"categories"` // not omitempty because we need to distinguish between [] and nil } -type VersionProjection struct { - From string `json:"from"` - To string `json:"to"` -} - // ResourceFilter can be used to limit what resources should be included in an operation. type ResourceFilter struct { // When given, the namespace filter will be applied to a resource's namespace. diff --git a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go index 3ab7c1d..b9fdd01 100644 --- a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go +++ b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go @@ -410,8 +410,10 @@ func (in *ResourceProjection) DeepCopyInto(out *ResourceProjection) { *out = *in if in.Versions != nil { in, out := &in.Versions, &out.Versions - *out = make([]VersionProjection, len(*in)) - copy(*out, *in) + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } } if in.ShortNames != nil { in, out := &in.ShortNames, &out.ShortNames diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go b/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go index 5d96291..84e268a 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/resourceprojection.go @@ -19,20 +19,20 @@ limitations under the License. package v1alpha1 import ( - syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + v1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" ) // ResourceProjectionApplyConfiguration represents a declarative configuration of the ResourceProjection type for use // with apply. type ResourceProjectionApplyConfiguration struct { - Group *string `json:"group,omitempty"` - Version *string `json:"version,omitempty"` - Versions []VersionProjectionApplyConfiguration `json:"versions,omitempty"` - Scope *syncagentv1alpha1.ResourceScope `json:"scope,omitempty"` - Kind *string `json:"kind,omitempty"` - Plural *string `json:"plural,omitempty"` - ShortNames []string `json:"shortNames,omitempty"` - Categories []string `json:"categories,omitempty"` + Group *string `json:"group,omitempty"` + Version *string `json:"version,omitempty"` + Versions map[string]string `json:"versions,omitempty"` + Scope *v1alpha1.ResourceScope `json:"scope,omitempty"` + Kind *string `json:"kind,omitempty"` + Plural *string `json:"plural,omitempty"` + ShortNames []string `json:"shortNames,omitempty"` + Categories []string `json:"categories,omitempty"` } // ResourceProjectionApplyConfiguration constructs a declarative configuration of the ResourceProjection type for use with @@ -57,15 +57,16 @@ func (b *ResourceProjectionApplyConfiguration) WithVersion(value string) *Resour return b } -// WithVersions adds the given value to the Versions field in the declarative configuration +// WithVersions puts the entries into the Versions field in the declarative configuration // and returns the receiver, so that objects can be build by chaining "With" function invocations. -// If called multiple times, values provided by each call will be appended to the Versions field. -func (b *ResourceProjectionApplyConfiguration) WithVersions(values ...*VersionProjectionApplyConfiguration) *ResourceProjectionApplyConfiguration { - for i := range values { - if values[i] == nil { - panic("nil value passed to WithVersions") - } - b.Versions = append(b.Versions, *values[i]) +// If called multiple times, the entries provided by each call will be put on the Versions field, +// overwriting an existing map entries in Versions field with the same key. +func (b *ResourceProjectionApplyConfiguration) WithVersions(entries map[string]string) *ResourceProjectionApplyConfiguration { + if b.Versions == nil && len(entries) > 0 { + b.Versions = make(map[string]string, len(entries)) + } + for k, v := range entries { + b.Versions[k] = v } return b } @@ -73,7 +74,7 @@ func (b *ResourceProjectionApplyConfiguration) WithVersions(values ...*VersionPr // WithScope sets the Scope field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Scope field is set to the value of the last call. -func (b *ResourceProjectionApplyConfiguration) WithScope(value syncagentv1alpha1.ResourceScope) *ResourceProjectionApplyConfiguration { +func (b *ResourceProjectionApplyConfiguration) WithScope(value v1alpha1.ResourceScope) *ResourceProjectionApplyConfiguration { b.Scope = &value return b } diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go b/sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go deleted file mode 100644 index d91482a..0000000 --- a/sdk/applyconfiguration/syncagent/v1alpha1/versionprojection.go +++ /dev/null @@ -1,48 +0,0 @@ -/* -Copyright The KCP 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. -*/ - -// Code generated by applyconfiguration-gen. DO NOT EDIT. - -package v1alpha1 - -// VersionProjectionApplyConfiguration represents a declarative configuration of the VersionProjection type for use -// with apply. -type VersionProjectionApplyConfiguration struct { - From *string `json:"from,omitempty"` - To *string `json:"to,omitempty"` -} - -// VersionProjectionApplyConfiguration constructs a declarative configuration of the VersionProjection type for use with -// apply. -func VersionProjection() *VersionProjectionApplyConfiguration { - return &VersionProjectionApplyConfiguration{} -} - -// WithFrom sets the From field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the From field is set to the value of the last call. -func (b *VersionProjectionApplyConfiguration) WithFrom(value string) *VersionProjectionApplyConfiguration { - b.From = &value - return b -} - -// WithTo sets the To field in the declarative configuration to the given value -// and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the To field is set to the value of the last call. -func (b *VersionProjectionApplyConfiguration) WithTo(value string) *VersionProjectionApplyConfiguration { - b.To = &value - return b -} diff --git a/sdk/applyconfiguration/utils.go b/sdk/applyconfiguration/utils.go index 3ad1e16..a8d3999 100644 --- a/sdk/applyconfiguration/utils.go +++ b/sdk/applyconfiguration/utils.go @@ -73,8 +73,6 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &syncagentv1alpha1.SourceResourceDescriptorApplyConfiguration{} case v1alpha1.SchemeGroupVersion.WithKind("TemplateExpression"): return &syncagentv1alpha1.TemplateExpressionApplyConfiguration{} - case v1alpha1.SchemeGroupVersion.WithKind("VersionProjection"): - return &syncagentv1alpha1.VersionProjectionApplyConfiguration{} } return nil From cf9825a0b6836eeac49f35518de305af572a6c7e Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 21:40:20 +0200 Subject: [PATCH 10/26] codegen On-behalf-of: @SAP christoph.mewes@sap.com --- .../syncagent/v1alpha1/zz_generated.deepcopy.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go index b9fdd01..8da6b2f 100644 --- a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go +++ b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go @@ -501,18 +501,3 @@ func (in *TemplateExpression) DeepCopy() *TemplateExpression { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *VersionProjection) DeepCopyInto(out *VersionProjection) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VersionProjection. -func (in *VersionProjection) DeepCopy() *VersionProjection { - if in == nil { - return nil - } - out := new(VersionProjection) - in.DeepCopyInto(out) - return out -} From 7077af07a7cdfd9b30df34447637f047aaede7e0 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 15:15:35 +0200 Subject: [PATCH 11/26] add discovery e2e tests On-behalf-of: @SAP christoph.mewes@sap.com --- test/e2e/discovery/discovery_test.go | 136 +++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 test/e2e/discovery/discovery_test.go diff --git a/test/e2e/discovery/discovery_test.go b/test/e2e/discovery/discovery_test.go new file mode 100644 index 0000000..6426364 --- /dev/null +++ b/test/e2e/discovery/discovery_test.go @@ -0,0 +1,136 @@ +//go:build e2e + +/* +Copyright 2025 The KCP 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 discovery + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/google/go-cmp/cmp" + + "github.com/kcp-dev/api-syncagent/internal/discovery" + "github.com/kcp-dev/api-syncagent/test/utils" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/tools/clientcmd" + ctrlruntime "sigs.k8s.io/controller-runtime" +) + +func TestDiscoverSingleVersionCRD(t *testing.T) { + testcases := []struct { + name string + crdFiles []string + groupKind schema.GroupKind + expectedVersions []string + expectedNames apiextensionsv1.CustomResourceDefinitionNames + }{ + { + name: "get non-CRD resource", + groupKind: schema.GroupKind{Group: "rbac.authorization.k8s.io", Kind: "Role"}, + expectedVersions: []string{"v1"}, + expectedNames: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "roles", + Singular: "role", + Kind: "Role", + ListKind: "RoleList", + }, + }, + { + name: "get CRD with single version", + crdFiles: []string{"test/crds/backup.yaml"}, + groupKind: schema.GroupKind{Group: "eksempel.no", Kind: "Backup"}, + expectedVersions: []string{"v1"}, + expectedNames: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "backups", + Singular: "backup", + Kind: "Backup", + ListKind: "BackupList", + }, + }, + { + name: "get CRD with multiple versions", + crdFiles: []string{"test/crds/crontab-multi-versions.yaml"}, + groupKind: schema.GroupKind{Group: "example.com", Kind: "CronTab"}, + expectedVersions: []string{"v1", "v2"}, + expectedNames: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "crontabs", + Singular: "crontab", + ShortNames: []string{"ct"}, + Kind: "CronTab", + ListKind: "CronTabList", + }, + }, + { + name: "get non-CRD with multiple versions", + groupKind: schema.GroupKind{Group: "autoscaling", Kind: "HorizontalPodAutoscaler"}, + expectedVersions: []string{"v1", "v2"}, + expectedNames: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "horizontalpodautoscalers", + Singular: "horizontalpodautoscaler", + ShortNames: []string{"hpa"}, + Kind: "HorizontalPodAutoscaler", + ListKind: "HorizontalPodAutoscalerList", + Categories: []string{"all"}, + }, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + kubeconfigFile, _, _ := utils.RunEnvtest(t, testcase.crdFiles) + kubeconfig, err := clientcmd.LoadFromFile(kubeconfigFile) + if err != nil { + t.Fatalf("Failed to load envtest kubeconfig: %v", err) + } + + restConfig, err := clientcmd.NewDefaultClientConfig(*kubeconfig, nil).ClientConfig() + if err != nil { + t.Fatalf("Failed to load envtest kubeconfig: %v", err) + } + + client, err := discovery.NewClient(restConfig) + if err != nil { + t.Fatalf("Failed to create discovery client: %v", err) + } + + crd, err := client.RetrieveCRD(ctx, testcase.groupKind) + if err != nil { + t.Fatalf("Failed to discover GK: %v", err) + } + + if diff := cmp.Diff(testcase.expectedNames, crd.Spec.Names); diff != "" { + t.Errorf("Did not get expected CRD names:\n\n%s", diff) + } + + var versions []string + for _, v := range crd.Spec.Versions { + versions = append(versions, v.Name) + } + + if diff := cmp.Diff(testcase.expectedVersions, versions); diff != "" { + t.Errorf("Did not get expected CRD versions:\n\n%s", diff) + } + }) + } +} From e08b7cfea11fe1a38d08859f9acedad0d2f50138 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 16:47:28 +0200 Subject: [PATCH 12/26] add more unit tests for CRD projections On-behalf-of: @SAP christoph.mewes@sap.com --- internal/projection/projection.go | 30 +- internal/projection/projection_test.go | 437 ++++++++++++++++++++++--- 2 files changed, 411 insertions(+), 56 deletions(-) diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 71fed1f..2251df3 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -100,7 +100,12 @@ func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *sync } // now we get to actually project something, if desired - result, err = projectCRD(result, pubRes) + result, err = projectCRDVersions(result, pubRes) + if err != nil { + return nil, err + } + + result, err = projectCRDNames(result, pubRes) if err != nil { return nil, err } @@ -168,16 +173,12 @@ func adjustStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (*apiex return crd, nil } -func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { +func projectCRDVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { projection := pubRes.Spec.Projection if projection == nil { return crd, nil } - if projection.Group != "" { - crd.Spec.Group = projection.Group - } - // We already validated that Version and Versions can be set at the same time. //nolint:staticcheck @@ -203,6 +204,7 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent if knownVersions.Has(version.Name) { return nil, fmt.Errorf("CRD contains multiple entries for %s after applying mutation rules", version.Name) } + knownVersions.Insert(version.Name) } // ensure proper Kubernetes-style version order @@ -211,6 +213,19 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent }) } + return crd, nil +} + +func projectCRDNames(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { + projection := pubRes.Spec.Projection + if projection == nil { + return crd, nil + } + + if projection.Group != "" { + crd.Spec.Group = projection.Group + } + if projection.Kind != "" { crd.Spec.Names.Kind = projection.Kind crd.Spec.Names.ListKind = projection.Kind + "List" @@ -235,5 +250,8 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent crd.Spec.Names.ShortNames = projection.ShortNames } + // re-calculate CRD name + crd.Name = fmt.Sprintf("%s.%s", crd.Spec.Names.Plural, crd.Spec.Group) + return crd, nil } diff --git a/internal/projection/projection_test.go b/internal/projection/projection_test.go index 964b58a..dcdd4a5 100644 --- a/internal/projection/projection_test.go +++ b/internal/projection/projection_test.go @@ -19,10 +19,11 @@ package projection import ( "testing" + "github.com/google/go-cmp/cmp" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/runtime/schema" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestPublishedResourceSourceGVK(t *testing.T) { @@ -53,92 +54,428 @@ func TestPublishedResourceSourceGVK(t *testing.T) { } } -func TestPublishedResourceProjectedGVK(t *testing.T) { - const ( - apiGroup = "testgroup" - version = "v1" - kind = "test" - ) - - pubRes := &syncagentv1alpha1.PublishedResource{ - Spec: syncagentv1alpha1.PublishedResourceSpec{ - Resource: syncagentv1alpha1.SourceResourceDescriptor{ - APIGroup: apiGroup, - Version: version, - Kind: kind, +func TestProjectCRD(t *testing.T) { + crdSingleVersion := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "things.example.com", + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: "example.com", + Scope: apiextensionsv1.ClusterScoped, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: "things", + Singular: "thing", + Kind: "Thing", + ListKind: "ThingList", + }, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ + { + Name: "v1", + Served: true, + Storage: true, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, }, }, } - crd := &apiextensionsv1.CustomResourceDefinition{ + crdMultiVersions := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "things.example.com", + }, Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Group: apiGroup, + Group: "example.com", + Scope: apiextensionsv1.ClusterScoped, Names: apiextensionsv1.CustomResourceDefinitionNames{ - Kind: kind, + Plural: "things", + Singular: "thing", + Kind: "Thing", + ListKind: "ThingList", }, Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ { - Name: version, + Name: "v1beta1", + Served: false, + Storage: false, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, + { + Name: "v1", + Served: true, + Storage: false, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, + { + Name: "v2alpha1", + Served: true, + Storage: false, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, + { + Name: "v2", Served: true, Storage: true, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, + { + Name: "v3", + Served: true, + Storage: false, }, }, }, } + patchCRD := func(base *apiextensionsv1.CustomResourceDefinition, patch func(*apiextensionsv1.CustomResourceDefinition)) *apiextensionsv1.CustomResourceDefinition { + crd := base.DeepCopy() + patch(crd) + + return crd + } + testcases := []struct { - name string - projection *syncagentv1alpha1.ResourceProjection - expected schema.GroupVersionKind + name string + crd *apiextensionsv1.CustomResourceDefinition + pubRes syncagentv1alpha1.PublishedResourceSpec + expected *apiextensionsv1.CustomResourceDefinition + expectErr bool }{ { - name: "no projection", - projection: nil, - expected: schema.GroupVersionKind{Group: apiGroup, Version: version, Kind: kind}, + name: "no projection on a single-version CRD", + crd: crdSingleVersion, + pubRes: syncagentv1alpha1.PublishedResourceSpec{}, + expected: crdSingleVersion, + }, + { + name: "no projection on a multi-version CRD", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{}, + expected: crdMultiVersions, + }, + { + name: "select a single version (deprecated)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Version: "v3", + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v3", + Served: true, + Storage: true, // should be flipped to true by the projection + }} + }), + }, + { + name: "select a single version (modern)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v3"}, + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v3", + Served: true, + Storage: true, + }} + }), }, { - name: "override version", - projection: &syncagentv1alpha1.ResourceProjection{Version: "v2"}, - expected: schema.GroupVersionKind{Group: apiGroup, Version: "v2", Kind: kind}, + name: "select a subset of versions", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v3", "v1"}, // note the order here + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + // Resulting CRD versions must be properly sorted regardless of the source rules. + + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v1", + Served: true, + Storage: false, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, { + Name: "v3", + Served: true, + Storage: true, // should be flipped to true by the projection + }} + }), }, { - name: "override kind", - projection: &syncagentv1alpha1.ResourceProjection{Kind: "dummy"}, - expected: schema.GroupVersionKind{Group: apiGroup, Version: version, Kind: "dummy"}, + name: "error: select a non-existing version (deprecated)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Version: "v4", + }, + }, + expectErr: true, }, { - name: "override both", - projection: &syncagentv1alpha1.ResourceProjection{Version: "v2", Kind: "dummy"}, - expected: schema.GroupVersionKind{Group: apiGroup, Version: "v2", Kind: "dummy"}, + name: "error: select a non-existing version (modern)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v4"}, + }, + }, + expectErr: true, }, { - name: "override group", - projection: &syncagentv1alpha1.ResourceProjection{Group: "projected.com"}, - expected: schema.GroupVersionKind{Group: "projected.com", Version: version, Kind: kind}, + name: "error: select no served version", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v1beta1"}, + }, + }, + expectErr: true, + }, + { + name: "auto-determine storage version", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v2", "v1"}, + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v1", + Served: true, + Storage: false, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }, { + Name: "v2", + Served: true, + Storage: true, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }} + }), + }, + { + name: "project single version (deprecated)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v3"}, + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Version: "v6", + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v6", + Served: true, + Storage: true, + }} + }), + }, + { + name: "project single version (modern)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v3"}, + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Versions: map[string]string{ + "v3": "v6", + }, + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v6", + Served: true, + Storage: true, + }} + }), + }, + { + name: "error: project multiple versions to the same version", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v3", "v1"}, + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Versions: map[string]string{ + "v3": "v6", + "v1": "v6", + }, + }, + }, + expectErr: true, + }, + { + name: "project multiple versions", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + Versions: []string{"v3", "v1"}, + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Versions: map[string]string{ + "v3": "v6", + "v1": "v7", + }, + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: "v6", + Served: true, + Storage: true, + }, { + Name: "v7", + Served: true, + Storage: false, + Subresources: &apiextensionsv1.CustomResourceSubresources{ + Status: &apiextensionsv1.CustomResourceSubresourceStatus{}, + }, + }} + }), + }, + { + name: "project API group", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Projection: &syncagentv1alpha1.ResourceProjection{ + Group: "new.example.com", + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Group = "new.example.com" + crd.Name = "things.new.example.com" + }), + }, + { + name: "project kind (auto-pluralizes)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Projection: &syncagentv1alpha1.ResourceProjection{ + Kind: "NewThing", + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Names.Kind = "NewThing" + crd.Spec.Names.ListKind = "NewThingList" + crd.Spec.Names.Singular = "newthing" + crd.Spec.Names.Plural = "newthings" + crd.Name = "newthings.example.com" + }), + }, + { + name: "project kind (explicit plural projection)", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Projection: &syncagentv1alpha1.ResourceProjection{ + Kind: "Foot", + Plural: "feet", + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Names.Kind = "Foot" + crd.Spec.Names.ListKind = "FootList" + crd.Spec.Names.Singular = "foot" + crd.Spec.Names.Plural = "feet" + crd.Name = "feet.example.com" + }), + }, + { + name: "project CRD properties", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Projection: &syncagentv1alpha1.ResourceProjection{ + Scope: syncagentv1alpha1.NamespaceScoped, + ShortNames: []string{"shorty"}, + Categories: []string{"e2e"}, + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Scope = apiextensionsv1.NamespaceScoped + crd.Spec.Names.ShortNames = []string{"shorty"} + crd.Spec.Names.Categories = []string{"e2e"} + }), + }, + { + name: "ensure new name takes both new group and new plural into account", + crd: crdMultiVersions, + pubRes: syncagentv1alpha1.PublishedResourceSpec{ + Projection: &syncagentv1alpha1.ResourceProjection{ + Group: "new.example.com", + Plural: "feet", + }, + }, + expected: patchCRD(crdMultiVersions, func(crd *apiextensionsv1.CustomResourceDefinition) { + crd.Spec.Group = "new.example.com" + crd.Spec.Names.Plural = "feet" + crd.Name = "feet.new.example.com" + }), }, } for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - pr := pubRes.DeepCopy() - pr.Spec.Projection = testcase.projection - - gvk, err := PublishedResourceProjectedGVK(crd, pr) - if err != nil { - t.Fatalf("Unexpected error: %v", err) + pr := &syncagentv1alpha1.PublishedResource{ + Spec: testcase.pubRes, } - if gvk.Group != testcase.expected.Group { - t.Errorf("Expected API group to be %q, but got %q.", testcase.expected.Group, gvk.Group) - } + projectedCRD, err := ProjectCRD(testcase.crd, pr) + if err != nil { + if !testcase.expectErr { + t.Fatalf("Unexpected error: %v", err) + } - if gvk.Version != testcase.expected.Version { - t.Errorf("Expected version to be %q, but got %q.", testcase.expected.Version, gvk.Version) + return + } else if testcase.expectErr { + t.Fatalf("Expected an error, but got a CRD instead:\n\n%+v", projectedCRD) } - if gvk.Kind != testcase.expected.Kind { - t.Errorf("Expected kind to be %q, but got %q.", testcase.expected.Kind, gvk.Kind) - } + compareSchemalessCRDs(t, testcase.expected, projectedCRD) }) } } + +func compareSchemalessCRDs(t *testing.T, expected, actual *apiextensionsv1.CustomResourceDefinition) { + if expected.Name != actual.Name { + t.Errorf("Expected CRD to be named %q, got %q.", expected.Name, actual.Name) + } + + if diff := cmp.Diff(expected.Spec.Names, actual.Spec.Names); diff != "" { + t.Errorf("Actual CRD names do not match expectations:\n\n%s", diff) + } + + for i, v := range actual.Spec.Versions { + v.Schema = nil + actual.Spec.Versions[i] = v + } + + if diff := cmp.Diff(expected.Spec.Versions, actual.Spec.Versions); diff != "" { + t.Errorf("Actual CRD versions do not match expectations:\n\n%s", diff) + } +} From 87d26cec359ead0174cc6344b1c2da0ac2b5b92c Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 19:30:39 +0200 Subject: [PATCH 13/26] add more e2e tests for the APIExport controller On-behalf-of: @SAP christoph.mewes@sap.com --- test/e2e/apiexport/apiexport_test.go | 375 +++++++++++++++++++++++++++ 1 file changed, 375 insertions(+) diff --git a/test/e2e/apiexport/apiexport_test.go b/test/e2e/apiexport/apiexport_test.go index 7265b80..c02a875 100644 --- a/test/e2e/apiexport/apiexport_test.go +++ b/test/e2e/apiexport/apiexport_test.go @@ -20,6 +20,8 @@ package apiexport import ( "context" + "slices" + "strings" "testing" "time" @@ -359,3 +361,376 @@ func TestExistingPermissionsClaimsAreKept(t *testing.T) { t.Fatalf("Failed to wait for APIExport to be updated: %v", err) } } + +func TestSchemasAreMerged(t *testing.T) { + const ( + apiExportName = "kcp.example.com" + ) + + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, "apiexport-schemas-are-updated", apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/crontab.yaml", + }) + + // set a random resource schema that is supposed to survive + orgClient := utils.GetClient(t, orgKubconfig) + apiExportKey := types.NamespacedName{Name: apiExportName} + + apiExport := &kcpapisv1alpha1.APIExport{} + if err := orgClient.Get(ctx, apiExportKey, apiExport); err != nil { + t.Fatalf("Failed to get APIExport: %v", err) + } + + foreignSchemaName := "v1.fakes.example.com" + managedSchemaSuffix := ".crontabs.example.com" + oldManagedSchemaName := "v0" + managedSchemaSuffix + apiExport.Spec.LatestResourceSchemas = []string{ + foreignSchemaName, // this is supposed to survive unchanged + oldManagedSchemaName, // this is supposed to be updated + } + + if err := orgClient.Update(ctx, apiExport); err != nil { + t.Fatalf("Failed to update APIExport: %v", err) + } + + // publish Crontabs + t.Logf("Publishing CRD…") + prCrontabs := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Versions: []string{"v1"}, + Kind: "CronTab", + }, + Related: []syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "super-secret", + Origin: "kcp", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.test.name", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.test.namespace", + }, + }, + }, + }, + }, + }, + } + + if err := envtestClient.Create(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // let the agent do its thing + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait for the APIExport to be updated + t.Logf("Waiting for APIExport to be updated…") + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + err = orgClient.Get(ctx, apiExportKey, apiExport) + if err != nil { + return false, err + } + + for _, schema := range apiExport.Spec.LatestResourceSchemas { + if strings.HasSuffix(schema, managedSchemaSuffix) && schema != oldManagedSchemaName { + return true, nil + } + } + + return false, nil + }) + if err != nil { + t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + } + + // check if the foreign schema is still present + if !slices.Contains(apiExport.Spec.LatestResourceSchemas, foreignSchemaName) { + t.Fatalf("Expected APIExport to still contain %s, but is %v instead.", foreignSchemaName, apiExport.Spec.LatestResourceSchemas) + } + + // sanity check + if len(apiExport.Spec.LatestResourceSchemas) != 2 { + t.Fatalf("Expected 2 schemas, but APIExport has %v instead.", apiExport.Spec.LatestResourceSchemas) + } +} + +func TestSchemaIsKeptWhenDeletingPublishedResource(t *testing.T) { + const ( + apiExportName = "kcp.example.com" + ) + + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, "apiexport-schema-is-kept", apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/backup.yaml", + "test/crds/crontab.yaml", + }) + + // publish Crontabs + t.Logf("Publishing Crontab CRD…") + crontabsSchemaSuffix := ".crontabs.example.com" + prCrontabs := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Versions: []string{"v1"}, + Kind: "CronTab", + }, + Related: []syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "super-secret", + Origin: "kcp", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.test.name", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.test.namespace", + }, + }, + }, + }, + }, + }, + } + + if err := envtestClient.Create(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // let the agent do its thing + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait for the APIExport to be contain the new ARS + t.Logf("Waiting for APIExport to be updated…") + + orgClient := utils.GetClient(t, orgKubconfig) + apiExport := &kcpapisv1alpha1.APIExport{} + apiExportKey := types.NamespacedName{Name: apiExportName} + + var crontabsSchemaName string + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + err = orgClient.Get(ctx, apiExportKey, apiExport) + if err != nil { + return false, err + } + + for _, schema := range apiExport.Spec.LatestResourceSchemas { + if strings.HasSuffix(schema, crontabsSchemaSuffix) { + crontabsSchemaName = schema + return true, nil + } + } + + return false, nil + }) + if err != nil { + t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + } + + // Now delete the PublishedResource + t.Logf("Unpublishing Crontab CRD…") + if err := envtestClient.Delete(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to delete PublishedResource: %v", err) + } + + // force a reconcile by creating another, different PR + t.Logf("Publishing Backup CRD…") + backupsSchemaSuffix := ".backups.eksempel.no" + prBackups := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-backups", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "eksempel.no", + Versions: []string{"v1"}, + Kind: "Backup", + }, + }, + } + + if err := envtestClient.Create(ctx, prBackups); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // wait for the APIExport to be contain the new ARS + t.Logf("Waiting for APIExport to be updated…") + err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + err = orgClient.Get(ctx, apiExportKey, apiExport) + if err != nil { + return false, err + } + + for _, schema := range apiExport.Spec.LatestResourceSchemas { + if strings.HasSuffix(schema, backupsSchemaSuffix) { + return true, nil + } + } + + return false, nil + }) + if err != nil { + t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + } + + // check that crontabs are still present + if !slices.Contains(apiExport.Spec.LatestResourceSchemas, crontabsSchemaName) { + t.Fatalf("Expected APIExport to still contain %s, but is %v instead.", crontabsSchemaName, apiExport.Spec.LatestResourceSchemas) + } +} + +func TestNewSchemasAreCreatedAsNeeded(t *testing.T) { + const ( + apiExportName = "kcp.example.com" + ) + + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, "apiexport-new-schemas-are-created", apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/crontab.yaml", + }) + + // publish Crontabs + t.Logf("Publishing CRD…") + managedSchemaSuffix := ".crontabs.example.com" + prCrontabs := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Versions: []string{"v1"}, + Kind: "CronTab", + }, + Related: []syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "super-secret", + Origin: "kcp", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.test.name", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.test.namespace", + }, + }, + }, + }, + }, + }, + } + + if err := envtestClient.Create(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // let the agent do its thing + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait for the APIExport to be updated + t.Logf("Waiting for APIExport to be updated…") + + orgClient := utils.GetClient(t, orgKubconfig) + apiExportKey := types.NamespacedName{Name: apiExportName} + apiExport := &kcpapisv1alpha1.APIExport{} + + var oldSchemaName string + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + err = orgClient.Get(ctx, apiExportKey, apiExport) + if err != nil { + return false, err + } + + for _, schema := range apiExport.Spec.LatestResourceSchemas { + if strings.HasSuffix(schema, managedSchemaSuffix) { + oldSchemaName = schema + return true, nil + } + } + + return false, nil + }) + if err != nil { + t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + } + + // Cause a new ARS to be created; usually this is because the underlying CRD changes, + // but here for simplicity we simply toggle the CRD's scope using projection. + if err := envtestClient.Get(ctx, ctrlruntimeclient.ObjectKeyFromObject(prCrontabs), prCrontabs); err != nil { + t.Fatalf("Failed to fetch current PublishedResource: %v", err) + } + + prCrontabs.Spec.Projection = &syncagentv1alpha1.ResourceProjection{ + Scope: syncagentv1alpha1.ClusterScoped, + } + + t.Logf("Changing PublishedResource…") + if err := envtestClient.Update(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to update PublishedResource: %v", err) + } + + // wait for the APIExport to be updated + t.Logf("Waiting for APIExport to be updated…") + err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + err = orgClient.Get(ctx, apiExportKey, apiExport) + if err != nil { + return false, err + } + + for _, schema := range apiExport.Spec.LatestResourceSchemas { + if strings.HasSuffix(schema, managedSchemaSuffix) && schema != oldSchemaName { + return true, nil + } + } + + return false, nil + }) + if err != nil { + t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + } + + // sanity check + if len(apiExport.Spec.LatestResourceSchemas) != 1 { + t.Fatalf("Expected 1 schema, but APIExport has %v instead.", apiExport.Spec.LatestResourceSchemas) + } +} From 23b85530b934239e02ba4bc7bb6be6130a4f5f65 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 19:43:37 +0200 Subject: [PATCH 14/26] fix: make apiresourceschema controller watch for CRD changes On-behalf-of: @SAP christoph.mewes@sap.com --- .../apiresourceschema/controller.go | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/internal/controller/apiresourceschema/controller.go b/internal/controller/apiresourceschema/controller.go index 931b277..f4e7232 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -37,12 +37,14 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/builder" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/kontext" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -87,10 +89,33 @@ func Add( WithOptions(controller.Options{MaxConcurrentReconciles: numWorkers}). // Watch for changes to PublishedResources on the local service cluster For(&syncagentv1alpha1.PublishedResource{}, builder.WithPredicates(predicate.ByLabels(prFilter))). + Watches(&apiextensionsv1.CustomResourceDefinition{}, handler.TypedEnqueueRequestsFromMapFunc(reconciler.enqueueMatchingPublishedResources)). Build(reconciler) + return err } +func (r *Reconciler) enqueueMatchingPublishedResources(ctx context.Context, obj ctrlruntimeclient.Object) []reconcile.Request { + crd := obj.(*apiextensionsv1.CustomResourceDefinition) + + pubResources := &syncagentv1alpha1.PublishedResourceList{} + if err := r.localClient.List(ctx, pubResources); err != nil { + runtime.HandleError(err) + return nil + } + + var requests []reconcile.Request + for _, pr := range pubResources.Items { + if pr.Spec.Resource.APIGroup == crd.Spec.Group && pr.Spec.Resource.Kind == crd.Spec.Names.Kind { + requests = append(requests, reconcile.Request{ + NamespacedName: ctrlruntimeclient.ObjectKeyFromObject(&pr), + }) + } + } + + return requests +} + func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { log := r.log.With("publishedresource", request) log.Debug("Processing") From bfcb66677c00b881dc8a1f07a0a83d4cf833d142 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 20:06:43 +0200 Subject: [PATCH 15/26] make selecting ARS easier by labelling them with the agent name On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/apiresourceschema/controller.go | 3 +++ sdk/apis/syncagent/v1alpha1/types.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/internal/controller/apiresourceschema/controller.go b/internal/controller/apiresourceschema/controller.go index f4e7232..c6df23d 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -210,6 +210,9 @@ func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.Sugar syncagentv1alpha1.SourceGenerationAnnotation: fmt.Sprintf("%d", projectedCRD.Generation), syncagentv1alpha1.AgentNameAnnotation: r.agentName, } + ars.Labels = map[string]string{ + syncagentv1alpha1.AgentNameLabel: r.agentName, + } ars.Spec.Group = converted.Spec.Group ars.Spec.Names = converted.Spec.Names ars.Spec.Scope = converted.Spec.Scope diff --git a/sdk/apis/syncagent/v1alpha1/types.go b/sdk/apis/syncagent/v1alpha1/types.go index e5c13f1..6f81328 100644 --- a/sdk/apis/syncagent/v1alpha1/types.go +++ b/sdk/apis/syncagent/v1alpha1/types.go @@ -20,6 +20,9 @@ const ( // AgentNameAnnotation records which Sync Agent has created an APIResourceSchema. AgentNameAnnotation = "syncagent.kcp.io/agent-name" + // AgentNameLabel records which Sync Agent has created an APIResourceSchema. + AgentNameLabel = "syncagent.kcp.io/agent-name" + // SourceGenerationAnnotation is the annotation on APIResourceSchemas that tells us // what generation of the CRD it was based on. This can be helpful in debugging, // as ARS resources cannot be updated, i.e. changes to CRDs are not reflected in ARS. From c19bcfe2d87a8a115a663c13fb05598e9d910ba4 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 20:07:11 +0200 Subject: [PATCH 16/26] this test obviously failed since we fixed the CRD watching issue earlier, adjust it to the new logic On-behalf-of: @SAP christoph.mewes@sap.com --- .../apiresourceschema_test.go | 60 +++++++++++-------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/test/e2e/apiresourceschema/apiresourceschema_test.go b/test/e2e/apiresourceschema/apiresourceschema_test.go index 9b25d9d..33a2032 100644 --- a/test/e2e/apiresourceschema/apiresourceschema_test.go +++ b/test/e2e/apiresourceschema/apiresourceschema_test.go @@ -36,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" ctrlruntime "sigs.k8s.io/controller-runtime" + ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) func TestARSAreCreated(t *testing.T) { @@ -147,51 +148,62 @@ func TestARSAreNotUpdated(t *testing.T) { // let the agent do its thing utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) - // wait for the APIExport to be updated - t.Logf("Waiting for APIExport to be updated…") + // check ARS + t.Logf("Waiting for APIResourceSchema to be created…") orgClient := utils.GetClient(t, orgKubconfig) - apiExportKey := types.NamespacedName{Name: apiExportName} - var arsName string err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { - apiExport := &kcpapisv1alpha1.APIExport{} - err = orgClient.Get(ctx, apiExportKey, apiExport) + schemas := &kcpapisv1alpha1.APIResourceSchemaList{} + err = orgClient.List(ctx, schemas, ctrlruntimeclient.HasLabels{syncagentv1alpha1.AgentNameLabel}) if err != nil { return false, err } - if len(apiExport.Spec.LatestResourceSchemas) == 0 { - return false, nil - } - - arsName = apiExport.Spec.LatestResourceSchemas[0] - - return true, nil + return len(schemas.Items) == 1, nil }) if err != nil { - t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + t.Fatalf("Failed to wait for APIResourceSchema to be created: %v", err) + } + + if err := envtestClient.Get(ctx, ctrlruntimeclient.ObjectKeyFromObject(pr), pr); err != nil { + t.Fatalf("Failed to fetch PublishedResource: %v", err) + } + + arsName := pr.Status.ResourceSchemaName + if arsName == "" { + t.Fatal("Expected PublishedResource status to contain ARS name, but value is empty.") } // update the CRD t.Logf("Updating CRD (same version, but new schema)…") utils.ApplyCRD(t, ctx, envtestClient, "test/crds/crontab-improved.yaml") - // give the agent some time to do nothing - time.Sleep(3 * time.Second) + // wait for the 2nd ARS to appear + t.Logf("Waiting for 2nd APIResourceSchema to be created…") + err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + schemas := &kcpapisv1alpha1.APIResourceSchemaList{} + err = orgClient.List(ctx, schemas, ctrlruntimeclient.HasLabels{syncagentv1alpha1.AgentNameLabel}) + if err != nil { + return false, err + } - // validate that the APIExport has *not* changed - apiExport := &kcpapisv1alpha1.APIExport{} - err = orgClient.Get(ctx, apiExportKey, apiExport) + return len(schemas.Items) == 2, nil + }) if err != nil { - t.Fatalf("APIExport disappeared: %v", err) + t.Fatalf("Failed to wait for 2nd APIResourceSchema to be created: %v", err) + } + + if err := envtestClient.Get(ctx, ctrlruntimeclient.ObjectKeyFromObject(pr), pr); err != nil { + t.Fatalf("Failed to fetch PublishedResource: %v", err) } - if l := len(apiExport.Spec.LatestResourceSchemas); l != 1 { - t.Fatalf("APIExport should still have 1 resource schema, but has %d.", l) + newARSName := pr.Status.ResourceSchemaName + if newARSName == "" { + t.Fatal("Expected PublishedResource status to contain ARS name, but value is empty.") } - if currentName := apiExport.Spec.LatestResourceSchemas[0]; currentName != arsName { - t.Fatalf("APIExport should still refer to the original ARS %q, but now contains %q.", arsName, currentName) + if newARSName == arsName { + t.Fatalf("Expected PublishedResource status to have been updated with new ARS name, but still contains %q.", arsName) } } From b3c56e8275cbc4b7a604ad2e48075b216fd9837f Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 20:14:40 +0200 Subject: [PATCH 17/26] gimps On-behalf-of: @SAP christoph.mewes@sap.com --- test/e2e/discovery/discovery_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/discovery/discovery_test.go b/test/e2e/discovery/discovery_test.go index 6426364..aeb40af 100644 --- a/test/e2e/discovery/discovery_test.go +++ b/test/e2e/discovery/discovery_test.go @@ -27,8 +27,8 @@ import ( "github.com/kcp-dev/api-syncagent/internal/discovery" "github.com/kcp-dev/api-syncagent/test/utils" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/clientcmd" ctrlruntime "sigs.k8s.io/controller-runtime" From 736a5a81026b19798ab8046a12e2cb590546979c Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 20:24:35 +0200 Subject: [PATCH 18/26] update docs On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/apiresourceschema/doc.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/controller/apiresourceschema/doc.go b/internal/controller/apiresourceschema/doc.go index ee46bc8..67ae39b 100644 --- a/internal/controller/apiresourceschema/doc.go +++ b/internal/controller/apiresourceschema/doc.go @@ -15,15 +15,13 @@ limitations under the License. */ /* -Package apiresourceschema contains a controller that watches for PublishedResources -and creates a matching APIResourceSchema (ARS) in kcp. The name of the generated -ARS is stored in the PublishedResource's status, so that the apiexport controller -can find and include it in the generated APIExport. +Package apiresourceschema contains a controller that watches for PublishedResources and CRDs +and creates a matching APIResourceSchema (ARS) and, optionally, an APIConversion in kcp. +The name of the generated ARS is stored in the PublishedResource's status, so that the +apiexport controller can find and include it in the generated APIExport. -The ARS name contains a hash over the GVK that the PublishedResource is pointing -to. This is to ensure that if an PublishedResource is created, then deleted, modified -with an editor and re-applied, it won't turn into the same ARS, as we cannot simply -turn an ARS for a Pod into an ARS for a StorageClass. +The ARS name contains a hash over the Group, Kind and spec of the projected CRD. This way any +changes to the original CRD or projection rules will result in a new ARS. There is no extra cleanup procedure in either of the clusters when a PublishedResource is deleted. This is to prevent accidental data loss in kcp in case a service owner From bce4988b73905e47a0dbef3d5ac29c8a4e4ecc86 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 21:13:48 +0200 Subject: [PATCH 19/26] enable reconcile logger to see when APIExports are updated On-behalf-of: @SAP christoph.mewes@sap.com --- cmd/api-syncagent/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/api-syncagent/main.go b/cmd/api-syncagent/main.go index 55d77d7..9febe46 100644 --- a/cmd/api-syncagent/main.go +++ b/cmd/api-syncagent/main.go @@ -27,6 +27,7 @@ import ( "github.com/kcp-dev/logicalcluster/v3" "github.com/spf13/pflag" "go.uber.org/zap" + reconcilerlog "k8c.io/reconciler/pkg/log" "github.com/kcp-dev/api-syncagent/internal/controller/apiexport" "github.com/kcp-dev/api-syncagent/internal/controller/apiresourceschema" @@ -80,6 +81,7 @@ func main() { // set the logger used by sigs.k8s.io/controller-runtime ctrlruntimelog.SetLogger(zapr.NewLogger(log.WithOptions(zap.AddCallerSkip(1)))) + reconcilerlog.SetLogger(sugar) if err := run(ctx, sugar, opts); err != nil { sugar.Fatalw("Sync Agent has encountered an error", zap.Error(err)) From dc7ac4709e81c9f6644a101f7d94ccb8e6578e12 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 14:23:48 +0200 Subject: [PATCH 20/26] fix docs On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/apiresourceschema/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/apiresourceschema/doc.go b/internal/controller/apiresourceschema/doc.go index 67ae39b..00bcde2 100644 --- a/internal/controller/apiresourceschema/doc.go +++ b/internal/controller/apiresourceschema/doc.go @@ -16,7 +16,7 @@ limitations under the License. /* Package apiresourceschema contains a controller that watches for PublishedResources and CRDs -and creates a matching APIResourceSchema (ARS) and, optionally, an APIConversion in kcp. +and creates a matching APIResourceSchema (ARS) in kcp. The name of the generated ARS is stored in the PublishedResource's status, so that the apiexport controller can find and include it in the generated APIExport. From caa9af7e36c4aa7a55e55971862c8283d1922109 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 14:25:19 +0200 Subject: [PATCH 21/26] rename function On-behalf-of: @SAP christoph.mewes@sap.com --- internal/controller/apiresourceschema/controller.go | 2 +- internal/projection/projection.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/apiresourceschema/controller.go b/internal/controller/apiresourceschema/controller.go index c6df23d..2e65db5 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -160,7 +160,7 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR } // project the CRD (i.e. strip unwanted versions, rename values etc.) - projectedCRD, err := projection.ApplyProjection(crd, pubResource) + projectedCRD, err := projection.ProjectCRD(crd, pubResource) if err != nil { return nil, fmt.Errorf("failed to apply projection rules: %w", err) } diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 2251df3..8e4dcc0 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -67,7 +67,7 @@ func getStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) string { // PublishedResourceProjectedGVK returns the effective GVK after the projection // rules have been applied according to the PublishedResource. func PublishedResourceProjectedGVK(originalCRD *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (schema.GroupVersionKind, error) { - projectedCRD, err := ApplyProjection(originalCRD, pubRes) + projectedCRD, err := ProjectCRD(originalCRD, pubRes) if err != nil { return schema.GroupVersionKind{}, fmt.Errorf("failed to project CRD: %w", err) } @@ -84,7 +84,7 @@ func PublishedResourceProjectedGVK(originalCRD *apiextensionsv1.CustomResourceDe }, nil } -func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { +func ProjectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { result := crd.DeepCopy() // reduce the CRD down to the selected versions From fc2b82cf76587ef22cd5300861d7c837cd75c132 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 14:40:32 +0200 Subject: [PATCH 22/26] update existing docs On-behalf-of: @SAP christoph.mewes@sap.com --- docs/content/publish-resources.md | 41 +++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/docs/content/publish-resources.md b/docs/content/publish-resources.md index bdc9c7e..971b904 100644 --- a/docs/content/publish-resources.md +++ b/docs/content/publish-resources.md @@ -18,8 +18,10 @@ For each of the CRDs on the service cluster that should be published, the servic `PublishedResource` object, which will contain both which CRD to publish, as well as numerous other important settings that influence the behaviour around handling the CRD. -When publishing a resource (CRD), exactly one version is published. All others are ignored from the -standpoint of the resource synchronization logic. +When publishing a resource (CRD), service owners can choose to restrict it to a subset of available +versions and even change API group, versions and names in transit (for example published a v1 from +the service cluster as v1beta1 within kcp). This process of changing the identity of a CRD is called +"projection" in the agent. All published resources together form the APIExport. When a service is enabled in a workspace (i.e. it is bound to it), users can manage objects for the projected resources described by the @@ -46,11 +48,18 @@ spec: resource: kind: Certificate apiGroup: cert-manager.io - version: v1 + versions: [v1] ``` However, you will most likely apply more configuration and use features described below. +You always have to select at least one version, and all selected versions must be marked as `served` +on the service cluster. If the storage version is selected to be published, it stays the storage +version in kcp. If no storage version is selected, the latest selected version becomes the storage +version. + +For more information refer to the [API lifecycle](api-lifecycle.md). + ### Filtering The Sync Agent can be instructed to only work on a subset of resources in kcp. This can be restricted @@ -70,16 +79,18 @@ spec: foo: bar ``` +The configuration above would mean the agent only synchronizes objects from `my-app` namespaces (in +each of the kcp workspaces) that also have a `foo=bar` label on them. + ### Schema -**Warning:** The actual CRD schema is always copied verbatim. All projections -etc. have to take into account that the resource contents must be expressible without changes to the -schema, so you cannot define entirely new fields in an object that are not defined by the original -CRD. +**Warning:** The actual CRD schema is always copied verbatim. All projections, mutations etc. have +to take into account that the resource contents must be expressible without changes to the schema, +so you cannot define entirely new fields in an object that are not defined by the original CRD. ### Projection -For stronger separation of concerns and to enable whitelabelling of services, the type meta for +For stronger separation of concerns and to enable whitelabelling of services, the type meta for CRDs can be projected, i.e. changed between the local service cluster and kcp. You could for example rename `Certificate` from cert-manager to `Sertifikat` inside kcp. @@ -103,10 +114,14 @@ metadata: spec: resource: ... projection: - version: v1beta1 + # all of these options are optional kind: Sertifikat plural: Sertifikater shortNames: [serts] + versions: + # old version => new version; + # this must not map multiple versions to the same new version. + v1: v1beta1 # categories: [management] # scope: Namespaced # change only when you know what you're doing ``` @@ -114,7 +129,7 @@ spec: Consumers (end users) in kcp would then ultimately see projected names only. Note that GVK projection applies only to the synced object itself and has no effect on the contents of these objects. To change the contents, use external solutions like Crossplane to transform objects. - +To change the contents, use *Mutations*. ### (Re-)Naming @@ -274,7 +289,7 @@ spec: resource: kind: Certificate apiGroup: cert-manager.io - version: v1 + versions: [v1] naming: # this is where our CA and Issuer live in this example @@ -360,7 +375,7 @@ spec: resource: kind: Certificate apiGroup: cert-manager.io - version: v1 + versions: [v1] naming: namespace: kube-system @@ -445,7 +460,7 @@ spec: resource: kind: Certificate apiGroup: cert-manager.io - version: v1 + versions: [v1] naming: # this is where our CA and Issuer live in this example From ff1c78800e97f04d66aeb5f55b08b268b0721e90 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 14:45:43 +0200 Subject: [PATCH 23/26] extend FAQ On-behalf-of: @SAP christoph.mewes@sap.com --- docs/content/faq.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/content/faq.md b/docs/content/faq.md index 03e7bc2..c9c1a09 100644 --- a/docs/content/faq.md +++ b/docs/content/faq.md @@ -17,18 +17,18 @@ Only if you have distinct API groups (and therefore also distinct `PublishedReso You cannot currently publish the same API group onto multiple kcp setups. See issue #13 for more information. -## What happens when CRDs are updated? - -At the moment, nothing. `APIResourceSchemas` in kcp are immutable and the Sync Agent currently does -not attempt to update existing schemas in an `APIExport`. If you add a _new_ CRD that you want to -publish, that's fine, it will be added to the `APIExport`. But changes to existing CRDs require -manual work. - -To trigger an update: - -* remove the `APIResourceSchema` from the `latestResourceSchemas`, -* delete the `APIResourceSchema` object in kcp, -* restart the api-syncagent +## Can I have additional resources in APIExports, unmanaged by the Sync Agent? + +Yes, you can. The agent will only ever change those resourceSchemas that match group/resource of +the configured `PublishedResources`. So if you configure the agent to publish +`cert-manager.io/Certificate`, this would "claim" all resource schemas ending in +`.certificates.cert-manager.io`. When updating the `APIExport`, the agent will only touch schemas +with this suffix and leave all others alone. + +This is also used when a `PublishedResource` is deleted: Since the `APIResourceSchema` remains in kcp, +but is no longer configured in the agent, the agent will simply ignore the schema in the `APIExport`. +This allows for async cleanup processes to happen before an admin ultimately removes the old +schema from the `APIExport`. ## Does the Sync Agent handle permission claims? From f66cfa894c8e7703315c9e3bdff16d1be2e93c3a Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 15:36:11 +0200 Subject: [PATCH 24/26] extend docs On-behalf-of: @SAP christoph.mewes@sap.com --- docs/content/api-lifecycle.md | 64 +++++++++++++++++++++++++++++++++++ docs/mkdocs.yml | 2 +- 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 docs/content/api-lifecycle.md diff --git a/docs/content/api-lifecycle.md b/docs/content/api-lifecycle.md new file mode 100644 index 0000000..6714439 --- /dev/null +++ b/docs/content/api-lifecycle.md @@ -0,0 +1,64 @@ +# API Lifecycle + +In only the rarest of cases will the first version of a CRD be also its final version. Instead usually +CRDs evolve over time and Kubernetes has strong, though sometimes hard to use, support for managing +different versions of CRDs and their resources. + +To understand how CRDs work in the context of the Sync Agent, it's important to first get familiar +with the [regular Kubernetes behaviour](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/) +regarding CRD versioning. + +## Basics + +The Sync Agent will, whenever a published CRD changes (this can also happen when the projection rules +inside a `PublishedResource` are updated), create a new `APIResourceSchema` (ARS) in kcp. The name and +version of this ARS are based on a hash of the projected CRD. Undoing a change would make the agent +re-use the previously created ARS (ARS are immutable). + +After every reconciliation, the list of latest resource schemas in the configured `APIExport` is +updated. For this the agent will find all ARS that belong to it (based on an ownership label) and +then merge them into the `APIExport`. Resource schemas for unknown group/resource combinations are +left untouched, so admins are free to add additional resource schemas to an `APIExport`. + +This means that every change to a CRD on the service cluster is applied practically immediately in +each workspace that consumes the `APIExport`. Administrators are wise to act carefully when working +with their CRDs on their service cluster. Sometimes it can make sense to turn-off the agent before +testing new CRDs, even though this will temporarily suspend the synchronization. + +## Single-Version CRDs + +A very common scenario is to only ever have a single version inside each CRD and keeping this version +perpetually backwards-compatible. As long as all consumers are aware that certain fields might not +be set yet in older objects, this scheme works out generally fine. + +The agent will handle this scenario just fine by itself. Whenever a CRD is updated, it will reflect +those changes back into a new `APIResourceSchema` and update the `APIExport`, making the changes +immediately available to all consumers. Since the agent itself doesn't much care for the contents of +objects, it itself is not affected by any structural changes in CRDs, as long as it is able to apply +them on the underlying Kubernetes cluster. + +## Multi-Version CRDs + +Having multiple versions in a single CRD is immediately much more work, since in Kubernetes all +versions of a CRD must be _losslessly_ convertible to every other version. Without CEL expressions +or a dedicated conversion webhook this is practically impossible to achieve. + +At the moment kcp does not support CEL-based conversions, and there is no support for configuring a +conversion webhook inside the Sync Agent either. This is because such a webhook would need to run +very close to the kcp shards and it's simply out of scope for such a component to be described and +deployed by the Sync Agent, let alone a trust nightmare for the kcp operators who would have to run +foreign webhooks on their cluster. + +Since both conversion mechanisms are not usable in the current state of kcp and the Sync Agent, +having multiple versions in a CRD can be difficult to manage. + +Generally the Sync Agent itself does not care much about the schemas of each CRD version or the +convertibility between them. The synchronization works by using unstructured clients to the storage +versison of the CRD on both sides (in kcp and on the service cluster). Which version is the storage +version is up to the CRD author. + +When publishing multiple versions of a CRD + +* only those versions marked as `served` can be picked and +* if no `storage` version is picked, the latest (highest) version will be chosen automatically as + the storage version in kcp. diff --git a/docs/mkdocs.yml b/docs/mkdocs.yml index 804df15..8ad481d 100644 --- a/docs/mkdocs.yml +++ b/docs/mkdocs.yml @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. - site_name: api-syncagent repo_url: https://github.com/kcp-dev/api-syncagent repo_name: kcp-dev/api-syncagent @@ -24,6 +23,7 @@ nav: - Getting Started: getting-started.md - Publishing Resources: publish-resources.md - Consuming Services: consuming-services.md + - API Lifecycle: api-lifecycle.md - FAQ: faq.md - Release Process: releasing.md From 5a2456fcd36632d2f16490fe10d8185cc6446ea4 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 15:41:12 +0200 Subject: [PATCH 25/26] gimps On-behalf-of: @SAP christoph.mewes@sap.com --- internal/projection/projection_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/projection/projection_test.go b/internal/projection/projection_test.go index dcdd4a5..ee9a106 100644 --- a/internal/projection/projection_test.go +++ b/internal/projection/projection_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" From 58721d67f3daa29bc1180fc3a52cbc33ed242be7 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Wed, 21 May 2025 16:58:21 +0200 Subject: [PATCH 26/26] fix missing conversion on multi-version ARS On-behalf-of: @SAP christoph.mewes@sap.com --- .../apiresourceschema/controller.go | 7 ++ .../apiresourceschema_test.go | 93 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/internal/controller/apiresourceschema/controller.go b/internal/controller/apiresourceschema/controller.go index 2e65db5..3655867 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -218,6 +218,13 @@ func (r *Reconciler) createAPIResourceSchema(ctx context.Context, log *zap.Sugar ars.Spec.Scope = converted.Spec.Scope ars.Spec.Versions = converted.Spec.Versions + if len(converted.Spec.Versions) > 1 { + ars.Spec.Conversion = &kcpdevv1alpha1.CustomResourceConversion{ + // as of kcp 0.27, there is no constant for this + Strategy: kcpdevv1alpha1.ConversionStrategyType("None"), + } + } + log.With("name", arsName).Info("Creating APIResourceSchema…") return r.kcpClient.Create(ctx, ars) diff --git a/test/e2e/apiresourceschema/apiresourceschema_test.go b/test/e2e/apiresourceschema/apiresourceschema_test.go index 33a2032..0b138fe 100644 --- a/test/e2e/apiresourceschema/apiresourceschema_test.go +++ b/test/e2e/apiresourceschema/apiresourceschema_test.go @@ -34,6 +34,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" ctrlruntime "sigs.k8s.io/controller-runtime" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -287,6 +288,98 @@ func TestARSOnlyContainsSelectedCRDVersion(t *testing.T) { } } +func TestMultiVersionCRD(t *testing.T) { + const ( + apiExportName = "example.com" + ) + + // force a non-standard order, because it should not matter for the sync + var selectedVersions = []string{"v2", "v1"} + + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, "ars-multi-versions", apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/crontab-multi-versions.yaml", + }) + + // publish Crontabs + t.Logf("Publishing CronTabs…") + pr := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Versions: selectedVersions, + Kind: "CronTab", + }, + }, + } + + if err := envtestClient.Create(ctx, pr); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // let the agent do its thing + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait for the APIExport to be updated + t.Logf("Waiting for APIExport to be updated…") + orgClient := utils.GetClient(t, orgKubconfig) + apiExportKey := types.NamespacedName{Name: apiExportName} + + var arsName string + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + apiExport := &kcpapisv1alpha1.APIExport{} + err = orgClient.Get(ctx, apiExportKey, apiExport) + if err != nil { + return false, err + } + + if len(apiExport.Spec.LatestResourceSchemas) == 0 { + return false, nil + } + + arsName = apiExport.Spec.LatestResourceSchemas[0] + + return true, nil + }) + if err != nil { + t.Fatalf("Failed to wait for APIExport to be updated: %v", err) + } + + // check the APIResourceSchema + ars := &kcpapisv1alpha1.APIResourceSchema{} + err = orgClient.Get(ctx, types.NamespacedName{Name: arsName}, ars) + if err != nil { + t.Fatalf("APIResourceSchema does not exist: %v", err) + } + + if len(ars.Spec.Versions) != len(selectedVersions) { + t.Fatalf("Expected %d versions to remain in the ARS, but found %d.", len(selectedVersions), len(ars.Spec.Versions)) + } + + // Projection tests already ensure the correct order, all we have to check + // here is that all versions are present (and more importantly that creating + // the multi-version ARS has worked at all). + expectedVersions := sets.New(selectedVersions...) + foundVersions := sets.New[string]() + + for _, v := range ars.Spec.Versions { + foundVersions.Insert(v.Name) + } + + if !expectedVersions.Equal(foundVersions) { + t.Fatalf("Expected versions %v, but ARS contains %v.", sets.List(expectedVersions), sets.List(foundVersions)) + } +} + func TestProjection(t *testing.T) { const ( apiExportName = "example.com"