Merge pull request #4766 from a7i/propagate-service-account-token-sec…

…rets

propagate `Secret` of type `kubernetes.io/service-account-token`

pkg/detector/detector.go

@@ -293,19 +293,6 @@ func (d *ResourceDetector) EventFilter(obj interface{}) bool {
 		}
 	}
 
-	if unstructObj, ok := obj.(*unstructured.Unstructured); ok {
-		switch unstructObj.GroupVersionKind() {
-		// The secret, with type 'kubernetes.io/service-account-token', is created along with `ServiceAccount` should be
-		// prevented from propagating.
-		// Refer to https://github.com/karmada-io/karmada/pull/1525#issuecomment-1091030659 for more details.
-		case corev1.SchemeGroupVersion.WithKind("Secret"):
-			secretType, found, _ := unstructured.NestedString(unstructObj.Object, "type")
-			if found && secretType == string(corev1.SecretTypeServiceAccountToken) {
-				return false
-			}
-		}
-	}
-
 	// Prevent configmap/extension-apiserver-authentication from propagating as it is generated
 	// and managed by kube-apiserver.
 	// Refer to https://github.com/karmada-io/karmada/issues/4228 for more details.

pkg/resourceinterpreter/default/native/prune/prune.go

@@ -29,10 +29,11 @@ import (
 )
 
 // pruneIrrelevantField is the function that prune irrelevant fields from Work Object.
-type irrelevantFieldPruneFunc func(workload *unstructured.Unstructured) error
+type irrelevantFieldPruneFunc func(*unstructured.Unstructured) error
 
 var kindIrrelevantFieldPruners = map[string]irrelevantFieldPruneFunc{
 	util.JobKind:            removeJobIrrelevantField,
+	util.SecretKind:         removeSecretIrrelevantField,
 	util.ServiceAccountKind: removeServiceAccountIrrelevantField,
 	util.ServiceKind:        removeServiceIrrelevantField,
 }
@@ -180,3 +181,12 @@ func removeServiceIrrelevantField(workload *unstructured.Unstructured) error {
 	}
 	return nil
 }
+
+// removeSecretIrrelevantField removes the data and service-account uid annotation from service-account token secrets managed by member-cluster controller-manager
+func removeSecretIrrelevantField(workload *unstructured.Unstructured) error {
+	if secretType, exists, _ := unstructured.NestedString(workload.Object, "type"); exists && secretType == string(corev1.SecretTypeServiceAccountToken) {
+		unstructured.RemoveNestedField(workload.Object, "metadata", "annotations", corev1.ServiceAccountUIDKey)
+		_ = unstructured.SetNestedField(workload.Object, nil, "data")
+	}
+	return nil
+}

pkg/resourceinterpreter/default/native/prune/prune_test.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	"github.com/karmada-io/karmada/pkg/util"
@@ -181,6 +182,46 @@ func TestRemoveIrrelevantField(t *testing.T) {
 				return false
 			},
 		},
+		{
+			name: "remove service-account token secret irrelevant fields",
+			workload: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"kind": util.SecretKind,
+					"metadata": map[string]interface{}{
+						corev1.ServiceAccountUIDKey: "123",
+					},
+					"type": string(corev1.SecretTypeServiceAccountToken),
+					"data": map[string]interface{}{
+						corev1.ServiceAccountTokenKey: "abc",
+					},
+				},
+			},
+			unexpectedFields: []field{
+				{"metadata", "annotations", corev1.ServiceAccountUIDKey},
+				{"data", corev1.ServiceAccountTokenKey},
+			},
+		},
+		{
+			name: "retains secret basic-auth fields",
+			workload: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"kind": util.SecretKind,
+					"metadata": map[string]interface{}{
+						"foo": "bar",
+					},
+					"type": string(corev1.SecretTypeBasicAuth),
+					"data": map[string]interface{}{
+						corev1.BasicAuthUsernameKey: "foo",
+						corev1.BasicAuthPasswordKey: "bar",
+					},
+				},
+			},
+			shouldNotRemoveFields: []field{
+				{"metadata", "foo"},
+				{"data", corev1.BasicAuthUsernameKey},
+				{"data", corev1.BasicAuthPasswordKey},
+			},
+		},
 	}
 
 	for _, tt := range tests {

pkg/resourceinterpreter/default/native/retain.go

@@ -18,6 +18,7 @@ package native
 
 import (
 	"fmt"
+	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
@@ -42,6 +43,7 @@ func getAllDefaultRetentionInterpreter() map[schema.GroupVersionKind]retentionIn
 	s[corev1.SchemeGroupVersion.WithKind(util.PersistentVolumeClaimKind)] = retainPersistentVolumeClaimFields
 	s[corev1.SchemeGroupVersion.WithKind(util.PersistentVolumeKind)] = retainPersistentVolumeFields
 	s[batchv1.SchemeGroupVersion.WithKind(util.JobKind)] = retainJobSelectorFields
+	s[corev1.SchemeGroupVersion.WithKind(util.SecretKind)] = retainSecretServiceAccountToken
 	return s
 }
 
@@ -160,3 +162,32 @@ func retainWorkloadReplicas(desired, observed *unstructured.Unstructured) (*unst
 
 	return desired, nil
 }
+
+func retainSecretServiceAccountToken(desired *unstructured.Unstructured, observed *unstructured.Unstructured) (retained *unstructured.Unstructured, err error) {
+	if secretType, exists, _ := unstructured.NestedString(desired.Object, "type"); exists && secretType == string(corev1.SecretTypeServiceAccountToken) {
+		// retain service-account.uid which is a unique per cluster
+		serviceAccountUIDPath := []string{"metadata", "annotations", corev1.ServiceAccountUIDKey}
+		uid, exist, err := unstructured.NestedString(observed.Object, serviceAccountUIDPath...)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get %s from desired.Object: %+v", corev1.ServiceAccountUIDKey, err)
+		}
+		if exist {
+			if err := unstructured.SetNestedField(desired.Object, uid, serviceAccountUIDPath...); err != nil {
+				return nil, fmt.Errorf("failed to set %s for %s %s/%s", strings.Join(serviceAccountUIDPath, "."), desired.GetKind(), desired.GetNamespace(), desired.GetName())
+			}
+		}
+
+		// retain token generated by cluster kube-controller-manager
+		data, exist, err := unstructured.NestedStringMap(observed.Object, "data")
+		if err != nil {
+			return nil, fmt.Errorf("failed to get .data from desired.Object: %+v", err)
+		}
+		if exist {
+			if err := unstructured.SetNestedStringMap(desired.Object, data, "data"); err != nil {
+				return nil, fmt.Errorf("failed to set data for %s %s/%s", desired.GetKind(), desired.GetNamespace(), desired.GetName())
+			}
+		}
+	}
+
+	return desired, nil
+}

pkg/resourceinterpreter/default/native/retain_test.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
@@ -101,10 +102,77 @@ func Test_retainK8sWorkloadReplicas(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := retainWorkloadReplicas(tt.args.desired, tt.args.observed)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("reflectPodDisruptionBudgetStatus() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("retainWorkloadReplicas() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			assert.Equalf(t, tt.want, got, "retainDeploymentFields(%v, %v)", tt.args.desired, tt.args.observed)
 		})
 	}
 }
+
+func Test_retainSecretServiceAccountToken(t *testing.T) {
+	createSecret := func(secretType corev1.SecretType, uuid, key, value string) *unstructured.Unstructured {
+		ret, _ := helper.ToUnstructured(&corev1.Secret{
+			Type: secretType,
+		})
+		return ret
+	}
+
+	type args struct {
+		desired  *unstructured.Unstructured
+		observed *unstructured.Unstructured
+	}
+	tests := []struct {
+		name string
+		args args
+		want *unstructured.Unstructured
+	}{
+		{
+			name: "secret data and uid are retained for type service-account-token",
+			args: args{
+				desired:  createSecret(corev1.SecretTypeServiceAccountToken, "111", corev1.ServiceAccountTokenKey, "desired-token"),
+				observed: createSecret(corev1.SecretTypeServiceAccountToken, "999", corev1.ServiceAccountTokenKey, "observed-token"),
+			},
+			want: createSecret(corev1.SecretTypeServiceAccountToken, "999", corev1.ServiceAccountTokenKey, "observed-token"),
+		},
+		{
+			name: "ignores missing uid and data for type service-account-token",
+			args: args{
+				desired:  &unstructured.Unstructured{Object: map[string]interface{}{"type": string(corev1.SecretTypeServiceAccountToken)}},
+				observed: &unstructured.Unstructured{Object: map[string]interface{}{"type": string(corev1.SecretTypeServiceAccountToken)}},
+			},
+			want: &unstructured.Unstructured{Object: map[string]interface{}{"type": string(corev1.SecretTypeServiceAccountToken)}},
+		},
+		{
+			name: "does not retain for type tls",
+			args: args{
+				desired:  createSecret(corev1.SecretTypeTLS, "111", corev1.TLSCertKey, "desired-cert"),
+				observed: createSecret(corev1.SecretTypeTLS, "999", corev1.TLSCertKey, "observed-cert"),
+			},
+			want: createSecret(corev1.SecretTypeTLS, "111", corev1.TLSCertKey, "desired-cert"),
+		},
+		{
+			name: "does not retain for type basic-auth",
+			args: args{
+				desired:  createSecret(corev1.SecretTypeBasicAuth, "111", corev1.BasicAuthUsernameKey, "desired-user"),
+				observed: createSecret(corev1.SecretTypeBasicAuth, "999", corev1.BasicAuthUsernameKey, "observed-user"),
+			},
+			want: createSecret(corev1.SecretTypeBasicAuth, "111", corev1.BasicAuthUsernameKey, "desired-user"),
+		},
+		{
+			name: "does not retain for type dockercfg",
+			args: args{
+				desired:  createSecret(corev1.SecretTypeDockercfg, "111", corev1.DockerConfigKey, "desired-docker-cfg"),
+				observed: createSecret(corev1.SecretTypeDockercfg, "999", corev1.DockerConfigKey, "observed-docker-cfg"),
+			},
+			want: createSecret(corev1.SecretTypeDockercfg, "111", corev1.DockerConfigKey, "desired-docker-cfg"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := retainSecretServiceAccountToken(tt.args.desired, tt.args.observed)
+			assert.Nil(t, err, "retainSecretServiceAccountToken() error = %v", err)
+			assert.Equalf(t, tt.want, got, "retainSecretServiceAccountToken(%v, %v)", tt.args.desired, tt.args.observed)
+		})
+	}
+}

pkg/util/constants.go

@@ -181,6 +181,8 @@ const (
 	ClusterRoleBindingKind = "ClusterRoleBinding"
 	// CRDKind indicates the target resource is a CustomResourceDefinition
 	CRDKind = "CustomResourceDefinition"
+	// SecretKind indicates the target resource is a Secret
+	SecretKind = "Secret"
 
 	// ServiceExportKind indicates the target resource is a serviceexport crd
 	ServiceExportKind = "ServiceExport"