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 https://github.com/kubernetes/kubernetes/issues/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

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",

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

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,7 +116,7 @@ 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)
 		},
 	}
@@ -122,6 +124,9 @@ func NewCmdToken(out io.Writer, errW io.Writer) *cobra.Command {
 		"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

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 {
+func CreateNewToken(client clientset.Interface, token string, tokenDuration time.Duration, usages []string, extraGroups []string, description string) error {
-	return UpdateOrCreateToken(client, token, true, tokenDuration, usages, description)
+	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)

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",

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",
+)

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)
+}

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)
+		}
+	}
+}

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.
+	// BootstrapGroupPattern is the valid regex pattern that all groups
-	BootstrapGroup = "system:bootstrappers"
+	// 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"
 )

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",
     ],
 )

plugin/pkg/auth/authenticator/token/bootstrap/bootstrap.go

@@ -23,11 +23,13 @@ import (
 	"crypto/subtle"
 	"fmt"
 	"regexp"
+	"strings"
 	"time"
 
 	"github.com/golang/glog"
 
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/kubernetes/pkg/api"
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
@@ -79,6 +81,7 @@ func tokenErrorf(s *api.Secret, format string, i ...interface{}) {
 //       token-id: ( token id )
 //       # Required key usage.
 //       usage-bootstrap-authentication: true
+//       auth-extra-groups: "system:bootstrappers:custom-group1,system:bootstrappers:custom-group2"
 //       # May also contain an expiry.
 //
 // Tokens are expected to be of the form:
@@ -134,9 +137,15 @@ func (t *TokenAuthenticator) AuthenticateToken(token string) (user.Info, bool, e
 		return nil, false, nil
 	}
 
+	groups, err := getGroups(secret)
+	if err != nil {
+		tokenErrorf(secret, "has invalid value for key %s: %v.", bootstrapapi.BootstrapTokenExtraGroupsKey, err)
+		return nil, false, nil
+	}
+
 	return &user.DefaultInfo{
 		Name:   bootstrapapi.BootstrapUserPrefix + string(id),
-		Groups: []string{bootstrapapi.BootstrapGroup},
+		Groups: groups,
 	}, true, nil
 }
 
@@ -184,3 +193,28 @@ func parseToken(s string) (string, string, error) {
 	}
 	return split[1], split[2], nil
 }
+
+// getGroups loads and validates the bootstrapapi.BootstrapTokenExtraGroupsKey
+// key from the bootstrap token secret, returning a list of group names or an
+// error if any of the group names are invalid.
+func getGroups(secret *api.Secret) ([]string, error) {
+	// always include the default group
+	groups := sets.NewString(bootstrapapi.BootstrapDefaultGroup)
+
+	// grab any extra groups and if there are none, return just the default
+	extraGroupsString := getSecretString(secret, bootstrapapi.BootstrapTokenExtraGroupsKey)
+	if extraGroupsString == "" {
+		return groups.List(), nil
+	}
+
+	// validate the names of the extra groups
+	for _, group := range strings.Split(extraGroupsString, ",") {
+		if err := bootstrapapi.ValidateBootstrapGroupName(group); err != nil {
+			return nil, err
+		}
+		groups.Insert(group)
+	}
+
+	// return the result as a deduplicated, sorted list
+	return groups.List(), nil
+}

plugin/pkg/auth/authenticator/token/bootstrap/bootstrap_test.go

@@ -84,6 +84,47 @@ func TestTokenAuthenticator(t *testing.T) {
 				Groups: []string{"system:bootstrappers"},
 			},
 		},
+		{
+			name: "valid token with extra group",
+			secrets: []*api.Secret{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: bootstrapapi.BootstrapTokenSecretPrefix + tokenID,
+					},
+					Data: map[string][]byte{
+						bootstrapapi.BootstrapTokenIDKey:               []byte(tokenID),
+						bootstrapapi.BootstrapTokenSecretKey:           []byte(tokenSecret),
+						bootstrapapi.BootstrapTokenUsageAuthentication: []byte("true"),
+						bootstrapapi.BootstrapTokenExtraGroupsKey:      []byte("system:bootstrappers:foo"),
+					},
+					Type: "bootstrap.kubernetes.io/token",
+				},
+			},
+			token: tokenID + "." + tokenSecret,
+			wantUser: &user.DefaultInfo{
+				Name:   "system:bootstrap:" + tokenID,
+				Groups: []string{"system:bootstrappers", "system:bootstrappers:foo"},
+			},
+		},
+		{
+			name: "invalid group",
+			secrets: []*api.Secret{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: bootstrapapi.BootstrapTokenSecretPrefix + tokenID,
+					},
+					Data: map[string][]byte{
+						bootstrapapi.BootstrapTokenIDKey:               []byte(tokenID),
+						bootstrapapi.BootstrapTokenSecretKey:           []byte(tokenSecret),
+						bootstrapapi.BootstrapTokenUsageAuthentication: []byte("true"),
+						bootstrapapi.BootstrapTokenExtraGroupsKey:      []byte("foo"),
+					},
+					Type: "bootstrap.kubernetes.io/token",
+				},
+			},
+			token:        tokenID + "." + tokenSecret,
+			wantNotFound: true,
+		},
 		{
 			name: "invalid secret name",
 			secrets: []*api.Secret{
@@ -247,3 +288,72 @@ func TestTokenAuthenticator(t *testing.T) {
 		}()
 	}
 }
+
+func TestGetGroups(t *testing.T) {
+	tests := []struct {
+		name         string
+		secret       *api.Secret
+		expectResult []string
+		expectError  bool
+	}{
+		{
+			name: "not set",
+			secret: &api.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "test"},
+				Data:       map[string][]byte{},
+			},
+			expectResult: []string{"system:bootstrappers"},
+		},
+		{
+			name: "set to empty value",
+			secret: &api.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "test"},
+				Data: map[string][]byte{
+					bootstrapapi.BootstrapTokenExtraGroupsKey: []byte(""),
+				},
+			},
+			expectResult: []string{"system:bootstrappers"},
+		},
+		{
+			name: "invalid prefix",
+			secret: &api.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "test"},
+				Data: map[string][]byte{
+					bootstrapapi.BootstrapTokenExtraGroupsKey: []byte("foo"),
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "valid",
+			secret: &api.Secret{
+				ObjectMeta: metav1.ObjectMeta{Name: "test"},
+				Data: map[string][]byte{
+					bootstrapapi.BootstrapTokenExtraGroupsKey: []byte("system:bootstrappers:foo,system:bootstrappers:bar,system:bootstrappers:bar"),
+				},
+			},
+			// expect the results in deduplicated, sorted order
+			expectResult: []string{
+				"system:bootstrappers",
+				"system:bootstrappers:bar",
+				"system:bootstrappers:foo",
+			},
+		},
+	}
+	for _, test := range tests {
+		result, err := getGroups(test.secret)
+		if test.expectError {
+			if err == nil {
+				t.Errorf("test %q expected an error, but didn't get one (result: %#v)", test.name, result)
+			}
+			continue
+		}
+		if err != nil {
+			t.Errorf("test %q return an unexpected error: %v", test.name, err)
+			continue
+		}
+		if !reflect.DeepEqual(result, test.expectResult) {
+			t.Errorf("test %q expected %#v, got %#v", test.name, test.expectResult, result)
+		}
+	}
+}