diff --git a/pkg/detector/detector.go b/pkg/detector/detector.go index 73a81fd68036..0b0f4b9bfd4e 100644 --- a/pkg/detector/detector.go +++ b/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. diff --git a/pkg/resourceinterpreter/default/native/prune/prune.go b/pkg/resourceinterpreter/default/native/prune/prune.go index 096aee77b3f1..7658dc9971db 100644 --- a/pkg/resourceinterpreter/default/native/prune/prune.go +++ b/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 +} diff --git a/pkg/resourceinterpreter/default/native/prune/prune_test.go b/pkg/resourceinterpreter/default/native/prune/prune_test.go index e4cc8a66b5fc..22cca12a98f6 100755 --- a/pkg/resourceinterpreter/default/native/prune/prune_test.go +++ b/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 { diff --git a/pkg/resourceinterpreter/default/native/retain.go b/pkg/resourceinterpreter/default/native/retain.go index 0e2ca3daa987..f167a5adaf10 100644 --- a/pkg/resourceinterpreter/default/native/retain.go +++ b/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 +} diff --git a/pkg/resourceinterpreter/default/native/retain_test.go b/pkg/resourceinterpreter/default/native/retain_test.go index 99c2a8eee900..3d30aab04d2c 100644 --- a/pkg/resourceinterpreter/default/native/retain_test.go +++ b/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) + }) + } +} diff --git a/pkg/util/constants.go b/pkg/util/constants.go index 3305ac260d9f..4f6c84a55f26 100644 --- a/pkg/util/constants.go +++ b/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"