karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.14k stars 812 forks source link

Propagate Service Account Token Secret #4752

Closed jklaw90 closed 1 month ago

jklaw90 commented 1 month ago

What happened: I made a PropagationPolicy for a secret with type kubernetes.io/service-account-token and it's ServiceAccount. It appears PR 1525 was made to ignore the secret. However, since 1.24 these secrets aren't automatically created anymore.

What you expected to happen: I'd expect for karmada to copy my secret and use my ResourceInterpreterCustomization to not copy the secret data. Additionally i'd like karmada to handle this case and and always remove the cluster specific secret data.

How to reproduce it (as minimally and precisely as possible):

kubectl create ns test
kubectl apply -n test -f - <<EOF
apiVersion: policy.karmada.io/v1alpha1
kind: PropagationPolicy
metadata:
  name: secret-propagation
  namespace: test
spec:
  resourceSelectors:
    - apiVersion: v1
      kind: Secret
      namespace: test
  placement:
    clusterAffinity:
      clusterNames:
        - member1
        - member2
        - member3
EOF 
kubectl create sa -n test sa-test
kubectl get secrets -n test  # NOTE no secrets different functionality from the pr linked above due to 1.24
kubectl apply -n test -f - <<EOF
apiVersion: v1
kind: Secret
type: kubernetes.io/service-account-token
metadata:
  name: sa-test-token
  annotations:
    kubernetes.io/service-account.name: sa-test
EOF

Anything else we need to know?:

Environment:

RainbowMango commented 1 month ago

Yes, the background of #1525 is that people don't need to propagate a secret with the type kubernetes.io/service-account-token. It's worth revisiting this now.

I'd expect for karmada to copy my secret and use my ResourceInterpreterCustomization to not copy the secret data.

What do you mean copy my secret? enable to propagate the secret? For the ResourceInterpreterCustomization, do you mean the retaintion?

whitewindmills commented 1 month ago

I'm still a little confused. Even if this secret is propagated, will it be of any help to you? You can also create this secret on the member cluster by your controller and use it. Karmada neither collects it's token and certs, nor synchronizes any configuration, this approach doesn't seem to make much sense. Similarly, will this behavior affect K8s versions below 1.24?

chaunceyjiang commented 1 month ago

Perhaps we can make it default that secrets(kubernetes.io/service-account-token) cannot be propagated to member clusters, but allow users to modify this behavior through ResourceInterpreterCustomization.

chaunceyjiang commented 1 month ago

/cc @RainbowMango @XiShanYongYe-Chang PTAL

XiShanYongYe-Chang commented 1 month ago

According to the current situation, users do propagate secrets of type kubernetes.io/service-account-token to member clusters, and the problem described in the comment does not occur.

For versions before 1.24, I think we can remove the kubernetes.io/service-account.uid annotation when propagating, so that the resource is not created and deleted frequently.

How do you think?

a7i commented 1 month ago

Correct, we have a tool (external to the cluster) that communicates via a long-lived service account token. Therefore we want the ServiceAccount and the Secret to be created in all member clusters. That's because with 1.24+, the Secret is no longer automatically created. However, we want each member cluster's kube-controller-manager to populate the .data.

For versions before 1.24, the member cluster is going to automatically create the Secret anyways and changes in #4766 should ignore the data and the uid.