Merge pull request #39677 from apprenda/kubeadm_104-lowercase_token

Automatic merge from submit-queue

kubeadm: must lower-case token portion used in DNS label.

**What this PR does / why we need it**: In Kubernetes, DNS labels must be lower-case. `kubeadm` doesn't care when creating certain objects through the API. This PR fixes that erroneous behavior.

**Which issue this PR fixes**: fixes https://github.com/kubernetes/kubeadm/issues/104

**Special notes for your reviewer**: /cc @luxas @mikedanese @dgoodwin

cmd/kubeadm/app/cmd/init.go

@@ -21,6 +21,7 @@ import (
 	"io"
 	"io/ioutil"
 	"path"
+	"strconv"
 
 	"github.com/renstrom/dedent"
 	"github.com/spf13/cobra"
@@ -207,9 +208,20 @@ func (i *Init) Run(out io.Writer) error {
 
 	// Exception:
 	if i.cfg.Discovery.Token != nil {
-		if err := kubemaster.PrepareTokenDiscovery(i.cfg.Discovery.Token); err != nil {
+		// Validate token
+		if valid, err := kubeadmutil.ValidateToken(i.cfg.Discovery.Token); valid == false {
 			return err
 		}
+
+		// Make sure there is at least one address
+		if len(i.cfg.Discovery.Token.Addresses) == 0 {
+			ip, err := netutil.ChooseHostInterface()
+			if err != nil {
+				return err
+			}
+			i.cfg.Discovery.Token.Addresses = []string{ip.String() + ":" + strconv.Itoa(kubeadmapiext.DefaultDiscoveryBindPort)}
+		}
+
 		if err := kubemaster.CreateTokenAuthFile(kubeadmutil.BearerToken(i.cfg.Discovery.Token)); err != nil {
 			return err
 		}

cmd/kubeadm/app/cmd/token.go

@@ -31,7 +31,7 @@ import (
 	kubemaster "k8s.io/kubernetes/cmd/kubeadm/app/master"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/pkg/api"
-	v1 "k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/fields"
 	"k8s.io/kubernetes/pkg/kubectl"
 )
@@ -108,7 +108,7 @@ func NewCmdTokenGenerate(out io.Writer) *cobra.Command {
 			the "init" and "join" commands.
 
 			You don't have to use this command in order to generate a token, you can do so
-			yourself as long as it's in the format "<6 characters>.<16 characters>". This
+			yourself as long as it's in the format "<6 characters>:<16 characters>". This
 			command is provided for convenience to generate tokens in that format.
 
 			You can also use "kubeadm init" without specifying a token, and it will
@@ -128,37 +128,29 @@ func RunCreateToken(out io.Writer, cmd *cobra.Command, tokenDuration time.Durati
 		return err
 	}
 
-	d := &kubeadmapi.TokenDiscovery{}
+	parsedID, parsedSecret, err := kubeadmutil.ParseToken(token)
-	if token != "" {
-		parsedID, parsedSecret, err := kubeadmutil.ParseToken(token)
-		if err != nil {
-			return err
-		}
-		d.ID = parsedID
-		d.Secret = parsedSecret
-	}
-	err = kubeadmutil.GenerateTokenIfNeeded(d)
 	if err != nil {
 		return err
 	}
+	td := &kubeadmapi.TokenDiscovery{ID: parsedID, Secret: parsedSecret}
 
-	err = kubeadmutil.UpdateOrCreateToken(client, d, tokenDuration)
+	err = kubeadmutil.UpdateOrCreateToken(client, td, tokenDuration)
 	if err != nil {
 		return err
 	}
-	fmt.Fprintln(out, kubeadmutil.BearerToken(d))
+	fmt.Fprintln(out, kubeadmutil.BearerToken(td))
 
 	return nil
 }
 
 func RunGenerateToken(out io.Writer) error {
-	d := &kubeadmapi.TokenDiscovery{}
+	td := &kubeadmapi.TokenDiscovery{}
-	err := kubeadmutil.GenerateToken(d)
+	err := kubeadmutil.GenerateToken(td)
 	if err != nil {
 		return err
 	}
 
-	fmt.Fprintln(out, kubeadmutil.BearerToken(d))
+	fmt.Fprintln(out, kubeadmutil.BearerToken(td))
 	return nil
 }
 
@@ -219,6 +211,10 @@ func RunListTokens(out io.Writer, errW io.Writer, cmd *cobra.Command) error {
 
 // RunDeleteToken removes a bootstrap token from the server.
 func RunDeleteToken(out io.Writer, cmd *cobra.Command, tokenId string) error {
+	if err := kubeadmutil.ParseTokenID(tokenId); err != nil {
+		return err
+	}
+
 	client, err := kubemaster.CreateClientFromFile(path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, "admin.conf"))
 	if err != nil {
 		return err

cmd/kubeadm/app/cmd/token_test.go

@@ -23,7 +23,7 @@ import (
 )
 
 const (
-	TokenExpectedRegex = "^\\S{6}\\.\\S{16}\n$"
+	TokenExpectedRegex = "^\\S{6}\\:\\S{16}\n$"
 )
 
 func TestRunGenerateToken(t *testing.T) {

cmd/kubeadm/app/discovery/BUILD

@@ -21,6 +21,7 @@ go_library(
         "//cmd/kubeadm/app/discovery/https:go_default_library",
         "//cmd/kubeadm/app/discovery/token:go_default_library",
         "//cmd/kubeadm/app/node:go_default_library",
+        "//cmd/kubeadm/app/util:go_default_library",
         "//pkg/client/unversioned/clientcmd:go_default_library",
         "//pkg/client/unversioned/clientcmd/api:go_default_library",
         "//vendor:github.com/spf13/pflag",

cmd/kubeadm/app/discovery/discovery.go

@@ -23,6 +23,7 @@ import (
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubenode "k8s.io/kubernetes/cmd/kubeadm/app/node"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd"
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 )
@@ -64,10 +65,15 @@ func runHTTPSDiscovery(hd *kubeadmapi.HTTPSDiscovery) (*clientcmdapi.Config, err
 
 // runTokenDiscovery executes token-based discovery.
 func runTokenDiscovery(td *kubeadmapi.TokenDiscovery) (*clientcmdapi.Config, error) {
+	if valid, err := kubeadmutil.ValidateToken(td); valid == false {
+		return nil, err
+	}
+
 	clusterInfo, err := kubenode.RetrieveTrustedClusterInfo(td)
 	if err != nil {
 		return nil, err
 	}
+
 	cfg, err := kubenode.EstablishMasterConnection(td, clusterInfo)
 	if err != nil {
 		return nil, err

cmd/kubeadm/app/master/BUILD

@@ -38,7 +38,6 @@ go_library(
         "//vendor:github.com/blang/semver",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
-        "//vendor:k8s.io/apimachinery/pkg/util/net",
         "//vendor:k8s.io/apimachinery/pkg/util/wait",
     ],
 )
@@ -50,13 +49,11 @@ go_test(
         "apiclient_test.go",
         "discovery_test.go",
         "manifests_test.go",
-        "tokens_test.go",
     ],
     library = ":go_default_library",
     tags = ["automanaged"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
-        "//cmd/kubeadm/app/util:go_default_library",
         "//pkg/api/v1:go_default_library",
         "//pkg/util/intstr:go_default_library",
     ],

cmd/kubeadm/app/master/tokens.go

@@ -21,30 +21,12 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"strconv"
 
-	netutil "k8s.io/apimachinery/pkg/util/net"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/uuid"
 )
 
-func PrepareTokenDiscovery(d *kubeadmapi.TokenDiscovery) error {
-	if len(d.Addresses) == 0 {
-		ip, err := netutil.ChooseHostInterface()
-		if err != nil {
-			return err
-		}
-		d.Addresses = []string{ip.String() + ":" + strconv.Itoa(kubeadmapiext.DefaultDiscoveryBindPort)}
-	}
-	if err := kubeadmutil.GenerateTokenIfNeeded(d); err != nil {
-		return fmt.Errorf("failed to generate token(s) [%v]", err)
-	}
-	return nil
-}
-
 func CreateTokenAuthFile(bt string) error {
 	tokenAuthFilePath := path.Join(kubeadmapi.GlobalEnvParams.HostPKIPath, "tokens.csv")
 	if err := os.MkdirAll(kubeadmapi.GlobalEnvParams.HostPKIPath, 0700); err != nil {

cmd/kubeadm/app/master/tokens_test.go

@@ -1,61 +0,0 @@
-/*
-Copyright 2016 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 master
-
-import (
-	"testing"
-
-	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-)
-
-func TestValidTokenPopulatesSecrets(t *testing.T) {
-	t.Run("provided", func(t *testing.T) {
-		expectedID := "123456"
-		expectedSecret := "0123456789abcdef"
-		s := &kubeadmapi.TokenDiscovery{
-			ID:     expectedID,
-			Secret: expectedSecret,
-		}
-
-		err := kubeadmutil.GenerateTokenIfNeeded(s)
-		if err != nil {
-			t.Errorf("GenerateTokenIfNeeded gave an error for a valid token: %v", err)
-		}
-		if s.ID != expectedID {
-			t.Errorf("GenerateTokenIfNeeded did not populate the TokenID correctly; expected [%s] but got [%s]", expectedID, s.ID)
-		}
-		if s.Secret != expectedSecret {
-			t.Errorf("GenerateTokenIfNeeded did not populate the Token correctly; expected %v but got %v", expectedSecret, s.Secret)
-		}
-	})
-
-	t.Run("not provided", func(t *testing.T) {
-		s := &kubeadmapi.TokenDiscovery{}
-
-		err := kubeadmutil.GenerateTokenIfNeeded(s)
-		if err != nil {
-			t.Errorf("GenerateTokenIfNeeded gave an error for a valid token: %v", err)
-		}
-		if s.ID == "" {
-			t.Errorf("GenerateTokenIfNeeded did not populate the TokenID correctly; expected ID to be non-empty")
-		}
-		if s.Secret == "" {
-			t.Errorf("GenerateTokenIfNeeded did not populate the Token correctly; expected Secret to be non-empty")
-		}
-	})
-}

cmd/kubeadm/app/util/tokens.go

@@ -36,13 +36,20 @@ import (
 
 const (
 	TokenIDBytes               = 3
-	TokenBytes                 = 8
+	TokenSecretBytes           = 8
 	BootstrapTokenSecretPrefix = "bootstrap-token-"
 	DefaultTokenDuration       = time.Duration(8) * time.Hour
 	tokenCreateRetries         = 5
 )
 
-func RandBytes(length int) (string, error) {
+var (
+	tokenIDRegexpString = "^([a-z0-9]{6})$"
+	tokenIDRegexp       = regexp.MustCompile(tokenIDRegexpString)
+	tokenRegexpString   = "^([a-z0-9]{6})\\:([a-z0-9]{16})$"
+	tokenRegexp         = regexp.MustCompile(tokenRegexpString)
+)
+
+func randBytes(length int) (string, error) {
 	b := make([]byte, length)
 	_, err := rand.Read(b)
 	if err != nil {
@@ -51,60 +58,55 @@ func RandBytes(length int) (string, error) {
 	return hex.EncodeToString(b), nil
 }
 
+// GenerateToken generates a new token with a token ID that is valid as a
+// Kubernetes DNS label.
+// For more info, see kubernetes/pkg/util/validation/validation.go.
 func GenerateToken(d *kubeadmapi.TokenDiscovery) error {
-	tokenID, err := RandBytes(TokenIDBytes)
+	tokenID, err := randBytes(TokenIDBytes)
 	if err != nil {
 		return err
 	}
 
-	token, err := RandBytes(TokenBytes)
+	token, err := randBytes(TokenSecretBytes)
 	if err != nil {
 		return err
 	}
 
-	d.ID = tokenID
+	d.ID = strings.ToLower(tokenID)
-	d.Secret = token
+	d.Secret = strings.ToLower(token)
 	return nil
 }
 
-var (
+// ParseTokenID tries and parse a valid token ID from a string.
-	tokenRegexpString = "^([a-zA-Z0-9]{6})\\.([a-zA-Z0-9]{16})$"
+// An error is returned in case of failure.
-	tokenRegexp       = regexp.MustCompile(tokenRegexpString)
+func ParseTokenID(s string) error {
-)
+	if !tokenIDRegexp.MatchString(s) {
-
+		return fmt.Errorf("token ID [%q] was not of form [%q]", s, tokenIDRegexpString)
-func GenerateTokenIfNeeded(d *kubeadmapi.TokenDiscovery) error {
-	ok, err := IsTokenValid(d)
-	if err != nil {
-		return err
 	}
-	if ok {
-		return nil
-	}
-	if err := GenerateToken(d); err != nil {
-		return err
-	}
-
 	return nil
+
 }
 
+// ParseToken tries and parse a valid token from a string.
+// A token ID and token secret are returned in case of success, an error otherwise.
 func ParseToken(s string) (string, string, error) {
 	split := tokenRegexp.FindStringSubmatch(s)
 	if len(split) != 3 {
-		return "", "", fmt.Errorf("token %q was not of form %q", s, tokenRegexpString)
+		return "", "", fmt.Errorf("token [%q] was not of form [%q]", s, tokenRegexpString)
 	}
 	return split[1], split[2], nil
 
 }
 
+// BearerToken returns a string representation of the passed token.
 func BearerToken(d *kubeadmapi.TokenDiscovery) string {
-	return fmt.Sprintf("%s.%s", d.ID, d.Secret)
+	return fmt.Sprintf("%s:%s", d.ID, d.Secret)
 }
 
-func IsTokenValid(d *kubeadmapi.TokenDiscovery) (bool, error) {
+// ValidateToken validates whether a token is well-formed.
-	if len(d.ID)+len(d.Secret) == 0 {
+// In case it's not, the corresponding error is returned as well.
-		return false, nil
+func ValidateToken(d *kubeadmapi.TokenDiscovery) (bool, error) {
-	}
+	if _, _, err := ParseToken(d.ID + ":" + d.Secret); err != nil {
-	if _, _, err := ParseToken(d.ID + "." + d.Secret); err != nil {
 		return false, err
 	}
 	return true, nil
@@ -127,8 +129,11 @@ func DiscoveryPort(d *kubeadmapi.TokenDiscovery) int32 {
 // UpdateOrCreateToken attempts to update a token with the given ID, or create if it does
 // not already exist.
 func UpdateOrCreateToken(client *clientset.Clientset, d *kubeadmapi.TokenDiscovery, tokenDuration time.Duration) error {
+	// Let's make sure
+	if valid, err := ValidateToken(d); !valid {
+		return err
+	}
 	secretName := fmt.Sprintf("%s%s", BootstrapTokenSecretPrefix, d.ID)
-
 	var lastErr error
 	for i := 0; i < tokenCreateRetries; i++ {
 		secret, err := client.Secrets(api.NamespaceSystem).Get(secretName, metav1.GetOptions{})
@@ -162,7 +167,11 @@ func UpdateOrCreateToken(client *clientset.Clientset, d *kubeadmapi.TokenDiscove
 		}
 
 	}
-	return fmt.Errorf("<util/tokens> unable to create bootstrap token after %d attempts [%v]", tokenCreateRetries, lastErr)
+	return fmt.Errorf(
+		"unable to create bootstrap token after %d attempts [%v]",
+		tokenCreateRetries,
+		lastErr,
+	)
 }
 
 func encodeTokenSecretData(d *kubeadmapi.TokenDiscovery, duration time.Duration) map[string][]byte {

cmd/kubeadm/app/util/tokens_test.go

@@ -22,30 +22,103 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
 
-func TestTokenParseErrors(t *testing.T) {
+func TestTokenParse(t *testing.T) {
 	invalidTokens := []string{
+		// invalid parcel size
 		"1234567890123456789012",
-		"12345.1234567890123456",
+		"12345:1234567890123456",
 		".1234567890123456",
-		"123456.1234567890.123456",
+		// invalid separation
+		"123456:1234567890.123456",
+		"abcdef.1234567890123456",
+		// invalid token id
+		"Abcdef:1234567890123456",
+		// invalid token secret
+		"123456:AABBCCDDEEFFGGHH",
 	}
 
 	for _, token := range invalidTokens {
 		if _, _, err := ParseToken(token); err == nil {
-			t.Errorf("generateTokenIfNeeded did not return an error for this invalid token: [%s]", token)
+			t.Errorf("ParseToken did not return an error for this invalid token: [%s]", token)
+		}
+	}
+
+	validTokens := []string{
+		"abcdef:1234567890123456",
+		"123456:aabbccddeeffgghh",
+	}
+
+	for _, token := range validTokens {
+		if _, _, err := ParseToken(token); err != nil {
+			t.Errorf("ParseToken returned an error for this valid token: [%s]", token)
+		}
+	}
+}
+
+func TestParseTokenID(t *testing.T) {
+	invalidTokenIDs := []string{
+		"",
+		"1234567890123456789012",
+		"12345",
+		"Abcdef",
+	}
+
+	for _, tokenID := range invalidTokenIDs {
+		if err := ParseTokenID(tokenID); err == nil {
+			t.Errorf("ParseTokenID did not return an error for this invalid token ID: [%q]", tokenID)
+		}
+	}
+
+	validTokens := []string{
+		"abcdef",
+		"123456",
+	}
+
+	for _, tokenID := range validTokens {
+		if err := ParseTokenID(tokenID); err != nil {
+			t.Errorf("ParseTokenID failed for a valid token ID [%q], err: %+v", tokenID, err)
+		}
+	}
+}
+
+func TestValidateToken(t *testing.T) {
+	invalidTokens := []*kubeadmapi.TokenDiscovery{
+		{ID: "", Secret: ""},
+		{ID: "1234567890123456789012", Secret: ""},
+		{ID: "", Secret: "1234567890123456789012"},
+		{ID: "12345", Secret: "1234567890123456"},
+		{ID: "Abcdef", Secret: "1234567890123456"},
+		{ID: "123456", Secret: "AABBCCDDEEFFGGHH"},
+	}
+
+	for _, token := range invalidTokens {
+		if valid, err := ValidateToken(token); valid == true || err == nil {
+			t.Errorf("ValidateToken did not return an error for this invalid token: [%s]", token)
+		}
+	}
+
+	validTokens := []*kubeadmapi.TokenDiscovery{
+		{ID: "abcdef", Secret: "1234567890123456"},
+		{ID: "123456", Secret: "aabbccddeeffgghh"},
+	}
+
+	for _, token := range validTokens {
+		if valid, err := ValidateToken(token); valid == false || err != nil {
+			t.Errorf("ValidateToken failed for a valid token [%s], valid: %t, err: %+v", token, valid, err)
 		}
 	}
 }
 
 func TestGenerateToken(t *testing.T) {
-	var cfg kubeadmapi.TokenDiscovery
+	td := &kubeadmapi.TokenDiscovery{}
-
+	if err := GenerateToken(td); err != nil {
-	GenerateToken(&cfg)
+		t.Fatalf("GenerateToken returned an unexpected error: %+v", err)
-	if len(cfg.ID) != 6 {
-		t.Errorf("failed GenerateToken first part length:\n\texpected: 6\n\t  actual: %d", len(cfg.ID))
 	}
-	if len(cfg.Secret) != 16 {
+	if len(td.ID) != 6 {
-		t.Errorf("failed GenerateToken first part length:\n\texpected: 16\n\t  actual: %d", len(cfg.Secret))
+		t.Errorf("failed GenerateToken first part length:\n\texpected: 6\n\t  actual: %d", len(td.ID))
+	}
+	if len(td.Secret) != 16 {
+		t.Errorf("failed GenerateToken second part length:\n\texpected: 16\n\t  actual: %d", len(td.Secret))
 	}
 }
 
@@ -59,12 +132,12 @@ func TestRandBytes(t *testing.T) {
 	}
 
 	for _, rt := range randTest {
-		actual, err := RandBytes(rt)
+		actual, err := randBytes(rt)
 		if err != nil {
-			t.Errorf("failed RandBytes: %v", err)
+			t.Errorf("failed randBytes: %v", err)
 		}
 		if len(actual) != rt*2 {
-			t.Errorf("failed RandBytes:\n\texpected: %d\n\t  actual: %d\n", rt*2, len(actual))
+			t.Errorf("failed randBytes:\n\texpected: %d\n\t  actual: %d\n", rt*2, len(actual))
 		}
 	}
 }

cmd/kubeadm/test/token_test.go

@@ -25,7 +25,7 @@ import (
 )
 
 const (
-	TokenExpectedRegex = "^\\S{6}\\.\\S{16}\n$"
+	TokenExpectedRegex = "^\\S{6}\\:\\S{16}\n$"
 )
 
 var kubeadmPath string
@@ -37,7 +37,7 @@ func init() {
 func TestCmdTokenGenerate(t *testing.T) {
 	stdout, _, err := RunCmd(kubeadmPath, "ex", "token", "generate")
 	if err != nil {
-		t.Errorf("'kubeadm ex token generate' exited uncleanly: %v", err)
+		t.Fatalf("'kubeadm ex token generate' exited uncleanly: %v", err)
 	}
 
 	matched, err := regexp.MatchString(TokenExpectedRegex, stdout)
@@ -45,7 +45,7 @@ func TestCmdTokenGenerate(t *testing.T) {
 		t.Fatalf("encountered an error while trying to match 'kubeadm ex token generate' stdout: %v", err)
 	}
 	if !matched {
-		t.Errorf("'kubeadm ex token generate' stdout did not match expected regex; wanted: [%s], got: [%s]", TokenExpectedRegex, stdout)
+		t.Errorf("'kubeadm ex token generate' stdout did not match expected regex; wanted: [%q], got: [%s]", TokenExpectedRegex, stdout)
 	}
 }