From 8a0ab331814d11a7c9a3c854f144667c7fe2688e Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 16:37:11 +0200 Subject: [PATCH 01/22] implement basic templating for PR naming rules On-behalf-of: @SAP christoph.mewes@sap.com --- internal/projection/naming.go | 69 ------ internal/sync/object_syncer.go | 13 +- internal/sync/syncer.go | 14 +- internal/sync/syncer_related.go | 4 +- internal/sync/templating/naming.go | 106 ++++++++++ .../templating}/naming_test.go | 13 +- internal/sync/templating/templating.go | 88 ++++++++ .../syncagent/v1alpha1/published_resource.go | 17 +- test/e2e/sync/primary_test.go | 196 +++++++++++++++++- test/e2e/sync/related_test.go | 4 +- 10 files changed, 428 insertions(+), 96 deletions(-) delete mode 100644 internal/projection/naming.go create mode 100644 internal/sync/templating/naming.go rename internal/{projection => sync/templating}/naming_test.go (89%) create mode 100644 internal/sync/templating/templating.go diff --git a/internal/projection/naming.go b/internal/projection/naming.go deleted file mode 100644 index 79e9e98..0000000 --- a/internal/projection/naming.go +++ /dev/null @@ -1,69 +0,0 @@ -/* -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 projection - -import ( - "fmt" - "strings" - - "github.com/kcp-dev/logicalcluster/v3" - - "github.com/kcp-dev/api-syncagent/internal/crypto" - syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" -) - -var DefaultNamingScheme = syncagentv1alpha1.ResourceNaming{ - Namespace: syncagentv1alpha1.PlaceholderRemoteClusterName, - Name: fmt.Sprintf("%s-%s", syncagentv1alpha1.PlaceholderRemoteNamespaceHash, syncagentv1alpha1.PlaceholderRemoteNameHash), -} - -func GenerateLocalObjectName(pr *syncagentv1alpha1.PublishedResource, object metav1.Object, clusterName logicalcluster.Name) types.NamespacedName { - naming := pr.Spec.Naming - if naming == nil { - naming = &syncagentv1alpha1.ResourceNaming{} - } - - replacer := strings.NewReplacer( - // order of elements is important here, "$fooHash" needs to be defined before "$foo" - syncagentv1alpha1.PlaceholderRemoteClusterName, clusterName.String(), - syncagentv1alpha1.PlaceholderRemoteNamespaceHash, crypto.ShortHash(object.GetNamespace()), - syncagentv1alpha1.PlaceholderRemoteNamespace, object.GetNamespace(), - syncagentv1alpha1.PlaceholderRemoteNameHash, crypto.ShortHash(object.GetName()), - syncagentv1alpha1.PlaceholderRemoteName, object.GetName(), - ) - - result := types.NamespacedName{} - - pattern := naming.Namespace - if pattern == "" { - pattern = DefaultNamingScheme.Namespace - } - - result.Namespace = replacer.Replace(pattern) - - pattern = naming.Name - if pattern == "" { - pattern = DefaultNamingScheme.Name - } - - result.Name = replacer.Replace(pattern) - - return result -} diff --git a/internal/sync/object_syncer.go b/internal/sync/object_syncer.go index 86c12fe..9b4e422 100644 --- a/internal/sync/object_syncer.go +++ b/internal/sync/object_syncer.go @@ -35,7 +35,7 @@ import ( ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) -type objectCreatorFunc func(source *unstructured.Unstructured) *unstructured.Unstructured +type objectCreatorFunc func(source *unstructured.Unstructured) (*unstructured.Unstructured, error) type objectSyncer struct { // When set, the syncer will create a label on the destination object that contains @@ -134,7 +134,11 @@ func (s *objectSyncer) applyMutations(source, dest syncSide) (syncSide, syncSide // the mutated names available. destObject := dest.object if destObject == nil { - destObject = s.destCreator(source.object) + var err error + destObject, err = s.destCreator(source.object) + if err != nil { + return source, dest, fmt.Errorf("failed to create destination object: %w", err) + } } sourceObj, err := s.mutator.MutateSpec(source.object.DeepCopy(), destObject) @@ -287,7 +291,10 @@ func (s *objectSyncer) syncObjectStatus(log *zap.SugaredLogger, source, dest syn func (s *objectSyncer) ensureDestinationObject(log *zap.SugaredLogger, source, dest syncSide) error { // create a copy of the source with GVK projected and renaming rules applied - destObj := s.destCreator(source.object) + destObj, err := s.destCreator(source.object) + if err != nil { + return fmt.Errorf("failed to create destination object: %w", err) + } // make sure the target namespace on the destination cluster exists if err := s.ensureNamespace(dest.ctx, log, dest.client, destObj.GetNamespace()); err != nil { diff --git a/internal/sync/syncer.go b/internal/sync/syncer.go index 80628b6..4cf239f 100644 --- a/internal/sync/syncer.go +++ b/internal/sync/syncer.go @@ -23,6 +23,7 @@ import ( "github.com/kcp-dev/api-syncagent/internal/mutation" "github.com/kcp-dev/api-syncagent/internal/projection" + "github.com/kcp-dev/api-syncagent/internal/sync/templating" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -152,7 +153,7 @@ func (s *ResourceSyncer) Process(ctx Context, remoteObj *unstructured.Unstructur agentName: s.agentName, subresources: s.subresources, // use the projection and renaming rules configured in the PublishedResource - destCreator: s.createLocalObjectCreator(ctx), + destCreator: s.newLocalObjectCreator(ctx), // for the main resource, status subresource handling is enabled (this // means _allowing_ status back-syncing, it still depends on whether the // status subresource even exists whether an update happens) @@ -214,8 +215,8 @@ func (s *ResourceSyncer) findLocalObject(ctx Context, remoteObj *unstructured.Un } } -func (s *ResourceSyncer) createLocalObjectCreator(ctx Context) objectCreatorFunc { - return func(remoteObj *unstructured.Unstructured) *unstructured.Unstructured { +func (s *ResourceSyncer) newLocalObjectCreator(ctx Context) objectCreatorFunc { + return func(remoteObj *unstructured.Unstructured) (*unstructured.Unstructured, error) { // map from the remote API into the actual, local API group destObj := remoteObj.DeepCopy() destObj.SetGroupVersionKind(s.destDummy.GroupVersionKind()) @@ -224,7 +225,10 @@ func (s *ResourceSyncer) createLocalObjectCreator(ctx Context) objectCreatorFunc destScope := syncagentv1alpha1.ResourceScope(s.localCRD.Spec.Scope) // map namespace/name - mappedName := projection.GenerateLocalObjectName(s.pubRes, remoteObj, ctx.clusterName) + mappedName, err := templating.GenerateLocalObjectName(s.pubRes, remoteObj, ctx.clusterName, ctx.workspacePath) + if err != nil { + return nil, fmt.Errorf("failed to generate local object name: %w", err) + } switch destScope { case syncagentv1alpha1.ClusterScoped: @@ -236,6 +240,6 @@ func (s *ResourceSyncer) createLocalObjectCreator(ctx Context) objectCreatorFunc destObj.SetName(mappedName.Name) } - return destObj + return destObj, nil } } diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 2dc86ea..e19b112 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -124,12 +124,12 @@ func (s *ResourceSyncer) processRelatedResource(log *zap.SugaredLogger, stateSto // in one place, on the service cluster side stateStore: stateStore, // how to create a new destination object - destCreator: func(source *unstructured.Unstructured) *unstructured.Unstructured { + destCreator: func(source *unstructured.Unstructured) (*unstructured.Unstructured, error) { dest := source.DeepCopy() dest.SetName(resolved.destination.Name) dest.SetNamespace(resolved.destination.Namespace) - return dest + return dest, nil }, // ConfigMaps and Secrets have no subresources subresources: nil, diff --git a/internal/sync/templating/naming.go b/internal/sync/templating/naming.go new file mode 100644 index 0000000..f04ddc4 --- /dev/null +++ b/internal/sync/templating/naming.go @@ -0,0 +1,106 @@ +/* +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 templating + +import ( + "fmt" + "strings" + + "github.com/kcp-dev/logicalcluster/v3" + + "github.com/kcp-dev/api-syncagent/internal/crypto" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" +) + +// localObjectNamingContext is the data available to Go templates when determining +// the local object name (the `naming` section of a PublishedResource). +type localObjectNamingContext struct { + // Object is the full remote object found in a kcp workspace. + Object map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf"). + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). + ClusterPath logicalcluster.Path +} + +func newLocalObjectNamingContext(object *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) localObjectNamingContext { + return localObjectNamingContext{ + Object: object.Object, + ClusterName: clusterName, + ClusterPath: clusterPath, + } +} + +var defaultNamingScheme = syncagentv1alpha1.ResourceNaming{ + Namespace: "{{ .ClusterName }}", + Name: "{{ .Object.metadata.namespace | sha3short }}-{{ .Object.metadata.name | sha3short }}", +} + +func GenerateLocalObjectName(pr *syncagentv1alpha1.PublishedResource, object *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) (types.NamespacedName, error) { + naming := pr.Spec.Naming + if naming == nil { + naming = &syncagentv1alpha1.ResourceNaming{} + } + + result := types.NamespacedName{} + + pattern := naming.Namespace + if pattern == "" { + pattern = defaultNamingScheme.Namespace + } + rendered, err := generateLocalObjectIdentifier(pattern, object, clusterName, clusterPath) + if err != nil { + return result, fmt.Errorf("invalid namespace naming: %w", err) + } + + result.Namespace = rendered + + pattern = naming.Name + if pattern == "" { + pattern = defaultNamingScheme.Name + } + rendered, err = generateLocalObjectIdentifier(pattern, object, clusterName, clusterPath) + if err != nil { + return result, fmt.Errorf("invalid name naming: %w", err) + } + + result.Name = rendered + + return result, nil +} + +func generateLocalObjectIdentifier(pattern string, object *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) (string, error) { + // modern Go template style + if strings.Contains(pattern, "{{") { + return Render(pattern, newLocalObjectNamingContext(object, clusterName, clusterPath)) + } + + // legacy $variable style, does also not support clusterPath + replacer := strings.NewReplacer( + // order of elements is important here, "$fooHash" needs to be defined before "$foo" + syncagentv1alpha1.PlaceholderRemoteClusterName, clusterName.String(), //nolint:staticcheck + syncagentv1alpha1.PlaceholderRemoteNamespaceHash, crypto.ShortHash(object.GetNamespace()), //nolint:staticcheck + syncagentv1alpha1.PlaceholderRemoteNamespace, object.GetNamespace(), //nolint:staticcheck + syncagentv1alpha1.PlaceholderRemoteNameHash, crypto.ShortHash(object.GetName()), //nolint:staticcheck + syncagentv1alpha1.PlaceholderRemoteName, object.GetName(), //nolint:staticcheck + ) + + return replacer.Replace(pattern), nil +} diff --git a/internal/projection/naming_test.go b/internal/sync/templating/naming_test.go similarity index 89% rename from internal/projection/naming_test.go rename to internal/sync/templating/naming_test.go index a8e1dd1..4d27ec8 100644 --- a/internal/projection/naming_test.go +++ b/internal/sync/templating/naming_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package projection +package templating import ( "testing" @@ -23,12 +23,11 @@ import ( syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" ) -func createNewObject(name, namespace string) metav1.Object { +func createNewObject(name, namespace string) *unstructured.Unstructured { obj := &unstructured.Unstructured{} obj.SetName(name) obj.SetNamespace(namespace) @@ -40,7 +39,8 @@ func TestGenerateLocalObjectName(t *testing.T) { testcases := []struct { name string clusterName string - remoteObject metav1.Object + clusterPath string + remoteObject *unstructured.Unstructured namingConfig *syncagentv1alpha1.ResourceNaming expected types.NamespacedName }{ @@ -96,7 +96,10 @@ func TestGenerateLocalObjectName(t *testing.T) { }, } - generatedName := GenerateLocalObjectName(pubRes, testcase.remoteObject, logicalcluster.Name(testcase.clusterName)) + generatedName, err := GenerateLocalObjectName(pubRes, testcase.remoteObject, logicalcluster.Name(testcase.clusterName), logicalcluster.NewPath(testcase.clusterPath)) + if err != nil { + t.Fatalf("Unexpected error: %v.", err) + } if generatedName.String() != testcase.expected.String() { t.Errorf("Expected %q, but got %q.", testcase.expected, generatedName) diff --git a/internal/sync/templating/templating.go b/internal/sync/templating/templating.go new file mode 100644 index 0000000..b580f6e --- /dev/null +++ b/internal/sync/templating/templating.go @@ -0,0 +1,88 @@ +/* +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 templating + +import ( + "bytes" + "encoding/hex" + "fmt" + "strings" + "text/template" + + "crypto/sha3" + "github.com/Masterminds/sprig/v3" + + "github.com/kcp-dev/api-syncagent/internal/crypto" +) + +func Render(tpl string, data any) (string, error) { + parsed, err := template.New("inline").Funcs(templateFuncMap()).Parse(tpl) + if err != nil { + return "", fmt.Errorf("failed to parse: %w", err) + } + + var buf bytes.Buffer + if err := parsed.Execute(&buf, data); err != nil { + return "", fmt.Errorf("failed to evaluate: %w", err) + } + + return strings.TrimSpace(buf.String()), nil +} + +func templateFuncMap() template.FuncMap { + funcs := sprig.TxtFuncMap() + funcs["join"] = strings.Join + funcs["sha3sum"] = sha3sum + funcs["sha3short"] = sha3short + + // shortHash is included for backwards compatibility with the old naming rules, + // new installations should not use it because it relies on SHA-1. Instead use + // sha3short. + funcs["shortHash"] = crypto.ShortHash + + return funcs +} + +func sha3sum(input string) string { + hash := sha3.Sum256([]byte(input)) + return hex.EncodeToString(hash[:]) +} + +// sha3short supports exactly 1 optional length argument. If not given, length defaults to 20. +func sha3short(input string, lengths ...int) (string, error) { + var length int + switch len(lengths) { + case 0: + length = 20 + case 1: + length = lengths[0] + default: + return "", fmt.Errorf("sha3short: expected at most one length argument, got %d", len(lengths)) + } + + if length <= 0 { + return "", fmt.Errorf("sha3short: invalid length %d", length) + } + + hash := sha3sum(input) + + if length > len(hash) { + length = len(hash) + } + + return hash[:length], nil +} diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index 4074500..aedf3dc 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -20,12 +20,21 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// All of these constants are used in the deprecated local naming scheme for +// PublishedResources. New code should not use them, but instead rely on +// Go templated expressions. + const ( - PlaceholderRemoteClusterName = "$remoteClusterName" - PlaceholderRemoteNamespace = "$remoteNamespace" + // Deprecated: Use Go templates instead. + PlaceholderRemoteClusterName = "$remoteClusterName" + // Deprecated: Use Go templates instead. + PlaceholderRemoteNamespace = "$remoteNamespace" + // Deprecated: Use Go templates instead. PlaceholderRemoteNamespaceHash = "$remoteNamespaceHash" - PlaceholderRemoteName = "$remoteName" - PlaceholderRemoteNameHash = "$remoteNameHash" + // Deprecated: Use Go templates instead. + PlaceholderRemoteName = "$remoteName" + // Deprecated: Use Go templates instead. + PlaceholderRemoteNameHash = "$remoteNameHash" ) // +genclient diff --git a/test/e2e/sync/primary_test.go b/test/e2e/sync/primary_test.go index 5de5db1..99c7246 100644 --- a/test/e2e/sync/primary_test.go +++ b/test/e2e/sync/primary_test.go @@ -64,6 +64,96 @@ func TestSyncSimpleObject(t *testing.T) { "test/crds/crontab.yaml", }) + // publish Crontabs and Backups + t.Logf("Publishing CRDs…") + prCrontabs := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Version: "v1", + Kind: "CronTab", + }, + // These rules make finding the local object easier, but should not be used in production. + Naming: &syncagentv1alpha1.ResourceNaming{ + Name: "{{ .Object.metadata.name }}", + Namespace: "synced-{{ .Object.metadata.namespace }}", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Group: kcpGroupName, + }, + }, + } + + if err := envtestClient.Create(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // start the agent in the background to update the APIExport with the CronTabs API + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait until the API is available + teamCtx := kontext.WithCluster(ctx, logicalcluster.Name(fmt.Sprintf("root:%s:team-1", orgWorkspace))) + kcpClient := utils.GetKcpAdminClusterClient(t) + utils.WaitForBoundAPI(t, teamCtx, kcpClient, schema.GroupVersionResource{ + Group: kcpGroupName, + Version: "v1", + Resource: "crontabs", + }) + + // create a Crontab object in a team workspace + t.Log("Creating CronTab in kcp…") + crontab := yamlToUnstructured(t, ` +apiVersion: kcp.example.com/v1 +kind: CronTab +metadata: + namespace: default + name: my-crontab +spec: + cronSpec: '* * *' + image: ubuntu:latest +`) + + if err := kcpClient.Create(teamCtx, crontab); err != nil { + t.Fatalf("Failed to create CronTab in kcp: %v", err) + } + + // wait for the agent to sync the object down into the service cluster + + t.Logf("Wait for CronTab to be synced…") + copy := &unstructured.Unstructured{} + copy.SetAPIVersion("example.com/v1") + copy.SetKind("CronTab") + + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 30*time.Second, false, func(ctx context.Context) (done bool, err error) { + copyKey := types.NamespacedName{Namespace: "synced-default", Name: "my-crontab"} + return envtestClient.Get(ctx, copyKey, copy) == nil, nil + }) + if err != nil { + t.Fatalf("Failed to wait for object to be synced down: %v", err) + } +} + +func TestSyncSimpleObjectOldNaming(t *testing.T) { + const ( + apiExportName = "kcp.example.com" + kcpGroupName = "kcp.example.com" + orgWorkspace = "sync-simple-deprecated" + ) + + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, orgWorkspace, apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/crontab.yaml", + }) + // publish Crontabs and Backups t.Logf("Publishing CRDs…") prCrontabs := &syncagentv1alpha1.PublishedResource{ @@ -136,6 +226,100 @@ spec: } } +func TestSyncWithDefaultNamingRules(t *testing.T) { + const ( + apiExportName = "kcp.example.com" + orgWorkspace = "sync-default-naming-rules" + ) + + ctx := context.Background() + ctrlruntime.SetLogger(logr.Discard()) + + // setup a test environment in kcp + orgKubconfig := utils.CreateOrganization(t, ctx, orgWorkspace, apiExportName) + + // start a service cluster + envtestKubeconfig, envtestClient, _ := utils.RunEnvtest(t, []string{ + "test/crds/crontab.yaml", + }) + + // publish Crontabs and Backups + t.Logf("Publishing CRDs…") + prCrontabs := &syncagentv1alpha1.PublishedResource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "publish-crontabs", + }, + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: "example.com", + Version: "v1", + Kind: "CronTab", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Group: "kcp.example.com", + }, + }, + } + + if err := envtestClient.Create(ctx, prCrontabs); err != nil { + t.Fatalf("Failed to create PublishedResource: %v", err) + } + + // start the agent in the background to update the APIExport with the CronTabs API + utils.RunAgent(ctx, t, "bob", orgKubconfig, envtestKubeconfig, apiExportName) + + // wait until the API is available + kcpClient := utils.GetKcpAdminClusterClient(t) + crontabsGVR := schema.GroupVersionResource{ + Group: "kcp.example.com", + Version: "v1", + Resource: "crontabs", + } + + // create a Crontab object in each team workspace, importantly using the same name and + // namespace in both workspaces + crontabYAML := ` +apiVersion: kcp.example.com/v1 +kind: CronTab +metadata: + namespace: default + name: my-crontab +spec: + cronSpec: '* * *' + image: ubuntu:latest +` + + t.Log("Creating CronTabs in kcp…") + for _, team := range []string{"team-1", "team-2"} { + teamCtx := kontext.WithCluster(ctx, logicalcluster.Name(fmt.Sprintf("root:%s:%s", orgWorkspace, team))) + utils.WaitForBoundAPI(t, teamCtx, kcpClient, crontabsGVR) + + if err := kcpClient.Create(teamCtx, yamlToUnstructured(t, crontabYAML)); err != nil { + t.Fatalf("Failed to create %s's CronTab in kcp: %v", team, err) + } + } + + // wait for the agent to sync both objects done, ensuring that we actually end + // up with 2 distinct objects + + t.Logf("Wait for CronTabs to be synced…") + crontabs := &unstructured.UnstructuredList{} + crontabs.SetAPIVersion("example.com/v1") + crontabs.SetKind("CronTabList") + + err := wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 30*time.Second, false, func(ctx context.Context) (done bool, err error) { + err = envtestClient.List(ctx, crontabs) + if err != nil { + return false, err + } + + return len(crontabs.Items) == 2, nil + }) + if err != nil { + t.Fatalf("Failed to wait for objects to be synced down: %v", err) + } +} + func TestLocalChangesAreKept(t *testing.T) { const ( apiExportName = "kcp.example.com" @@ -168,8 +352,8 @@ func TestLocalChangesAreKept(t *testing.T) { }, // These rules make finding the local object easier, but should not be used in production. Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteName", - Namespace: "synced-$remoteNamespace", + Name: "{{ .Object.metadata.name }}", + Namespace: "synced-{{ .Object.metadata.namespace }}", }, Projection: &syncagentv1alpha1.ResourceProjection{ Group: kcpGroupName, @@ -374,8 +558,8 @@ func TestResourceFilter(t *testing.T) { }, // These rules make finding the local object easier, but should not be used in production. Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteName", - Namespace: "synced-$remoteNamespace", + Name: "{{ .Object.metadata.name }}", + Namespace: "synced-{{ .Object.metadata.namespace }}", }, Projection: &syncagentv1alpha1.ResourceProjection{ Group: kcpGroupName, @@ -495,8 +679,8 @@ func TestSyncingOverlyLongNames(t *testing.T) { }, // These rules make finding the local object easier, but should not be used in production. Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteName", - Namespace: "synced-$remoteNamespace", + Name: "{{ .Object.metadata.name }}", + Namespace: "synced-{{ .Object.metadata.namespace }}", }, Projection: &syncagentv1alpha1.ResourceProjection{ Group: kcpGroupName, diff --git a/test/e2e/sync/related_test.go b/test/e2e/sync/related_test.go index 60433a9..220457e 100644 --- a/test/e2e/sync/related_test.go +++ b/test/e2e/sync/related_test.go @@ -481,8 +481,8 @@ func TestSyncRelatedObjects(t *testing.T) { }, // These rules make finding the local object easier, but should not be used in production. Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteName", - Namespace: "synced-$remoteNamespace", + Name: "{{ .Object.metadata.name }}", + Namespace: "synced-{{ .Object.metadata.namespace }}", }, Projection: &syncagentv1alpha1.ResourceProjection{ Group: "kcp.example.com", From 060d4250308b6eeb15d291972ac305ef2809d921 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 16:54:09 +0200 Subject: [PATCH 02/22] implement support for templated related resource refs On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/syncer_related.go | 30 ++++++++++-- internal/sync/templating/related.go | 48 +++++++++++++++++++ .../syncagent/v1alpha1/published_resource.go | 2 + 3 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 internal/sync/templating/related.go diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index e19b112..3242bcd 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -28,6 +28,7 @@ import ( "go.uber.org/zap" "github.com/kcp-dev/api-syncagent/internal/mutation" + "github.com/kcp-dev/api-syncagent/internal/sync/templating" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" corev1 "k8s.io/api/core/v1" @@ -249,8 +250,9 @@ func resolveRelatedResourceObjects(relatedOrigin, relatedDest syncSide, relRes s func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, spec syncagentv1alpha1.RelatedResourceObjectSpec) (map[string]string, error) { switch { + //nolint:staticcheck case spec.Reference != nil: - originNamespace, err := resolveObjectReference(relatedOrigin.object, *spec.Reference) + originNamespace, err := resolveObjectReference(relatedOrigin.object, *spec.Reference) //nolint:staticcheck if err != nil { return nil, err } @@ -259,7 +261,7 @@ func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, return nil, nil } - destNamespace, err := resolveObjectReference(relatedDest.object, *spec.Reference) + destNamespace, err := resolveObjectReference(relatedDest.object, *spec.Reference) //nolint:staticcheck if err != nil { return nil, err } @@ -495,5 +497,27 @@ func applyTemplate(relatedOrigin, relatedDest syncSide, tpl syncagentv1alpha1.Te } func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, tpl syncagentv1alpha1.TemplateExpression) (originValue, destValue string, err error) { - return "", "", errors.New("not yet implemented") + // clusterName and workspacePath are only set on the kcp side of the sync. + clusterName := relatedDest.clusterName + workspacePath := relatedDest.workspacePath + if clusterName == "" { + clusterName = relatedOrigin.clusterName + workspacePath = relatedOrigin.workspacePath + } + + // evaluate the template for the origin object side + ctx := templating.NewRelatedObjectContext(relatedOrigin.object, clusterName, workspacePath) + originValue, err = templating.Render(tpl.Template, ctx) + if err != nil { + return "", "", fmt.Errorf("failed to evaluate template on origin side: %w", err) + } + + // and once more on the other side + ctx = templating.NewRelatedObjectContext(relatedDest.object, clusterName, workspacePath) + destValue, err = templating.Render(tpl.Template, ctx) + if err != nil { + return "", "", fmt.Errorf("failed to evaluate template on destination side: %w", err) + } + + return originValue, destValue, nil } diff --git a/internal/sync/templating/related.go b/internal/sync/templating/related.go new file mode 100644 index 0000000..fa29a58 --- /dev/null +++ b/internal/sync/templating/related.go @@ -0,0 +1,48 @@ +/* +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 templating + +import ( + "github.com/kcp-dev/logicalcluster/v3" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// relatedObjectContext is the data available to Go templates when determining +// the local object name (the `naming` section of a PublishedResource). +type relatedObjectContext struct { + // Object is the primary object belonging to the related object. Since related + // object templates are evaluated twice (once for the origin side and once + // for the destination side), object is the primary object on the side the + // template is evaluated for. + Object map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") + // of the kcp workspace that the synchronization is currently processing. This + // value is set for both evaluations, regardless of side. + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). This + // value is set for both evaluations, regardless of side. + ClusterPath logicalcluster.Path +} + +func NewRelatedObjectContext(object *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) relatedObjectContext { + return relatedObjectContext{ + Object: object.Object, + ClusterName: clusterName, + ClusterPath: clusterPath, + } +} diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index aedf3dc..b67f336 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -210,6 +210,8 @@ type RelatedResourceObjectSpec struct { Selector *RelatedResourceObjectSelector `json:"selector,omitempty"` // Reference points to a field inside the main object. This reference is // evaluated on both source and destination sides to find the related object. + // + // Deprecated: Use Go templates instead. Reference *RelatedResourceObjectReference `json:"reference,omitempty"` // Template is a Go templated string that can make use of variables to // construct the resulting string. From e7e0fc9a780b430b5744e699d0205a8479eade30 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 17:27:06 +0200 Subject: [PATCH 03/22] implement templating in label selectors On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/syncer_related.go | 87 +++++++++++++++---- internal/sync/templating/related.go | 25 ++++++ .../syncagent/v1alpha1/published_resource.go | 11 ++- 3 files changed, 103 insertions(+), 20 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 3242bcd..f6b7201 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -24,6 +24,7 @@ import ( "slices" "strings" + "github.com/kcp-dev/logicalcluster/v3" "github.com/tidwall/gjson" "go.uber.org/zap" @@ -68,7 +69,7 @@ func (s *ResourceSyncer) processRelatedResource(log *zap.SugaredLogger, stateSto dest syncSide ) - if relRes.Origin == "service" { + if relRes.Origin == syncagentv1alpha1.RelatedResourceOriginService { origin = local dest = remote } else { @@ -159,7 +160,7 @@ func (s *ResourceSyncer) processRelatedResource(log *zap.SugaredLogger, stateSto // now that the related object was successfully synced, we can remember its details on the // main object - if relRes.Origin == "service" { + if relRes.Origin == syncagentv1alpha1.RelatedResourceOriginService { // TODO: Improve this logic, the added index is just a hack until we find a better solution // to let the user know about the related object (this annotation is not relevant for the // syncing logic, it's purely for the end-user). @@ -211,6 +212,7 @@ func resolveRelatedResourceObjects(relatedOrigin, relatedDest syncSide, relRes s // resolving the originNamespace first allows us to scope down any .List() calls later originNamespace := relatedOrigin.object.GetNamespace() destNamespace := relatedDest.object.GetNamespace() + origin := relRes.Origin namespaceMap := map[string]string{ originNamespace: destNamespace, @@ -218,7 +220,7 @@ func resolveRelatedResourceObjects(relatedOrigin, relatedDest syncSide, relRes s if nsSpec := relRes.Object.Namespace; nsSpec != nil { var err error - namespaceMap, err = resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest, *nsSpec) + namespaceMap, err = resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest, origin, *nsSpec) if err != nil { return nil, fmt.Errorf("failed to resolve namespace: %w", err) } @@ -248,7 +250,7 @@ func resolveRelatedResourceObjects(relatedOrigin, relatedDest syncSide, relRes s return objects, nil } -func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, spec syncagentv1alpha1.RelatedResourceObjectSpec) (map[string]string, error) { +func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, spec syncagentv1alpha1.RelatedResourceObjectSpec) (map[string]string, error) { switch { //nolint:staticcheck case spec.Reference != nil: @@ -294,7 +296,7 @@ func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, for _, namespace := range namespaces.Items { name := namespace.Name - destinationName, err := applyRewrites(relatedOrigin, relatedDest, name, spec.Selector.Rewrite) + destinationName, err := applySelectorRewrites(relatedOrigin, relatedDest, name, spec.Selector.Rewrite) if err != nil { return nil, fmt.Errorf("failed to rewrite origin namespace: %w", err) } @@ -305,7 +307,7 @@ func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, return namespaceMap, nil case spec.Template != nil: - originValue, destValue, err := applyTemplateBothSides(relatedOrigin, relatedDest, *spec.Template) + originValue, destValue, err := applyTemplateBothSides(relatedOrigin, relatedDest, origin, *spec.Template) if err != nil { return nil, fmt.Errorf("failed to apply template: %w", err) } @@ -391,7 +393,12 @@ func resolveRelatedResourceObjectsInNamespace(relatedOrigin, relatedDest syncSid originObjects.SetAPIVersion("v1") // we only support ConfigMaps and Secrets, both are in core/v1 originObjects.SetKind(relRes.Kind) - selector, err := metav1.LabelSelectorAsSelector(&spec.Selector.LabelSelector) + labelSelector, err := templateLabelSelector(relatedOrigin, relatedDest, relRes.Origin, &spec.Selector.LabelSelector) + if err != nil { + return nil, fmt.Errorf("failed to apply templates to label selector: %w", err) + } + + selector, err := metav1.LabelSelectorAsSelector(labelSelector) if err != nil { return nil, fmt.Errorf("invalid selector configured: %w", err) } @@ -409,7 +416,7 @@ func resolveRelatedResourceObjectsInNamespace(relatedOrigin, relatedDest syncSid for _, originObject := range originObjects.Items { name := originObject.GetName() - destinationName, err := applyRewrites(relatedOrigin, relatedDest, name, spec.Selector.Rewrite) + destinationName, err := applySelectorRewrites(relatedOrigin, relatedDest, name, spec.Selector.Rewrite) if err != nil { return nil, fmt.Errorf("failed to rewrite origin name: %w", err) } @@ -420,7 +427,7 @@ func resolveRelatedResourceObjectsInNamespace(relatedOrigin, relatedDest syncSid return nameMap, nil case spec.Template != nil: - originValue, destValue, err := applyTemplateBothSides(relatedOrigin, relatedDest, *spec.Template) + originValue, destValue, err := applyTemplateBothSides(relatedOrigin, relatedDest, relRes.Origin, *spec.Template) if err != nil { return nil, fmt.Errorf("failed to apply template: %w", err) } @@ -468,7 +475,7 @@ func resolveReference(jsonData []byte, ref syncagentv1alpha1.RelatedResourceObje return strVal, nil } -func applyRewrites(relatedOrigin, relatedDest syncSide, value string, rewrite syncagentv1alpha1.RelatedResourceSelectorRewrite) (string, error) { +func applySelectorRewrites(relatedOrigin, relatedDest syncSide, value string, rewrite syncagentv1alpha1.RelatedResourceSelectorRewrite) (string, error) { switch { case rewrite.Regex != nil: return applyRegularExpression(value, *rewrite.Regex) @@ -496,14 +503,8 @@ func applyTemplate(relatedOrigin, relatedDest syncSide, tpl syncagentv1alpha1.Te return "", errors.New("not yet implemented") } -func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, tpl syncagentv1alpha1.TemplateExpression) (originValue, destValue string, err error) { - // clusterName and workspacePath are only set on the kcp side of the sync. - clusterName := relatedDest.clusterName - workspacePath := relatedDest.workspacePath - if clusterName == "" { - clusterName = relatedOrigin.clusterName - workspacePath = relatedOrigin.workspacePath - } +func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, tpl syncagentv1alpha1.TemplateExpression) (originValue, destValue string, err error) { + clusterName, workspacePath := clusterIdent(relatedOrigin, relatedDest, origin) // evaluate the template for the origin object side ctx := templating.NewRelatedObjectContext(relatedOrigin.object, clusterName, workspacePath) @@ -521,3 +522,53 @@ func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, tpl syncagentv1 return originValue, destValue, nil } + +// templateLabelSelector applies Go templating logic to all keys and values in the MatchLabels of +// a label selector. +func templateLabelSelector(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, selector *metav1.LabelSelector) (*metav1.LabelSelector, error) { + clusterName, workspacePath := clusterIdent(relatedOrigin, relatedDest, origin) + + localObject := relatedOrigin.object + remoteObject := relatedDest.object + if origin == syncagentv1alpha1.RelatedResourceOriginKcp { + localObject = relatedDest.object + remoteObject = relatedOrigin.object + } + + ctx := templating.NewRelatedObjectLabelContext(localObject, remoteObject, clusterName, workspacePath) + + newMatchLabels := map[string]string{} + for key, value := range selector.MatchLabels { + if strings.Contains(key, "{{") { + rendered, err := templating.Render(key, ctx) + if err != nil { + return nil, fmt.Errorf("failed to evaluate key as template: %w", err) + } + + key = rendered + } + + if strings.Contains(value, "{{") { + rendered, err := templating.Render(value, ctx) + if err != nil { + return nil, fmt.Errorf("failed to evaluate value as template: %w", err) + } + + value = rendered + } + + newMatchLabels[key] = value + } + + selector.MatchLabels = newMatchLabels + + return selector, nil +} + +func clusterIdent(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin) (logicalcluster.Name, logicalcluster.Path) { + if origin == syncagentv1alpha1.RelatedResourceOriginKcp { + return relatedOrigin.clusterName, relatedOrigin.workspacePath + } + + return relatedDest.clusterName, relatedDest.workspacePath +} diff --git a/internal/sync/templating/related.go b/internal/sync/templating/related.go index fa29a58..45d06bb 100644 --- a/internal/sync/templating/related.go +++ b/internal/sync/templating/related.go @@ -46,3 +46,28 @@ func NewRelatedObjectContext(object *unstructured.Unstructured, clusterName logi ClusterPath: clusterPath, } } + +// relatedObjectContext is the data available to Go templates in the keys and values +// of a label selector for a related object. +type relatedObjectLabelContext struct { + // LocalObject ist the primary object copy on the local side of the sync + // (i.e. on the service cluster). + LocalObject map[string]any + // RemoteObject is the primary object original, in kcp. + RemoteObject map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") + // of the kcp workspace that the synchronization is currently processing + // (where the remote object exists). + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). + ClusterPath logicalcluster.Path +} + +func NewRelatedObjectLabelContext(localObject, remoteObject *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) relatedObjectLabelContext { + return relatedObjectLabelContext{ + LocalObject: localObject.Object, + RemoteObject: remoteObject.Object, + ClusterName: clusterName, + ClusterPath: clusterPath, + } +} diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index b67f336..28e0fe2 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -167,6 +167,13 @@ type ResourceTemplateMutation struct { Template string `json:"template"` } +type RelatedResourceOrigin string + +const ( + RelatedResourceOriginService RelatedResourceOrigin = "service" + RelatedResourceOriginKcp RelatedResourceOrigin = "kcp" +) + type RelatedResourceSpec struct { // Identifier is a unique name for this related resource. The name must be unique within one // PublishedResource and is the key by which consumers (end users) can identify and consume the @@ -174,8 +181,8 @@ type RelatedResourceSpec struct { // The identifier must be an alphanumeric string. Identifier string `json:"identifier"` - // "service" or "kcp" - Origin string `json:"origin"` + // +kubebuilder:validation:Enum=service;kcp + Origin RelatedResourceOrigin `json:"origin"` // ConfigMap or Secret Kind string `json:"kind"` From 3299827ca38f1811a224da04a413ac43e8be9091 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 17:55:09 +0200 Subject: [PATCH 04/22] implement templating for label-selector based rewrites On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/syncer_related.go | 45 ++++++++++++++-------------- internal/sync/templating/related.go | 46 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index f6b7201..2093f74 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -24,7 +24,6 @@ import ( "slices" "strings" - "github.com/kcp-dev/logicalcluster/v3" "github.com/tidwall/gjson" "go.uber.org/zap" @@ -296,7 +295,7 @@ func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, for _, namespace := range namespaces.Items { name := namespace.Name - destinationName, err := applySelectorRewrites(relatedOrigin, relatedDest, name, spec.Selector.Rewrite) + destinationName, err := applySelectorRewrites(relatedOrigin, relatedDest, origin, name, nil, spec.Selector.Rewrite) if err != nil { return nil, fmt.Errorf("failed to rewrite origin namespace: %w", err) } @@ -416,7 +415,7 @@ func resolveRelatedResourceObjectsInNamespace(relatedOrigin, relatedDest syncSid for _, originObject := range originObjects.Items { name := originObject.GetName() - destinationName, err := applySelectorRewrites(relatedOrigin, relatedDest, name, spec.Selector.Rewrite) + destinationName, err := applySelectorRewrites(relatedOrigin, relatedDest, relRes.Origin, name, &originObject, spec.Selector.Rewrite) if err != nil { return nil, fmt.Errorf("failed to rewrite origin name: %w", err) } @@ -475,12 +474,18 @@ func resolveReference(jsonData []byte, ref syncagentv1alpha1.RelatedResourceObje return strVal, nil } -func applySelectorRewrites(relatedOrigin, relatedDest syncSide, value string, rewrite syncagentv1alpha1.RelatedResourceSelectorRewrite) (string, error) { +// applyTemplate is used after a label selector has been applied and a list of namespaces or objects +// has been selected. To map these to the destination side, rewrites can be applied, and these are +// first applied to all found namespaces (in which case, the value parameter here is the namespace +// name and originRelatedObject is nil) and then again to all found objects (in which case the value +// parameter is the object's name and originRelatedObject is set). In both cases the rewrite is supposed +// to return a string. +func applySelectorRewrites(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, value string, originRelatedObject *unstructured.Unstructured, rewrite syncagentv1alpha1.RelatedResourceSelectorRewrite) (string, error) { switch { case rewrite.Regex != nil: return applyRegularExpression(value, *rewrite.Regex) case rewrite.Template != nil: - return applyTemplate(relatedOrigin, relatedDest, *rewrite.Template, value) + return applyTemplate(relatedOrigin, relatedDest, origin, *rewrite.Template, value, originRelatedObject) default: return "", errors.New("invalid rewrite: no mechanism configured") } @@ -499,22 +504,25 @@ func applyRegularExpression(value string, re syncagentv1alpha1.RegularExpression return expr.ReplaceAllString(value, re.Replacement), nil } -func applyTemplate(relatedOrigin, relatedDest syncSide, tpl syncagentv1alpha1.TemplateExpression, value string) (string, error) { - return "", errors.New("not yet implemented") +func applyTemplate(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, tpl syncagentv1alpha1.TemplateExpression, value string, originRelatedObject *unstructured.Unstructured) (string, error) { + localSide, remoteSide := remapSyncSides(relatedOrigin, relatedDest, origin) + ctx := templating.NewRelatedObjectLabelRewriteContext(value, localSide.object, remoteSide.object, originRelatedObject, remoteSide.clusterName, remoteSide.workspacePath) + + return templating.Render(tpl.Template, ctx) } func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, tpl syncagentv1alpha1.TemplateExpression) (originValue, destValue string, err error) { - clusterName, workspacePath := clusterIdent(relatedOrigin, relatedDest, origin) + _, remoteSide := remapSyncSides(relatedOrigin, relatedDest, origin) // evaluate the template for the origin object side - ctx := templating.NewRelatedObjectContext(relatedOrigin.object, clusterName, workspacePath) + ctx := templating.NewRelatedObjectContext(relatedOrigin.object, remoteSide.clusterName, remoteSide.workspacePath) originValue, err = templating.Render(tpl.Template, ctx) if err != nil { return "", "", fmt.Errorf("failed to evaluate template on origin side: %w", err) } // and once more on the other side - ctx = templating.NewRelatedObjectContext(relatedDest.object, clusterName, workspacePath) + ctx = templating.NewRelatedObjectContext(relatedDest.object, remoteSide.clusterName, remoteSide.workspacePath) destValue, err = templating.Render(tpl.Template, ctx) if err != nil { return "", "", fmt.Errorf("failed to evaluate template on destination side: %w", err) @@ -526,16 +534,9 @@ func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, origin syncagen // templateLabelSelector applies Go templating logic to all keys and values in the MatchLabels of // a label selector. func templateLabelSelector(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, selector *metav1.LabelSelector) (*metav1.LabelSelector, error) { - clusterName, workspacePath := clusterIdent(relatedOrigin, relatedDest, origin) - - localObject := relatedOrigin.object - remoteObject := relatedDest.object - if origin == syncagentv1alpha1.RelatedResourceOriginKcp { - localObject = relatedDest.object - remoteObject = relatedOrigin.object - } + localSide, remoteSide := remapSyncSides(relatedOrigin, relatedDest, origin) - ctx := templating.NewRelatedObjectLabelContext(localObject, remoteObject, clusterName, workspacePath) + ctx := templating.NewRelatedObjectLabelContext(localSide.object, remoteSide.object, remoteSide.clusterName, remoteSide.workspacePath) newMatchLabels := map[string]string{} for key, value := range selector.MatchLabels { @@ -565,10 +566,10 @@ func templateLabelSelector(relatedOrigin, relatedDest syncSide, origin syncagent return selector, nil } -func clusterIdent(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin) (logicalcluster.Name, logicalcluster.Path) { +func remapSyncSides(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin) (localSide, remoteSide syncSide) { if origin == syncagentv1alpha1.RelatedResourceOriginKcp { - return relatedOrigin.clusterName, relatedOrigin.workspacePath + return relatedOrigin, relatedDest } - return relatedDest.clusterName, relatedDest.workspacePath + return relatedDest, relatedOrigin } diff --git a/internal/sync/templating/related.go b/internal/sync/templating/related.go index 45d06bb..99f1884 100644 --- a/internal/sync/templating/related.go +++ b/internal/sync/templating/related.go @@ -71,3 +71,49 @@ func NewRelatedObjectLabelContext(localObject, remoteObject *unstructured.Unstru ClusterPath: clusterPath, } } + +// relatedObjectLabelRewriteContext is the data available to Go templates when +// mapping the found namespace names and objects names from having evaluated a +// label selector previously. +type relatedObjectLabelRewriteContext struct { + // Value is either the a found namespace name (when a label selector was + // used to select the source namespaces for related objects) or the name of + // a found object (when a label selector was used to find objects). In the + // former case, the template should return the new namespace to use on the + // destination side, in the latter case it should return the new object name + // to use on the destination side. + Value string + // When a rewrite is used to rewrite object names, RelatedObject is the + // original related object (found on the origin side). This enables you to + // ignore the given Value entirely and just select anything from the object + // itself. + // RelatedObject is nil when the rewrite is performed for a namespace. + RelatedObject map[string]any + // LocalObject ist the primary object copy on the local side of the sync + // (i.e. on the service cluster). + LocalObject map[string]any + // RemoteObject is the primary object original, in kcp. + RemoteObject map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") + // of the kcp workspace that the synchronization is currently processing + // (where the remote object exists). + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). + ClusterPath logicalcluster.Path +} + +func NewRelatedObjectLabelRewriteContext(value string, localObject, remoteObject, relatedObject *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) relatedObjectLabelRewriteContext { + ctx := relatedObjectLabelRewriteContext{ + Value: value, + LocalObject: localObject.Object, + RemoteObject: remoteObject.Object, + ClusterName: clusterName, + ClusterPath: clusterPath, + } + + if relatedObject != nil { + ctx.RelatedObject = relatedObject.Object + } + + return ctx +} From 7a980ed12e8eee432176a04cf49ef63ea64137a4 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 17:56:15 +0200 Subject: [PATCH 05/22] lint On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/syncer_related.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 2093f74..2f58268 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -364,8 +364,9 @@ func resolveRelatedResourceObjectsInNamespaces(relatedOrigin, relatedDest syncSi func resolveRelatedResourceObjectsInNamespace(relatedOrigin, relatedDest syncSide, relRes syncagentv1alpha1.RelatedResourceSpec, spec syncagentv1alpha1.RelatedResourceObjectSpec, namespace string) (map[string]string, error) { switch { + //nolint:staticcheck case spec.Reference != nil: - originName, err := resolveObjectReference(relatedOrigin.object, *spec.Reference) + originName, err := resolveObjectReference(relatedOrigin.object, *spec.Reference) //nolint:staticcheck if err != nil { return nil, err } @@ -374,7 +375,7 @@ func resolveRelatedResourceObjectsInNamespace(relatedOrigin, relatedDest syncSid return nil, nil } - destName, err := resolveObjectReference(relatedDest.object, *spec.Reference) + destName, err := resolveObjectReference(relatedDest.object, *spec.Reference) //nolint:staticcheck if err != nil { return nil, err } From 4fd0b28368518914754c9340c054804d49af52da Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 18:00:08 +0200 Subject: [PATCH 06/22] codegen On-behalf-of: @SAP christoph.mewes@sap.com --- .../crd/kcp.io/syncagent.kcp.io_publishedresources.yaml | 8 +++++++- .../syncagent/v1alpha1/relatedresourcespec.go | 8 ++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index 75641c1..ac878b8 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -438,6 +438,8 @@ spec: description: |- Reference points to a field inside the main object. This reference is evaluated on both source and destination sides to find the related object. + + Deprecated: Use Go templates instead. properties: path: description: |- @@ -553,6 +555,8 @@ spec: description: |- Reference points to a field inside the main object. This reference is evaluated on both source and destination sides to find the related object. + + Deprecated: Use Go templates instead. properties: path: description: |- @@ -665,7 +669,9 @@ spec: type: object type: object origin: - description: '"service" or "kcp"' + enum: + - service + - kcp type: string required: - identifier diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go index c552766..8ea51a6 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go @@ -18,11 +18,15 @@ limitations under the License. package v1alpha1 +import ( + v1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" +) + // RelatedResourceSpecApplyConfiguration represents a declarative configuration of the RelatedResourceSpec type for use // with apply. type RelatedResourceSpecApplyConfiguration struct { Identifier *string `json:"identifier,omitempty"` - Origin *string `json:"origin,omitempty"` + Origin *v1alpha1.RelatedResourceOrigin `json:"origin,omitempty"` Kind *string `json:"kind,omitempty"` Object *RelatedResourceObjectApplyConfiguration `json:"object,omitempty"` Mutation *ResourceMutationSpecApplyConfiguration `json:"mutation,omitempty"` @@ -45,7 +49,7 @@ func (b *RelatedResourceSpecApplyConfiguration) WithIdentifier(value string) *Re // WithOrigin sets the Origin 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 Origin field is set to the value of the last call. -func (b *RelatedResourceSpecApplyConfiguration) WithOrigin(value string) *RelatedResourceSpecApplyConfiguration { +func (b *RelatedResourceSpecApplyConfiguration) WithOrigin(value v1alpha1.RelatedResourceOrigin) *RelatedResourceSpecApplyConfiguration { b.Origin = &value return b } From 17d19ce8a9bdf316ed79a469e26c6c599ef95e74 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 19:08:16 +0200 Subject: [PATCH 07/22] bump minimum Go version for the agent to Go 1.24 On-behalf-of: @SAP christoph.mewes@sap.com --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 9ebc02d..614fca9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/kcp-dev/api-syncagent -go 1.23.0 +go 1.24.0 replace github.com/kcp-dev/api-syncagent/sdk => ./sdk From 22c2d7a4281cfc32766563bdb60b6ff169028f9a Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 19:38:47 +0200 Subject: [PATCH 08/22] adjust naming tests to new sha3 defaults On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/templating/naming_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/sync/templating/naming_test.go b/internal/sync/templating/naming_test.go index 4d27ec8..f180a59 100644 --- a/internal/sync/templating/naming_test.go +++ b/internal/sync/templating/naming_test.go @@ -49,35 +49,35 @@ func TestGenerateLocalObjectName(t *testing.T) { clusterName: "testcluster", remoteObject: createNewObject("objname", "objnamespace"), namingConfig: nil, - expected: types.NamespacedName{Namespace: "testcluster", Name: "e75ee3d444e238331f6a-8b09d63c82efb771a2c5"}, + expected: types.NamespacedName{Namespace: "testcluster", Name: "2928c2aa0e510f017f07-73d08a1ba188f1340dae"}, }, { name: "custom static namespace pattern", clusterName: "testcluster", remoteObject: createNewObject("objname", "objnamespace"), namingConfig: &syncagentv1alpha1.ResourceNaming{Namespace: "foobar"}, - expected: types.NamespacedName{Namespace: "foobar", Name: "e75ee3d444e238331f6a-8b09d63c82efb771a2c5"}, + expected: types.NamespacedName{Namespace: "foobar", Name: "2928c2aa0e510f017f07-73d08a1ba188f1340dae"}, }, { name: "custom dynamic namespace pattern", clusterName: "testcluster", remoteObject: createNewObject("objname", "objnamespace"), namingConfig: &syncagentv1alpha1.ResourceNaming{Namespace: "foobar-$remoteClusterName"}, - expected: types.NamespacedName{Namespace: "foobar-testcluster", Name: "e75ee3d444e238331f6a-8b09d63c82efb771a2c5"}, + expected: types.NamespacedName{Namespace: "foobar-testcluster", Name: "2928c2aa0e510f017f07-73d08a1ba188f1340dae"}, }, { name: "plain, unhashed values should be available in patterns", clusterName: "testcluster", remoteObject: createNewObject("objname", "objnamespace"), namingConfig: &syncagentv1alpha1.ResourceNaming{Namespace: "$remoteNamespace"}, - expected: types.NamespacedName{Namespace: "objnamespace", Name: "e75ee3d444e238331f6a-8b09d63c82efb771a2c5"}, + expected: types.NamespacedName{Namespace: "objnamespace", Name: "2928c2aa0e510f017f07-73d08a1ba188f1340dae"}, }, { name: "configured but empty patterns", clusterName: "testcluster", remoteObject: createNewObject("objname", "objnamespace"), namingConfig: &syncagentv1alpha1.ResourceNaming{Namespace: "", Name: ""}, - expected: types.NamespacedName{Namespace: "testcluster", Name: "e75ee3d444e238331f6a-8b09d63c82efb771a2c5"}, + expected: types.NamespacedName{Namespace: "testcluster", Name: "2928c2aa0e510f017f07-73d08a1ba188f1340dae"}, }, { name: "custom dynamic name pattern", From 95bde59db79bc265082dcb8df12cf20145a59522 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 19:38:56 +0200 Subject: [PATCH 09/22] use t.Context() in tests On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/context_test.go | 5 ++--- internal/sync/state_store_test.go | 3 +-- internal/sync/syncer_test.go | 4 ++-- test/e2e/apiexport/apiexport_test.go | 10 +++++----- test/e2e/apiresourceschema/apiresourceschema_test.go | 12 ++++++------ test/e2e/discovery/discovery_test.go | 3 +-- test/e2e/sync/primary_test.go | 12 ++++++------ test/e2e/sync/related_test.go | 2 +- 8 files changed, 24 insertions(+), 27 deletions(-) diff --git a/internal/sync/context_test.go b/internal/sync/context_test.go index 51199c1..461369e 100644 --- a/internal/sync/context_test.go +++ b/internal/sync/context_test.go @@ -17,7 +17,6 @@ limitations under the License. package sync import ( - "context" "testing" "github.com/kcp-dev/logicalcluster/v3" @@ -27,9 +26,9 @@ import ( func TestNewContext(t *testing.T) { clusterName := logicalcluster.Name("foo") - ctx := kontext.WithCluster(context.Background(), clusterName) + ctx := kontext.WithCluster(t.Context(), clusterName) - combinedCtx := NewContext(context.Background(), ctx) + combinedCtx := NewContext(t.Context(), ctx) if combinedCtx.clusterName != clusterName { t.Fatalf("Expected function to recognize the cluster name in the context, but got %q", combinedCtx.clusterName) diff --git a/internal/sync/state_store_test.go b/internal/sync/state_store_test.go index 6ea50bc..fcc7a7e 100644 --- a/internal/sync/state_store_test.go +++ b/internal/sync/state_store_test.go @@ -17,7 +17,6 @@ limitations under the License. package sync import ( - "context" "testing" dummyv1alpha1 "github.com/kcp-dev/api-syncagent/internal/sync/apis/dummy/v1alpha1" @@ -37,7 +36,7 @@ func TestStateStoreBasics(t *testing.T) { }, withKind("RemoteThing")) serviceClusterClient := buildFakeClient() - ctx := context.Background() + ctx := t.Context() stateNamespace := "kcp-system" primaryObjectSide := syncSide{ diff --git a/internal/sync/syncer_test.go b/internal/sync/syncer_test.go index 93baf54..074cfad 100644 --- a/internal/sync/syncer_test.go +++ b/internal/sync/syncer_test.go @@ -907,7 +907,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { t.Fatalf("Failed to create syncer: %v", err) } - localCtx := context.Background() + localCtx := t.Context() remoteCtx := kontext.WithCluster(localCtx, clusterName) ctx := NewContext(localCtx, remoteCtx) @@ -1213,7 +1213,7 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { t.Fatalf("Failed to create syncer: %v", err) } - localCtx := context.Background() + localCtx := t.Context() remoteCtx := kontext.WithCluster(localCtx, clusterName) ctx := NewContext(localCtx, remoteCtx) diff --git a/test/e2e/apiexport/apiexport_test.go b/test/e2e/apiexport/apiexport_test.go index c02a875..cac539b 100644 --- a/test/e2e/apiexport/apiexport_test.go +++ b/test/e2e/apiexport/apiexport_test.go @@ -45,7 +45,7 @@ func TestPermissionsClaims(t *testing.T) { apiExportName = "kcp.example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -247,7 +247,7 @@ func TestExistingPermissionsClaimsAreKept(t *testing.T) { apiExportName = "kcp.example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -367,7 +367,7 @@ func TestSchemasAreMerged(t *testing.T) { apiExportName = "kcp.example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -476,7 +476,7 @@ func TestSchemaIsKeptWhenDeletingPublishedResource(t *testing.T) { apiExportName = "kcp.example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -614,7 +614,7 @@ func TestNewSchemasAreCreatedAsNeeded(t *testing.T) { apiExportName = "kcp.example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp diff --git a/test/e2e/apiresourceschema/apiresourceschema_test.go b/test/e2e/apiresourceschema/apiresourceschema_test.go index 0b138fe..ac5ad1e 100644 --- a/test/e2e/apiresourceschema/apiresourceschema_test.go +++ b/test/e2e/apiresourceschema/apiresourceschema_test.go @@ -45,7 +45,7 @@ func TestARSAreCreated(t *testing.T) { apiExportName = "example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -116,7 +116,7 @@ func TestARSAreNotUpdated(t *testing.T) { apiExportName = "example.com" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -214,7 +214,7 @@ func TestARSOnlyContainsSelectedCRDVersion(t *testing.T) { theVersion = "v1" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -296,7 +296,7 @@ func TestMultiVersionCRD(t *testing.T) { // force a non-standard order, because it should not matter for the sync var selectedVersions = []string{"v2", "v1"} - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -386,7 +386,7 @@ func TestProjection(t *testing.T) { originalVersion = "v1" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -489,7 +489,7 @@ func TestNonCRDResource(t *testing.T) { originalVersion = "v1" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp diff --git a/test/e2e/discovery/discovery_test.go b/test/e2e/discovery/discovery_test.go index aeb40af..b551336 100644 --- a/test/e2e/discovery/discovery_test.go +++ b/test/e2e/discovery/discovery_test.go @@ -19,7 +19,6 @@ limitations under the License. package discovery import ( - "context" "testing" "github.com/go-logr/logr" @@ -95,7 +94,7 @@ func TestDiscoverSingleVersionCRD(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) kubeconfigFile, _, _ := utils.RunEnvtest(t, testcase.crdFiles) diff --git a/test/e2e/sync/primary_test.go b/test/e2e/sync/primary_test.go index 99c7246..0958f19 100644 --- a/test/e2e/sync/primary_test.go +++ b/test/e2e/sync/primary_test.go @@ -53,7 +53,7 @@ func TestSyncSimpleObject(t *testing.T) { orgWorkspace = "sync-simple" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -143,7 +143,7 @@ func TestSyncSimpleObjectOldNaming(t *testing.T) { orgWorkspace = "sync-simple-deprecated" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -232,7 +232,7 @@ func TestSyncWithDefaultNamingRules(t *testing.T) { orgWorkspace = "sync-default-naming-rules" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -327,7 +327,7 @@ func TestLocalChangesAreKept(t *testing.T) { orgWorkspace = "sync-undo-local-changes" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -533,7 +533,7 @@ func TestResourceFilter(t *testing.T) { orgWorkspace = "sync-resource-filter" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp @@ -654,7 +654,7 @@ func TestSyncingOverlyLongNames(t *testing.T) { orgWorkspace = "sync-long-names" ) - ctx := context.Background() + ctx := t.Context() ctrlruntime.SetLogger(logr.Discard()) // setup a test environment in kcp diff --git a/test/e2e/sync/related_test.go b/test/e2e/sync/related_test.go index 220457e..acc901e 100644 --- a/test/e2e/sync/related_test.go +++ b/test/e2e/sync/related_test.go @@ -457,7 +457,7 @@ func TestSyncRelatedObjects(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - ctx := context.Background() + ctx := t.Context() // setup a test environment in kcp orgKubconfig := utils.CreateOrganization(t, ctx, testcase.workspace, apiExportName) From 9fbf31dfa86fb0a1fe050703de3730195090fa7c Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 19:41:37 +0200 Subject: [PATCH 10/22] add unit test for template-based naming On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/templating/naming_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/sync/templating/naming_test.go b/internal/sync/templating/naming_test.go index f180a59..a72541c 100644 --- a/internal/sync/templating/naming_test.go +++ b/internal/sync/templating/naming_test.go @@ -86,6 +86,14 @@ func TestGenerateLocalObjectName(t *testing.T) { namingConfig: &syncagentv1alpha1.ResourceNaming{Name: "foobar-$remoteName"}, expected: types.NamespacedName{Namespace: "testcluster", Name: "foobar-objname"}, }, + { + name: "Go templates", + clusterName: "testcluster", + clusterPath: "root:test:team", + remoteObject: createNewObject("objname", "objnamespace"), + namingConfig: &syncagentv1alpha1.ResourceNaming{Name: "{{ .ClusterPath }}-{{ .Object.metadata.name }}"}, + expected: types.NamespacedName{Namespace: "testcluster", Name: "root:test:team-objname"}, + }, } for _, testcase := range testcases { From 710bb377edc9f9f473ffc4b9e9bc0f447737c28b Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 20:21:55 +0200 Subject: [PATCH 11/22] make the current side available to templates to enable complex logic On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/syncer_related.go | 12 ++++++++++-- internal/sync/templating/related.go | 12 +++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 2f58268..6dd720d 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -516,14 +516,14 @@ func applyTemplateBothSides(relatedOrigin, relatedDest syncSide, origin syncagen _, remoteSide := remapSyncSides(relatedOrigin, relatedDest, origin) // evaluate the template for the origin object side - ctx := templating.NewRelatedObjectContext(relatedOrigin.object, remoteSide.clusterName, remoteSide.workspacePath) + ctx := templating.NewRelatedObjectContext(relatedOrigin.object, origin, remoteSide.clusterName, remoteSide.workspacePath) originValue, err = templating.Render(tpl.Template, ctx) if err != nil { return "", "", fmt.Errorf("failed to evaluate template on origin side: %w", err) } // and once more on the other side - ctx = templating.NewRelatedObjectContext(relatedDest.object, remoteSide.clusterName, remoteSide.workspacePath) + ctx = templating.NewRelatedObjectContext(relatedDest.object, oppositeSide(origin), remoteSide.clusterName, remoteSide.workspacePath) destValue, err = templating.Render(tpl.Template, ctx) if err != nil { return "", "", fmt.Errorf("failed to evaluate template on destination side: %w", err) @@ -574,3 +574,11 @@ func remapSyncSides(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha return relatedDest, relatedOrigin } + +func oppositeSide(origin syncagentv1alpha1.RelatedResourceOrigin) syncagentv1alpha1.RelatedResourceOrigin { + if origin == syncagentv1alpha1.RelatedResourceOriginKcp { + return syncagentv1alpha1.RelatedResourceOriginService + } + + return syncagentv1alpha1.RelatedResourceOriginKcp +} diff --git a/internal/sync/templating/related.go b/internal/sync/templating/related.go index 99f1884..1c0d483 100644 --- a/internal/sync/templating/related.go +++ b/internal/sync/templating/related.go @@ -19,12 +19,17 @@ package templating import ( "github.com/kcp-dev/logicalcluster/v3" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -// relatedObjectContext is the data available to Go templates when determining -// the local object name (the `naming` section of a PublishedResource). +// relatedObjectContext is the data available to Go templates when evaluating +// the origin of a related object. type relatedObjectContext struct { + // Side is set to either one of the possible origin values to indicate for + // which cluster the template is currently being evaluated for. + Side syncagentv1alpha1.RelatedResourceOrigin // Object is the primary object belonging to the related object. Since related // object templates are evaluated twice (once for the origin side and once // for the destination side), object is the primary object on the side the @@ -39,8 +44,9 @@ type relatedObjectContext struct { ClusterPath logicalcluster.Path } -func NewRelatedObjectContext(object *unstructured.Unstructured, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) relatedObjectContext { +func NewRelatedObjectContext(object *unstructured.Unstructured, side syncagentv1alpha1.RelatedResourceOrigin, clusterName logicalcluster.Name, clusterPath logicalcluster.Path) relatedObjectContext { return relatedObjectContext{ + Side: side, Object: object.Object, ClusterName: clusterName, ClusterPath: clusterPath, From 7ca0303a3b416a868a46b07613c230858bca659e Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Thu, 22 May 2025 20:36:42 +0200 Subject: [PATCH 12/22] finally re-enable the exotic related resources tests On-behalf-of: @SAP christoph.mewes@sap.com --- test/e2e/sync/related_test.go | 372 +++++++++++++++------------------- 1 file changed, 162 insertions(+), 210 deletions(-) diff --git a/test/e2e/sync/related_test.go b/test/e2e/sync/related_test.go index acc901e..0670592 100644 --- a/test/e2e/sync/related_test.go +++ b/test/e2e/sync/related_test.go @@ -177,216 +177,168 @@ func TestSyncRelatedObjects(t *testing.T) { ////////////////////////////////////////////////////////////////////////////////////////////// - // { - // name: "sync referenced Secret up into a new namespace", - // workspace: "sync-referenced-secret-up-namespace", - // mainResource: crds.Crontab{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-crontab", - // Namespace: "default", - // }, - // Spec: crds.CrontabSpec{ - // CronSpec: "* * *", - // Image: "ubuntu:latest", - // }, - // }, - // relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ - // Identifier: "credentials", - // Origin: "service", - // Kind: "Secret", - // Object: syncagentv1alpha1.RelatedResourceObject{ - // RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "my-credentials", - // }, - // }, - // }, - // }, - // Destination: syncagentv1alpha1.RelatedResourceDestination{ - // RelatedResourceDestinationSpec: syncagentv1alpha1.RelatedResourceDestinationSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "my-credentials", - // }, - // }, - // }, - // Namespace: &syncagentv1alpha1.RelatedResourceDestinationSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "new-namespace", - // }, - // }, - // }, - // }, - // }, - // sourceRelatedObject: corev1.Secret{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-credentials", - // Namespace: "synced-default", - // }, - // Data: map[string][]byte{ - // "password": []byte("hunter2"), - // }, - // Type: corev1.SecretTypeOpaque, - // }, - - // expectedSyncedRelatedObject: corev1.Secret{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-credentials", - // Namespace: "new-namespace", - // }, - // Data: map[string][]byte{ - // "password": []byte("hunter2"), - // }, - // Type: corev1.SecretTypeOpaque, - // }, - // }, - - // ////////////////////////////////////////////////////////////////////////////////////////////// - - // { - // name: "sync referenced Secret down into a new namespace", - // workspace: "sync-referenced-secret-down-namespace", - // mainResource: crds.Crontab{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-crontab", - // Namespace: "default", - // }, - // Spec: crds.CrontabSpec{ - // CronSpec: "* * *", - // Image: "ubuntu:latest", - // }, - // }, - // relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ - // Identifier: "credentials", - // Origin: "kcp", - // Kind: "Secret", - // Object: syncagentv1alpha1.RelatedResourceObject{ - // RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "my-credentials", - // }, - // }, - // }, - // }, - // Destination: syncagentv1alpha1.RelatedResourceDestination{ - // RelatedResourceDestinationSpec: syncagentv1alpha1.RelatedResourceDestinationSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "my-credentials", - // }, - // }, - // }, - // Namespace: &syncagentv1alpha1.RelatedResourceDestinationSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "new-namespace", - // }, - // }, - // }, - // }, - // }, - // sourceRelatedObject: corev1.Secret{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-credentials", - // Namespace: "default", - // }, - // Data: map[string][]byte{ - // "password": []byte("hunter2"), - // }, - // Type: corev1.SecretTypeOpaque, - // }, - - // expectedSyncedRelatedObject: corev1.Secret{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-credentials", - // Namespace: "new-namespace", - // }, - // Data: map[string][]byte{ - // "password": []byte("hunter2"), - // }, - // Type: corev1.SecretTypeOpaque, - // }, - // }, - - // ////////////////////////////////////////////////////////////////////////////////////////////// - - // { - // name: "sync referenced Secret up from a foreign namespace", - // workspace: "sync-referenced-secret-up-foreign-namespace", - // mainResource: crds.Crontab{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-crontab", - // Namespace: "default", - // }, - // Spec: crds.CrontabSpec{ - // CronSpec: "* * *", - // Image: "ubuntu:latest", - // }, - // }, - // relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ - // Identifier: "credentials", - // Origin: "service", - // Kind: "Secret", - // Object: syncagentv1alpha1.RelatedResourceObject{ - // RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "my-credentials", - // }, - // }, - // }, - // Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "other-namespace", - // }, - // }, - // }, - // }, - // Destination: syncagentv1alpha1.RelatedResourceDestination{ - // RelatedResourceDestinationSpec: syncagentv1alpha1.RelatedResourceDestinationSpec{ - // Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - // Path: "metadata.name", // irrelevant - // Regex: &syncagentv1alpha1.RegularExpression{ - // Replacement: "my-credentials", - // }, - // }, - // }, - // }, - // }, - // sourceRelatedObject: corev1.Secret{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-credentials", - // Namespace: "other-namespace", - // }, - // Data: map[string][]byte{ - // "password": []byte("hunter2"), - // }, - // Type: corev1.SecretTypeOpaque, - // }, - - // expectedSyncedRelatedObject: corev1.Secret{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "my-credentials", - // Namespace: "default", - // }, - // Data: map[string][]byte{ - // "password": []byte("hunter2"), - // }, - // Type: corev1.SecretTypeOpaque, - // }, - // }, + { + name: "sync referenced Secret up into a new namespace", + workspace: "sync-referenced-secret-up-namespace", + mainResource: crds.Crontab{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-crontab", + Namespace: "default", + }, + Spec: crds.CrontabSpec{ + CronSpec: "* * *", + Image: "ubuntu:latest", + }, + }, + relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ + Identifier: "credentials", + Origin: "service", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "my-credentials", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: `{{ if eq .Side "kcp" }}new-namespace{{ else }}{{ .Object.metadata.namespace }}{{ end }}`, + }, + }, + }, + }, + sourceRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "synced-default", + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + + expectedSyncedRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "new-namespace", + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + }, + + ////////////////////////////////////////////////////////////////////////////////////////////// + + { + name: "sync referenced Secret down into a new namespace", + workspace: "sync-referenced-secret-down-namespace", + mainResource: crds.Crontab{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-crontab", + Namespace: "default", + }, + Spec: crds.CrontabSpec{ + CronSpec: "* * *", + Image: "ubuntu:latest", + }, + }, + relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ + Identifier: "credentials", + Origin: "kcp", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "my-credentials", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: `{{ if eq .Side "kcp" }}{{ .Object.metadata.namespace }}{{ else }}new-namespace{{ end }}`, + }, + }, + }, + }, + sourceRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "default", + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + + expectedSyncedRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "new-namespace", + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + }, + + ////////////////////////////////////////////////////////////////////////////////////////////// + + { + name: "sync referenced Secret up from a foreign namespace", + workspace: "sync-referenced-secret-up-foreign-namespace", + mainResource: crds.Crontab{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-crontab", + Namespace: "default", + }, + Spec: crds.CrontabSpec{ + CronSpec: "* * *", + Image: "ubuntu:latest", + }, + }, + relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ + Identifier: "credentials", + Origin: "service", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "my-credentials", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: `{{ if eq .Side "kcp" }}{{ .Object.metadata.namespace }}{{ else }}other-namespace{{ end }}`, + }, + }, + }, + }, + sourceRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "other-namespace", + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + + expectedSyncedRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "default", + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + }, ////////////////////////////////////////////////////////////////////////////////////////////// From 46be4fbfbece03c1eb6d2bc81b10826dac8cefb1 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Fri, 23 May 2025 15:09:56 +0200 Subject: [PATCH 13/22] add basic test for templated label selectors On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/templating/templating.go | 1 - test/e2e/sync/related_test.go | 91 ++++++++++++++++++++++---- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/internal/sync/templating/templating.go b/internal/sync/templating/templating.go index b580f6e..d4a947b 100644 --- a/internal/sync/templating/templating.go +++ b/internal/sync/templating/templating.go @@ -45,7 +45,6 @@ func Render(tpl string, data any) (string, error) { func templateFuncMap() template.FuncMap { funcs := sprig.TxtFuncMap() - funcs["join"] = strings.Join funcs["sha3sum"] = sha3sum funcs["sha3short"] = sha3short diff --git a/test/e2e/sync/related_test.go b/test/e2e/sync/related_test.go index 0670592..fe59a34 100644 --- a/test/e2e/sync/related_test.go +++ b/test/e2e/sync/related_test.go @@ -90,11 +90,9 @@ func TestSyncRelatedObjects(t *testing.T) { Kind: "Secret", Object: syncagentv1alpha1.RelatedResourceObject{ RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ - Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - Path: "metadata.name", // irrelevant - Regex: &syncagentv1alpha1.RegularExpression{ - Replacement: "my-credentials", - }, + Template: &syncagentv1alpha1.TemplateExpression{ + // same fixed value on both sides + Template: "my-credentials", }, }, }, @@ -143,11 +141,9 @@ func TestSyncRelatedObjects(t *testing.T) { Kind: "Secret", Object: syncagentv1alpha1.RelatedResourceObject{ RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ - Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ - Path: "metadata.name", // irrelevant - Regex: &syncagentv1alpha1.RegularExpression{ - Replacement: "my-credentials", - }, + Template: &syncagentv1alpha1.TemplateExpression{ + // same fixed value on both sides + Template: "my-credentials", }, }, }, @@ -368,9 +364,9 @@ func TestSyncRelatedObjects(t *testing.T) { }, }, Rewrite: syncagentv1alpha1.RelatedResourceSelectorRewrite{ - // TODO: Use template instead of regex once that is implemented. - Regex: &syncagentv1alpha1.RegularExpression{ - Replacement: "my-credentials", + Template: &syncagentv1alpha1.TemplateExpression{ + // same fixed name on both sides + Template: "my-credentials", }, }, }, @@ -405,6 +401,75 @@ func TestSyncRelatedObjects(t *testing.T) { Type: corev1.SecretTypeOpaque, }, }, + + ////////////////////////////////////////////////////////////////////////////////////////////// + + { + name: "find Secret based on templated label selector", + workspace: "sync-templated-selected-secret-up", + mainResource: crds.Crontab{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-crontab", + Namespace: "default", + }, + Spec: crds.CrontabSpec{ + CronSpec: "* * *", + Image: "ubuntu:latest", + }, + }, + relatedConfig: syncagentv1alpha1.RelatedResourceSpec{ + Identifier: "credentials", + Origin: "service", + Kind: "Secret", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Selector: &syncagentv1alpha1.RelatedResourceObjectSelector{ + LabelSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + // include some nasty whitespace + ` {{ list "fi" "nd" | join "-" }} `: ` +{{ lower "ME" }} + `, + }, + }, + Rewrite: syncagentv1alpha1.RelatedResourceSelectorRewrite{ + Template: &syncagentv1alpha1.TemplateExpression{ + // same fixed name on both sides + Template: "my-credentials", + }, + }, + }, + }, + }, + }, + sourceRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unknown-name", + Namespace: "synced-default", + Labels: map[string]string{ + "fi-nd": "me", + }, + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + + expectedSyncedRelatedObject: corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-credentials", + Namespace: "default", + Labels: map[string]string{ + "fi-nd": "me", + }, + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + Type: corev1.SecretTypeOpaque, + }, + }, } for _, testcase := range testcases { From 632e05e0db64875b50e46a0619d4b3e1837ac5a8 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Mon, 26 May 2025 14:53:43 +0200 Subject: [PATCH 14/22] extend documentation On-behalf-of: @SAP christoph.mewes@sap.com --- docs/content/publish-resources/.pages | 1 + docs/content/publish-resources/index.md | 255 +++++++++++++++---- docs/content/publish-resources/templating.md | 87 +++++++ internal/sync/templating/related.go | 4 +- 4 files changed, 298 insertions(+), 49 deletions(-) create mode 100644 docs/content/publish-resources/templating.md diff --git a/docs/content/publish-resources/.pages b/docs/content/publish-resources/.pages index 3caec12..e342023 100644 --- a/docs/content/publish-resources/.pages +++ b/docs/content/publish-resources/.pages @@ -1,4 +1,5 @@ nav: - index.md + - templating.md - api-lifecycle.md - technical-details.md diff --git a/docs/content/publish-resources/index.md b/docs/content/publish-resources/index.md index edb89fa..85c9a7d 100644 --- a/docs/content/publish-resources/index.md +++ b/docs/content/publish-resources/index.md @@ -137,20 +137,7 @@ Since the Sync Agent ingests resources from many different Kubernetes clusters ( them onto a single cluster, resources have to be renamed to prevent collisions and also follow the conventions of whatever tooling ultimately processes the resources locally. -The renaming is configured in `spec.naming`. In there, renaming patterns are configured, where -pre-defined placeholders can be used, for example `foo-$placeholder`. The following placeholders -are available: - -* `$remoteClusterName` – the workspace's cluster name (e.g. "1084s8ceexsehjm2") -* `$remoteNamespace` – the original namespace used by the consumer inside the workspace -* `$remoteNamespaceHash` – first 20 hex characters of the SHA-1 hash of `$remoteNamespace` -* `$remoteName` – the original name of the object inside the workspace (rarely used to construct - local namespace names) -* `$remoteNameHash` – first 20 hex characters of the SHA-1 hash of `$remoteName` - -If nothing is configured, the default ensures that no collisions will happen: Each workspace in -kcp will create a namespace on the local cluster, with a combination of namespace and name hashes -used for the actual resource names. +This snippet shows the implicit default configuration: ```yaml apiVersion: syncagent.kcp.io/v1alpha1 @@ -160,11 +147,60 @@ metadata: spec: resource: ... naming: - # This is the implicit default configuration. - namespace: "$remoteClusterName" - name: "cert-$remoteNamespaceHash-$remoteNameHash" + namespace: '{{ .ClusterName }}' + name: '{{ .Object.metadata.namespace | sha3short }}-{{ .Object.metadata.name | sha3short }}' +``` + +This configuration ensures that no collisions will happen: Each workspace in +kcp will create a namespace on the local cluster, with a combination of namespace and name hashes +used for the actual resource names. + +You can override the name or namespaces rules, or both. It is your responsibility to ensure no +naming conflicts can happen on the service cluster, as the agent cannot determine this automatically. + +#### Templating + +In `spec.naming`, [Go template expressions](https://pkg.go.dev/text/template) are used to construct +the desired name of the object's copy. In the templates used here, the following data is injected by +the agent: + +```go +type localObjectNamingContext struct { + // Object is the full remote object found in a kcp workspace. + Object map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf"). + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). + ClusterPath logicalcluster.Path +} ``` +For more details about the templating, see the [Templating](templating.md) documentation. + +#### Legacy Naming Rules + +Go templates for naming rules have been added in v0.3 of the agent. Previous versions used a +`$variable`-based approach, which since has been deprecated. You are encouraged to migrate your +PublishedResources over to Go templates. + +The following table shows the available variables and their modern replacements: + +| Deprecated Variable | Go Template | Description | +| ---------------------- | ----------------------------------------------- | ----------- | +| `$remoteClusterName` | `{{ .ClusterName }}` | the workspace's cluster name (e.g. "1084s8ceexsehjm2") | +| `$remoteNamespace` | `{{ .Object.metadata.namespace }}` | the original namespace used by the consumer inside the workspace | +| `$remoteNamespaceHash` | `{{ .Object.metadata.namespace \| shortHash }}` | first 20 hex characters of the SHA-1 hash of `$remoteNamespace` | +| `$remoteName` | `{{ .Object.metadata.name }}` | the original name of the object inside the workspace (rarely used to construct local namespace names) | +| `$remoteNameHash` | `{{ .Object.metadata.name \| shortHash }}` | first 20 hex characters of the SHA-1 hash of `$remoteName` | + +Note that `ClusterPath` was never available in `$variable` form. + +Note also that the `shortHash` function exists only for backwards compatibility with the old +`$variable` syntax. The new default is to use SHA-3 instead (via the `sha3short` function). When +migrating from the old syntax, you can use the `shortHash` function to ensure new objects are placed +in the old locations. New setups should however use explicitly named functions for hashing, like +`sha3sum` or `sha3short`. `sha3short` takes an optional length parameter that defaults to 20. + ### Mutation Besides projecting the type meta, changes to object contents are also nearly always required. @@ -216,7 +252,7 @@ usual path, without a leading dot. ```yaml template: path: "json.path[expression]" - template: "{{ .LocalObject.ObjectMeta.Namespace }}" + template: "{{ .LocalObject.metadata.namespace }}" ``` {% endraw %} @@ -275,7 +311,7 @@ PublishedResource) and the path will yield 2 ready to use values (`my-secret` an The value selected by the path expression must be a string (or number, but it will be coalesced into a string) and can then be further adjusted by applying a regular expression to it. -References can only ever select 1 related object. Their upside is that they are simple to understand +References can only ever select one related object. Their upside is that they are simple to understand and easy to use, but require a "link" in the primary object that would point to the related object. Here's an example on how to use references to locate the related object. @@ -295,7 +331,7 @@ spec: # this is where our CA and Issuer live in this example namespace: kube-system # need to adjust it to prevent collions (normally clustername is the namespace) - name: "$remoteClusterName-$remoteNamespaceHash-$remoteNameHash" + name: "{{ .ClusterName }}-{{ .Object.metadata.namespace | sha3short }}-{{ .Object.metadata.name | sha3short }}" related: - # unique name for this related resource. The name must be unique within @@ -313,7 +349,7 @@ spec: # configure where in the parent object we can find the child object object: - # Object can use either reference, labelSelector or expressions. In this + # Object can use either reference, labelSelector or template. In this # example we use references. reference: # This path is evaluated in both the local and remote objects, to figure out @@ -332,6 +368,59 @@ spec: # replacement: '...' ``` +#### Templates + +Similar to references, [Go templates](https://pkg.go.dev/text/template) can also be used to determine +the names of related objects on both sides of the sync. In fact, templates can be thought of as more +powerful references since they allow for minimal logic to be embedded in them. Templates also do not +necessarily have to select a value from the object (like a reference does), but can use any kind of +logic to determine the names. + +Like references, templates can also only be used to select a single object per related resource. + +A template gets the following data injected into it: + +```go +type localObjectNamingContext struct { + // Side is set to either one of the possible origin values to indicate for + // which cluster the template is currently being evaluated for. + Side syncagentv1alpha1.RelatedResourceOrigin + // Object is the primary object belonging to the related object. Since related + // object templates are evaluated twice (once for the origin side and once + // for the destination side), object is the primary object on the side the + // template is evaluated for. + Object map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") + // of the kcp workspace that the synchronization is currently processing. This + // value is set for both evaluations, regardless of side. + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). This + // value is set for both evaluations, regardless of side. + ClusterPath logicalcluster.Path +} +``` + +In the simplest form, a template can replace a reference: + +* reference: `.spec.secretName` +* Go template: `{{ .Object.spec.secretName }}` + +Just like with references, the configured template is evaluated twice, once for each side of the +synchronization. You can use the `Side` variable to allow for fully customized names on each side: + +```yaml +spec: + ... + related: + - identifier: tls-secret + # ..omitting other fields.. + object: + template: + template: `{{ if eq .Side "kcp" }}name-in-kcp{{ else }}name-on-service-cluster{{ end }}` +``` + +See [Templating](templating.md) for more information on how to use templates in PublishedResources. + #### Label Selectors In some cases, the primary object does not have a link to its child/children objects. In these cases, @@ -362,7 +451,7 @@ is assumed. However you can actually also use label selectors to find the origin dynamically. So you can configure two label selectors, and then agent will first use the namespace selector to find all applicable namespaces, and then use the other label selector _in each of the applicable namespaces_ to finally locate the related objects. How useful this is depends a lot on -how crazy the underlying operators on the service clusters are. +how peculiar the underlying operators on the service clusters are. Here is an example on how to use label selectors: @@ -379,7 +468,7 @@ spec: naming: namespace: kube-system - name: "$remoteClusterName-$remoteNamespaceHash-$remoteNameHash" + name: "{{ .ClusterName }}-{{ .Object.metadata.namespace | sha3short }}-{{ .Object.metadata.name | sha3short }}" related: - identifier: tls-secrets @@ -399,9 +488,18 @@ spec: matchLabels: my-key: my-value another: pair + # Within matchLabels, keys and values are treated as Go templates. + # In this example, since the Secret originates on the service cluster + # (see "origin" above), we use LocalObject to determine the value + # for the selector. In case the object was heavily mutated during the + # sync, this will give access to the mutated values on the service + # cluster side. + '{{ shasum "test" }}': '{{ .LocalObject.spec.username }}' # You also need to provide rules on how objects found by this selector - # should be named on the destination side of the sync. + # should be named on the destination side of the sync. You can choose + # to define a rewrite rule that keeps the original name from the origin + # side, but this may leak undesirable internals to the users. # Rewrites are either using regular expressions or templated strings, # never both. # The rewrite config is applied to each individual found object. @@ -427,13 +525,84 @@ spec: # replacement: '...' ``` -#### Templates +There are two possible usages of Go templates when using label selectors. See [Templating](templating.md) +for more information on how to use templates in PublishedResources in general. + +##### Selector Templates + +Each template rendered as part of a `matchLabels` selector gets the following data injected: + +```go +type relatedObjectLabelContext struct { + // LocalObject is the primary object copy on the local side of the sync + // (i.e. on the service cluster). + LocalObject map[string]any + // RemoteObject is the primary object original, in kcp. + RemoteObject map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") + // of the kcp workspace that the synchronization is currently processing + // (where the remote object exists). + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). + ClusterPath logicalcluster.Path +} +``` + +Note that in contrast to the `template` way of selecting objects, the templates here in the label +selector are only evaluated once, on the origin side of the sync. The names of the destination side +are determined using the rewrite mechanism (which might also be a Go template, see next section). + +##### Rewrite Rules + +Each found related object on the origin side needs to have its own name on the destination side. To +map from the origin to the destination side, regular expressions (see example snippet) or Go +templatess can be used. + +If a template is configured, it is evaluated once for every found related object. The template gets +the following data injected into it: + +```go +type relatedObjectLabelRewriteContext struct { + // Value is either the a found namespace name (when a label selector was + // used to select the source namespaces for related objects) or the name of + // a found object (when a label selector was used to find objects). In the + // former case, the template should return the new namespace to use on the + // destination side, in the latter case it should return the new object name + // to use on the destination side. + Value string + // When a rewrite is used to rewrite object names, RelatedObject is the + // original related object (found on the origin side). This enables you to + // ignore the given Value entirely and just select anything from the object + // itself. + // RelatedObject is nil when the rewrite is performed for a namespace. + RelatedObject map[string]any + // LocalObject is the primary object copy on the local side of the sync + // (i.e. on the service cluster). + LocalObject map[string]any + // RemoteObject is the primary object original, in kcp. + RemoteObject map[string]any + // ClusterName is the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") + // of the kcp workspace that the synchronization is currently processing + // (where the remote object exists). + ClusterName logicalcluster.Name + // ClusterPath is the workspace path (e.g. "root:customer:projectx"). + ClusterPath logicalcluster.Path +} +``` -The third option to configure how to find/create related objects are templates. These are simple -Go template strings (like `{% raw %}{{ .Variable }}{% endraw %}`) that allow to easily configure static values with a -sprinkling of dynamic values. +Regarding `Value`: The agent allows to individually configure rules for finding object _names_ and +object _namespaces_. Often the namespace is not configured because the related objects live in the +same namespace as their owning, primary object. -This feature has not been fully implemented yet. +When a label selector is configured to find namespaces, the rewrite template will be evaluated once +for each found namespace. In this case the `.Value` is the name of the found namespace. Remember, the +template's job is to map the found namespace to the new namespace on the destination side of the sync. + +Once the namespaces have been determined, the agent will look for matching objects in each namespace +individually. For each namespace it will again follow the configured source, may it be a selector, +template or reference. If again a label selector is used, it will be applied in each namespace and +the configured rewrite rule will be evaluated once per found object. In this case, `.Value` is the +name of found object. ## Examples @@ -466,28 +635,20 @@ spec: # this is where our CA and Issuer live in this example namespace: kube-system # need to adjust it to prevent collions (normally clustername is the namespace) - name: "$remoteClusterName-$remoteNamespaceHash-$remoteNameHash" + name: "{{ .ClusterName }}-{{ .Object.metadata.namespace | sha3short }}-{{ .Object.metadata.name | sha3short }}" related: - origin: service # service or kcp - kind: Secret # for now, only "Secret" and "ConfigMap" are supported; - # there is no GVK projection for related resources + kind: Secret # for now, only "Secret" and "ConfigMap" are supported; + # there is no GVK projection for related resources # configure where in the parent object we can find # the name/namespace of the related resource (the child) - reference: - name: - # This path is evaluated in both the local and remote objects, to figure out - # the local and remote names for the related object. This saves us from having - # to remember mutated fields before their mutation (similar to the last-known - # annotation). - path: spec.secretName - # namespace part is optional; if not configured, - # Sync Agent assumes the same namespace as the owning resource - # namespace: - # path: spec.secretName - # regex: - # pattern: '...' - # replacement: '...' + object: + # This template is evaluated in both the local and remote objects, to figure out + # the local and remote names for the related object. This saves us from having + # to remember mutated fields before their mutation (similar to the last-known + # annotation). + template: + template: '{{ .Object.spec.secretName }}' ``` - diff --git a/docs/content/publish-resources/templating.md b/docs/content/publish-resources/templating.md new file mode 100644 index 0000000..3551362 --- /dev/null +++ b/docs/content/publish-resources/templating.md @@ -0,0 +1,87 @@ +# Templating + +`PublishedResources` allow to use [Go templates](https://pkg.go.dev/text/template) in a number of +places. A simple template could look like `{{ .Object.spec.secretName | sha3sum }}`. + +## General Usage + +Users are encouraged to get familiar with the [Go documentation](https://pkg.go.dev/text/template) +on templates. + +Specifically within the agent, the following rules apply when a template is evaluated: + +* All templates must evaluate successfully. Any error will cancel the synchronization process for + that object, potentially leaving it in a half-finished overall state. +* Templates should not output random values, as those can lead to reconcile loops and higher load + on the service cluster. +* Any leading and trailing whitespace will be automatically trimmed from the template's output. + +## Functions + +Templates can make use of all functions provided by [sprig/v3](https://masterminds.github.io/sprig/), +for example `join` or `b64enc`. The agent then adds the following functions: + +* `sha3sum STRING`
Returns the hex-encoded SHA3-256 hash (32 characters long). +* `sha3short STRING [LENGTH=20]`
Returns the first `LENGTH` characters of the hex-encoded SHA3-256 hash. +* `shortHash STRING`
Returns the first 20 characters of the hex-encoded SHA-1 hash. + This function is only available for backwards compatibility when migrating `$variable`-based + naming rules to use Go templates. New setups should not use this function, but one of the explicitly + named ones, like `sha256sum` or `sha3sum`. + +## Context + +Depending on where a template is used, different data is available inside the template. The following +is a summary of those different values: + +### Primary Object Naming Rules + +This is for templates used in `.spec.naming`: + +| Name | Type | Description | +| ------------- | --------------------- | ----------- | +| `Object` | `map[string]any` | the full remote object found in a kcp workspace | +| `ClusterName` | `logicalcluster.Name` | the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") | +| `ClusterPath` | `logicalcluster.Path` | the workspace path (e.g. "root:customer:projectx") | + +### Related Object Template Source + +This is for templates used in `.spec.related[*].object.template` and +`.spec.related[*].object.namespace.template`: + +| Name | Type | Description | +| ------------- | --------------------- | ----------- | +| `Side` | `string` | set to either one of the possible origin values (`kcp` or `origin`) to indicate for which cluster the template is currently being evaluated for | +| `Object` | `map[string]any` | the primary object belonging to the related object. Since related object templates are evaluated twice (once for the origin side and once for the destination side), object is the primary object on the side the template is evaluated for | +| `ClusterName` | `logicalcluster.Name` | the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") of the kcp workspace that the synchronization is currently processing; this value is set for both evaluations, regardless of side | +| `ClusterPath` | `logicalcluster.Path` | the workspace path (e.g. "root:customer:projectx"); this value is set for both evaluations, regardless of side | + +These templates are evaluated once on each side of the synchronization. + +### Related Object Label Selectors + +This is for templates used in `.spec.related[*].object.selector.matchLabels` and +`.spec.related[*].object.namespace.selector.matchLabels`, both keys and values: + +| Name | Type | Description | +| -------------- | --------------------- | ----------- | +| `LocalObject` | `map[string]any` | the primary object copy on the local side of the sync (i.e. on the service cluster) | +| `RemoteObject` | `map[string]any` | the primary object original, in kcp | +| `ClusterName` | `logicalcluster.Name` | the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") of the kcp workspace that the synchronization is currently processing (where the remote object exists) | +| `ClusterPath` | `logicalcluster.Path` | the workspace path (e.g. "root:customer:projectx") | + +If a template for a key evaluates to an empty string, the key-value combination will be omitted from +the final selector. Empty values however are allowed. + +### Related Object Label Selector Rewrites + +This is for templates used in `.spec.related[*].object.selector.rewrite.template` and +`.spec.related[*].object.namespace.selector.rewrite.template`: + +| Name | Type | Description | +| --------------- | --------------------- | ----------- | +| `Value` | `string` | Either the a found namespace name (when a label selector was used to select the source namespaces for related objects) or the name of a found object (when a label selector was used to find objects). In the former case, the template should return the new namespace to use on the destination side, in the latter case it should return the new object name to use on the destination side. | +| `RelatedObject` | `map[string]any` | When a rewrite is used to rewrite object names, RelatedObject is the original related object (found on the origin side). This enables you to ignore the given Value entirely and just select anything from the object itself. RelatedObject is `nil` when the rewrite is performed for a namespace. | +| `LocalObject` | `map[string]any` | the primary object copy on the local side of the sync (i.e. on the service cluster) | +| `RemoteObject` | `map[string]any` | the primary object original, in kcp | +| `ClusterName` | `logicalcluster.Name` | the internal cluster identifier (e.g. "34hg2j4gh24jdfgf") of the kcp workspace that the synchronization is currently processing (where the remote object exists) | +| `ClusterPath` | `logicalcluster.Path` | the workspace path (e.g. "root:customer:projectx") | diff --git a/internal/sync/templating/related.go b/internal/sync/templating/related.go index 1c0d483..24f1d17 100644 --- a/internal/sync/templating/related.go +++ b/internal/sync/templating/related.go @@ -56,7 +56,7 @@ func NewRelatedObjectContext(object *unstructured.Unstructured, side syncagentv1 // relatedObjectContext is the data available to Go templates in the keys and values // of a label selector for a related object. type relatedObjectLabelContext struct { - // LocalObject ist the primary object copy on the local side of the sync + // LocalObject is the primary object copy on the local side of the sync // (i.e. on the service cluster). LocalObject map[string]any // RemoteObject is the primary object original, in kcp. @@ -95,7 +95,7 @@ type relatedObjectLabelRewriteContext struct { // itself. // RelatedObject is nil when the rewrite is performed for a namespace. RelatedObject map[string]any - // LocalObject ist the primary object copy on the local side of the sync + // LocalObject is the primary object copy on the local side of the sync // (i.e. on the service cluster). LocalObject map[string]any // RemoteObject is the primary object original, in kcp. From 9b10cfa7af54e4bd396f0fff5493713bba365244 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Mon, 26 May 2025 15:18:53 +0200 Subject: [PATCH 15/22] as just documented, empty keys in label selectors are dropped On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/syncer_related.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 6dd720d..1f660c6 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -559,7 +559,9 @@ func templateLabelSelector(relatedOrigin, relatedDest syncSide, origin syncagent value = rendered } - newMatchLabels[key] = value + if key != "" { + newMatchLabels[key] = value + } } selector.MatchLabels = newMatchLabels From 6af7becd02b259fad4ac3bea4fb1432fb0f8a63e Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Mon, 26 May 2025 15:37:29 +0200 Subject: [PATCH 16/22] extend docs On-behalf-of: @SAP christoph.mewes@sap.com --- docs/content/publish-resources/templating.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/content/publish-resources/templating.md b/docs/content/publish-resources/templating.md index 3551362..ec3af12 100644 --- a/docs/content/publish-resources/templating.md +++ b/docs/content/publish-resources/templating.md @@ -15,6 +15,8 @@ Specifically within the agent, the following rules apply when a template is eval * Templates should not output random values, as those can lead to reconcile loops and higher load on the service cluster. * Any leading and trailing whitespace will be automatically trimmed from the template's output. +* All "objects" mentioned in this documentation refer technically to an `unstructured.Unstructured` + value's `.Object` field, i.e. the JSON-decoded representation of a Kubernetes object. ## Functions From 3e8fa72cd8c0e9e58bdceebbd115310ae9fdac57 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 27 May 2025 11:44:50 +0200 Subject: [PATCH 17/22] add new field to dummy Thing type for testing On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/apis/dummy/v1alpha1/thing.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/sync/apis/dummy/v1alpha1/thing.go b/internal/sync/apis/dummy/v1alpha1/thing.go index 5ffb1fe..dd85d99 100644 --- a/internal/sync/apis/dummy/v1alpha1/thing.go +++ b/internal/sync/apis/dummy/v1alpha1/thing.go @@ -34,6 +34,7 @@ type Thing struct { type ThingSpec struct { Username string `json:"username"` + Kink string `json:"kink"` Address string `json:"address,omitempty"` } From 29cf6496f79a7838d3ee20bdf8e8efb2bf4498c9 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 27 May 2025 11:45:10 +0200 Subject: [PATCH 18/22] codegen On-behalf-of: @SAP christoph.mewes@sap.com --- .../dummy.example.com_namespacedthings.yaml | 3 ++ .../sync/crd/dummy.example.com_things.yaml | 3 ++ .../dummy.example.com_thingwithstatuses.yaml | 3 ++ ...ample.com_thingwithstatussubresources.yaml | 3 ++ internal/sync/syncer_test.go | 42 +++++++++---------- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/internal/sync/crd/dummy.example.com_namespacedthings.yaml b/internal/sync/crd/dummy.example.com_namespacedthings.yaml index baf1dcc..5a8455e 100644 --- a/internal/sync/crd/dummy.example.com_namespacedthings.yaml +++ b/internal/sync/crd/dummy.example.com_namespacedthings.yaml @@ -40,9 +40,12 @@ spec: properties: address: type: string + kink: + type: string username: type: string required: + - kink - username type: object required: diff --git a/internal/sync/crd/dummy.example.com_things.yaml b/internal/sync/crd/dummy.example.com_things.yaml index b30edb8..d490de3 100644 --- a/internal/sync/crd/dummy.example.com_things.yaml +++ b/internal/sync/crd/dummy.example.com_things.yaml @@ -40,9 +40,12 @@ spec: properties: address: type: string + kink: + type: string username: type: string required: + - kink - username type: object required: diff --git a/internal/sync/crd/dummy.example.com_thingwithstatuses.yaml b/internal/sync/crd/dummy.example.com_thingwithstatuses.yaml index 13ad83a..c6d078e 100644 --- a/internal/sync/crd/dummy.example.com_thingwithstatuses.yaml +++ b/internal/sync/crd/dummy.example.com_thingwithstatuses.yaml @@ -40,9 +40,12 @@ spec: properties: address: type: string + kink: + type: string username: type: string required: + - kink - username type: object status: diff --git a/internal/sync/crd/dummy.example.com_thingwithstatussubresources.yaml b/internal/sync/crd/dummy.example.com_thingwithstatussubresources.yaml index a314b9f..a486454 100644 --- a/internal/sync/crd/dummy.example.com_thingwithstatussubresources.yaml +++ b/internal/sync/crd/dummy.example.com_thingwithstatussubresources.yaml @@ -40,9 +40,12 @@ spec: properties: address: type: string + kink: + type: string username: type: string required: + - kink - username type: object status: diff --git a/internal/sync/syncer_test.go b/internal/sync/syncer_test.go index 074cfad..e34dbbf 100644 --- a/internal/sync/syncer_test.go +++ b/internal/sync/syncer_test.go @@ -193,7 +193,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -222,7 +222,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -271,7 +271,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -330,7 +330,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -368,7 +368,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -397,7 +397,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Miss Scarlet", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Miss Scarlet"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Miss Scarlet"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -464,7 +464,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -513,7 +513,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"annotations":{"existing-annotation":"annotation-value"},"labels":{"existing-label":"label-value"},"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"annotations":{"existing-annotation":"annotation-value"},"labels":{"existing-label":"label-value"},"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -557,7 +557,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { }, }), // last state annotation is "space optimized" and so does not include the ignored labels and annotations - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"annotations":{"existing-annotation":"new-annotation-value","new-annotation":"hei-verden"},"labels":{"existing-label":"new-label-value","new-label":"hello-world"},"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"annotations":{"existing-annotation":"new-annotation-value","new-annotation":"hei-verden"},"labels":{"existing-label":"new-label-value","new-label":"hello-world"},"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -592,7 +592,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -622,7 +622,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { }, }), // last state annotation is "space optimized" and so does not include the ignored labels and annotations - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -661,7 +661,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Address: "Hotdogstr. 13", // we assume this field was set by a local controller/webhook, unrelated to the Sync Agent }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -692,7 +692,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Address: "Hotdogstr. 13", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Miss Scarlet"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Miss Scarlet"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -737,7 +737,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -771,7 +771,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -850,7 +850,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ ObjectMeta: metav1.ObjectMeta{ @@ -881,7 +881,7 @@ func TestSyncerProcessingSingleResourceWithoutStatus(t *testing.T) { Username: "Colonel Mustard", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, } @@ -1076,7 +1076,7 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { CurrentVersion: "v1", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.ThingWithStatusSubresource{ ObjectMeta: metav1.ObjectMeta{ @@ -1111,7 +1111,7 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { CurrentVersion: "v1", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, ///////////////////////////////////////////////////////////////////////////////// @@ -1152,7 +1152,7 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { CurrentVersion: "v1", }, }), - existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + existingState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, expectedRemoteObject: newUnstructured(&dummyv1alpha1.ThingWithStatusSubresource{ ObjectMeta: metav1.ObjectMeta{ @@ -1187,7 +1187,7 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { CurrentVersion: "v1", }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"kink":"","username":"Colonel Mustard"}}`, }, } From 3369eb94d8172acc49bb88c4a8ea2bf3e3f61e27 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 27 May 2025 11:46:00 +0200 Subject: [PATCH 19/22] add tests to ensure reference semantics On-behalf-of: @SAP christoph.mewes@sap.com --- internal/sync/init_test.go | 4 + internal/sync/syncer_related.go | 2 +- internal/sync/syncer_related_test.go | 234 +++++++++++++++++++++++++++ 3 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 internal/sync/syncer_related_test.go diff --git a/internal/sync/init_test.go b/internal/sync/init_test.go index 44075fd..36b8e33 100644 --- a/internal/sync/init_test.go +++ b/internal/sync/init_test.go @@ -21,6 +21,7 @@ import ( dummyv1alpha1 "github.com/kcp-dev/api-syncagent/internal/sync/apis/dummy/v1alpha1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -32,6 +33,9 @@ func init() { if err := dummyv1alpha1.AddToScheme(testScheme); err != nil { panic(err) } + if err := corev1.AddToScheme(testScheme); err != nil { + panic(err) + } } var nonEmptyTime = metav1.Time{ diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 1f660c6..6f48463 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -457,7 +457,7 @@ func resolveObjectReference(object *unstructured.Unstructured, ref syncagentv1al func resolveReference(jsonData []byte, ref syncagentv1alpha1.RelatedResourceObjectReference) (string, error) { gval := gjson.Get(string(jsonData), ref.Path) if !gval.Exists() { - return "", fmt.Errorf("cannot find %s in document", ref.Path) + return "", nil } // this does apply some coalescing, like turning numbers into strings diff --git a/internal/sync/syncer_related_test.go b/internal/sync/syncer_related_test.go new file mode 100644 index 0000000..1761644 --- /dev/null +++ b/internal/sync/syncer_related_test.go @@ -0,0 +1,234 @@ +/* +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 sync + +import ( + "testing" + + dummyv1alpha1 "github.com/kcp-dev/api-syncagent/internal/sync/apis/dummy/v1alpha1" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestResolveRelatedResourceObjects(t *testing.T) { + // in kcp + primaryObject := newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "original-value", + Kink: "taxreturns", + }, + }, withKind("RemoteThing")) + + // on the service cluster + primaryObjectCopy := newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "mutated-value", + Kink: "", + }, + }) + + // Create a secret that can be found by using a good reference, so we can ensure that references + // do indeed work; all other subtests here ensure that reference support can deal with broken refs. + dummySecret := newUnstructured(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "dummy-namespace", + Name: "mutated-value", + }, + }) + + kcpClient := buildFakeClient(primaryObject) + serviceClusterClient := buildFakeClient(primaryObjectCopy, dummySecret) + ctx := t.Context() + + // Now we configure origin/dest as if we're syncing a Secret up from the service cluster to kcp, + // i.e. origin=service. + + originSide := syncSide{ + ctx: ctx, + client: serviceClusterClient, + object: primaryObjectCopy, + } + + destSide := syncSide{ + ctx: ctx, + client: kcpClient, + object: primaryObject, + // Since this is a just a regular kube client, we do not need to set clusterName/clusterPath. + } + + testcases := []struct { + name string + objectSpec syncagentv1alpha1.RelatedResourceObject + expectedSecrets int + }{ + { + name: "valid reference to an existing object", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.username", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "dummy-namespace", + }, + }, + }, + expectedSecrets: 1, + }, + { + name: "valid template to an existing object", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "{{ .Object.spec.username }}", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "dummy-namespace", + }, + }, + }, + expectedSecrets: 1, + }, + { + name: "valid reference but target object doesn't exist [yet?]", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.username", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "nonexisting-namespace", + }, + }, + }, + expectedSecrets: 0, + }, + { + name: "valid template but target object doesn't exist [yet?]", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "{{ .Object.spec.username }}", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "nonexisting-namespace", + }, + }, + }, + expectedSecrets: 0, + }, + { + name: "valid reference to an empty field", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.kink", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "dummy-namespace", + }, + }, + }, + expectedSecrets: 0, + }, + { + name: "valid template to an empty field", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "{{ .Object.spec.kink }}", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "dummy-namespace", + }, + }, + }, + expectedSecrets: 0, + }, + { + name: "referring to an omitempty field", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "spec.address", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "dummy-namespace", + }, + }, + }, + expectedSecrets: 0, + }, + { + name: "templating an omitempty field", + objectSpec: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "{{ .Object.spec.address }}", + }, + }, + Namespace: &syncagentv1alpha1.RelatedResourceObjectSpec{ + Template: &syncagentv1alpha1.TemplateExpression{ + Template: "dummy-namespace", + }, + }, + }, + expectedSecrets: 0, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + pubRes := syncagentv1alpha1.RelatedResourceSpec{ + Identifier: "test", + Origin: syncagentv1alpha1.RelatedResourceOriginService, + Kind: "Secret", + Object: testcase.objectSpec, + } + + foundObjects, err := resolveRelatedResourceObjects(originSide, destSide, pubRes) + if err != nil { + t.Fatalf("Failed to resolve related objects: %v", err) + } + if len(foundObjects) != testcase.expectedSecrets { + t.Fatalf("Expected %d related object (Secret) to be found, but found %d.", testcase.expectedSecrets, len(foundObjects)) + } + }) + } +} From 18756a03c2db4a91b036e06830ca618cfb229183 Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 27 May 2025 16:30:50 +0200 Subject: [PATCH 20/22] fix typo On-behalf-of: @SAP christoph.mewes@sap.com --- docs/content/publish-resources/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/publish-resources/index.md b/docs/content/publish-resources/index.md index 85c9a7d..935d48b 100644 --- a/docs/content/publish-resources/index.md +++ b/docs/content/publish-resources/index.md @@ -556,7 +556,7 @@ are determined using the rewrite mechanism (which might also be a Go template, s Each found related object on the origin side needs to have its own name on the destination side. To map from the origin to the destination side, regular expressions (see example snippet) or Go -templatess can be used. +templates can be used. If a template is configured, it is evaluated once for every found related object. The template gets the following data injected into it: From 28ecd450523ea569f126c823f172852dae1e020b Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 27 May 2025 16:32:19 +0200 Subject: [PATCH 21/22] explain nolints On-behalf-of: @SAP christoph.mewes@sap.com --- internal/projection/projection.go | 4 ++-- internal/sync/syncer_related.go | 2 +- internal/sync/templating/naming.go | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/projection/projection.go b/internal/projection/projection.go index 8e4dcc0..5b57405 100644 --- a/internal/projection/projection.go +++ b/internal/projection/projection.go @@ -116,7 +116,7 @@ func ProjectCRD(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagent func stripUnwantedVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes *syncagentv1alpha1.PublishedResource) (*apiextensionsv1.CustomResourceDefinition, error) { src := pubRes.Spec.Resource - //nolint:staticcheck + //nolint:staticcheck // .Version is deprecated, but we still support it for now. if src.Version != "" && len(src.Versions) > 0 { return nil, errors.New("cannot configure both .version and .versions in as the source of a PublishedResource") } @@ -181,7 +181,7 @@ func projectCRDVersions(crd *apiextensionsv1.CustomResourceDefinition, pubRes *s // We already validated that Version and Versions can be set at the same time. - //nolint:staticcheck + //nolint:staticcheck // .Version is deprecated, but we still support it for now. 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) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 6f48463..b2a427d 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -251,7 +251,7 @@ func resolveRelatedResourceObjects(relatedOrigin, relatedDest syncSide, relRes s func resolveRelatedResourceOriginNamespaces(relatedOrigin, relatedDest syncSide, origin syncagentv1alpha1.RelatedResourceOrigin, spec syncagentv1alpha1.RelatedResourceObjectSpec) (map[string]string, error) { switch { - //nolint:staticcheck + //nolint:staticcheck // .Reference is deprecated, but we still support it for now. case spec.Reference != nil: originNamespace, err := resolveObjectReference(relatedOrigin.object, *spec.Reference) //nolint:staticcheck if err != nil { diff --git a/internal/sync/templating/naming.go b/internal/sync/templating/naming.go index f04ddc4..4877039 100644 --- a/internal/sync/templating/naming.go +++ b/internal/sync/templating/naming.go @@ -92,7 +92,8 @@ func generateLocalObjectIdentifier(pattern string, object *unstructured.Unstruct return Render(pattern, newLocalObjectNamingContext(object, clusterName, clusterPath)) } - // legacy $variable style, does also not support clusterPath + // Legacy $variable style, does also not support clusterPath; + // note that all of these constants are deprecated already. replacer := strings.NewReplacer( // order of elements is important here, "$fooHash" needs to be defined before "$foo" syncagentv1alpha1.PlaceholderRemoteClusterName, clusterName.String(), //nolint:staticcheck From 1b5b99550078aedf2401a741fae5cc57fe56a44d Mon Sep 17 00:00:00 2001 From: Christoph Mewes Date: Tue, 27 May 2025 19:23:32 +0200 Subject: [PATCH 22/22] make apiresourceschema e2e tests more stable On-behalf-of: @SAP christoph.mewes@sap.com --- .../apiresourceschema_test.go | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/test/e2e/apiresourceschema/apiresourceschema_test.go b/test/e2e/apiresourceschema/apiresourceschema_test.go index ac5ad1e..c4d2108 100644 --- a/test/e2e/apiresourceschema/apiresourceschema_test.go +++ b/test/e2e/apiresourceschema/apiresourceschema_test.go @@ -166,14 +166,20 @@ func TestARSAreNotUpdated(t *testing.T) { 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) + // prevent race condition of the controller first creating the ARS, then updating the PR, + // by waiting again for the status to be updated. + err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 30*time.Second, false, func(ctx context.Context) (done bool, err error) { + if err := envtestClient.Get(ctx, ctrlruntimeclient.ObjectKeyFromObject(pr), pr); err != nil { + return false, nil + } + + return pr.Status.ResourceSchemaName != "", nil + }) + if err != nil { + t.Fatalf("Failed to wait for PublishedResource status to be updated: %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)…") @@ -194,17 +200,20 @@ func TestARSAreNotUpdated(t *testing.T) { 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) - } + // wait for the status to contain the new ARS name + err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 1*time.Minute, false, func(ctx context.Context) (done bool, err error) { + if err := envtestClient.Get(ctx, ctrlruntimeclient.ObjectKeyFromObject(pr), pr); err != nil { + return false, nil + } - newARSName := pr.Status.ResourceSchemaName - if newARSName == "" { - t.Fatal("Expected PublishedResource status to contain ARS name, but value is empty.") + return pr.Status.ResourceSchemaName != arsName, nil + }) + if err != nil { + t.Fatalf("Failed to wait for PublishedResource status to be updated: %v", err) } - if newARSName == arsName { - t.Fatalf("Expected PublishedResource status to have been updated with new ARS name, but still contains %q.", arsName) + if pr.Status.ResourceSchemaName == "" { + t.Fatal("Expected PublishedResource status to contain ARS name, but value is empty.") } }