Merge pull request #50933 from mattmoyer/bootstrap-token-groups

Automatic merge from submit-queue (batch tested with PRs 49861, 50933, 51380, 50688, 51305) Add configurable groups to bootstrap tokens. **What this PR does / why we need it**: This change adds support for authenticating bootstrap tokens into a configurable set of extra groups in addition to `system:bootstrappers`. Previously, bootstrap tokens could only ever authenticate to the `system:bootstrappers` group. Groups are specified as a comma-separated list in the `auth-extra-groups` key of the `bootstrap.kubernetes.io/token` Secret, and must begin with the prefix `system:bootstrapper:` (and match a validation regex that checks against our normal convention). Whether or not any extra groups are configured, `system:bootstrappers` will still be added. This also adds a `--groups` flag for `kubeadm token create`, which sets the `auth-extra-groups` key on the resulting Secret. The default is to not set the key. `kubeadm token list` is also updated to include a `EXTRA GROUPS` output column. **Which issue this PR fixes**: fixes #49306 **Special notes for your reviewer**: The use case for this is in #49306. Comments on the feature itself are probably better over there. It will be part of how HA/self-hosting kubeadm bootstraps new master nodes (post 1.8). **Release note**: ```release-note Add support for configurable groups for bootstrap token authentication. ``` cc @luxas @kubernetes/sig-cluster-lifecycle-api-reviews @kubernetes/sig-auth-api-reviews /kind feature
kubernetes · Aug 28, 2017 · 915b772 · 915b772
2 parents ecd910e + c7996a7
commit 915b772
Show file tree

Hide file tree

Showing 12 changed files with 298 additions and 17 deletions.
diff --git a/cmd/kubeadm/app/cmd/BUILD b/cmd/kubeadm/app/cmd/BUILD
@@ -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",

diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go
@@ -346,7 +346,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 {
 		return err
 	}
 	// Create RBAC rules that makes the bootstrap tokens able to post CSRs

diff --git a/cmd/kubeadm/app/cmd/token.go b/cmd/kubeadm/app/cmd/token.go
@@ -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"
@@ -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{
@@ -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)
@@ -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
@@ -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
 	}
@@ -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")
 	for _, secret := range secrets.Items {
 		tokenId := getSecretString(&secret, bootstrapapi.BootstrapTokenIDKey)
 		if len(tokenId) == 0 {
@@ -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

diff --git a/cmd/kubeadm/app/phases/bootstraptoken/node/token.go b/cmd/kubeadm/app/phases/bootstraptoken/node/token.go
@@ -18,6 +18,7 @@ package node
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"k8s.io/api/core/v1"
@@ -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
@@ -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
 			}
@@ -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
@@ -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)

diff --git a/cmd/kubeadm/app/phases/bootstraptoken/node/token_test.go b/cmd/kubeadm/app/phases/bootstraptoken/node/token_test.go
@@ -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",

diff --git a/pkg/bootstrap/api/BUILD b/pkg/bootstrap/api/BUILD
@@ -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"],
@@ -26,3 +28,9 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["helpers_test.go"],
+    library = ":go_default_library",
+)
diff --git a/pkg/bootstrap/api/helpers.go b/pkg/bootstrap/api/helpers.go
@@ -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)
+}
diff --git a/pkg/bootstrap/api/helpers_test.go b/pkg/bootstrap/api/helpers_test.go
@@ -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)
+		}
+	}
+}
diff --git a/pkg/bootstrap/api/types.go b/pkg/bootstrap/api/types.go
@@ -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-"
@@ -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"
 
@@ -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"
+	// 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]"
+
+	// BootstrapDefaultGroup is the default group for bootstrapping bearer
+	// tokens (in addition to any groups from BootstrapTokenExtraGroupsKey).
+	BootstrapDefaultGroup = "system:bootstrappers"
 )
diff --git a/plugin/pkg/auth/authenticator/token/bootstrap/BUILD b/plugin/pkg/auth/authenticator/token/bootstrap/BUILD
@@ -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",
     ],
 )