-
Notifications
You must be signed in to change notification settings - Fork 39.9k
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
Make secrets at cluster startup. #5470
Conversation
Whatever you're trying to do with that script, it needs to go in |
Is it a |
Good question on naming, but the original intent of having it created on the master still holds. I was originally thinking of calling these things "system objects" and not "addon objects", so maybe this would precipitate that change. |
The pod json/yaml for the scheduler and the controller-manager should go into, say: I don't believe that any change is needed in Right now, there is no ordering between the different addon objects. This should still "just work". That is, create all the object in any order. Without a scheduler, none of the objects will start except the scheduler, which has a @zmerlynn tell me if above is correct. |
The question is how to get the variables we need (tokens) from kube-up to kube-addons. Option 1: In the Option 2: Actually generates the tokens in kube-addons.sh script, and append them to |
The part where you're adding the env variable and generation of the token to Why do you need two more tokens, btw? |
I want each system component to have its own identity so each component can have only the privileges it needs. |
@zmerlynn I've moved most of my changes into kube-addons.sh @ArtfulCoder you can use these secrets to authorize scheduler and replication controller. @vishh you can use these secrets to authorize monitoring pods that need to talk to the apiserver. |
for a in scheduler controller_manager logging monitoring dns | ||
do | ||
token=$(dd if=/dev/urandom bs=128 count=1 2>/dev/null | base64 | tr -d "=+/" | dd bs=32 count=1 2>/dev/null) | ||
echo "${token},kubernetes_builtin:$a,kubernetes_builtin:$a" >> "${known_tokens_file}" |
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.
@smarterclayton ISTR you suggesting a format for namespacing usernames but I can't find the issue now. Is there already a standard used in openshift? Here I used kubernetes_builtin:$USER
, but I am open to whatever.
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.
We ended up with normal usernames being unnamedspaced (so they can inherit the preferred username from the bedrock identity provider), and namespaced system/service account usernames with a "prefix:" (like "system:" or "project:"). That means ":" is disallowed in normal usernames for us.
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.
We had very specific users names and we are using the system: prefix to scope deeper roles.
https://github.com/openshift/origin/blob/master/pkg/cmd/server/bootstrappolicy/constants.go
https://github.com/openshift/origin/blob/master/pkg/cmd/server/admin/default_certs.go
Would be good to settle on a pattern for prefixing as we go (don't have to change here).
On Mar 27, 2015, at 7:18 PM, Jordan Liggitt notifications@github.com wrote:
In cluster/gce/configure-vm.sh:
@@ -260,6 +260,13 @@ function create-salt-auth() {
kubelet_auth_file="/srv/salt-overlay/salt/kubelet/kubernetes_auth"
(umask 077;
echo "{"BearerToken": "${KUBELET_TOKEN}", "Insecure": true }" > "${kubelet_auth_file}")
+
Generate tokens for other "service accounts". Append to known_tokens.
- for a in scheduler controller_manager logging monitoring dns
- do
- token=$(dd if=/dev/urandom bs=128 count=1 2>/dev/null | base64 | tr -d "=+/" | dd bs=32 count=1 2>/dev/null)
- echo "${token},kubernetes_builtin:$a,kubernetes_builtin:$a" >> "${known_tokens_file}"
We ended up with normal usernames being unnamedspaced (so they can inherit the preferred username from the bedrock identity provider), and namespaced system/service account usernames with a "prefix:" (like "system:" or "project:"). That means ":" is disallowed in normal usernames for us.—
Reply to this email directly or view it on GitHub.
A step to fixing #5921 |
This looks like it should work and actually require no GKE side changes. |
@erictune: Ack. I will update heapster as soon as this PR is merged. |
252a66d
to
aa91596
Compare
@zmerlynn would you mind reviewing? |
Happy to. |
obj="$1" | ||
|
||
for tries in {1..5}; do | ||
if ( echo "${obj}" | ${KUBECTL} --server="127.0.0.1:8080" create -f - ); then |
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.
Any reason not to pass --validate
here as well?
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.
ok. there was a bug in kubectl with validate and some objects, but it is since fixed.
@liggitt This seems relevant to your interests |
@@ -19,6 +19,39 @@ | |||
# managed result is of that. Start everything below that directory. | |||
KUBECTL=/usr/local/bin/kubectl | |||
|
|||
function create-kubernetesauth-secret() { |
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.
Blah, I didn't enforce it well in create-object
, but you can fix it while you're in here. :)
These should all be local
. Probably local -r token=$1
, etc, since they look readonly.
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.
ok
for a in "system:scheduler" "system:controller_manager" "system:logging" "system:monitoring" "system:dns" | ||
do | ||
token=$(dd if=/dev/urandom bs=128 count=1 2>/dev/null | base64 | tr -d "=+/" | dd bs=32 count=1 2>/dev/null) | ||
echo "${token},$a,$a" >> "${known_tokens_file}" |
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.
Nit: I try to follow ${foo} for all variables. (so ${a}
. Or ${account}
). And below.
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.
ok
I did my best to channel @filbranden. The actual code LGTM, it's just style nits. |
Needs rebase onto #6384 |
930e8d6
to
59b3868
Compare
These secrets will be used in subsequent PRs by: scheduler, controller-manager, monitoring services, logging services, and skydns. Each of these services will then be able to stop using kubernetes-ro or host networking.
Okay, rebased. Testing. |
Okay, kube-up.sh works, and expected addons and new secrets are created. |
LGTM. |
Make secrets at cluster startup.
Thanks. |
Changes approach taken in kubernetes#5470 Instead of creating a kubernetes_auth file, which we are tring to get away from, creates a kubeconfig file, which is the new hotness. Instead of creating the kubeconfig file in the kube-addon script on the master, it creates it at the time of salt-overlay generation. More information is available at this time. In particular, the master certs and master address are handy at this point, so those are included in the kubeconfig file. The kube-addons script is simplified because the secret objects are now just plain olf yaml files, which it knows how to create, just like it creates pods and services. Kubectl is used to generate the kubeconfig file. This ensures correct format and is more self-documenting, and matches how the admin credentials are done. TODO(erictune): do this for kubelet.
@ArtfulCoder