From 6eda5cca4ab2bd083470f096d61a8000217e0ce1 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Wed, 19 Mar 2025 11:55:56 +0100 Subject: [PATCH 1/7] feat: add optional published resources Signed-off-by: Karol Szwaj On-behalf-of: @SAP karol.szwaj@sap.com --- internal/sync/syncer_related.go | 14 +- internal/sync/syncer_test.go | 276 ++++++++++++++++++ .../syncagent/v1alpha1/published_resource.go | 3 + 3 files changed, 287 insertions(+), 6 deletions(-) diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 2dc86ea..651e370 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -40,13 +40,15 @@ import ( func (s *ResourceSyncer) processRelatedResources(log *zap.SugaredLogger, stateStore ObjectStateStore, remote, local syncSide) (requeue bool, err error) { for _, relatedResource := range s.pubRes.Spec.Related { - requeue, err := s.processRelatedResource(log.With("identifier", relatedResource.Identifier), stateStore, remote, local, relatedResource) - if err != nil { - return false, fmt.Errorf("failed to process related resource %s: %w", relatedResource.Identifier, err) - } + if !relatedResource.Optional { + requeue, err := s.processRelatedResource(log.With("identifier", relatedResource.Identifier), stateStore, remote, local, relatedResource) + if err != nil { + return false, fmt.Errorf("failed to process related resource %s: %w", relatedResource.Identifier, err) + } - if requeue { - return true, nil + if requeue { + return true, nil + } } } diff --git a/internal/sync/syncer_test.go b/internal/sync/syncer_test.go index 93baf54..b9796a7 100644 --- a/internal/sync/syncer_test.go +++ b/internal/sync/syncer_test.go @@ -1304,6 +1304,282 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { }) } } +func TestSyncerProcessingRelatedResources(t *testing.T) { + type testcase struct { + name string + remoteAPIGroup string + localCRD *apiextensionsv1.CustomResourceDefinition + pubRes *syncagentv1alpha1.PublishedResource + remoteObject *unstructured.Unstructured + localObject *unstructured.Unstructured + existingState string + performRequeues bool + expectedRemoteObject *unstructured.Unstructured + expectedLocalObject *unstructured.Unstructured + expectedState string + customVerification func(t *testing.T, requeue bool, processErr error, finalRemoteObject *unstructured.Unstructured, finalLocalObject *unstructured.Unstructured, testcase testcase) + } + + clusterName := logicalcluster.Name("testcluster") + + remoteThingPR := &syncagentv1alpha1.PublishedResource{ + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: dummyv1alpha1.GroupName, + Version: dummyv1alpha1.GroupVersion, + Kind: "Thing", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Kind: "RemoteThing", + }, + // include explicit naming rules to be independent of possible changes to the defaults + Naming: &syncagentv1alpha1.ResourceNaming{ + Name: "$remoteClusterName-$remoteName", // Things are Cluster-scoped + }, + Related: []syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "mandatory-secret", + Origin: "service", + Kind: "Thing", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "spec.otherTest.name", + }, + Namespace: &syncagentv1alpha1.ResourceLocator{ + Path: "spec.otherTest.namespace", + }, + }, + Optional: false, + }, + { + Identifier: "optional-secret", + Origin: "kcp", + Kind: "Thing", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "spec.test.name", + }, + Namespace: &syncagentv1alpha1.ResourceLocator{ + Path: "spec.test.namespace", + }, + }, + Optional: true, + }, + }, + }, + } + + testcases := []testcase{ + { + name: "optional related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: remoteThingPR, + performRequeues: true, + + remoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + localObject: nil, + existingState: "", + + expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Finalizers: []string{ + deletionFinalizer, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + }, + { + name: "mandatory related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: remoteThingPR, + performRequeues: true, + + remoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + localObject: nil, + existingState: "", + + expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Finalizers: []string{ + deletionFinalizer, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + }, + } + + const stateNamespace = "kcp-system" + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + localClient := buildFakeClient(testcase.localObject) + remoteClient := buildFakeClient(testcase.remoteObject) + + syncer, err := NewResourceSyncer( + // zap.Must(zap.NewDevelopment()).Sugar(), + zap.NewNop().Sugar(), + localClient, + remoteClient, + testcase.pubRes, + testcase.localCRD, + testcase.remoteAPIGroup, + nil, + stateNamespace, + "textor-the-doctor", + ) + if err != nil { + t.Fatalf("Failed to create syncer: %v", err) + } + + localCtx := context.Background() + remoteCtx := kontext.WithCluster(localCtx, clusterName) + ctx := NewContext(localCtx, remoteCtx) + + // setup a custom state backend that we can prime + var backend *kubernetesBackend + syncer.newObjectStateStore = func(primaryObject, stateCluster syncSide) ObjectStateStore { + // .Process() is called multiple times, but we want the state to persist between reconciles. + if backend == nil { + backend = newKubernetesBackend(stateNamespace, primaryObject, stateCluster) + if testcase.existingState != "" { + if err := backend.Put(testcase.remoteObject, clusterName, []byte(testcase.existingState)); err != nil { + t.Fatalf("Failed to prime state store: %v", err) + } + } + } + + return &objectStateStore{ + backend: backend, + } + } + + var requeue bool + + if testcase.performRequeues { + target := testcase.remoteObject.DeepCopy() + + for i := 0; true; i++ { + if i > 20 { + t.Fatalf("Detected potential infinite loop, stopping after %d requeues.", i) + } + + requeue, err = syncer.Process(ctx, target) + if err != nil { + break + } + + if !requeue { + break + } + + if err = remoteClient.Get(remoteCtx, ctrlruntimeclient.ObjectKeyFromObject(target), target); err != nil { + // it's possible for the processing to have deleted the remote object, + // so a NotFound is valid here + if apierrors.IsNotFound(err) { + break + } + + t.Fatalf("Failed to get updated remote object: %v", err) + } + } + } else { + requeue, err = syncer.Process(ctx, testcase.remoteObject) + } + + finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteObject, testcase.expectedRemoteObject) + if getErr != nil { + t.Fatalf("Failed to get final remote object: %v", getErr) + } + + finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localObject, testcase.expectedLocalObject) + if getErr != nil { + t.Fatalf("Failed to get final local object: %v", getErr) + } + + if testcase.customVerification != nil { + testcase.customVerification(t, requeue, err, finalRemoteObject, finalLocalObject, testcase) + } else { + if err != nil { + t.Fatalf("Processing failed: %v", err) + } + + assertObjectsEqual(t, "local", testcase.expectedLocalObject, finalLocalObject) + assertObjectsEqual(t, "remote", testcase.expectedRemoteObject, finalRemoteObject) + + if testcase.expectedState != "" { + if backend == nil { + t.Fatal("Cannot check object state, state store was never instantiated.") + } + + finalState, err := backend.Get(testcase.expectedRemoteObject, clusterName) + if err != nil { + t.Fatalf("Failed to get final state: %v", err) + } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { + t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) + } + } + } + }) + } +} func assertObjectsEqual(t *testing.T, kind string, expected, actual *unstructured.Unstructured) { if expected == nil { diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index 613924c..8cce3f4 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -178,6 +178,9 @@ type RelatedResourceSpec struct { // Mutation configures optional transformation rules for the related resource. // Status mutations are only performed when the related resource originates in kcp. Mutation *ResourceMutationSpec `json:"mutation,omitempty"` + + // Optional indicates whether the related resource must be referenced. + Optional bool `json:"optional,omitempty"` } // RelatedResourceSource configures how the related resource can be found on the origin side From a47eef5c300d68820fd88c8959ce1c8ed5903987 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Thu, 20 Mar 2025 11:48:01 +0100 Subject: [PATCH 2/7] update codegen Signed-off-by: Karol Szwaj On-behalf-of: @SAP karol.szwaj@sap.com --- .../syncagent.kcp.io_publishedresources.yaml | 6 + internal/sync/syncer_test.go | 276 ------------------ .../syncagent/v1alpha1/relatedresourcespec.go | 8 + 3 files changed, 14 insertions(+), 276 deletions(-) diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index 41b3ec6..de6a5f1 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -410,6 +410,7 @@ spec: type: object type: array type: object +<<<<<<< HEAD object: description: |- Object describes how the related resource can be found on the origin side @@ -652,6 +653,11 @@ spec: type: string type: object type: object +======= + optional: + description: Optional indicates whether the related resource must be referenced. + type: boolean +>>>>>>> 2f5479b (update codegen) origin: description: '"service" or "kcp"' type: string diff --git a/internal/sync/syncer_test.go b/internal/sync/syncer_test.go index b9796a7..93baf54 100644 --- a/internal/sync/syncer_test.go +++ b/internal/sync/syncer_test.go @@ -1304,282 +1304,6 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { }) } } -func TestSyncerProcessingRelatedResources(t *testing.T) { - type testcase struct { - name string - remoteAPIGroup string - localCRD *apiextensionsv1.CustomResourceDefinition - pubRes *syncagentv1alpha1.PublishedResource - remoteObject *unstructured.Unstructured - localObject *unstructured.Unstructured - existingState string - performRequeues bool - expectedRemoteObject *unstructured.Unstructured - expectedLocalObject *unstructured.Unstructured - expectedState string - customVerification func(t *testing.T, requeue bool, processErr error, finalRemoteObject *unstructured.Unstructured, finalLocalObject *unstructured.Unstructured, testcase testcase) - } - - clusterName := logicalcluster.Name("testcluster") - - remoteThingPR := &syncagentv1alpha1.PublishedResource{ - Spec: syncagentv1alpha1.PublishedResourceSpec{ - Resource: syncagentv1alpha1.SourceResourceDescriptor{ - APIGroup: dummyv1alpha1.GroupName, - Version: dummyv1alpha1.GroupVersion, - Kind: "Thing", - }, - Projection: &syncagentv1alpha1.ResourceProjection{ - Kind: "RemoteThing", - }, - // include explicit naming rules to be independent of possible changes to the defaults - Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteClusterName-$remoteName", // Things are Cluster-scoped - }, - Related: []syncagentv1alpha1.RelatedResourceSpec{ - { - Identifier: "mandatory-secret", - Origin: "service", - Kind: "Thing", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "spec.otherTest.name", - }, - Namespace: &syncagentv1alpha1.ResourceLocator{ - Path: "spec.otherTest.namespace", - }, - }, - Optional: false, - }, - { - Identifier: "optional-secret", - Origin: "kcp", - Kind: "Thing", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "spec.test.name", - }, - Namespace: &syncagentv1alpha1.ResourceLocator{ - Path: "spec.test.namespace", - }, - }, - Optional: true, - }, - }, - }, - } - - testcases := []testcase{ - { - name: "optional related resource does not exist", - remoteAPIGroup: "remote.example.corp", - localCRD: loadCRD("things"), - pubRes: remoteThingPR, - performRequeues: true, - - remoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - localObject: nil, - existingState: "", - - expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - Finalizers: []string{ - deletionFinalizer, - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testcluster-my-test-thing", - Labels: map[string]string{ - agentNameLabel: "textor-the-doctor", - remoteObjectClusterLabel: "testcluster", - remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", - }, - Annotations: map[string]string{ - remoteObjectNameAnnotation: "my-test-thing", - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, - }, - { - name: "mandatory related resource does not exist", - remoteAPIGroup: "remote.example.corp", - localCRD: loadCRD("things"), - pubRes: remoteThingPR, - performRequeues: true, - - remoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - localObject: nil, - existingState: "", - - expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - Finalizers: []string{ - deletionFinalizer, - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testcluster-my-test-thing", - Labels: map[string]string{ - agentNameLabel: "textor-the-doctor", - remoteObjectClusterLabel: "testcluster", - remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", - }, - Annotations: map[string]string{ - remoteObjectNameAnnotation: "my-test-thing", - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, - }, - } - - const stateNamespace = "kcp-system" - - for _, testcase := range testcases { - t.Run(testcase.name, func(t *testing.T) { - localClient := buildFakeClient(testcase.localObject) - remoteClient := buildFakeClient(testcase.remoteObject) - - syncer, err := NewResourceSyncer( - // zap.Must(zap.NewDevelopment()).Sugar(), - zap.NewNop().Sugar(), - localClient, - remoteClient, - testcase.pubRes, - testcase.localCRD, - testcase.remoteAPIGroup, - nil, - stateNamespace, - "textor-the-doctor", - ) - if err != nil { - t.Fatalf("Failed to create syncer: %v", err) - } - - localCtx := context.Background() - remoteCtx := kontext.WithCluster(localCtx, clusterName) - ctx := NewContext(localCtx, remoteCtx) - - // setup a custom state backend that we can prime - var backend *kubernetesBackend - syncer.newObjectStateStore = func(primaryObject, stateCluster syncSide) ObjectStateStore { - // .Process() is called multiple times, but we want the state to persist between reconciles. - if backend == nil { - backend = newKubernetesBackend(stateNamespace, primaryObject, stateCluster) - if testcase.existingState != "" { - if err := backend.Put(testcase.remoteObject, clusterName, []byte(testcase.existingState)); err != nil { - t.Fatalf("Failed to prime state store: %v", err) - } - } - } - - return &objectStateStore{ - backend: backend, - } - } - - var requeue bool - - if testcase.performRequeues { - target := testcase.remoteObject.DeepCopy() - - for i := 0; true; i++ { - if i > 20 { - t.Fatalf("Detected potential infinite loop, stopping after %d requeues.", i) - } - - requeue, err = syncer.Process(ctx, target) - if err != nil { - break - } - - if !requeue { - break - } - - if err = remoteClient.Get(remoteCtx, ctrlruntimeclient.ObjectKeyFromObject(target), target); err != nil { - // it's possible for the processing to have deleted the remote object, - // so a NotFound is valid here - if apierrors.IsNotFound(err) { - break - } - - t.Fatalf("Failed to get updated remote object: %v", err) - } - } - } else { - requeue, err = syncer.Process(ctx, testcase.remoteObject) - } - - finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteObject, testcase.expectedRemoteObject) - if getErr != nil { - t.Fatalf("Failed to get final remote object: %v", getErr) - } - - finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localObject, testcase.expectedLocalObject) - if getErr != nil { - t.Fatalf("Failed to get final local object: %v", getErr) - } - - if testcase.customVerification != nil { - testcase.customVerification(t, requeue, err, finalRemoteObject, finalLocalObject, testcase) - } else { - if err != nil { - t.Fatalf("Processing failed: %v", err) - } - - assertObjectsEqual(t, "local", testcase.expectedLocalObject, finalLocalObject) - assertObjectsEqual(t, "remote", testcase.expectedRemoteObject, finalRemoteObject) - - if testcase.expectedState != "" { - if backend == nil { - t.Fatal("Cannot check object state, state store was never instantiated.") - } - - finalState, err := backend.Get(testcase.expectedRemoteObject, clusterName) - if err != nil { - t.Fatalf("Failed to get final state: %v", err) - } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { - t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) - } - } - } - }) - } -} func assertObjectsEqual(t *testing.T, kind string, expected, actual *unstructured.Unstructured) { if expected == nil { diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go index 08173b2..0e560e4 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go @@ -73,3 +73,11 @@ func (b *RelatedResourceSpecApplyConfiguration) WithMutation(value *ResourceMuta b.Mutation = value return b } + +// WithOptional sets the Optional 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 Optional field is set to the value of the last call. +func (b *RelatedResourceSpecApplyConfiguration) WithOptional(value bool) *RelatedResourceSpecApplyConfiguration { + b.Optional = &value + return b +} From 05e013c2c71fe0ca27f86943ab723fb8e3df16a4 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Thu, 20 Mar 2025 14:00:13 +0100 Subject: [PATCH 3/7] add related resources tests Signed-off-by: Karol Szwaj On-behalf-of: @SAP karol.szwaj@sap.com --- internal/sync/init_test.go | 4 + internal/sync/syncer_test.go | 288 +++++++++++++++++++++++++++++++++++ 2 files changed, 292 insertions(+) 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_test.go b/internal/sync/syncer_test.go index 93baf54..f47643a 100644 --- a/internal/sync/syncer_test.go +++ b/internal/sync/syncer_test.go @@ -30,6 +30,7 @@ import ( "github.com/kcp-dev/api-syncagent/internal/test/diff" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1305,6 +1306,293 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { } } +func TestSyncerProcessingRelatedResources(t *testing.T) { + type testcase struct { + name string + remoteAPIGroup string + localCRD *apiextensionsv1.CustomResourceDefinition + pubRes *syncagentv1alpha1.PublishedResource + remoteObject *unstructured.Unstructured + localObject *unstructured.Unstructured + existingState string + performRequeues bool + expectedRemoteObject *unstructured.Unstructured + expectedLocalObject *unstructured.Unstructured + expectedState string + customVerification func(t *testing.T, requeue bool, processErr error, finalRemoteObject *unstructured.Unstructured, finalLocalObject *unstructured.Unstructured, testcase testcase) + } + + clusterName := logicalcluster.Name("testcluster") + + remoteThingPR := &syncagentv1alpha1.PublishedResource{ + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: dummyv1alpha1.GroupName, + Version: dummyv1alpha1.GroupVersion, + Kind: "Thing", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Kind: "RemoteThing", + }, + // include explicit naming rules to be independent of possible changes to the defaults + Naming: &syncagentv1alpha1.ResourceNaming{ + Name: "$remoteClusterName-$remoteName", // Things are Cluster-scoped + }, + Related: []syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "mandatory-credentials", + Origin: "service", + Kind: "Secret", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "metadata.name", // irrelevant + Regex: &syncagentv1alpha1.RegexResourceLocator{ + Replacement: "mandatory-credentials", + }, + }, + }, + }, + { + Identifier: "optional-secret", + Origin: "service", + Kind: "Secret", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "metadata.name", // irrelevant + Regex: &syncagentv1alpha1.RegexResourceLocator{ + Replacement: "optional-credentials", + }, + }, + }, + Optional: false, + }, + }, + }, + } + + testcases := []testcase{ + { + name: "optional related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: remoteThingPR, + performRequeues: true, + + remoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + localObject: nil, + existingState: "", + + expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Finalizers: []string{ + deletionFinalizer, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + }, + { + name: "mandatory related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: remoteThingPR, + performRequeues: true, + + remoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + localObject: nil, + existingState: "", + + expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Finalizers: []string{ + deletionFinalizer, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, + }, + } + + const stateNamespace = "kcp-system" + credentials := newUnstructured(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mandatory-credentials", + Namespace: stateNamespace, + Labels: map[string]string{ + "hello": "world", + }, + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + }) + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + localClient := buildFakeClient(testcase.localObject, credentials) + remoteClient := buildFakeClient(testcase.remoteObject) + + syncer, err := NewResourceSyncer( + // zap.Must(zap.NewDevelopment()).Sugar(), + zap.NewNop().Sugar(), + localClient, + remoteClient, + testcase.pubRes, + testcase.localCRD, + testcase.remoteAPIGroup, + nil, + stateNamespace, + "textor-the-doctor", + ) + if err != nil { + t.Fatalf("Failed to create syncer: %v", err) + } + + localCtx := context.Background() + remoteCtx := kontext.WithCluster(localCtx, clusterName) + ctx := NewContext(localCtx, remoteCtx) + + // setup a custom state backend that we can prime + var backend *kubernetesBackend + syncer.newObjectStateStore = func(primaryObject, stateCluster syncSide) ObjectStateStore { + // .Process() is called multiple times, but we want the state to persist between reconciles. + if backend == nil { + backend = newKubernetesBackend(stateNamespace, primaryObject, stateCluster) + if testcase.existingState != "" { + if err := backend.Put(testcase.remoteObject, clusterName, []byte(testcase.existingState)); err != nil { + t.Fatalf("Failed to prime state store: %v", err) + } + } + } + + return &objectStateStore{ + backend: backend, + } + } + + var requeue bool + + if testcase.performRequeues { + target := testcase.remoteObject.DeepCopy() + + for i := 0; true; i++ { + if i > 20 { + t.Fatalf("Detected potential infinite loop, stopping after %d requeues.", i) + } + + requeue, err = syncer.Process(ctx, target) + if err != nil { + break + } + + if !requeue { + break + } + + if err = remoteClient.Get(remoteCtx, ctrlruntimeclient.ObjectKeyFromObject(target), target); err != nil { + // it's possible for the processing to have deleted the remote object, + // so a NotFound is valid here + if apierrors.IsNotFound(err) { + break + } + + t.Fatalf("Failed to get updated remote object: %v", err) + } + } + } else { + requeue, err = syncer.Process(ctx, testcase.remoteObject) + } + + finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteObject, testcase.expectedRemoteObject) + if getErr != nil { + t.Fatalf("Failed to get final remote object: %v", getErr) + } + + finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localObject, testcase.expectedLocalObject) + if getErr != nil { + t.Fatalf("Failed to get final local object: %v", getErr) + } + + if testcase.customVerification != nil { + testcase.customVerification(t, requeue, err, finalRemoteObject, finalLocalObject, testcase) + } else { + if err != nil { + t.Fatalf("Processing failed: %v", err) + } + + assertObjectsEqual(t, "local", testcase.expectedLocalObject, finalLocalObject) + assertObjectsEqual(t, "remote", testcase.expectedRemoteObject, finalRemoteObject) + + if testcase.expectedState != "" { + if backend == nil { + t.Fatal("Cannot check object state, state store was never instantiated.") + } + + finalState, err := backend.Get(testcase.expectedRemoteObject, clusterName) + if err != nil { + t.Fatalf("Failed to get final state: %v", err) + } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { + t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) + } + } + } + }) + } +} + func assertObjectsEqual(t *testing.T, kind string, expected, actual *unstructured.Unstructured) { if expected == nil { if actual != nil { From 779d9ff50bb35d6a16f0973612eee2ba0b957cd8 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Fri, 21 Mar 2025 12:39:19 +0100 Subject: [PATCH 4/7] move the tests to seperate file Signed-off-by: Karol Szwaj On-behalf-of: @SAP karol.szwaj@sap.com --- internal/sync/syncer_related_test.go | 368 +++++++++++++++++++++++++++ internal/sync/syncer_test.go | 288 --------------------- 2 files changed, 368 insertions(+), 288 deletions(-) create mode 100644 internal/sync/syncer_related_test.go diff --git a/internal/sync/syncer_related_test.go b/internal/sync/syncer_related_test.go new file mode 100644 index 0000000..cc6d8b0 --- /dev/null +++ b/internal/sync/syncer_related_test.go @@ -0,0 +1,368 @@ +/* +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 ( + "bytes" + "context" + "testing" + + "github.com/kcp-dev/logicalcluster/v3" + "go.uber.org/zap" + + dummyv1alpha1 "github.com/kcp-dev/api-syncagent/internal/sync/apis/dummy/v1alpha1" + "github.com/kcp-dev/api-syncagent/internal/test/diff" + syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" + + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/kontext" +) + +func TestSyncerProcessingRelatedResources(t *testing.T) { + const stateNamespace = "kcp-system" + + type testcase struct { + name string + remoteAPIGroup string + localCRD *apiextensionsv1.CustomResourceDefinition + pubRes *syncagentv1alpha1.PublishedResource + remoteObject *unstructured.Unstructured + localObject *unstructured.Unstructured + existingState string + performRequeues bool + expectedRemoteObject *unstructured.Unstructured + expectedLocalObject *unstructured.Unstructured + expectedState string + customVerification func(t *testing.T, requeue bool, processErr error, finalRemoteObject *unstructured.Unstructured, finalLocalObject *unstructured.Unstructured, testcase testcase) + } + + clusterName := logicalcluster.Name("testcluster") + + remoteThingPR := &syncagentv1alpha1.PublishedResource{ + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: dummyv1alpha1.GroupName, + Version: dummyv1alpha1.GroupVersion, + Kind: "NamespacedThing", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Kind: "RemoteThing", + }, + // include explicit naming rules to be independent of possible changes to the defaults + Naming: &syncagentv1alpha1.ResourceNaming{ + Name: "$remoteClusterName-$remoteName", // Things are Cluster-scoped + }, + Related: []syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "mandatory-credentials", + Origin: "kcp", + Kind: "Secret", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "metadata.name", + Regex: &syncagentv1alpha1.RegexResourceLocator{ + Replacement: "mandatory-credentials", + }, + }, + }, + }, + { + Identifier: "optional-secret", + Origin: "service", + Kind: "Secret", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "metadata.name", + Regex: &syncagentv1alpha1.RegexResourceLocator{ + Replacement: "optional-credentials", + }, + }, + }, + Optional: true, + }, + }, + }, + } + + testcases := []testcase{ + { + name: "optional related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: remoteThingPR, + performRequeues: true, + + remoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Namespace: stateNamespace, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + localObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Namespace: stateNamespace, + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + remoteObjectNamespaceAnnotation: stateNamespace, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + existingState: "", + + expectedRemoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Namespace: stateNamespace, + Finalizers: []string{ + deletionFinalizer, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + expectedLocalObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Namespace: stateNamespace, + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + remoteObjectNamespaceAnnotation: stateNamespace, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing","namespace":"kcp-system"},"spec":{"username":"Colonel Mustard"}}`, + }, + { + name: "mandatory related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: remoteThingPR, + performRequeues: true, + + remoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Namespace: stateNamespace, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + localObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Namespace: stateNamespace, + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + remoteObjectNamespaceAnnotation: stateNamespace, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + existingState: "", + + expectedRemoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-thing", + Namespace: stateNamespace, + Finalizers: []string{ + deletionFinalizer, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }, withGroupKind("remote.example.corp", "RemoteThing")), + expectedLocalObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster-my-test-thing", + Namespace: stateNamespace, + Labels: map[string]string{ + agentNameLabel: "textor-the-doctor", + remoteObjectClusterLabel: "testcluster", + remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", + }, + Annotations: map[string]string{ + remoteObjectNameAnnotation: "my-test-thing", + remoteObjectNamespaceAnnotation: stateNamespace, + }, + }, + Spec: dummyv1alpha1.ThingSpec{ + Username: "Colonel Mustard", + }, + }), + expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing","namespace":"kcp-system"},"spec":{"username":"Colonel Mustard"}}`, + }, + } + + credentials := newUnstructured(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mandatory-credentials", + Namespace: stateNamespace, + Labels: map[string]string{ + "hello": "world", + }, + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + }) + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + localClient := buildFakeClient(testcase.localObject, credentials) + remoteClient := buildFakeClient(testcase.remoteObject, credentials) + + syncer, err := NewResourceSyncer( + // zap.Must(zap.NewDevelopment()).Sugar(), + zap.NewNop().Sugar(), + localClient, + remoteClient, + testcase.pubRes, + testcase.localCRD, + testcase.remoteAPIGroup, + nil, + stateNamespace, + "textor-the-doctor", + ) + if err != nil { + t.Fatalf("Failed to create syncer: %v", err) + } + + localCtx := context.Background() + remoteCtx := kontext.WithCluster(localCtx, clusterName) + ctx := NewContext(localCtx, remoteCtx) + + // setup a custom state backend that we can prime + var backend *kubernetesBackend + syncer.newObjectStateStore = func(primaryObject, stateCluster syncSide) ObjectStateStore { + // .Process() is called multiple times, but we want the state to persist between reconciles. + if backend == nil { + backend = newKubernetesBackend(stateNamespace, primaryObject, stateCluster) + if testcase.existingState != "" { + if err := backend.Put(testcase.remoteObject, clusterName, []byte(testcase.existingState)); err != nil { + t.Fatalf("Failed to prime state store: %v", err) + } + } + } + + return &objectStateStore{ + backend: backend, + } + } + + var requeue bool + + if testcase.performRequeues { + target := testcase.remoteObject.DeepCopy() + + for i := 0; true; i++ { + if i > 20 { + t.Fatalf("Detected potential infinite loop, stopping after %d requeues.", i) + } + + requeue, err = syncer.Process(ctx, target) + if err != nil { + break + } + + if !requeue { + break + } + + if err = remoteClient.Get(remoteCtx, ctrlruntimeclient.ObjectKeyFromObject(target), target); err != nil { + // it's possible for the processing to have deleted the remote object, + // so a NotFound is valid here + if apierrors.IsNotFound(err) { + break + } + + t.Fatalf("Failed to get updated remote object: %v", err) + } + } + } else { + requeue, err = syncer.Process(ctx, testcase.remoteObject) + } + + finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteObject, testcase.expectedRemoteObject) + if getErr != nil { + t.Fatalf("Failed to get final remote object: %v", getErr) + } + + finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localObject, testcase.expectedLocalObject) + if getErr != nil { + t.Fatalf("Failed to get final local object: %v", getErr) + } + + if testcase.customVerification != nil { + testcase.customVerification(t, requeue, err, finalRemoteObject, finalLocalObject, testcase) + } else { + if err != nil { + t.Fatalf("Processing failed: %v", err) + } + + assertObjectsEqual(t, "local", testcase.expectedLocalObject, finalLocalObject) + assertObjectsEqual(t, "remote", testcase.expectedRemoteObject, finalRemoteObject) + + if testcase.expectedState != "" { + if backend == nil { + t.Fatal("Cannot check object state, state store was never instantiated.") + } + + finalState, err := backend.Get(testcase.expectedRemoteObject, clusterName) + if err != nil { + t.Fatalf("Failed to get final state: %v", err) + } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { + t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) + } + } + } + }) + } +} diff --git a/internal/sync/syncer_test.go b/internal/sync/syncer_test.go index f47643a..93baf54 100644 --- a/internal/sync/syncer_test.go +++ b/internal/sync/syncer_test.go @@ -30,7 +30,6 @@ import ( "github.com/kcp-dev/api-syncagent/internal/test/diff" syncagentv1alpha1 "github.com/kcp-dev/api-syncagent/sdk/apis/syncagent/v1alpha1" - corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1306,293 +1305,6 @@ func TestSyncerProcessingSingleResourceWithStatus(t *testing.T) { } } -func TestSyncerProcessingRelatedResources(t *testing.T) { - type testcase struct { - name string - remoteAPIGroup string - localCRD *apiextensionsv1.CustomResourceDefinition - pubRes *syncagentv1alpha1.PublishedResource - remoteObject *unstructured.Unstructured - localObject *unstructured.Unstructured - existingState string - performRequeues bool - expectedRemoteObject *unstructured.Unstructured - expectedLocalObject *unstructured.Unstructured - expectedState string - customVerification func(t *testing.T, requeue bool, processErr error, finalRemoteObject *unstructured.Unstructured, finalLocalObject *unstructured.Unstructured, testcase testcase) - } - - clusterName := logicalcluster.Name("testcluster") - - remoteThingPR := &syncagentv1alpha1.PublishedResource{ - Spec: syncagentv1alpha1.PublishedResourceSpec{ - Resource: syncagentv1alpha1.SourceResourceDescriptor{ - APIGroup: dummyv1alpha1.GroupName, - Version: dummyv1alpha1.GroupVersion, - Kind: "Thing", - }, - Projection: &syncagentv1alpha1.ResourceProjection{ - Kind: "RemoteThing", - }, - // include explicit naming rules to be independent of possible changes to the defaults - Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteClusterName-$remoteName", // Things are Cluster-scoped - }, - Related: []syncagentv1alpha1.RelatedResourceSpec{ - { - Identifier: "mandatory-credentials", - Origin: "service", - Kind: "Secret", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "metadata.name", // irrelevant - Regex: &syncagentv1alpha1.RegexResourceLocator{ - Replacement: "mandatory-credentials", - }, - }, - }, - }, - { - Identifier: "optional-secret", - Origin: "service", - Kind: "Secret", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "metadata.name", // irrelevant - Regex: &syncagentv1alpha1.RegexResourceLocator{ - Replacement: "optional-credentials", - }, - }, - }, - Optional: false, - }, - }, - }, - } - - testcases := []testcase{ - { - name: "optional related resource does not exist", - remoteAPIGroup: "remote.example.corp", - localCRD: loadCRD("things"), - pubRes: remoteThingPR, - performRequeues: true, - - remoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - localObject: nil, - existingState: "", - - expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - Finalizers: []string{ - deletionFinalizer, - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testcluster-my-test-thing", - Labels: map[string]string{ - agentNameLabel: "textor-the-doctor", - remoteObjectClusterLabel: "testcluster", - remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", - }, - Annotations: map[string]string{ - remoteObjectNameAnnotation: "my-test-thing", - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, - }, - { - name: "mandatory related resource does not exist", - remoteAPIGroup: "remote.example.corp", - localCRD: loadCRD("things"), - pubRes: remoteThingPR, - performRequeues: true, - - remoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - localObject: nil, - existingState: "", - - expectedRemoteObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - Finalizers: []string{ - deletionFinalizer, - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - expectedLocalObject: newUnstructured(&dummyv1alpha1.Thing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testcluster-my-test-thing", - Labels: map[string]string{ - agentNameLabel: "textor-the-doctor", - remoteObjectClusterLabel: "testcluster", - remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", - }, - Annotations: map[string]string{ - remoteObjectNameAnnotation: "my-test-thing", - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing"},"spec":{"username":"Colonel Mustard"}}`, - }, - } - - const stateNamespace = "kcp-system" - credentials := newUnstructured(&corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mandatory-credentials", - Namespace: stateNamespace, - Labels: map[string]string{ - "hello": "world", - }, - }, - Data: map[string][]byte{ - "password": []byte("hunter2"), - }, - }) - for _, testcase := range testcases { - t.Run(testcase.name, func(t *testing.T) { - localClient := buildFakeClient(testcase.localObject, credentials) - remoteClient := buildFakeClient(testcase.remoteObject) - - syncer, err := NewResourceSyncer( - // zap.Must(zap.NewDevelopment()).Sugar(), - zap.NewNop().Sugar(), - localClient, - remoteClient, - testcase.pubRes, - testcase.localCRD, - testcase.remoteAPIGroup, - nil, - stateNamespace, - "textor-the-doctor", - ) - if err != nil { - t.Fatalf("Failed to create syncer: %v", err) - } - - localCtx := context.Background() - remoteCtx := kontext.WithCluster(localCtx, clusterName) - ctx := NewContext(localCtx, remoteCtx) - - // setup a custom state backend that we can prime - var backend *kubernetesBackend - syncer.newObjectStateStore = func(primaryObject, stateCluster syncSide) ObjectStateStore { - // .Process() is called multiple times, but we want the state to persist between reconciles. - if backend == nil { - backend = newKubernetesBackend(stateNamespace, primaryObject, stateCluster) - if testcase.existingState != "" { - if err := backend.Put(testcase.remoteObject, clusterName, []byte(testcase.existingState)); err != nil { - t.Fatalf("Failed to prime state store: %v", err) - } - } - } - - return &objectStateStore{ - backend: backend, - } - } - - var requeue bool - - if testcase.performRequeues { - target := testcase.remoteObject.DeepCopy() - - for i := 0; true; i++ { - if i > 20 { - t.Fatalf("Detected potential infinite loop, stopping after %d requeues.", i) - } - - requeue, err = syncer.Process(ctx, target) - if err != nil { - break - } - - if !requeue { - break - } - - if err = remoteClient.Get(remoteCtx, ctrlruntimeclient.ObjectKeyFromObject(target), target); err != nil { - // it's possible for the processing to have deleted the remote object, - // so a NotFound is valid here - if apierrors.IsNotFound(err) { - break - } - - t.Fatalf("Failed to get updated remote object: %v", err) - } - } - } else { - requeue, err = syncer.Process(ctx, testcase.remoteObject) - } - - finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteObject, testcase.expectedRemoteObject) - if getErr != nil { - t.Fatalf("Failed to get final remote object: %v", getErr) - } - - finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localObject, testcase.expectedLocalObject) - if getErr != nil { - t.Fatalf("Failed to get final local object: %v", getErr) - } - - if testcase.customVerification != nil { - testcase.customVerification(t, requeue, err, finalRemoteObject, finalLocalObject, testcase) - } else { - if err != nil { - t.Fatalf("Processing failed: %v", err) - } - - assertObjectsEqual(t, "local", testcase.expectedLocalObject, finalLocalObject) - assertObjectsEqual(t, "remote", testcase.expectedRemoteObject, finalRemoteObject) - - if testcase.expectedState != "" { - if backend == nil { - t.Fatal("Cannot check object state, state store was never instantiated.") - } - - finalState, err := backend.Get(testcase.expectedRemoteObject, clusterName) - if err != nil { - t.Fatalf("Failed to get final state: %v", err) - } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { - t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) - } - } - } - }) - } -} - func assertObjectsEqual(t *testing.T, kind string, expected, actual *unstructured.Unstructured) { if expected == nil { if actual != nil { From f68f7764fbb29f6e6e67d304dc4242e2eeb77e36 Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Wed, 26 Mar 2025 13:25:20 +0100 Subject: [PATCH 5/7] add the unit tests for related resources Signed-off-by: Karol Szwaj On-behalf-of: @SAP karol.szwaj@sap.com --- .../syncagent.kcp.io_publishedresources.yaml | 3 + internal/sync/syncer_related.go | 14 ++- internal/sync/syncer_related_test.go | 89 +++++++++---------- .../syncagent/v1alpha1/published_resource.go | 3 - .../syncagent/v1alpha1/relatedresourcespec.go | 16 ++-- 5 files changed, 61 insertions(+), 64 deletions(-) diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index de6a5f1..e7fd211 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -410,6 +410,7 @@ spec: type: object type: array type: object +<<<<<<< HEAD <<<<<<< HEAD object: description: |- @@ -658,6 +659,8 @@ spec: description: Optional indicates whether the related resource must be referenced. type: boolean >>>>>>> 2f5479b (update codegen) +======= +>>>>>>> 05c1537 (add the unit tests for related resources) origin: description: '"service" or "kcp"' type: string diff --git a/internal/sync/syncer_related.go b/internal/sync/syncer_related.go index 651e370..2dc86ea 100644 --- a/internal/sync/syncer_related.go +++ b/internal/sync/syncer_related.go @@ -40,15 +40,13 @@ import ( func (s *ResourceSyncer) processRelatedResources(log *zap.SugaredLogger, stateStore ObjectStateStore, remote, local syncSide) (requeue bool, err error) { for _, relatedResource := range s.pubRes.Spec.Related { - if !relatedResource.Optional { - requeue, err := s.processRelatedResource(log.With("identifier", relatedResource.Identifier), stateStore, remote, local, relatedResource) - if err != nil { - return false, fmt.Errorf("failed to process related resource %s: %w", relatedResource.Identifier, err) - } + requeue, err := s.processRelatedResource(log.With("identifier", relatedResource.Identifier), stateStore, remote, local, relatedResource) + if err != nil { + return false, fmt.Errorf("failed to process related resource %s: %w", relatedResource.Identifier, err) + } - if requeue { - return true, nil - } + if requeue { + return true, nil } } diff --git a/internal/sync/syncer_related_test.go b/internal/sync/syncer_related_test.go index cc6d8b0..ef5bc26 100644 --- a/internal/sync/syncer_related_test.go +++ b/internal/sync/syncer_related_test.go @@ -37,6 +37,25 @@ import ( "sigs.k8s.io/controller-runtime/pkg/kontext" ) +func newPublishedResources(relatedResources []syncagentv1alpha1.RelatedResourceSpec) *syncagentv1alpha1.PublishedResource { + return &syncagentv1alpha1.PublishedResource{ + Spec: syncagentv1alpha1.PublishedResourceSpec{ + Resource: syncagentv1alpha1.SourceResourceDescriptor{ + APIGroup: dummyv1alpha1.GroupName, + Version: dummyv1alpha1.GroupVersion, + Kind: "NamespacedThing", + }, + Projection: &syncagentv1alpha1.ResourceProjection{ + Kind: "RemoteThing", + }, + Naming: &syncagentv1alpha1.ResourceNaming{ + Name: "$remoteClusterName-$remoteName", + }, + Related: relatedResources, + }, + } +} + func TestSyncerProcessingRelatedResources(t *testing.T) { const stateNamespace = "kcp-system" @@ -57,34 +76,12 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { clusterName := logicalcluster.Name("testcluster") - remoteThingPR := &syncagentv1alpha1.PublishedResource{ - Spec: syncagentv1alpha1.PublishedResourceSpec{ - Resource: syncagentv1alpha1.SourceResourceDescriptor{ - APIGroup: dummyv1alpha1.GroupName, - Version: dummyv1alpha1.GroupVersion, - Kind: "NamespacedThing", - }, - Projection: &syncagentv1alpha1.ResourceProjection{ - Kind: "RemoteThing", - }, - // include explicit naming rules to be independent of possible changes to the defaults - Naming: &syncagentv1alpha1.ResourceNaming{ - Name: "$remoteClusterName-$remoteName", // Things are Cluster-scoped - }, - Related: []syncagentv1alpha1.RelatedResourceSpec{ - { - Identifier: "mandatory-credentials", - Origin: "kcp", - Kind: "Secret", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "metadata.name", - Regex: &syncagentv1alpha1.RegexResourceLocator{ - Replacement: "mandatory-credentials", - }, - }, - }, - }, + testcases := []testcase{ + { + name: "optional related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: newPublishedResources([]syncagentv1alpha1.RelatedResourceSpec{ { Identifier: "optional-secret", Origin: "service", @@ -97,20 +94,9 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { }, }, }, - Optional: true, }, - }, - }, - } - - testcases := []testcase{ - { - name: "optional related resource does not exist", - remoteAPIGroup: "remote.example.corp", - localCRD: loadCRD("things"), - pubRes: remoteThingPR, + }), performRequeues: true, - remoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-thing", @@ -173,12 +159,25 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing","namespace":"kcp-system"},"spec":{"username":"Colonel Mustard"}}`, }, { - name: "mandatory related resource does not exist", - remoteAPIGroup: "remote.example.corp", - localCRD: loadCRD("things"), - pubRes: remoteThingPR, + name: "mandatory related resource does not exist", + remoteAPIGroup: "remote.example.corp", + localCRD: loadCRD("things"), + pubRes: newPublishedResources([]syncagentv1alpha1.RelatedResourceSpec{ + { + Identifier: "mandatory-credentials", + Origin: "kcp", + Kind: "Secret", + Reference: syncagentv1alpha1.RelatedResourceReference{ + Name: syncagentv1alpha1.ResourceLocator{ + Path: "metadata.name", + Regex: &syncagentv1alpha1.RegexResourceLocator{ + Replacement: "mandatory-credentials", + }, + }, + }, + }, + }), performRequeues: true, - remoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-thing", diff --git a/sdk/apis/syncagent/v1alpha1/published_resource.go b/sdk/apis/syncagent/v1alpha1/published_resource.go index 8cce3f4..613924c 100644 --- a/sdk/apis/syncagent/v1alpha1/published_resource.go +++ b/sdk/apis/syncagent/v1alpha1/published_resource.go @@ -178,9 +178,6 @@ type RelatedResourceSpec struct { // Mutation configures optional transformation rules for the related resource. // Status mutations are only performed when the related resource originates in kcp. Mutation *ResourceMutationSpec `json:"mutation,omitempty"` - - // Optional indicates whether the related resource must be referenced. - Optional bool `json:"optional,omitempty"` } // RelatedResourceSource configures how the related resource can be found on the origin side diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go index 0e560e4..6fdbc18 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go @@ -21,11 +21,19 @@ package v1alpha1 // RelatedResourceSpecApplyConfiguration represents a declarative configuration of the RelatedResourceSpec type for use // with apply. type RelatedResourceSpecApplyConfiguration struct { +<<<<<<< HEAD Identifier *string `json:"identifier,omitempty"` Origin *string `json:"origin,omitempty"` Kind *string `json:"kind,omitempty"` Object *RelatedResourceObjectApplyConfiguration `json:"object,omitempty"` Mutation *ResourceMutationSpecApplyConfiguration `json:"mutation,omitempty"` +======= + Identifier *string `json:"identifier,omitempty"` + Origin *string `json:"origin,omitempty"` + Kind *string `json:"kind,omitempty"` + Reference *RelatedResourceReferenceApplyConfiguration `json:"reference,omitempty"` + Mutation *ResourceMutationSpecApplyConfiguration `json:"mutation,omitempty"` +>>>>>>> 05c1537 (add the unit tests for related resources) } // RelatedResourceSpecApplyConfiguration constructs a declarative configuration of the RelatedResourceSpec type for use with @@ -73,11 +81,3 @@ func (b *RelatedResourceSpecApplyConfiguration) WithMutation(value *ResourceMuta b.Mutation = value return b } - -// WithOptional sets the Optional 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 Optional field is set to the value of the last call. -func (b *RelatedResourceSpecApplyConfiguration) WithOptional(value bool) *RelatedResourceSpecApplyConfiguration { - b.Optional = &value - return b -} From 4b21f3585dde504dc6201d79015571bd9af91bdf Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Tue, 8 Apr 2025 16:17:04 +0200 Subject: [PATCH 6/7] update tests Signed-off-by: Karol Szwaj On-behalf-of: @SAP karol.szwaj@sap.com --- internal/sync/syncer_related_test.go | 184 ++++++++++++--------------- 1 file changed, 81 insertions(+), 103 deletions(-) diff --git a/internal/sync/syncer_related_test.go b/internal/sync/syncer_related_test.go index ef5bc26..ea005e9 100644 --- a/internal/sync/syncer_related_test.go +++ b/internal/sync/syncer_related_test.go @@ -55,36 +55,36 @@ func newPublishedResources(relatedResources []syncagentv1alpha1.RelatedResourceS }, } } - func TestSyncerProcessingRelatedResources(t *testing.T) { const stateNamespace = "kcp-system" type testcase struct { - name string - remoteAPIGroup string - localCRD *apiextensionsv1.CustomResourceDefinition - pubRes *syncagentv1alpha1.PublishedResource - remoteObject *unstructured.Unstructured - localObject *unstructured.Unstructured - existingState string - performRequeues bool - expectedRemoteObject *unstructured.Unstructured - expectedLocalObject *unstructured.Unstructured - expectedState string - customVerification func(t *testing.T, requeue bool, processErr error, finalRemoteObject *unstructured.Unstructured, finalLocalObject *unstructured.Unstructured, testcase testcase) + name string + remoteAPIGroup string + localCRD *apiextensionsv1.CustomResourceDefinition + pubRes *syncagentv1alpha1.PublishedResource + remoteRelatedSecret *unstructured.Unstructured + localRelatedSecret *unstructured.Unstructured + remoteObject *unstructured.Unstructured + localObject *unstructured.Unstructured + existingState string + performRequeues bool + expectedRemoteRelatedSecret *unstructured.Unstructured + expectedLocalRelatedSecret *unstructured.Unstructured + expectedState string } clusterName := logicalcluster.Name("testcluster") testcases := []testcase{ { - name: "optional related resource does not exist", + name: "optional related resource of kcp origin does not exist in the source", remoteAPIGroup: "remote.example.corp", localCRD: loadCRD("things"), pubRes: newPublishedResources([]syncagentv1alpha1.RelatedResourceSpec{ { Identifier: "optional-secret", - Origin: "service", + Origin: "kcp", Kind: "Secret", Reference: syncagentv1alpha1.RelatedResourceReference{ Name: syncagentv1alpha1.ResourceLocator{ @@ -96,7 +96,9 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { }, }, }), - performRequeues: true, + performRequeues: true, + remoteRelatedSecret: nil, + localRelatedSecret: nil, remoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-thing", @@ -124,42 +126,13 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { Username: "Colonel Mustard", }, }), - existingState: "", - - expectedRemoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", - Namespace: stateNamespace, - Finalizers: []string{ - deletionFinalizer, - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - expectedLocalObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testcluster-my-test-thing", - Namespace: stateNamespace, - Labels: map[string]string{ - agentNameLabel: "textor-the-doctor", - remoteObjectClusterLabel: "testcluster", - remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", - }, - Annotations: map[string]string{ - remoteObjectNameAnnotation: "my-test-thing", - remoteObjectNamespaceAnnotation: stateNamespace, - }, - }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", - }, - }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing","namespace":"kcp-system"},"spec":{"username":"Colonel Mustard"}}`, + existingState: "", + expectedRemoteRelatedSecret: nil, + expectedLocalRelatedSecret: nil, + expectedState: "", }, { - name: "mandatory related resource does not exist", + name: "mandatory related resource of kcp origin exists in the source side", remoteAPIGroup: "remote.example.corp", localCRD: loadCRD("things"), pubRes: newPublishedResources([]syncagentv1alpha1.RelatedResourceSpec{ @@ -178,6 +151,30 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { }, }), performRequeues: true, + remoteRelatedSecret: newUnstructured(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mandatory-credentials", + Namespace: stateNamespace, + Labels: map[string]string{ + "hello": "world", + }, + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + }), + localRelatedSecret: newUnstructured(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mandatory-credentials", + Namespace: stateNamespace, + Labels: map[string]string{ + "hello": "world", + }, + }, + Data: map[string][]byte{ + "password": []byte("hunter2"), + }, + }), remoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-thing", @@ -206,57 +203,41 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { }, }), existingState: "", - - expectedRemoteObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + expectedRemoteRelatedSecret: newUnstructured(&corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "my-test-thing", + Name: "mandatory-credentials", Namespace: stateNamespace, + Labels: map[string]string{ + "hello": "world", + }, Finalizers: []string{ deletionFinalizer, }, }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", + Data: map[string][]byte{ + "password": []byte("hunter2"), }, - }, withGroupKind("remote.example.corp", "RemoteThing")), - expectedLocalObject: newUnstructured(&dummyv1alpha1.NamespacedThing{ + }), + expectedLocalRelatedSecret: newUnstructured(&corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "testcluster-my-test-thing", + Name: "mandatory-credentials", Namespace: stateNamespace, Labels: map[string]string{ - agentNameLabel: "textor-the-doctor", - remoteObjectClusterLabel: "testcluster", - remoteObjectNameHashLabel: "c346c8ceb5d104cc783d09b95e8ea7032c190948", - }, - Annotations: map[string]string{ - remoteObjectNameAnnotation: "my-test-thing", - remoteObjectNamespaceAnnotation: stateNamespace, + "hello": "world", }, }, - Spec: dummyv1alpha1.ThingSpec{ - Username: "Colonel Mustard", + Data: map[string][]byte{ + "password": []byte("hunter2"), }, }), - expectedState: `{"apiVersion":"remote.example.corp/v1alpha1","kind":"RemoteThing","metadata":{"name":"my-test-thing","namespace":"kcp-system"},"spec":{"username":"Colonel Mustard"}}`, + expectedState: `{"apiVersion":"v1","data":{"password":"aHVudGVyMg=="},"kind":"Secret","metadata":{"labels":{"hello":"world"},"name":"mandatory-credentials","namespace":"kcp-system"}}`, }, } - credentials := newUnstructured(&corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "mandatory-credentials", - Namespace: stateNamespace, - Labels: map[string]string{ - "hello": "world", - }, - }, - Data: map[string][]byte{ - "password": []byte("hunter2"), - }, - }) for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - localClient := buildFakeClient(testcase.localObject, credentials) - remoteClient := buildFakeClient(testcase.remoteObject, credentials) + localClient := buildFakeClient(testcase.localObject, testcase.localRelatedSecret) + remoteClient := buildFakeClient(testcase.remoteObject, testcase.remoteRelatedSecret) syncer, err := NewResourceSyncer( // zap.Must(zap.NewDevelopment()).Sugar(), @@ -326,42 +307,39 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { } } } else { - requeue, err = syncer.Process(ctx, testcase.remoteObject) + _, err = syncer.Process(ctx, testcase.remoteObject) } - finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteObject, testcase.expectedRemoteObject) + finalRemoteObject, getErr := getFinalObjectVersion(remoteCtx, remoteClient, testcase.remoteRelatedSecret, testcase.expectedRemoteRelatedSecret) if getErr != nil { t.Fatalf("Failed to get final remote object: %v", getErr) } - finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localObject, testcase.expectedLocalObject) + finalLocalObject, getErr := getFinalObjectVersion(localCtx, localClient, testcase.localRelatedSecret, testcase.expectedLocalRelatedSecret) if getErr != nil { t.Fatalf("Failed to get final local object: %v", getErr) } - if testcase.customVerification != nil { - testcase.customVerification(t, requeue, err, finalRemoteObject, finalLocalObject, testcase) - } else { - if err != nil { - t.Fatalf("Processing failed: %v", err) - } + if err != nil { + t.Fatalf("Processing failed: %v", err) + } - assertObjectsEqual(t, "local", testcase.expectedLocalObject, finalLocalObject) - assertObjectsEqual(t, "remote", testcase.expectedRemoteObject, finalRemoteObject) + assertObjectsEqual(t, "local", testcase.expectedLocalRelatedSecret, finalLocalObject) + assertObjectsEqual(t, "remote", testcase.expectedRemoteRelatedSecret, finalRemoteObject) - if testcase.expectedState != "" { - if backend == nil { - t.Fatal("Cannot check object state, state store was never instantiated.") - } + if testcase.expectedState != "" { + if backend == nil { + t.Fatal("Cannot check object state, state store was never instantiated.") + } - finalState, err := backend.Get(testcase.expectedRemoteObject, clusterName) - if err != nil { - t.Fatalf("Failed to get final state: %v", err) - } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { - t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) - } + finalState, err := backend.Get(testcase.expectedRemoteRelatedSecret, clusterName) + if err != nil { + t.Fatalf("Failed to get final state: %v", err) + } else if !bytes.Equal(finalState, []byte(testcase.expectedState)) { + t.Fatalf("States do not match:\n%s", diff.StringDiff(testcase.expectedState, string(finalState))) } } + }) } } From 39d2db1c41480f037fb789b06072167bacf8cd9a Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Fri, 11 Apr 2025 17:38:30 +0200 Subject: [PATCH 7/7] update crds Signed-off-by: Karol Szwaj --- .../syncagent.kcp.io_publishedresources.yaml | 9 ------ internal/sync/syncer_related_test.go | 31 ++++++++++--------- .../syncagent/v1alpha1/relatedresourcespec.go | 8 ----- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml index e7fd211..41b3ec6 100644 --- a/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml +++ b/deploy/crd/kcp.io/syncagent.kcp.io_publishedresources.yaml @@ -410,8 +410,6 @@ spec: type: object type: array type: object -<<<<<<< HEAD -<<<<<<< HEAD object: description: |- Object describes how the related resource can be found on the origin side @@ -654,13 +652,6 @@ spec: type: string type: object type: object -======= - optional: - description: Optional indicates whether the related resource must be referenced. - type: boolean ->>>>>>> 2f5479b (update codegen) -======= ->>>>>>> 05c1537 (add the unit tests for related resources) origin: description: '"service" or "kcp"' type: string diff --git a/internal/sync/syncer_related_test.go b/internal/sync/syncer_related_test.go index ea005e9..7e4cbcf 100644 --- a/internal/sync/syncer_related_test.go +++ b/internal/sync/syncer_related_test.go @@ -83,14 +83,16 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { localCRD: loadCRD("things"), pubRes: newPublishedResources([]syncagentv1alpha1.RelatedResourceSpec{ { - Identifier: "optional-secret", + Identifier: "optional-credentials", Origin: "kcp", Kind: "Secret", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "metadata.name", - Regex: &syncagentv1alpha1.RegexResourceLocator{ - Replacement: "optional-credentials", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "metadata.name", + Regex: &syncagentv1alpha1.RegularExpression{ + Replacement: "optional-credentials", + }, }, }, }, @@ -140,11 +142,13 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { Identifier: "mandatory-credentials", Origin: "kcp", Kind: "Secret", - Reference: syncagentv1alpha1.RelatedResourceReference{ - Name: syncagentv1alpha1.ResourceLocator{ - Path: "metadata.name", - Regex: &syncagentv1alpha1.RegexResourceLocator{ - Replacement: "mandatory-credentials", + Object: syncagentv1alpha1.RelatedResourceObject{ + RelatedResourceObjectSpec: syncagentv1alpha1.RelatedResourceObjectSpec{ + Reference: &syncagentv1alpha1.RelatedResourceObjectReference{ + Path: "metadata.name", + Regex: &syncagentv1alpha1.RegularExpression{ + Replacement: "mandatory-credentials", + }, }, }, }, @@ -240,13 +244,12 @@ func TestSyncerProcessingRelatedResources(t *testing.T) { remoteClient := buildFakeClient(testcase.remoteObject, testcase.remoteRelatedSecret) syncer, err := NewResourceSyncer( - // zap.Must(zap.NewDevelopment()).Sugar(), - zap.NewNop().Sugar(), + zap.Must(zap.NewDevelopment()).Sugar(), + //zap.NewNop().Sugar(), localClient, remoteClient, testcase.pubRes, testcase.localCRD, - testcase.remoteAPIGroup, nil, stateNamespace, "textor-the-doctor", diff --git a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go index 6fdbc18..08173b2 100644 --- a/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go +++ b/sdk/applyconfiguration/syncagent/v1alpha1/relatedresourcespec.go @@ -21,19 +21,11 @@ package v1alpha1 // RelatedResourceSpecApplyConfiguration represents a declarative configuration of the RelatedResourceSpec type for use // with apply. type RelatedResourceSpecApplyConfiguration struct { -<<<<<<< HEAD Identifier *string `json:"identifier,omitempty"` Origin *string `json:"origin,omitempty"` Kind *string `json:"kind,omitempty"` Object *RelatedResourceObjectApplyConfiguration `json:"object,omitempty"` Mutation *ResourceMutationSpecApplyConfiguration `json:"mutation,omitempty"` -======= - Identifier *string `json:"identifier,omitempty"` - Origin *string `json:"origin,omitempty"` - Kind *string `json:"kind,omitempty"` - Reference *RelatedResourceReferenceApplyConfiguration `json:"reference,omitempty"` - Mutation *ResourceMutationSpecApplyConfiguration `json:"mutation,omitempty"` ->>>>>>> 05c1537 (add the unit tests for related resources) } // RelatedResourceSpecApplyConfiguration constructs a declarative configuration of the RelatedResourceSpec type for use with