Merge pull request #916 from Danil-Grigorev/secret-diff-fix

🐛 Fix diff check on secret sync

internal/controllers/capiprovider_controller_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package controllers
 
 import (
+	"time"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -187,6 +189,14 @@ var _ = Describe("Reconcile CAPIProvider", func() {
 			g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(provider), provider)).ToNot(HaveOccurred())
 			g.Expect(conditions.IsTrue(provider, turtlesv1.RancherCredentialsSecretCondition))
 		}).Should(Succeed())
+
+		resourceVersion := ""
+		Eventually(func(g Gomega) {
+			g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(doSecret), doSecret)).ToNot(HaveOccurred())
+			previousVersion := resourceVersion
+			resourceVersion = doSecret.GetResourceVersion()
+			g.Expect(previousVersion).To(Equal(resourceVersion))
+		}, time.Minute, 10*time.Second).Should(Succeed())
 	})
 
 	It("Should reflect missing infrastructure digitalocean provider credential secret in the status", func() {

internal/sync/client.go

@@ -39,7 +39,7 @@ func setKind(cl client.Client, obj client.Object) error {
 }
 
 // Patch will only patch mirror object in the cluster.
-func Patch(ctx context.Context, cl client.Client, obj client.Object) error {
+func Patch(ctx context.Context, cl client.Client, obj client.Object, options ...client.PatchOption) error {
 	log := log.FromContext(ctx)
 
 	obj.SetManagedFields(nil)
@@ -50,10 +50,13 @@ func Patch(ctx context.Context, cl client.Client, obj client.Object) error {
 
 	log.Info(fmt.Sprintf("Updating %s: %s", obj.GetObjectKind().GroupVersionKind().Kind, client.ObjectKeyFromObject(obj)))
 
-	return cl.Patch(ctx, obj, client.Apply, []client.PatchOption{
+	patchOptions := []client.PatchOption{
 		client.ForceOwnership,
 		client.FieldOwner(fieldOwner),
-	}...)
+	}
+	patchOptions = append(patchOptions, options...)
+
+	return cl.Patch(ctx, obj, client.Apply, patchOptions...)
 }
 
 // PatchStatus will only patch the status subresource of the provided object.

internal/sync/core.go

@@ -22,7 +22,6 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/client-go/util/retry"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -60,15 +59,13 @@ func (s *DefaultSynchronizer) Get(ctx context.Context) error {
 }
 
 // Apply applies the destination object to the cluster.
-func (s *DefaultSynchronizer) Apply(ctx context.Context, reterr *error) {
+func (s *DefaultSynchronizer) Apply(ctx context.Context, reterr *error, options ...client.PatchOption) {
 	log := log.FromContext(ctx)
 	uid := s.Destination.GetUID()
 
 	setOwnerReference(s.Source, s.Destination)
 
-	if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		return Patch(ctx, s.client, s.Destination)
-	}); err != nil {
+	if err := Patch(ctx, s.client, s.Destination, options...); err != nil {
 		*reterr = kerrors.NewAggregate([]error{*reterr, err})
 		log.Error(*reterr, fmt.Sprintf("Unable to patch object: %s", *reterr))
 	}

internal/sync/interface.go

@@ -31,7 +31,7 @@ type Sync interface {
 	Template(source *turtlesv1.CAPIProvider) client.Object
 	Get(ctx context.Context) error
 	Sync(ctx context.Context) error
-	Apply(ctx context.Context, reterr *error)
+	Apply(ctx context.Context, reterr *error, options ...client.PatchOption)
 }
 
 // List contains a list of syncers to apply the syncing logic.

internal/sync/interface_test.go

@@ -42,7 +42,7 @@ func (m *MockSynchronizer) Sync(ctx context.Context) error {
 	return m.syncronizerr
 }
 
-func (m *MockSynchronizer) Apply(ctx context.Context, reterr *error) {
+func (m *MockSynchronizer) Apply(ctx context.Context, reterr *error, options ...client.PatchOption) {
 	*reterr = m.applyErr
 }
 

internal/sync/secret_mapper_sync.go

@@ -304,19 +304,6 @@ func (s *SecretMapperSync) Sync(ctx context.Context) error {
 		return nil
 	}
 
-	allSet := true
-
-	for k, v := range s.SecretSync.Secret.StringData {
-		if b64value, found := s.RancherSecret.Data[k]; !found || base64.StdEncoding.EncodeToString([]byte(v)) != string(b64value) {
-			allSet = false
-			break
-		}
-	}
-
-	if allSet {
-		s.SecretSync.Secret.StringData = map[string]string{}
-	}
-
 	log.Info(fmt.Sprintf("Credential keys from %s (%s) are successfully mapped to secret %s",
 		client.ObjectKeyFromObject(s.RancherSecret).String(),
 		cmp.Or(s.Source.Spec.Credentials.RancherCloudCredential, s.Source.Spec.Credentials.RancherCloudCredentialNamespaceName),
@@ -329,6 +316,12 @@ func (s *SecretMapperSync) Sync(ctx context.Context) error {
 	return nil
 }
 
+// Apply performs SSA patch of the secret mapper resources, using different FieldOwner from default
+// to avoid collisions with patches performed by variable syncer on the same secret resource.
+func (s *SecretMapperSync) Apply(ctx context.Context, reterr *error, options ...client.PatchOption) {
+	s.DefaultSynchronizer.Apply(ctx, reterr, append(options, client.FieldOwner("secret-mapper-sync"))...)
+}
+
 // Into maps the secret keys from source secret data according to credentials map.
 func Into(provider string, from map[string][]byte, to map[string]string) error {
 	providerValues := knownProviderRequirements[provider]

internal/sync/secret_sync.go

@@ -19,7 +19,6 @@ package sync
 import (
 	"cmp"
 	"context"
-	"encoding/base64"
 	"maps"
 	"strconv"
 
@@ -90,19 +89,6 @@ func (s *SecretSync) SyncObjects() {
 	setFeatures(s.DefaultSynchronizer.Source)
 
 	s.Secret.StringData = s.DefaultSynchronizer.Source.Status.Variables
-
-	allSet := true
-
-	for k, v := range s.DefaultSynchronizer.Source.Status.Variables {
-		if b64value, found := s.Secret.Data[k]; !found || base64.StdEncoding.EncodeToString([]byte(v)) != string(b64value) {
-			allSet = false
-			break
-		}
-	}
-
-	if allSet {
-		s.Secret.StringData = map[string]string{}
-	}
 }
 
 func setVariables(capiProvider *turtlesv1.CAPIProvider) {