kubernetes-sigs / cluster-api-provider-nested

Cluster API Provider for Nested Clusters
Apache License 2.0
298 stars 65 forks source link

🌱 Support for kube-api-access volume mount #348

Closed wondywang closed 1 year ago

wondywang commented 1 year ago

What this PR does / why we need it: This PR continue to improve the feature gated support for VirtualCluster to work with any Kubernetes version 1.21+ and when the RootCACertConfigMap feature is enabled in 1.20 clusters. To achieve this, before creating the pPod, the content in the kube-api-access volume is changed. Change it to directly mount the tenant cluster secret instead of applying the token of the super cluster.

Before:

  volumes:
  - name: kube-api-access-sjz6l
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          expirationSeconds: 3607
          path: token
      - configMap:
          items:
          - key: ca.crt
            path: ca.crt
          name: kube-root-ca.crt
      - downwardAPI:
          items:
          - fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
            path: namespace

After:

  volumes:
  - name: kube-api-access-f7ww9
    projected:
      defaultMode: 420
      sources:
      - secret:
          name: default-token-v2wlt

Here is the reference to the implementation in the Kubernetes: https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/serviceaccount/admission.go

Follow-up needs to be considered: After Kubernetes version 1.24+, KCM will no longer distribute ServiceAccount tokens. I plan to add a token manager in the ServiceAccount sync controller to create ServiceAccount tokens and keep them in the validity period.

Which issue(s) this PR fixes:

282

/kind feature

christopherhein commented 1 year ago

This is looking great so far, this doesn’t do the TokenManagement aspects but I see your note that this can come later.

Fei-Guo commented 1 year ago

@wondywang.

wondywang commented 1 year ago

@wondywang.

  • Please fix lint/test failures.
  • Please add UTs for the change.
  • Please add a feature gate to guard your change entirely.

Ok, I will do it. If adding a new gate, I will prefer separate out a new file. I actually thought about doing that.

wondywang commented 1 year ago

@Fei-Guo @christopherhein

PTAL, thanks.

Fei-Guo commented 1 year ago

LGTM. @christopherhein, take another look?

christopherhein commented 1 year ago

Overall looks great, @Fei-Guo & @ravisantoshgudimetla added the same feedback I would. Thank you @wondywang for taking the lead on this.

wondywang commented 1 year ago

Overall looks great, @Fei-Guo & @ravisantoshgudimetla added the same feedback I would. Thank you @wondywang for taking the lead on this.

@christopherhein @ravisantoshgudimetla That's great, thanks! I will make some change later.

ravisantoshgudimetla commented 1 year ago

@wondywang - go-imports are failing. Can you fix and push the commit. LGTM from code standpoint.

wondywang commented 1 year ago

@wondywang - go-imports are failing. Can you fix and push the commit. LGTM from code standpoint.

Sorry, I've been a little busy lately. I will get it done as soon as possible. @ravisantoshgudimetla

wondywang commented 1 year ago

@Fei-Guo @christopherhein PTAL.

And I also update serviceAccount token manager in this commit. This part of the logic is an idea of mine (maybe not complete). Can be used as a reference. I plan not to mention PR for now. Wait for @ravisantoshgudimetla to finish it. What do you think?

https://github.com/wondywang/cluster-api-provider-nested/commit/ad8545a9efd52ebd85b7d0aa735bd29835a0424d

Fei-Guo commented 1 year ago

/lgtm /approve

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Fei-Guo, wondywang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[virtualcluster/OWNERS](https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/virtualcluster/OWNERS)~~ [Fei-Guo] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment