From 2285d3569ccbf443c9d087fe7565c8c789c11f45 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 15 May 2025 18:05:41 +0200 Subject: [PATCH 01/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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 f332be3e9a4f38107822eee7a677f3af06532ba0 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 20:53:58 +0200 Subject: [PATCH 09/22] add API conversion to PublishedResources On-behalf-of: @SAP christoph.mewes@sap.com --- .../syncagent/v1alpha1/published_resource.go | 11 +++++ sdk/go.mod | 2 + sdk/go.sum | 43 +++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index 9c7b870..ce402db 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -17,6 +17,8 @@ limitations under the License. package v1alpha1 import ( + kcpapisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -87,6 +89,15 @@ type PublishedResourceSpec struct { // directions during the synchronization. Mutation *ResourceMutationSpec `json:"mutation,omitempty"` + // Conversions specify rules to convert between different API versions in the selected CRD. + // This field is required when more than one version is published into kcp. + // + // The from and to versions in each conversion refer to the local CRD versions, i.e. before + // any projection rules are applied. They will be automatically mutated during reconciliation. + Conversions []kcpapisv1alpha1.APIVersionConversion `json:"conversions,omitempty"` + + // Related describes additional objects that belong to a primary object. These related objects + // can be synced along the primary object in both directions of the sync. Related []RelatedResourceSpec `json:"related,omitempty"` } diff --git a/sdk/go.mod b/sdk/go.mod index 1459091..6699195 100644 --- a/sdk/go.mod +++ b/sdk/go.mod @@ -5,6 +5,7 @@ go 1.23.0 require ( github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250223115924-431177b024f3 github.com/kcp-dev/client-go v0.0.0-20250223133118-3dea338dc267 + github.com/kcp-dev/kcp/sdk v0.27.1 github.com/kcp-dev/logicalcluster/v3 v3.0.5 k8s.io/apimachinery v0.31.6 k8s.io/client-go v0.31.6 @@ -51,6 +52,7 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/api v0.31.6 // indirect + k8s.io/apiextensions-apiserver v0.31.6 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect diff --git a/sdk/go.sum b/sdk/go.sum index 20d3e2b..23da2e6 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -1,3 +1,13 @@ +github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= +github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= +github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= +github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= +github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= +github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM= +github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -22,6 +32,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= +github.com/google/cel-go v0.20.1 h1:nDx9r8S3L4pE61eDdt8igGj8rf5kjYR3ILxWIpWNi84= +github.com/google/cel-go v0.20.1/go.mod h1:kWcIzTsPX0zmQ+H3TirHstLLf9ep5QTsZBN9u4dOYLg= github.com/google/gnostic-models v0.6.9 h1:MU/8wDLif2qCXZmzncUQ/BOfxWfthHi63KqpoNbWqVw= github.com/google/gnostic-models v0.6.9/go.mod h1:CiWsm0s6BSQd1hRn8/QmxqB6BesYcbSZxsz9b0KuDBw= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= @@ -34,6 +46,8 @@ github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad h1:a6HEuzUHeKH6hwfN/Z github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad/go.mod h1:vavhavw2zAxS5dIdcRluK6cSGGPlZynqzFM8NdvU144= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= +github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= @@ -42,6 +56,8 @@ github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250223115924-431177b024f3 h1:YwNX7 github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250223115924-431177b024f3/go.mod h1:n0+EV+LGKl1MXXqGbGcn0AaBv7hdKsdazSYuq8nM8Us= github.com/kcp-dev/client-go v0.0.0-20250223133118-3dea338dc267 h1:Ec2/Mh7mVvboBFol0S8u30arfA7oyk/VtHL9Xojjvfs= github.com/kcp-dev/client-go v0.0.0-20250223133118-3dea338dc267/go.mod h1:1lEs8b8BYzGrMr7Q8Fs7cNVaDAWogu5lLkz5t6HtRLI= +github.com/kcp-dev/kcp/sdk v0.27.1 h1:jBVdrZoJd5hy2RqaBnmCCzldimwOqDkf8FXtNq5HaWA= +github.com/kcp-dev/kcp/sdk v0.27.1/go.mod h1:3eRgW42d81Ng60DbG1xbne0FSS2znpcN/GUx4rqJgUo= github.com/kcp-dev/logicalcluster/v3 v3.0.5 h1:JbYakokb+5Uinz09oTXomSUJVQsqfxEvU4RyHUYxHOU= github.com/kcp-dev/logicalcluster/v3 v3.0.5/go.mod h1:EWBUBxdr49fUB1cLMO4nOdBWmYifLbP1LfoL20KkXYY= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= @@ -68,10 +84,22 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/prometheus/client_golang v1.19.1 h1:wZWJDwK+NameRJuPGDhlnFgx8e8HN3XHQeLaYJFJBOE= +github.com/prometheus/client_golang v1.19.1/go.mod h1:mP78NwGzrVks5S2H6ab8+ZZGJLZUq1hoULYBAYBw1Ho= +github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E= +github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY= +github.com/prometheus/common v0.55.0 h1:KEi6DK7lXW/m7Ig5i47x0vRzuBsHuvJdi5ee6Y3G1dc= +github.com/prometheus/common v0.55.0/go.mod h1:2SECS4xJG1kd8XF9IcM1gMX6510RAEL65zxzNImwdc8= +github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= +github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= +github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= +github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stoewer/go-strcase v1.2.0 h1:Z2iHWqGXH00XYgqDmNgQbIBxf3wrNq0F3feEy0ainaU= +github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= @@ -83,6 +111,8 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc h1:mCRnTeVUjcrhlRmO0VK8a6k6Rrf6TF9htwo2pJVSjIU= +golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -96,6 +126,8 @@ golang.org/x/oauth2 v0.28.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.12.0 h1:MHc5BpPuC30uJk597Ri8TV3CNZcTLu6B6z4lJy+g6Jw= +golang.org/x/sync v0.12.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -119,6 +151,11 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d h1:VBu5YqKPv6XiJ199exd8Br+Aetz+o08F+PLMnwJQHAY= +google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 h1:7whR9kGa5LUwFtpLm2ArCEejtnxlGeLbAyjFY8sGNFw= +google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157/go.mod h1:99sLkeliLXfdj2J75X3Ho+rrVCaJze0uwN7zDDkjPVU= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 h1:BwIjyKYGsK9dMCBOorzRri8MQwmi7mT9rGHsCEinZkA= +google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY= google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY= google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -134,10 +171,16 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.31.6 h1:ocWG/UhC9Mqp5oEfYWy9wCddbZiZyBAFTlBt0LVlhDg= k8s.io/api v0.31.6/go.mod h1:i16xSiKMgVIVhsJMxfWq0mJbXA+Z7KhjPgYmwT41hl4= +k8s.io/apiextensions-apiserver v0.31.6 h1:v9sqyWlrgFZpAPdEb/bEiXfM98TfSppwRF0X/uWKXh0= +k8s.io/apiextensions-apiserver v0.31.6/go.mod h1:QVH3CFwqzGZtwsxPYzJlA/Qiwgb5FXmRMGls3CjzvbI= k8s.io/apimachinery v0.31.6 h1:Pn96A0wHD0X8+l7QTdAzdLQPrpav1s8rU6A+v2/9UEY= k8s.io/apimachinery v0.31.6/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo= +k8s.io/apiserver v0.31.6 h1:FEhEGLsz1PbMOHeQZDbOUlMh36zRZbjgKwJCoMhdGmw= +k8s.io/apiserver v0.31.6/go.mod h1:dpFh+xqFQ02O8vLYCIqoiV7sJIpZsUULeNuag6Y9HGo= k8s.io/client-go v0.31.6 h1:51HT40qVIZ13BrHKeWxFuU52uoPnFhxTYJnv4+LTgp4= k8s.io/client-go v0.31.6/go.mod h1:MEq7JQJelUQ0/4fMoPEUrc/OOFyGo/9LmGA38H6O6xY= +k8s.io/component-base v0.31.6 h1:FgI25PuZtCp2n7AFpOaDpMQOLieFdrpAbpeoZu7VhDI= +k8s.io/component-base v0.31.6/go.mod h1:aVRrh8lAI1kSShFmwcKLhc3msQoUcmFWPBDf0sXaISM= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag= From 193da9a2c51b639b81865c6c4b6f6ee2ec860a91 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 20:54:32 +0200 Subject: [PATCH 10/22] codegen On-behalf-of: @SAP christoph.mewes@sap.com --- .../syncagent.kcp.io_publishedresources.yaml | 70 +++++++++++++++++++ .../v1alpha1/zz_generated.deepcopy.go | 9 +++ .../v1alpha1/publishedresourcespec.go | 15 ++++ 3 files changed, 94 insertions(+) diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index 427314b..12b3eef 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -47,6 +47,73 @@ spec: PublishedResourceSpec describes the desired resource publication from a service cluster to kcp. properties: + conversions: + description: |- + Conversions specify rules to convert between different API versions in the selected CRD. + This field is required when more than one version is published into kcp. + + The from and to versions in each conversion refer to the local CRD versions, i.e. before + any projection rules are applied. They will be automatically mutated during reconciliation. + items: + description: |- + APIVersionConversion contains rules to convert between two specific API versions in an + APIResourceSchema. Additionally, to avoid data loss when round-tripping from a version that + contains a new field to one that doesn't and back again, you can specify a list of fields to + preserve (these are stored in annotations). + properties: + from: + description: from is the source version. + minLength: 1 + pattern: ^v[1-9][0-9]*([a-z]+[1-9][0-9]*)?$ + type: string + preserve: + description: |- + preserve contains a list of JSONPath expressions to fields to preserve in the originating version + of the object, relative to its root, such as '.spec.name.first'. + items: + type: string + type: array + rules: + description: rules contains field-specific conversion expressions. + items: + description: APIConversionRule specifies how to convert a single field. + properties: + destination: + description: |- + destination is a JSONPath expression to the field in the target version of the object, relative to + its root, such as '.spec.name.first'. + minLength: 1 + type: string + field: + description: |- + field is a JSONPath expression to the field in the originating version of the object, relative to its root, such + as '.spec.name.first'. + minLength: 1 + type: string + transformation: + description: |- + transformation is an optional CEL expression used to execute user-specified rules to transform the + originating field -- identified by 'self' -- to the destination field. + type: string + required: + - destination + - field + type: object + type: array + x-kubernetes-list-map-keys: + - destination + x-kubernetes-list-type: map + to: + description: to is the target version. + minLength: 1 + pattern: ^v[1-9][0-9]*([a-z]+[1-9][0-9]*)?$ + type: string + required: + - from + - rules + - to + type: object + type: array enableWorkspacePaths: description: |- EnableWorkspacePaths toggles whether the Sync Agent will not just store the kcp @@ -340,6 +407,9 @@ spec: type: array type: object related: + description: |- + Related describes additional objects that belong to a primary object. These related objects + can be synced along the primary object in both directions of the sync. items: properties: identifier: diff --git a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go index 3ab7c1d..5a9314f 100644 --- a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go +++ b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go @@ -21,6 +21,8 @@ limitations under the License. package v1alpha1 import ( + apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -108,6 +110,13 @@ func (in *PublishedResourceSpec) DeepCopyInto(out *PublishedResourceSpec) { *out = new(ResourceMutationSpec) (*in).DeepCopyInto(*out) } + if in.Conversions != nil { + in, out := &in.Conversions, &out.Conversions + *out = make([]apisv1alpha1.APIVersionConversion, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Related != nil { in, out := &in.Related, &out.Related *out = make([]RelatedResourceSpec, len(*in)) diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/publishedresourcespec.go b/sdk/applyconfiguration/syncagent/v1alpha1/publishedresourcespec.go index a733ddf..332af3c 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/publishedresourcespec.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/publishedresourcespec.go @@ -18,6 +18,10 @@ limitations under the License. package v1alpha1 +import ( + apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" +) + // PublishedResourceSpecApplyConfiguration represents a declarative configuration of the PublishedResourceSpec type for use // with apply. type PublishedResourceSpecApplyConfiguration struct { @@ -27,6 +31,7 @@ type PublishedResourceSpecApplyConfiguration struct { EnableWorkspacePaths *bool `json:"enableWorkspacePaths,omitempty"` Projection *ResourceProjectionApplyConfiguration `json:"projection,omitempty"` Mutation *ResourceMutationSpecApplyConfiguration `json:"mutation,omitempty"` + Conversions []apisv1alpha1.APIVersionConversion `json:"conversions,omitempty"` Related []RelatedResourceSpecApplyConfiguration `json:"related,omitempty"` } @@ -84,6 +89,16 @@ func (b *PublishedResourceSpecApplyConfiguration) WithMutation(value *ResourceMu return b } +// WithConversions adds the given value to the Conversions 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 Conversions field. +func (b *PublishedResourceSpecApplyConfiguration) WithConversions(values ...apisv1alpha1.APIVersionConversion) *PublishedResourceSpecApplyConfiguration { + for i := range values { + b.Conversions = append(b.Conversions, values[i]) + } + return b +} + // WithRelated adds the given value to the Related 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 Related field. From d0d8c1f7ded98e8ae06f8ba51ea1afa21db19230 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 20:57:32 +0200 Subject: [PATCH 11/22] add reconciling config for APIConversions On-behalf-of: @SAP christoph.mewes@sap.com --- hack/reconciling.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/reconciling.yaml b/hack/reconciling.yaml index 321400d..be8db5b 100644 --- a/hack/reconciling.yaml +++ b/hack/reconciling.yaml @@ -20,4 +20,4 @@ boilerplate: hack/boilerplate/generated/boilerplate.go.txt resourceTypes: # kcp-dev/v1alpha1 - { package: github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1, importAlias: kcpdevv1alpha1, resourceName: APIExport } - - { package: github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1, importAlias: kcpdevv1alpha1, resourceName: APIResourceSchema } + - { package: github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1, importAlias: kcpdevv1alpha1, resourceName: APIConversion } From 3acf0d1b116948c86ee43eb304daf26eb31db315 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 20:58:03 +0200 Subject: [PATCH 12/22] codegen On-behalf-of: @SAP christoph.mewes@sap.com --- .../reconciling/zz_generated_reconcile.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/resources/reconciling/zz_generated_reconcile.go b/internal/resources/reconciling/zz_generated_reconcile.go index 0a22759..ad3a730 100644 --- a/internal/resources/reconciling/zz_generated_reconcile.go +++ b/internal/resources/reconciling/zz_generated_reconcile.go @@ -64,28 +64,28 @@ func ReconcileAPIExports(ctx context.Context, namedFactories []NamedAPIExportRec return nil } -// APIResourceSchemaReconciler defines an interface to create/update APIResourceSchemas. -type APIResourceSchemaReconciler = func(existing *kcpdevv1alpha1.APIResourceSchema) (*kcpdevv1alpha1.APIResourceSchema, error) +// APIConversionReconciler defines an interface to create/update APIConversions. +type APIConversionReconciler = func(existing *kcpdevv1alpha1.APIConversion) (*kcpdevv1alpha1.APIConversion, error) -// NamedAPIResourceSchemaReconcilerFactory returns the name of the resource and the corresponding Reconciler function. -type NamedAPIResourceSchemaReconcilerFactory = func() (name string, reconciler APIResourceSchemaReconciler) +// NamedAPIConversionReconcilerFactory returns the name of the resource and the corresponding Reconciler function. +type NamedAPIConversionReconcilerFactory = func() (name string, reconciler APIConversionReconciler) -// APIResourceSchemaObjectWrapper adds a wrapper so the APIResourceSchemaReconciler matches ObjectReconciler. +// APIConversionObjectWrapper adds a wrapper so the APIConversionReconciler matches ObjectReconciler. // This is needed as Go does not support function interface matching. -func APIResourceSchemaObjectWrapper(reconciler APIResourceSchemaReconciler) reconciling.ObjectReconciler { +func APIConversionObjectWrapper(reconciler APIConversionReconciler) reconciling.ObjectReconciler { return func(existing ctrlruntimeclient.Object) (ctrlruntimeclient.Object, error) { if existing != nil { - return reconciler(existing.(*kcpdevv1alpha1.APIResourceSchema)) + return reconciler(existing.(*kcpdevv1alpha1.APIConversion)) } - return reconciler(&kcpdevv1alpha1.APIResourceSchema{}) + return reconciler(&kcpdevv1alpha1.APIConversion{}) } } -// ReconcileAPIResourceSchemas will create and update the APIResourceSchemas coming from the passed APIResourceSchemaReconciler slice. -func ReconcileAPIResourceSchemas(ctx context.Context, namedFactories []NamedAPIResourceSchemaReconcilerFactory, namespace string, client ctrlruntimeclient.Client, objectModifiers ...reconciling.ObjectModifier) error { +// ReconcileAPIConversions will create and update the APIConversions coming from the passed APIConversionReconciler slice. +func ReconcileAPIConversions(ctx context.Context, namedFactories []NamedAPIConversionReconcilerFactory, namespace string, client ctrlruntimeclient.Client, objectModifiers ...reconciling.ObjectModifier) error { for _, factory := range namedFactories { name, reconciler := factory() - reconcileObject := APIResourceSchemaObjectWrapper(reconciler) + reconcileObject := APIConversionObjectWrapper(reconciler) reconcileObject = reconciling.CreateWithNamespace(reconcileObject, namespace) reconcileObject = reconciling.CreateWithName(reconcileObject, name) @@ -93,8 +93,8 @@ func ReconcileAPIResourceSchemas(ctx context.Context, namedFactories []NamedAPIR reconcileObject = objectModifier(reconcileObject) } - if err := reconciling.EnsureNamedObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, reconcileObject, client, &kcpdevv1alpha1.APIResourceSchema{}, false); err != nil { - return fmt.Errorf("failed to ensure APIResourceSchema %s/%s: %w", namespace, name, err) + if err := reconciling.EnsureNamedObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, reconcileObject, client, &kcpdevv1alpha1.APIConversion{}, false); err != nil { + return fmt.Errorf("failed to ensure APIConversion %s/%s: %w", namespace, name, err) } } From 3640fe5227885890d9ac67cb0ce2b81e0d7e58ed Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 21:21:52 +0200 Subject: [PATCH 13/22] 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 12b3eef..27e9eb7 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -391,20 +391,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: description: |- 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 ce402db..fecc277 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -306,7 +306,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"` @@ -330,11 +330,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 5a9314f..0101365 100644 --- a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go +++ b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go @@ -419,8 +419,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 50e77d1e4213faaee362c6a6afd4c55fed541128 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 21:40:20 +0200 Subject: [PATCH 14/22] 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 0101365..2c16276 100644 --- a/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go +++ b/sdk/apis/syncagent/v1alpha1/zz_generated.deepcopy.go @@ -510,18 +510,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 92d0af9072c8753401f12b32f082207c65ac40de Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 16 May 2025 21:40:45 +0200 Subject: [PATCH 15/22] add logic to apiresourceschema ctrl to maintain a matching APIConversion object in kcp On-behalf-of: @SAP christoph.mewes@sap.com --- .../apiresourceschema/controller.go | 27 +++++++++++- .../apiresourceschema/reconciler.go | 42 +++++++++++++++++++ internal/projection/projection.go | 29 +++++++++++-- 3 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 internal/controller/apiresourceschema/reconciler.go diff --git a/internal/controller/apiresourceschema/controller.go b/internal/controller/apiresourceschema/controller.go index 931b277..3ba3563 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -28,6 +28,7 @@ import ( "github.com/kcp-dev/api-syncagent/internal/crypto" "github.com/kcp-dev/api-syncagent/internal/discovery" "github.com/kcp-dev/api-syncagent/internal/projection" + "github.com/kcp-dev/api-syncagent/internal/resources/reconciling" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" kcpdevv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" @@ -135,16 +136,28 @@ 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) } // generate a unique name for this exact state of the CRD arsName := r.getAPIResourceSchemaName(projectedCRD) + wsCtx := kontext.WithCluster(ctx, r.lcName) + + projectedConversions, err := projection.ProjectConversionRules(pubResource) + if err != nil { + return nil, fmt.Errorf("failed to apply projection rules to conversions: %w", err) + } + + // only reconcile if there are rules because APIConversions must contain at least one conversion + if len(projectedConversions) > 0 { + if err := r.reconcileConversions(wsCtx, arsName, projectedConversions); err != nil { + return nil, fmt.Errorf("failed to reconcile APIConversions: %w", err) + } + } // 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{}) @@ -169,6 +182,8 @@ func (r *Reconciler) reconcile(ctx context.Context, log *zap.SugaredLogger, pubR } } + // reconcile a matching APIConversion object + return nil, nil } @@ -206,3 +221,11 @@ func (r *Reconciler) getAPIResourceSchemaName(crd *apiextensionsv1.CustomResourc // 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) } + +func (r *Reconciler) reconcileConversions(ctx context.Context, arsName string, rules []kcpdevv1alpha1.APIVersionConversion) error { + factories := []reconciling.NamedAPIConversionReconcilerFactory{ + r.createAPIConversionReconciler(arsName, r.agentName, rules), + } + + return reconciling.ReconcileAPIConversions(ctx, factories, "", r.kcpClient) +} diff --git a/internal/controller/apiresourceschema/reconciler.go b/internal/controller/apiresourceschema/reconciler.go new file mode 100644 index 0000000..7318da9 --- /dev/null +++ b/internal/controller/apiresourceschema/reconciler.go @@ -0,0 +1,42 @@ +/* +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 apiresourceschema + +import ( + "github.com/kcp-dev/api-syncagent/internal/resources/reconciling" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + + kcpapisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + kcpdevv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" +) + +func (r *Reconciler) createAPIConversionReconciler(name string, agentName string, rules []kcpapisv1alpha1.APIVersionConversion) reconciling.NamedAPIConversionReconcilerFactory { + return func() (string, reconciling.APIConversionReconciler) { + return name, func(existing *kcpdevv1alpha1.APIConversion) (*kcpdevv1alpha1.APIConversion, error) { + if existing.Annotations == nil { + existing.Annotations = map[string]string{} + } + existing.Annotations[syncagentv1alpha1.AgentNameAnnotation] = agentName + + existing.Spec = kcpdevv1alpha1.APIConversionSpec{ + Conversions: rules, + } + + return existing, nil + } + } +} diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 71fed1f..12e8819 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -24,6 +24,8 @@ import ( syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + kcpapisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" @@ -67,7 +69,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 +86,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 @@ -100,7 +102,7 @@ func ApplyProjection(crd *apiextensionsv1.CustomResourceDefinition, pubRes *sync } // now we get to actually project something, if desired - result, err = projectCRD(result, pubRes) + result, err = projectCRDNames(result, pubRes) if err != nil { return nil, err } @@ -168,7 +170,7 @@ func adjustStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (*apiex return crd, nil } -func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { +func projectCRDNames(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { projection := pubRes.Spec.Projection if projection == nil { return crd, nil @@ -237,3 +239,22 @@ func projectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent return crd, nil } + +func ProjectConversionRules(pubRes *syncagentv1alpha1.PublishedResource) ([]kcpapisv1alpha1.APIVersionConversion, error) { + result := pubRes.DeepCopy().Spec.Conversions + + if proj := pubRes.Spec.Projection; proj != nil { + for idx, conversion := range result { + if projectedFrom := proj.Versions[conversion.From]; projectedFrom != "" { + conversion.From = projectedFrom + } + if projectedTo := proj.Versions[conversion.To]; projectedTo != "" { + conversion.To = projectedTo + } + + result[idx] = conversion + } + } + + return result, nil +} From 589d82921e89e5743ce847d111882ea3eadf3280 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 15:15:35 +0200 Subject: [PATCH 16/22] 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 7bf4455a5d3849770762db49c8fdb5d87c3cee34 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 16:47:28 +0200 Subject: [PATCH 17/22] add more unit tests for CRD projections On-behalf-of: @SAP christoph.mewes@sap.com --- internal/projection/projection.go | 28 +- internal/projection/projection_test.go | 437 ++++++++++++++++++++++--- 2 files changed, 410 insertions(+), 55 deletions(-) diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 12e8819..b80579a 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -102,6 +102,11 @@ func ProjectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent } // now we get to actually project something, if desired + result, err = projectCRDVersions(result, pubRes) + if err != nil { + return nil, err + } + result, err = projectCRDNames(result, pubRes) if err != nil { return nil, err @@ -170,16 +175,12 @@ func adjustStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (*apiex return crd, nil } -func projectCRDNames(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 @@ -205,6 +206,7 @@ func projectCRDNames(crd *apiextensionsv1.CustomResourceDefinition, pubRes *sync 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 @@ -213,6 +215,19 @@ func projectCRDNames(crd *apiextensionsv1.CustomResourceDefinition, pubRes *sync }) } + 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" @@ -237,6 +252,9 @@ func projectCRDNames(crd *apiextensionsv1.CustomResourceDefinition, pubRes *sync 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 f03b86357cf0ec0ddc702453aa91c4d3e9d1aace Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 16:59:37 +0200 Subject: [PATCH 18/22] test conversion rule projection On-behalf-of: @SAP christoph.mewes@sap.com --- internal/projection/projection_test.go | 96 ++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/internal/projection/projection_test.go b/internal/projection/projection_test.go index dcdd4a5..5f1335c 100644 --- a/internal/projection/projection_test.go +++ b/internal/projection/projection_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + kcpapisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -479,3 +480,98 @@ func compareSchemalessCRDs(t *testing.T, expected, actual *apiextensionsv1.Custo t.Errorf("Actual CRD versions do not match expectations:\n\n%s", diff) } } + +func TestProjectConversions(t *testing.T) { + testcases := []struct { + name string + conversions []kcpapisv1alpha1.APIVersionConversion + projection *syncagentv1alpha1.ResourceProjection + expected []kcpapisv1alpha1.APIVersionConversion + expectErr bool + }{ + { + name: "no projections at all", + conversions: []kcpapisv1alpha1.APIVersionConversion{{ + From: "v1", + To: "v2", + Rules: []kcpapisv1alpha1.APIConversionRule{}, + }}, + expected: []kcpapisv1alpha1.APIVersionConversion{{ + From: "v1", + To: "v2", + Rules: []kcpapisv1alpha1.APIConversionRule{}, + }}, + }, + { + name: "project versions", + conversions: []kcpapisv1alpha1.APIVersionConversion{{ + From: "v1", + To: "v2", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "a"}}, + }, { + From: "v2", + To: "v3", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "b"}}, + }, { + From: "v3", + To: "v1", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "c"}}, + }, { + From: "v4", + To: "v2", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "d"}}, + }}, + projection: &syncagentv1alpha1.ResourceProjection{ + Versions: map[string]string{ + "v1": "v4", + "v2": "v1", + "v3": "v3", // project to itself + "v4": "v2", + }, + }, + expected: []kcpapisv1alpha1.APIVersionConversion{{ + From: "v4", + To: "v1", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "a"}}, + }, { + From: "v1", + To: "v3", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "b"}}, + }, { + From: "v3", + To: "v4", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "c"}}, + }, { + From: "v2", + To: "v1", + Rules: []kcpapisv1alpha1.APIConversionRule{{Field: "d"}}, + }}, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + pr := &syncagentv1alpha1.PublishedResource{ + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Conversions: testcase.conversions, + Projection: testcase.projection, + }, + } + + rules, err := ProjectConversionRules(pr) + if err != nil { + if !testcase.expectErr { + t.Fatalf("Unexpected error: %v", err) + } + + return + } else if testcase.expectErr { + t.Fatalf("Expected an error, but got rules instead:\n\n%+v", rules) + } + + if diff := cmp.Diff(testcase.expected, rules); diff != "" { + t.Fatalf("Result does not match expectations:\n\n%s", diff) + } + }) + } +} From dfccf049c03eba8a62d9efb2162e6ecbeb2e8d04 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 19:30:39 +0200 Subject: [PATCH 19/22] 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 14023e1f53b54e78f5e4cb03fc8b5fd7b3445172 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 19:43:37 +0200 Subject: [PATCH 20/22] 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 3ba3563..e4e5c07 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -38,12 +38,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" @@ -88,10 +90,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 e911d9b2037a54d25793f1d252e280cbf1150d2f Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 20:06:43 +0200 Subject: [PATCH 21/22] 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 e4e5c07..af5003b 100644 --- a/internal/controller/apiresourceschema/controller.go +++ b/internal/controller/apiresourceschema/controller.go @@ -225,6 +225,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 5f5c89d918bd796284fa1b872db397005c306e53 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 20 May 2025 20:07:11 +0200 Subject: [PATCH 22/22] 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) } }