Merge pull request #35075 from pipejakob/token_validation_fix

Automatic merge from submit-queue

If token validation fails, give the user the expected format.

If a user specifies their own token to kubeadm, and it fails validation, the error they currently receive isn't the friendliest. This first change adds messaging for the expected token format, with more improvements to follow as part of #33930. It also adds some unit tests to document the behavior we have currently.

CC: @mikedanese, @krousey, @kubernetes/sig-cluster-lifecycle

Part of #33930.

cmd/kubeadm/app/util/tokens.go

@@ -66,9 +66,8 @@ func UseGivenTokenIfValid(s *kubeadmapi.Secrets) (bool, error) {
 	}
 	fmt.Println("<util/tokens> validating provided token")
 	givenToken := strings.Split(strings.ToLower(s.GivenToken), ".")
-	// TODO(phase1+) print desired format
 	// TODO(phase1+) could also print more specific messages in each case
-	invalidErr := "<util/tokens> provided token is invalid - %s"
+	invalidErr := "<util/tokens> provided token does not match expected <6 characters>.<16 characters> format - %s"
 	if len(givenToken) != 2 {
 		return false, fmt.Errorf(invalidErr, "not in 2-part dot-separated format")
 	}

cmd/kubeadm/app/util/tokens_test.go

@@ -0,0 +1,85 @@
+/*
+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 util
+
+import (
+	"bytes"
+	"testing"
+
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+)
+
+func TestUsingEmptyTokenFails(t *testing.T) {
+	// Simulates what happens when you omit --token on the CLI
+	s := newSecretsWithToken("")
+
+	given, err := UseGivenTokenIfValid(s)
+	if err != nil {
+		t.Errorf("UseGivenTokenIfValid returned an error when the token was omitted: %v", err)
+	}
+	if given {
+		t.Error("UseGivenTokenIfValid returned given = true when the token was omitted; expected false")
+	}
+}
+
+func TestTokenValidationFailures(t *testing.T) {
+	invalidTokens := []string{
+		"1234567890123456789012",
+		"12345.1234567890123456",
+		".1234567890123456",
+		"123456.1234567890.123456",
+	}
+
+	for _, token := range invalidTokens {
+		s := newSecretsWithToken(token)
+		_, err := UseGivenTokenIfValid(s)
+
+		if err == nil {
+			t.Errorf("UseGivenTokenIfValid did not return an error for this invalid token: [%s]", token)
+		}
+	}
+}
+
+func TestValidTokenPopulatesSecrets(t *testing.T) {
+	s := newSecretsWithToken("123456.0123456789AbCdEf")
+	expectedToken := []byte("0123456789abcdef")
+	expectedTokenID := "123456"
+	expectedBearerToken := "0123456789abcdef"
+
+	given, err := UseGivenTokenIfValid(s)
+	if err != nil {
+		t.Errorf("UseGivenTokenIfValid gave an error for a valid token: %v", err)
+	}
+	if !given {
+		t.Error("UseGivenTokenIfValid returned given = false when given a valid token")
+	}
+	if s.TokenID != expectedTokenID {
+		t.Errorf("UseGivenTokenIfValid did not populate the TokenID correctly; expected [%s] but got [%s]", expectedTokenID, s.TokenID)
+	}
+	if s.BearerToken != expectedBearerToken {
+		t.Errorf("UseGivenTokenIfValid did not populate the BearerToken correctly; expected [%s] but got [%s]", expectedBearerToken, s.BearerToken)
+	}
+	if !bytes.Equal(s.Token, expectedToken) {
+		t.Errorf("UseGivenTokenIfValid did not populate the Token correctly; expected %v but got %v", expectedToken, s.Token)
+	}
+}
+
+func newSecretsWithToken(token string) *kubeadmapi.Secrets {
+	s := new(kubeadmapi.Secrets)
+	s.GivenToken = token
+	return s
+}