separate the manifests and operations for klusterlet operator

Signed-off-by: zhujian <jiazhu@redhat.com>

hack/copy-crds.sh

@@ -9,7 +9,7 @@ done
 
 for f in $SPOKE_CRD_FILES
 do
-    cp $f ./manifests/klusterlet/
+    cp $f ./manifests/klusterlet/managed/
 done
 
 cp $CLUSTER_MANAGER_CRD_FILE ./deploy/cluster-manager/config/crds/

hack/verify-crds.sh

@@ -9,7 +9,7 @@ done
 
 for f in $SPOKE_CRD_FILES
 do
-    diff -N $f ./manifests/klusterlet/$(basename $f) || ( echo 'crd content is incorrect' && false )
+    diff -N $f ./manifests/klusterlet/managed/$(basename $f) || ( echo 'crd content is incorrect' && false )
 done
 
 diff -N $CLUSTER_MANAGER_CRD_FILE ./deploy/cluster-manager/config/crds/$(basename $CLUSTER_MANAGER_CRD_FILE) || ( echo 'crd content is incorrect' && false )

manifests/fs.go

@@ -5,9 +5,7 @@ import "embed"
 //go:embed cluster-manager
 var ClusterManagerManifestFiles embed.FS
 
-//go:embed klusterlet
-//go:embed klusterlet/detached
+//go:embed klusterlet/management
+//go:embed klusterlet/managed
 //go:embed klusterletkube111
 var KlusterletManifestFiles embed.FS
-
-// var Klusterlet111ManifestFiles embed.FS

manifests/klusterlet/klusterlet-registration-clusterrole.yaml → manifests/klusterlet/managed/klusterlet-registration-clusterrole.yaml

@@ -5,10 +5,13 @@ metadata:
   name: open-cluster-management:{{ .KlusterletName }}-registration:agent
 rules:
 # Allow agent to get/list/watch nodes and configmaps.
+# list nodes to calculates the capacity and allocatable resources of the managed cluster
 - apiGroups: [""]
-  resources: ["nodes", "configmaps"]
+  resources: ["nodes"]
   verbs: ["get", "list", "watch"]
+
 # Allow agent to get/list/watch/create/delete/update/patch secrets.
+# TODO(zhujian7): consider to delete when addon agents can be deployed on the management cluster.
 - apiGroups: [""]
   resources: ["secrets"]
   verbs: ["get", "list", "watch", "create", "delete", "update", "patch"]

manifests/klusterlet/klusterlet-work-clusterrole.yaml → manifests/klusterlet/managed/klusterlet-work-clusterrole.yaml

@@ -24,9 +24,9 @@ rules:
   resources: ["subjectaccessreviews"]
   verbs: ["create"]
 # Allow agent to create events
-- apiGroups: ["", "events.k8s.io"]
-  resources: ["events"]
-  verbs: ["create", "patch", "update"]
+# - apiGroups: ["", "events.k8s.io"]
+#   resources: ["events"]
+#   verbs: ["create", "patch", "update"]
 # Allow agent to managed appliedmanifestworks
 - apiGroups: ["work.open-cluster-management.io"]
   resources: ["appliedmanifestworks"]

manifests/klusterlet/klusterlet-registration-role.yaml → manifests/klusterlet/management/klusterlet-registration-role.yaml

@@ -5,6 +5,9 @@ metadata:
   name: open-cluster-management:{{ .KlusterletName }}-registration:agent
   namespace: {{ .KlusterletNamespace }}
 rules:
+# leader election needs to operate configmaps, create hub-kubeconfig external-managed-registration/work secrets
+# TODO(zhujian7): may be replaced by a clusterrole to grant secret operation for others namespaces when addon
+# agents are supported running on the management cluster
 - apiGroups: [""]
   resources: ["configmaps", "secrets"]
   verbs: ["get", "list", "watch", "create", "delete", "update", "patch"]

manifests/klusterlet/detached/klusterlet-work-role-management.yaml → manifests/klusterlet/management/klusterlet-work-role.yaml

@@ -5,11 +5,12 @@ metadata:
   name: open-cluster-management:{{ .KlusterletName }}-work:agent
   namespace: {{ .KlusterletNamespace }}
 rules:
+# leader election needs to operate configmaps
+# - apiGroups: [""]
+#   resources: ["configmaps"]
+#   verbs: ["get", "list", "watch", "create", "delete", "update", "patch"]
 - apiGroups: [""]
-  resources: ["configmaps"]
-  verbs: ["get", "list", "watch", "create", "delete", "update", "patch"]
-- apiGroups: [""]
-  resources: ["secrets"]
+  resources: ["configmaps", "secrets"]
   verbs: ["get", "list", "watch"]
 - apiGroups: ["", "events.k8s.io"]
   resources: ["events"]

pkg/helpers/helpers.go

@@ -22,6 +22,7 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/kubernetes"
 	admissionclient "k8s.io/client-go/kubernetes/typed/admissionregistration/v1"
+	coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/util/retry"
@@ -601,10 +602,58 @@ func KlusterletNamespace(mode operatorapiv1.InstallMode, klusterletName, specNam
 		return klusterletName
 	}
 
-	if specNamespace == "" {
+	if len(specNamespace) == 0 {
 		// If namespace is not set, use the default namespace
 		return KlusterletDefaultNamespace
 	}
 
 	return specNamespace
 }
+
+// SyncSecret forked from https://github.com/openshift/library-go/blob/d9cdfbd844ea08465b938c46a16bed2ea23207e4/pkg/operator/resource/resourceapply/core.go#L357,
+// add an addition targetClient parameter to support sync secret to another cluster.
+func SyncSecret(client, targetClient coreclientv1.SecretsGetter, recorder events.Recorder,
+	sourceNamespace, sourceName, targetNamespace, targetName string, ownerRefs []metav1.OwnerReference) (*corev1.Secret, bool, error) {
+	source, err := client.Secrets(sourceNamespace).Get(context.TODO(), sourceName, metav1.GetOptions{})
+	switch {
+	case errors.IsNotFound(err):
+		if _, getErr := targetClient.Secrets(targetNamespace).Get(context.TODO(), targetName, metav1.GetOptions{}); getErr != nil && errors.IsNotFound(getErr) {
+			return nil, true, nil
+		}
+		deleteErr := targetClient.Secrets(targetNamespace).Delete(context.TODO(), targetName, metav1.DeleteOptions{})
+		if errors.IsNotFound(deleteErr) {
+			return nil, false, nil
+		}
+		if deleteErr == nil {
+			recorder.Eventf("TargetSecretDeleted", "Deleted target secret %s/%s because source config does not exist", targetNamespace, targetName)
+			return nil, true, nil
+		}
+		return nil, false, deleteErr
+	case err != nil:
+		return nil, false, err
+	default:
+		if source.Type == corev1.SecretTypeServiceAccountToken {
+
+			// Make sure the token is already present, otherwise we have to wait before creating the target
+			if len(source.Data[corev1.ServiceAccountTokenKey]) == 0 {
+				return nil, false, fmt.Errorf("secret %s/%s doesn't have a token yet", source.Namespace, source.Name)
+			}
+
+			if source.Annotations != nil {
+				// When syncing a service account token we have to remove the SA annotation to disable injection into copies
+				delete(source.Annotations, corev1.ServiceAccountNameKey)
+				// To make it clean, remove the dormant annotations as well
+				delete(source.Annotations, corev1.ServiceAccountUIDKey)
+			}
+
+			// SecretTypeServiceAccountToken implies required fields and injection which we do not want in copies
+			source.Type = corev1.SecretTypeOpaque
+		}
+
+		source.Namespace = targetNamespace
+		source.Name = targetName
+		source.ResourceVersion = ""
+		source.OwnerReferences = ownerRefs
+		return resourceapply.ApplySecret(targetClient, recorder, source)
+	}
+}

pkg/helpers/helpers_test.go

@@ -8,7 +8,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/openshift/library-go/pkg/assets"
+	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
 	operatorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
@@ -20,10 +22,12 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/client-go/kubernetes/fake"
 	fakekube "k8s.io/client-go/kubernetes/fake"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	clientcmdlatest "k8s.io/client-go/tools/clientcmd/api/latest"
 	fakeapiregistration "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake"
+
 	opereatorfake "open-cluster-management.io/api/client/operator/clientset/versioned/fake"
 	operatorapiv1 "open-cluster-management.io/api/operator/v1"
 	"open-cluster-management.io/registration-operator/manifests"
@@ -970,3 +974,167 @@ func TestKlusterletNamespace(t *testing.T) {
 		})
 	}
 }
+
+func TestSyncSecret(t *testing.T) {
+	tt := []struct {
+		name                        string
+		sourceNamespace, sourceName string
+		targetNamespace, targetName string
+		ownerRefs                   []metav1.OwnerReference
+		existingObjects             []runtime.Object
+		expectedSecret              *corev1.Secret
+		expectedChanged             bool
+		expectedErr                 error
+	}{
+		{
+			name:            "syncing existing secret succeeds when the target is missing",
+			sourceNamespace: "sourceNamespace",
+			sourceName:      "sourceName",
+			targetNamespace: "targetNamespace",
+			targetName:      "targetName",
+			ownerRefs:       nil,
+			existingObjects: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "sourceNamespace",
+						Name:      "sourceName",
+					},
+					Type: corev1.SecretTypeOpaque,
+					Data: map[string][]byte{"foo": []byte("bar")},
+				},
+			},
+			expectedSecret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "targetNamespace",
+					Name:      "targetName",
+				},
+				Type: corev1.SecretTypeOpaque,
+				Data: map[string][]byte{"foo": []byte("bar")},
+			},
+			expectedChanged: true,
+			expectedErr:     nil,
+		},
+		{
+			name:            "syncing existing secret succeeds when the target is present and needs update",
+			sourceNamespace: "sourceNamespace",
+			sourceName:      "sourceName",
+			targetNamespace: "targetNamespace",
+			targetName:      "targetName",
+			ownerRefs:       nil,
+			existingObjects: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "sourceNamespace",
+						Name:      "sourceName",
+					},
+					Type: corev1.SecretTypeOpaque,
+					Data: map[string][]byte{"foo": []byte("bar2")},
+				},
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "targetNamespace",
+						Name:      "targetName",
+					},
+					Type: corev1.SecretTypeOpaque,
+					Data: map[string][]byte{"foo": []byte("bar1")},
+				},
+			},
+			expectedSecret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "targetNamespace",
+					Name:      "targetName",
+				},
+				Type: corev1.SecretTypeOpaque,
+				Data: map[string][]byte{"foo": []byte("bar2")},
+			},
+			expectedChanged: true,
+			expectedErr:     nil,
+		},
+		{
+			name:            "syncing missing source secret doesn't fail",
+			sourceNamespace: "sourceNamespace",
+			sourceName:      "sourceName",
+			targetNamespace: "targetNamespace",
+			targetName:      "targetName",
+			ownerRefs:       nil,
+			existingObjects: []runtime.Object{},
+			expectedSecret:  nil,
+			expectedChanged: true,
+			expectedErr:     nil,
+		},
+		{
+			name:            "syncing service account token doesn't sync without the token being present",
+			sourceNamespace: "sourceNamespace",
+			sourceName:      "sourceName",
+			targetNamespace: "targetNamespace",
+			targetName:      "targetName",
+			ownerRefs:       nil,
+			existingObjects: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "sourceNamespace",
+						Name:      "sourceName",
+					},
+					Type: corev1.SecretTypeServiceAccountToken,
+					Data: map[string][]byte{"foo": []byte("bar")},
+				},
+			},
+			expectedSecret:  nil,
+			expectedChanged: false,
+			expectedErr:     fmt.Errorf("secret sourceNamespace/sourceName doesn't have a token yet"),
+		},
+		{
+			name:            "syncing service account token strips \"managed\" annotations",
+			sourceNamespace: "sourceNamespace",
+			sourceName:      "sourceName",
+			targetNamespace: "targetNamespace",
+			targetName:      "targetName",
+			ownerRefs:       nil,
+			existingObjects: []runtime.Object{
+				&corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "sourceNamespace",
+						Name:      "sourceName",
+						Annotations: map[string]string{
+							corev1.ServiceAccountNameKey: "foo",
+							corev1.ServiceAccountUIDKey:  "bar",
+						},
+					},
+					Type: corev1.SecretTypeServiceAccountToken,
+					Data: map[string][]byte{"token": []byte("top-secret")},
+				},
+			},
+			expectedSecret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "targetNamespace",
+					Name:      "targetName",
+				},
+				Type: corev1.SecretTypeOpaque,
+				Data: map[string][]byte{"token": []byte("top-secret")},
+			},
+			expectedChanged: true,
+			expectedErr:     nil,
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			client := fake.NewSimpleClientset(tc.existingObjects...)
+			clientTarget := fake.NewSimpleClientset()
+			secret, changed, err := SyncSecret(client.CoreV1(), clientTarget.CoreV1(), events.NewInMemoryRecorder("test"), tc.sourceNamespace, tc.sourceName, tc.targetNamespace, tc.targetName, tc.ownerRefs)
+
+			if !reflect.DeepEqual(err, tc.expectedErr) {
+				t.Errorf("%s: expected error %v, got %v", tc.name, tc.expectedErr, err)
+				return
+			}
+
+			if !equality.Semantic.DeepEqual(secret, tc.expectedSecret) {
+				t.Errorf("%s: secrets differ: %s", tc.name, cmp.Diff(tc.expectedSecret, secret))
+			}
+
+			if changed != tc.expectedChanged {
+				t.Errorf("%s: expected changed %t, got %t", tc.name, tc.expectedChanged, changed)
+			}
+		})
+	}
+}