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.

Sign up for GitHub

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

Add configurable groups to bootstrap tokens. #50933

Merged
merged 4 commits into from
Aug 28, 2017
Merged

Add configurable groups to bootstrap tokens. #50933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
33e02af
Add extra group constants and validation to `pkg/bootstrap/api`.
mattmoyer Aug 22, 2017
fd5c00b
Implement `auth-extra-groups` in bootstrap token authenticator.
mattmoyer Aug 22, 2017
77f1b72
kubeadm: add `--groups` flag for `kubeadm token create`.
mattmoyer Aug 22, 2017
c7996a7
kubeadm: add extra group info to `token list`.
mattmoyer Aug 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/kubeadm/app/cmd/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/fields:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/version:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library",
"//vendor/k8s.io/client-go/kubernetes:go_default_library",
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (i *Init) Run(out io.Writer) error {

// Create the default node bootstrap token
tokenDescription := "The default bootstrap token generated by 'kubeadm init'."
if err := nodebootstraptokenphase.UpdateOrCreateToken(client, i.cfg.Token, false, i.cfg.TokenTTL, kubeadmconstants.DefaultTokenUsages, tokenDescription); err != nil {
if err := nodebootstraptokenphase.UpdateOrCreateToken(client, i.cfg.Token, false, i.cfg.TokenTTL, kubeadmconstants.DefaultTokenUsages, []string{}, tokenDescription); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

The default kubeadm token should have a prefix of system:bootstrappers:kubeadm:default-node or something.
Please add that to constants.go
We should also migrate the RBAC-creation code to create a ClusterRoleBinding depending on version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you okay doing that in a followup PR? I wanted to make sure we're careful about upgrading the cluster role binding since it's possible that an upgraded cluster would have nodes using tokens created without the new default group.

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with that.

Copy link
Member

Choose a reason for hiding this comment

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

The default kubeadm token should have a prefix of system:bootstrappers:kubeadm:default-node or something.

That seems like you're using a group to represent an individual token... each token already has its own unique username... why not use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there's a slight difference where you might have a single logical node pool (maybe system:bootstrappers:high-integrity-nodes). That group name might be used in a custom CSR autoapprover to assign a particular class of node identity to those nodes.

You might rotate through multiple tokens for different nodes within that pool, e.g., a token you used to bootstrap an initial set of nodes, and then another token you use to bootstrap additional nodes later on.

Copy link
Member

Choose a reason for hiding this comment

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

So this is for the default token group, sorry for not making that clear.
ClusterRoleBindings will be bound to this group; and the default token will be added to this group by default.

return err
}
// Create RBAC rules that makes the bootstrap tokens able to post CSRs
Expand Down
34 changes: 29 additions & 5 deletions cmd/kubeadm/app/cmd/token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/sets"
clientset "k8s.io/client-go/kubernetes"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
Expand Down Expand Up @@ -87,6 +88,7 @@ func NewCmdToken(out io.Writer, errW io.Writer) *cobra.Command {
"dry-run", dryRun, "Whether to enable dry-run mode or not")

var usages []string
var extraGroups []string
var tokenDuration time.Duration
var description string
createCmd := &cobra.Command{
Expand Down Expand Up @@ -114,14 +116,17 @@ func NewCmdToken(out io.Writer, errW io.Writer) *cobra.Command {
fmt.Fprintln(errW, "[kubeadm] WARNING: starting in 1.8, tokens expire after 24 hours by default (if you require a non-expiring token use --ttl 0)")
}

err = RunCreateToken(out, client, token, tokenDuration, usages, description)
err = RunCreateToken(out, client, token, tokenDuration, usages, extraGroups, description)
kubeadmutil.CheckErr(err)
},
}
createCmd.Flags().DurationVar(&tokenDuration,
"ttl", kubeadmconstants.DefaultTokenDuration, "The duration before the token is automatically deleted (e.g. 1s, 2m, 3h). 0 means 'never expires'.")
createCmd.Flags().StringSliceVar(&usages,
"usages", kubeadmconstants.DefaultTokenUsages, "The ways in which this token can be used. Valid options: [signing,authentication].")
createCmd.Flags().StringSliceVar(&extraGroups,
"groups", []string{},
fmt.Sprintf("Extra groups that this token will authenticate as when used for authentication. Must match %q.", bootstrapapi.BootstrapGroupPattern))
createCmd.Flags().StringVar(&description,
"description", "", "A human friendly description of how this token is used.")
tokenCmd.AddCommand(createCmd)
Expand Down Expand Up @@ -192,7 +197,7 @@ func NewCmdTokenGenerate(out io.Writer) *cobra.Command {
}

// RunCreateToken generates a new bootstrap token and stores it as a secret on the server.
func RunCreateToken(out io.Writer, client clientset.Interface, token string, tokenDuration time.Duration, usages []string, description string) error {
func RunCreateToken(out io.Writer, client clientset.Interface, token string, tokenDuration time.Duration, usages []string, extraGroups []string, description string) error {

if len(token) == 0 {
var err error
Expand All @@ -207,8 +212,22 @@ func RunCreateToken(out io.Writer, client clientset.Interface, token string, tok
}
}

// adding groups only makes sense for authentication
var usagesSet sets.String
usagesSet.Insert(usages...)
if len(extraGroups) > 0 && !usagesSet.Has("authentication") {
return fmt.Errorf("--groups cannot be specified unless --usages includes \"authentication\"")
}

// validate any extra group names
for _, group := range extraGroups {
if err := bootstrapapi.ValidateBootstrapGroupName(group); err != nil {
return err
}
}

// TODO: Validate usages here so we don't allow something unsupported
err := tokenphase.CreateNewToken(client, token, tokenDuration, usages, description)
err := tokenphase.CreateNewToken(client, token, tokenDuration, usages, extraGroups, description)
if err != nil {
return err
}
Expand Down Expand Up @@ -246,7 +265,7 @@ func RunListTokens(out io.Writer, errW io.Writer, client clientset.Interface) er
}

w := tabwriter.NewWriter(out, 10, 4, 3, ' ', 0)
fmt.Fprintln(w, "TOKEN\tTTL\tEXPIRES\tUSAGES\tDESCRIPTION")
fmt.Fprintln(w, "TOKEN\tTTL\tEXPIRES\tUSAGES\tDESCRIPTION\tEXTRA GROUPS")
Copy link
Member

Choose a reason for hiding this comment

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

nit: column headers shouldn't have whitespace in the titles

for _, secret := range secrets.Items {
tokenId := getSecretString(&secret, bootstrapapi.BootstrapTokenIDKey)
if len(tokenId) == 0 {
Expand Down Expand Up @@ -304,7 +323,12 @@ func RunListTokens(out io.Writer, errW io.Writer, client clientset.Interface) er
if len(description) == 0 {
description = "<none>"
}
fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", tokenutil.BearerToken(td), ttl, expires, usageString, description)

groups := getSecretString(&secret, bootstrapapi.BootstrapTokenExtraGroupsKey)
if len(groups) == 0 {
groups = "<none>"
}
fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n", tokenutil.BearerToken(td), ttl, expires, usageString, description, groups)
}
w.Flush()
return nil
Expand Down
17 changes: 11 additions & 6 deletions cmd/kubeadm/app/phases/bootstraptoken/node/token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package node

import (
"fmt"
"strings"
"time"

"k8s.io/api/core/v1"
Expand All @@ -33,12 +34,12 @@ const tokenCreateRetries = 5
// TODO(mattmoyer): Move CreateNewToken, UpdateOrCreateToken and encodeTokenSecretData out of this package to client-go for a generic abstraction and client for a Bootstrap Token

// CreateNewToken tries to create a token and fails if one with the same ID already exists
func CreateNewToken(client clientset.Interface, token string, tokenDuration time.Duration, usages []string, description string) error {
return UpdateOrCreateToken(client, token, true, tokenDuration, usages, description)
func CreateNewToken(client clientset.Interface, token string, tokenDuration time.Duration, usages []string, extraGroups []string, description string) error {
return UpdateOrCreateToken(client, token, true, tokenDuration, usages, extraGroups, description)
}

// UpdateOrCreateToken attempts to update a token with the given ID, or create if it does not already exist.
func UpdateOrCreateToken(client clientset.Interface, token string, failIfExists bool, tokenDuration time.Duration, usages []string, description string) error {
func UpdateOrCreateToken(client clientset.Interface, token string, failIfExists bool, tokenDuration time.Duration, usages []string, extraGroups []string, description string) error {
tokenID, tokenSecret, err := tokenutil.ParseToken(token)
if err != nil {
return err
Expand All @@ -52,7 +53,7 @@ func UpdateOrCreateToken(client clientset.Interface, token string, failIfExists
return fmt.Errorf("a token with id %q already exists", tokenID)
}
// Secret with this ID already exists, update it:
secret.Data = encodeTokenSecretData(tokenID, tokenSecret, tokenDuration, usages, description)
secret.Data = encodeTokenSecretData(tokenID, tokenSecret, tokenDuration, usages, extraGroups, description)
if _, err := client.CoreV1().Secrets(metav1.NamespaceSystem).Update(secret); err == nil {
return nil
}
Expand All @@ -67,7 +68,7 @@ func UpdateOrCreateToken(client clientset.Interface, token string, failIfExists
Name: secretName,
},
Type: v1.SecretType(bootstrapapi.SecretTypeBootstrapToken),
Data: encodeTokenSecretData(tokenID, tokenSecret, tokenDuration, usages, description),
Data: encodeTokenSecretData(tokenID, tokenSecret, tokenDuration, usages, extraGroups, description),
}
if _, err := client.CoreV1().Secrets(metav1.NamespaceSystem).Create(secret); err == nil {
return nil
Expand All @@ -85,12 +86,16 @@ func UpdateOrCreateToken(client clientset.Interface, token string, failIfExists
}

// encodeTokenSecretData takes the token discovery object and an optional duration and returns the .Data for the Secret
func encodeTokenSecretData(tokenID, tokenSecret string, duration time.Duration, usages []string, description string) map[string][]byte {
func encodeTokenSecretData(tokenID, tokenSecret string, duration time.Duration, usages []string, extraGroups []string, description string) map[string][]byte {
data := map[string][]byte{
bootstrapapi.BootstrapTokenIDKey: []byte(tokenID),
bootstrapapi.BootstrapTokenSecretKey: []byte(tokenSecret),
}

if len(extraGroups) > 0 {
data[bootstrapapi.BootstrapTokenExtraGroupsKey] = []byte(strings.Join(extraGroups, ","))
}

if duration > 0 {
// Get the current time, add the specified duration, and format it accordingly
durationString := time.Now().Add(duration).Format(time.RFC3339)
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/phases/bootstraptoken/node/token_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestEncodeTokenSecretData(t *testing.T) {
{token: &kubeadmapi.TokenDiscovery{ID: "foo", Secret: "bar"}, t: time.Second}, // should use default
}
for _, rt := range tests {
actual := encodeTokenSecretData(rt.token.ID, rt.token.Secret, rt.t, []string{}, "")
actual := encodeTokenSecretData(rt.token.ID, rt.token.Secret, rt.t, []string{}, []string{}, "")
if !bytes.Equal(actual["token-id"], []byte(rt.token.ID)) {
t.Errorf(
"failed EncodeTokenSecretData:\n\texpected: %s\n\t actual: %s",
Expand Down
8 changes: 8 additions & 0 deletions pkg/bootstrap/api/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ package(default_visibility = ["//visibility:public"])
load(
"@io_bazel_rules_go//go:def.bzl",
"go_library",
"go_test",
)

go_library(
name = "go_default_library",
srcs = [
"doc.go",
"helpers.go",
"types.go",
],
deps = ["//vendor/k8s.io/api/core/v1:go_default_library"],
Expand All @@ -26,3 +28,9 @@ filegroup(
srcs = [":package-srcs"],
tags = ["automanaged"],
)

go_test(
name = "go_default_test",
srcs = ["helpers_test.go"],
library = ":go_default_library",
)
34 changes: 34 additions & 0 deletions pkg/bootstrap/api/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package api

import (
"fmt"
"regexp"
)

var bootstrapGroupRegexp = regexp.MustCompile(`\A` + BootstrapGroupPattern + `\z`)

// ValidateBootstrapGroupName checks if the provided group name is a valid
// bootstrap group name. Returns nil if valid or a validation error if invalid.
// TODO(mattmoyer): this validation should migrate out to client-go (see https://github.com/kubernetes/client-go/issues/114)
func ValidateBootstrapGroupName(name string) error {
if bootstrapGroupRegexp.Match([]byte(name)) {
return nil
}
return fmt.Errorf("bootstrap group %q is invalid (must match %s)", name, BootstrapGroupPattern)
}
52 changes: 52 additions & 0 deletions pkg/bootstrap/api/helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package api

import (
"strings"
"testing"
)

func TestValidateBootstrapGroupName(t *testing.T) {
tests := []struct {
name string
input string
valid bool
}{
{"valid", "system:bootstrappers:foo", true},
{"valid nested", "system:bootstrappers:foo:bar:baz", true},
{"valid with dashes and number", "system:bootstrappers:foo-bar-42", true},
{"invalid uppercase", "system:bootstrappers:Foo", false},
{"missing prefix", "foo", false},
{"prefix with no body", "system:bootstrappers:", false},
{"invalid spaces", "system:bootstrappers: ", false},
{"invalid asterisk", "system:bootstrappers:*", false},
{"trailing colon", "system:bootstrappers:foo:", false},
{"trailing dash", "system:bootstrappers:foo-", false},
{"script tags", "system:bootstrappers:<script> alert(\"scary?!\") </script>", false},
{"too long", "system:bootstrappers:" + strings.Repeat("x", 300), false},
}
for _, test := range tests {
err := ValidateBootstrapGroupName(test.input)
if err != nil && test.valid {
t.Errorf("test %q: ValidateBootstrapGroupName(%q) returned unexpected error: %v", test.name, test.input, err)
}
if err == nil && !test.valid {
t.Errorf("test %q: ValidateBootstrapGroupName(%q) was supposed to return an error but didn't", test.name, test.input)
}
}
}
18 changes: 15 additions & 3 deletions pkg/bootstrap/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ const (
// describes what the bootstrap token is used for. Optional.
BootstrapTokenDescriptionKey = "description"

// BootstrapTokenExtraGroupsKey is a comma-separated list of group names.
// The bootstrap token will authenticate as these groups in addition to the
// "system:bootstrappers" group.
BootstrapTokenExtraGroupsKey = "auth-extra-groups"

// BootstrapTokenUsagePrefix is the prefix for the other usage constants that specifies different
// functions of a bootstrap token
BootstrapTokenUsagePrefix = "usage-bootstrap-"
Expand All @@ -63,7 +68,8 @@ const (
// BootstrapTokenUsageAuthentication signals that this token should be used
// as a bearer token to authenticate against the Kubernetes API. The bearer
// token takes the form "<token-id>.<token-secret>" and authenticates as the
// user "system:bootstrap:<token-id>" in the group "system:bootstrappers".
// user "system:bootstrap:<token-id>" in the "system:bootstrappers" group
// as well as any groups specified using BootstrapTokenExtraGroupsKey.
// Value must be "true". Any other value is assumed to be false. Optional.
BootstrapTokenUsageAuthentication = "usage-bootstrap-authentication"

Expand All @@ -80,6 +86,12 @@ const (
// authenticate as. The full username given is "system:bootstrap:<token-id>".
BootstrapUserPrefix = "system:bootstrap:"

// BootstrapGroup is the group bootstrapping bearer tokens authenticate in.
BootstrapGroup = "system:bootstrappers"
Copy link
Member

Choose a reason for hiding this comment

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

Have you looked up and replaced all occurances of this const? I think there should be some more places...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was all I could find.

// BootstrapGroupPattern is the valid regex pattern that all groups
// assigned to a bootstrap token by BootstrapTokenExtraGroupsKey must match.
// See also ValidateBootstrapGroupName().
BootstrapGroupPattern = "system:bootstrappers:[a-z0-9:-]{0,255}[a-z0-9]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: usually for stuff like this we use a DNS-ish type of regexp. That means that any segment shouldn't start with a number or -. It is a bit strange to allow system:bootstrappers:-foo here, for example.

But I don't see any real problem to be honest so it is just a nit.


// BootstrapDefaultGroup is the default group for bootstrapping bearer
// tokens (in addition to any groups from BootstrapTokenExtraGroupsKey).
BootstrapDefaultGroup = "system:bootstrappers"
)
1 change: 1 addition & 0 deletions plugin/pkg/auth/authenticator/token/bootstrap/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ go_library(
"//pkg/client/listers/core/internalversion:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
],
)
Expand Down
Loading