syncer: readd compatibility for v0.6.0 ns locators

Author: sttts

pkg/syncer/shared/namespace.go

@@ -41,15 +41,16 @@ type NamespaceLocator struct {
 }
 
 type SyncTargetLocator struct {
-	Workspace logicalcluster.Name `json:"workspace"`
-	Name      string              `json:"name"`
-	UID       types.UID           `json:"uid"`
+	Workspace      string    `json:"workspace,omitempty"`
+	DeprecatedPath string    `json:"path,omitempty"`
+	Name           string    `json:"name"`
+	UID            types.UID `json:"uid"`
 }
 
 func NewNamespaceLocator(workspace, syncTargetWorkspace logicalcluster.Name, syncTargetUID types.UID, workloadLogicalClusterName, upstreamNamespace string) NamespaceLocator {
 	return NamespaceLocator{
 		SyncTarget: SyncTargetLocator{
-			Workspace: syncTargetWorkspace,
+			Workspace: syncTargetWorkspace.String(),
 			Name:      workloadLogicalClusterName,
 			UID:       syncTargetUID,
 		},
@@ -58,6 +59,18 @@ func NewNamespaceLocator(workspace, syncTargetWorkspace logicalcluster.Name, syn
 	}
 }
 
+func NewNamespaceLocatorV060(workspace, syncTargetWorkspace logicalcluster.Name, syncTargetUID types.UID, workloadLogicalClusterName, upstreamNamespace string) NamespaceLocator {
+	return NamespaceLocator{
+		SyncTarget: SyncTargetLocator{
+			DeprecatedPath: syncTargetWorkspace.String(),
+			Name:           workloadLogicalClusterName,
+			UID:            syncTargetUID,
+		},
+		Workspace: workspace,
+		Namespace: upstreamNamespace,
+	}
+}
+
 func LocatorFromAnnotations(annotations map[string]string) (*NamespaceLocator, bool, error) {
 	annotation, ok := annotations[NamespaceLocatorAnnotation]
 	if !ok {
@@ -67,6 +80,13 @@ func LocatorFromAnnotations(annotations map[string]string) (*NamespaceLocator, b
 	if err := json.Unmarshal([]byte(annotation), &locator); err != nil {
 		return nil, false, err
 	}
+
+	// get us from v0.6.0 locators (using syncTarget.path) to v0.6.1+ (using syncTarget.workspace)
+	if locator.SyncTarget.Workspace == "" {
+		locator.SyncTarget.Workspace = locator.SyncTarget.DeprecatedPath
+	}
+	locator.SyncTarget.DeprecatedPath = ""
+
 	return &locator, true, nil
 }
 

pkg/syncer/shared/namespace_test.go

@@ -0,0 +1,101 @@
+/*
+Copyright 2022 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 shared
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/kcp-dev/logicalcluster"
+)
+
+func TestLocatorFromAnnotations(t *testing.T) {
+	tests := []struct {
+		name        string
+		annotations map[string]string
+		want        *NamespaceLocator
+		wantFound   bool
+		wantErrs    []string
+	}{
+		{
+			name:      "no annotation",
+			wantFound: false,
+		},
+		{
+			name: "garbage",
+			annotations: map[string]string{
+				NamespaceLocatorAnnotation: "garbage",
+			},
+			wantErrs: []string{"invalid character"},
+		},
+		{
+			name: "happy case",
+			annotations: map[string]string{
+				NamespaceLocatorAnnotation: `{"syncTarget":{"workspace":"test-workspace","name":"test-name","uid":"test-uid"},"workspace":"test-workspace","namespace":"test-namespace"}`,
+			},
+			want: &NamespaceLocator{
+				SyncTarget: SyncTargetLocator{
+					Workspace: "test-workspace",
+					Name:      "test-name",
+					UID:       "test-uid",
+				},
+				Workspace: logicalcluster.New("test-workspace"),
+				Namespace: "test-namespace",
+			},
+			wantFound: true,
+		},
+		{
+			name: "format up to v0.6.0",
+			annotations: map[string]string{
+				NamespaceLocatorAnnotation: `{"syncTarget":{"path":"test-workspace","name":"test-name","uid":"test-uid"},"workspace":"test-workspace","namespace":"test-namespace"}`,
+			},
+			want: &NamespaceLocator{
+				SyncTarget: SyncTargetLocator{
+					Workspace: "test-workspace",
+					Name:      "test-name",
+					UID:       "test-uid",
+				},
+				Workspace: logicalcluster.New("test-workspace"),
+				Namespace: "test-namespace",
+			},
+			wantFound: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, gotFound, err := LocatorFromAnnotations(tt.annotations)
+			if (err != nil) != (len(tt.wantErrs) > 0) {
+				t.Errorf("LocatorFromAnnotations() error = %q, wantErrs %v", err.Error(), tt.wantErrs)
+				return
+			} else if err != nil {
+				for _, wantErr := range tt.wantErrs {
+					if !strings.Contains(err.Error(), wantErr) {
+						t.Errorf("LocatorFromAnnotations() error = %q, wantErrs %q", err.Error(), wantErr)
+						return
+					}
+				}
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("LocatorFromAnnotations() got = %v, want %v", got, tt.want)
+			}
+			if gotFound != tt.wantFound {
+				t.Errorf("LocatorFromAnnotations() gotFound = %v, want %v", gotFound, tt.wantFound)
+			}
+		})
+	}
+}

pkg/syncer/spec/spec_process.go

@@ -114,17 +114,30 @@ func (c *Controller) process(ctx context.Context, gvr schema.GroupVersionResourc
 	}
 	clusterName, name := clusters.SplitClusterAwareKey(clusterAwareName)
 
+	namespaceGvr := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}
+
 	desiredNSLocator := shared.NewNamespaceLocator(clusterName, c.syncTargetClusterName, c.syncTargetUID, c.syncTargetName, upstreamNamespace)
 	jsonNSLocator, err := json.Marshal(desiredNSLocator)
 	if err != nil {
 		return err
 	}
-
-	namespaceGvr := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}
 	downstreamNamespaces, err := c.downstreamInformers.ForResource(namespaceGvr).Informer().GetIndexer().ByIndex(byNamespaceLocatorIndexName, string(jsonNSLocator))
 	if err != nil {
 		return err
 	}
+	if len(downstreamNamespaces) == 0 {
+		// case for up to v0.6.0 where we used syncTarget.path in namespace locators
+		desiredNSLocator := shared.NewNamespaceLocatorV060(clusterName, c.syncTargetClusterName, c.syncTargetUID, c.syncTargetName, upstreamNamespace)
+		jsonNSLocator, err := json.Marshal(desiredNSLocator)
+		if err != nil {
+			return err
+		}
+		downstreamNamespaces, err = c.downstreamInformers.ForResource(namespaceGvr).Informer().GetIndexer().ByIndex(byNamespaceLocatorIndexName, string(jsonNSLocator))
+		if err != nil {
+			return err
+		}
+	}
+
 	var downstreamNamespace string
 
 	if len(downstreamNamespaces) == 1 {

pkg/syncer/spec/spec_process_test.go

@@ -926,6 +926,51 @@ func TestSyncerProcess(t *testing.T) {
 			expectActionsOnFrom:                 []clienttesting.Action{},
 			expectActionsOnTo:                   []clienttesting.Action{},
 		},
+		"old v0.6.0 namespace locator exists downstream": {
+			upstreamLogicalCluster: "root:org:ws",
+			fromNamespace: namespace("test", "root:org:ws", map[string]string{
+				"state.workload.kcp.dev/us-west1": "Sync",
+			}, nil),
+			gvr: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"},
+			toResources: []runtime.Object{
+				namespace("kcp-0123456789", "", map[string]string{
+					"state.workload.kcp.dev/us-west1": "Sync",
+				},
+					map[string]string{
+						"kcp.dev/namespace-locator": `{"syncTarget":{"path":"root:org:ws","name":"us-west1","uid":"syncTargetUID"},"workspace":"root:org:ws","namespace":"test"}`,
+					}),
+				secret("foo", "test", "root:org:ws",
+					map[string]string{"state.workload.kcp.dev/us-west1": "Sync"},
+					nil,
+					map[string][]byte{
+						"a": []byte("b"),
+					}),
+			},
+			fromResources: []runtime.Object{
+				secretWithFinalizers("foo", "test", "root:org:ws",
+					map[string]string{
+						"state.workload.kcp.dev/us-west1": "Sync",
+						"something":                       "else"},
+					nil,
+					[]string{"workload.kcp.dev/syncer-us-west1"},
+					map[string][]byte{
+						"a": []byte("b"),
+					}),
+			},
+			resourceToProcessLogicalClusterName: "root:org:ws",
+			resourceToProcessName:               "foo",
+			syncTargetName:                      "us-west1",
+
+			expectActionsOnFrom: []clienttesting.Action{},
+			expectActionsOnTo: []clienttesting.Action{
+				patchSecretAction(
+					"foo",
+					"kcp-0123456789",
+					types.ApplyPatchType,
+					[]byte(`{"apiVersion":"v1","data":{"a":"Yg=="},"kind":"Secret","metadata":{"creationTimestamp":null,"labels":{"internal.workload.kcp.dev/cluster":"us-west1","something":"else"},"name":"foo","namespace":"kcp-0123456789"},"type":"kubernetes.io/service-account-token"}`),
+				),
+			},
+		},
 	}
 
 	for name, tc := range tests {
@@ -987,11 +1032,7 @@ func TestSyncerProcess(t *testing.T) {
 
 			key := tc.fromNamespace.Name + "/" + clusters.ToClusterAwareKey(logicalcluster.New(tc.resourceToProcessLogicalClusterName), tc.resourceToProcessName)
 			err = controller.process(context.Background(),
-				schema.GroupVersionResource{
-					Group:    "apps",
-					Version:  "v1",
-					Resource: "deployments",
-				},
+				tc.gvr,
 				key,
 			)
 			if tc.expectError {
@@ -1055,13 +1096,18 @@ func deployment(name, namespace, clusterName string, labels, annotations map[str
 }
 
 func secret(name, namespace, clusterName string, labels, annotations map[string]string, data map[string][]byte) *corev1.Secret {
+	return secretWithFinalizers(name, namespace, clusterName, labels, annotations, nil, data)
+}
+
+func secretWithFinalizers(name, namespace, clusterName string, labels, annotations map[string]string, finalizers []string, data map[string][]byte) *corev1.Secret {
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        name,
 			Namespace:   namespace,
 			ClusterName: clusterName,
 			Labels:      labels,
 			Annotations: annotations,
+			Finalizers:  finalizers,
 		},
 		Data:       data,
 		StringData: nil,
@@ -1196,3 +1242,21 @@ func deleteDeploymentAction(name, namespace string, subresources ...string) clie
 		DeleteOptions: metav1.DeleteOptions{},
 	}
 }
+
+func secretAction(verb, namespace string, subresources ...string) clienttesting.ActionImpl {
+	return clienttesting.ActionImpl{
+		Namespace:   namespace,
+		Verb:        verb,
+		Resource:    schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"},
+		Subresource: strings.Join(subresources, "/"),
+	}
+}
+
+func patchSecretAction(name, namespace string, patchType types.PatchType, patch []byte, subresources ...string) clienttesting.PatchActionImpl {
+	return clienttesting.PatchActionImpl{
+		ActionImpl: secretAction("patch", namespace, subresources...),
+		Name:       name,
+		PatchType:  patchType,
+		Patch:      patch,
+	}
+}