-
Notifications
You must be signed in to change notification settings - Fork 213
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
base: master
Are you sure you want to change the base?
Conversation
…eissner/stlsbootstrap-linux
…eissner/stlsbootstrap-linux
…eissner/stlsbootstrap-linux
…eissner/stlsbootstrap-linux
…eissner/stlsbootstrap-linux-msi
@@ -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}}" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, keep forgetting
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
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
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:
Some important things to note:
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Release note: