Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions pkg/detector/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 11 additions & 1 deletion pkg/resourceinterpreter/default/native/prune/prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
}
41 changes: 41 additions & 0 deletions pkg/resourceinterpreter/default/native/prune/prune_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
31 changes: 31 additions & 0 deletions pkg/resourceinterpreter/default/native/retain.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package native

import (
"fmt"
"strings"

appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
a7i marked this conversation as resolved.
Show resolved Hide resolved
}
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)
}
a7i marked this conversation as resolved.
Show resolved Hide resolved
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
}
70 changes: 69 additions & 1 deletion pkg/resourceinterpreter/default/native/retain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch. thanks.

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)
})
}
}
2 changes: 2 additions & 0 deletions pkg/util/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading