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

refactor: updated STLS bootstrap integration with linux CSE #4386

Open
wants to merge 70 commits into
base: master
Choose a base branch
from

Conversation

cameronmeissner
Copy link
Collaborator

@cameronmeissner cameronmeissner commented May 9, 2024

What type of PR is this?
/kind feature

What this PR does / why we need it:
adds support for the new design of secure TLS bootstrapping to linux CSE/customdata.

follow-up items:

  • add start-kubelet.sh to the VHD during builds
  • download the secure TLS bootstrap client during VHD builds

Some important things to note:

  • with this change, we now start kubelet without waiting for the startup to complete in the secure TLS bootstrapping case. this is due to the fact that CSE will not have access to MSI tokens via IMDS because of how RP assigns kubelet identities to both VMSS and standalone VMs, thus we want kubelet to delegate to the secure bootstrap client after CSE has exited with code 0, and eventually after RP has attached the kubelet identity to the VM. this also means that there will now be a longer delay between when CSE exits successfully and when the node is able to register with the API server due to the time it takes CSE success to propagate back to RP and for RP to perform the PATCH on the VMSS/SAVM to assign it the kubelet identity.
  • this change does not effect the current state of vanilla TLS bootstrapping and will continue functioning the exact same way as it does now when we're not using secure bootstrapping
  • this change will have it such that vanilla TLS bootstrapping is used as a fall-back in case secure TLS bootstrapping fails to bootstrap a signed kubelet client certificate after a pre-configured timeout

Which issue(s) this PR fixes:

Fixes #

Requirements:

Special notes for your reviewer:

Release note:

none

Cameron Meissner added 30 commits April 5, 2024 08:03
@cameronmeissner cameronmeissner changed the title feat: STLS bootstrap integration with linux CSE with MSI support feat: STLS bootstrap integration with linux CSE May 14, 2024
@cameronmeissner cameronmeissner changed the title feat: STLS bootstrap integration with linux CSE refactor: updated STLS bootstrap integration with linux CSE May 17, 2024
@@ -118,7 +118,7 @@ NO_PROXY_URLS="{{GetNoProxy}}"
PROXY_VARS="{{GetProxyVariables}}"
ENABLE_TLS_BOOTSTRAPPING="{{EnableTLSBootstrapping}}"
ENABLE_SECURE_TLS_BOOTSTRAPPING="{{EnableSecureTLSBootstrapping}}"
CUSTOM_SECURE_TLS_BOOTSTRAP_AAD_SERVER_APP_ID="{{GetCustomSecureTLSBootstrapAADServerAppID}}"
CUSTOM_SECURE_TLS_BOOTSTRAP_AAD_RESOURCE="{{GetCustomSecureTLSBootstrapAADResource}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

cse_cmd.sh var naming change, want to loop in @Bryce-Soghigian @tallaxes here for viz, however, I dont think this variable is being used yet that the renaming would break them?

still tagging them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, keep forgetting

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack, and appreciated. Not used yet, so should be fine - but only as long as not setting it at all is tolerated by the rest of the logic.

@@ -293,6 +293,27 @@ configureCNIIPTables() {
fi
}

configureKubeletSecureTLSBootstrap() {
# default AAD resource here so we can minimze bootstrap contract surface
AAD_RESOURCE="6dae42f8-4368-4678-94ff-3960e28e3630"
Copy link
Collaborator

Choose a reason for hiding this comment

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

If user doesn't set CUSTOM_SECURE_TLS_BOOTSTRAP_AAD_RESOURCE, AAD_RESOURCE is defaulted to 6dae42f8-4368-4678-94ff-3960e28e3630. Does it mean this AAD_RESOURCE is always available in every user's subscription?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, as far as I know - this is the app ID of AKS's AAD server, I've spoken with Baichao and Weinong about this:

https://azure.github.io/kubelogin/concepts/aks.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

though will be a good idea to confirm this is correct gov environments as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants