Merge pull request #41754 from jbeda/bootstrap-secret-name

Automatic merge from submit-queue (batch tested with PRs 41709, 41685, 41754, 41759, 37237)

Ignore Bootstrap Token secrets that don't use predictable names.

cmd/kubeadm/app/cmd/token.go

@@ -227,7 +227,7 @@ func RunDeleteToken(out io.Writer, cmd *cobra.Command, tokenId string) error {
 		return err
 	}
 
-	tokenSecretName := fmt.Sprintf("%s%s", kubeadmconstants.BootstrapTokenSecretPrefix, tokenId)
+	tokenSecretName := fmt.Sprintf("%s%s", bootstrapapi.BootstrapTokenSecretPrefix, tokenId)
 	if err := client.Secrets(metav1.NamespaceSystem).Delete(tokenSecretName, nil); err != nil {
 		return fmt.Errorf("failed to delete bootstrap token [%v]", err)
 	}

cmd/kubeadm/app/constants/constants.go

@@ -73,8 +73,6 @@ const (
 
 	// DefaultTokenDuration specifies the default amount of time that a bootstrap token will be valid
 	DefaultTokenDuration = time.Duration(8) * time.Hour
-	// BootstrapTokenSecretPrefix the the prefix that will be used for the Secrets that are created as type bootstrap.kubernetes.io/token
-	BootstrapTokenSecretPrefix = "bootstrap-token-"
 
 	// CSVTokenBootstrapUser is currently the user the bootstrap token in the .csv file
 	// TODO: This should change to something more official and supported

cmd/kubeadm/app/phases/token/bootstrap.go

@@ -25,7 +25,6 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/pkg/api/v1"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	tokenutil "k8s.io/kubernetes/cmd/kubeadm/app/util/token"
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
 )
@@ -41,7 +40,7 @@ func UpdateOrCreateToken(client *clientset.Clientset, d *kubeadmapi.TokenDiscove
 	if valid, err := tokenutil.ValidateToken(d); !valid {
 		return err
 	}
-	secretName := fmt.Sprintf("%s%s", kubeadmconstants.BootstrapTokenSecretPrefix, d.ID)
+	secretName := fmt.Sprintf("%s%s", bootstrapapi.BootstrapTokenSecretPrefix, d.ID)
 	var lastErr error
 	for i := 0; i < tokenCreateRetries; i++ {
 		secret, err := client.Secrets(metav1.NamespaceSystem).Get(secretName, metav1.GetOptions{})

pkg/bootstrap/api/types.go

@@ -21,6 +21,12 @@ import (
 )
 
 const (
+	// BootstrapTokenSecretPrefix is the prefix for bootstrap token names.
+	// Bootstrap tokens secrets must be named in the form
+	// `bootstrap-token-<token-id>`.  This is the prefix to be used before the
+	// token ID.
+	BootstrapTokenSecretPrefix = "bootstrap-token-"
+
 	// SecretTypeBootstrapToken is used during the automated bootstrap process (first
 	// implemented by kubeadm). It stores tokens that are used to sign well known
 	// ConfigMaps. They may also eventually be used for authentication.

pkg/controller/bootstrap/BUILD

@@ -22,6 +22,7 @@ go_test(
     deps = [
         "//pkg/bootstrap/api:go_default_library",
         "//vendor:github.com/davecgh/go-spew/spew",
+        "//vendor:github.com/stretchr/testify/assert",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
         "//vendor:k8s.io/client-go/kubernetes/fake",

pkg/controller/bootstrap/bootstrapsigner.go

@@ -277,7 +277,9 @@ func (e *BootstrapSigner) getTokens() map[string]string {
 
 		// Check and warn for duplicate secrets. Behavior here will be undefined.
 		if _, ok := ret[tokenID]; ok {
-			glog.V(3).Infof("Duplicate bootstrap tokens found for id %s, ignoring on in %s/%s", tokenID, secret.Namespace, secret.Name)
+			// This should never happen as we ensure a consistent secret name.
+			// But leave this in here just in case.
+			glog.V(1).Infof("Duplicate bootstrap tokens found for id %s, ignoring on in %s/%s", tokenID, secret.Namespace, secret.Name)
 			continue
 		}
 

pkg/controller/bootstrap/bootstrapsigner_test.go

@@ -34,6 +34,8 @@ func init() {
 	spew.Config.DisableMethods = true
 }
 
+const testTokenID = "abc123"
+
 func newBootstrapSigner() (*BootstrapSigner, *fake.Clientset) {
 	options := DefaultBootstrapSignerOptions()
 	cl := fake.NewSimpleClientset()
@@ -69,7 +71,7 @@ func TestSimpleSign(t *testing.T) {
 	cm := newConfigMap("", "")
 	signer.configMaps.Add(cm)
 
-	secret := newTokenSecret("tokenID", "tokenSecret")
+	secret := newTokenSecret(testTokenID, "tokenSecret")
 	addSecretSigningUsage(secret, "true")
 	signer.secrets.Add(secret)
 
@@ -78,7 +80,7 @@ func TestSimpleSign(t *testing.T) {
 	expected := []core.Action{
 		core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "configmaps"},
 			api.NamespacePublic,
-			newConfigMap("tokenID", "eyJhbGciOiJIUzI1NiIsImtpZCI6InRva2VuSUQifQ..QAvK9DAjF0hSyASEkH1MOTB5rJMmbWEY9j-z1NSYILE")),
+			newConfigMap(testTokenID, "eyJhbGciOiJIUzI1NiIsImtpZCI6ImFiYzEyMyJ9..QSxpUG7Q542CirTI2ECPSZjvBOJURUW5a7XqFpNI958")),
 	}
 
 	verifyActions(t, expected, cl.Actions())
@@ -87,10 +89,10 @@ func TestSimpleSign(t *testing.T) {
 func TestNoSignNeeded(t *testing.T) {
 	signer, cl := newBootstrapSigner()
 
-	cm := newConfigMap("tokenID", "eyJhbGciOiJIUzI1NiIsImtpZCI6InRva2VuSUQifQ..QAvK9DAjF0hSyASEkH1MOTB5rJMmbWEY9j-z1NSYILE")
+	cm := newConfigMap(testTokenID, "eyJhbGciOiJIUzI1NiIsImtpZCI6ImFiYzEyMyJ9..QSxpUG7Q542CirTI2ECPSZjvBOJURUW5a7XqFpNI958")
 	signer.configMaps.Add(cm)
 
-	secret := newTokenSecret("tokenID", "tokenSecret")
+	secret := newTokenSecret(testTokenID, "tokenSecret")
 	addSecretSigningUsage(secret, "true")
 	signer.secrets.Add(secret)
 
@@ -102,10 +104,10 @@ func TestNoSignNeeded(t *testing.T) {
 func TestUpdateSignature(t *testing.T) {
 	signer, cl := newBootstrapSigner()
 
-	cm := newConfigMap("tokenID", "old signature")
+	cm := newConfigMap(testTokenID, "old signature")
 	signer.configMaps.Add(cm)
 
-	secret := newTokenSecret("tokenID", "tokenSecret")
+	secret := newTokenSecret(testTokenID, "tokenSecret")
 	addSecretSigningUsage(secret, "true")
 	signer.secrets.Add(secret)
 
@@ -114,7 +116,7 @@ func TestUpdateSignature(t *testing.T) {
 	expected := []core.Action{
 		core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "configmaps"},
 			api.NamespacePublic,
-			newConfigMap("tokenID", "eyJhbGciOiJIUzI1NiIsImtpZCI6InRva2VuSUQifQ..QAvK9DAjF0hSyASEkH1MOTB5rJMmbWEY9j-z1NSYILE")),
+			newConfigMap(testTokenID, "eyJhbGciOiJIUzI1NiIsImtpZCI6ImFiYzEyMyJ9..QSxpUG7Q542CirTI2ECPSZjvBOJURUW5a7XqFpNI958")),
 	}
 
 	verifyActions(t, expected, cl.Actions())
@@ -123,7 +125,7 @@ func TestUpdateSignature(t *testing.T) {
 func TestRemoveSignature(t *testing.T) {
 	signer, cl := newBootstrapSigner()
 
-	cm := newConfigMap("tokenID", "old signature")
+	cm := newConfigMap(testTokenID, "old signature")
 	signer.configMaps.Add(cm)
 
 	signer.signConfigMap()

pkg/controller/bootstrap/common_test.go

@@ -32,7 +32,7 @@ func newTokenSecret(tokenID, tokenSecret string) *v1.Secret {
 	return &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:       metav1.NamespaceSystem,
-			Name:            "secretName",
+			Name:            bootstrapapi.BootstrapTokenSecretPrefix + tokenID,
 			ResourceVersion: "1",
 		},
 		Type: bootstrapapi.SecretTypeBootstrapToken,

pkg/controller/bootstrap/util.go

@@ -17,6 +17,7 @@ limitations under the License.
 package bootstrap
 
 import (
+	"regexp"
 	"time"
 
 	"github.com/golang/glog"
@@ -25,6 +26,9 @@ import (
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
 )
 
+var namePattern = `^` + regexp.QuoteMeta(bootstrapapi.BootstrapTokenSecretPrefix) + `([a-z0-9]{6})$`
+var nameRegExp = regexp.MustCompile(namePattern)
+
 // getSecretString gets a string value from a secret.  If there is an error or
 // if the key doesn't exist, an empty string is returned.
 func getSecretString(secret *v1.Secret, key string) string {
@@ -36,13 +40,33 @@ func getSecretString(secret *v1.Secret, key string) string {
 	return string(data)
 }
 
+// parseSecretName parses the name of the secret to extract the secret ID.
+func parseSecretName(name string) (secretID string, ok bool) {
+	r := nameRegExp.FindStringSubmatch(name)
+	if r == nil {
+		return "", false
+	}
+	return r[1], true
+}
+
 func validateSecretForSigning(secret *v1.Secret) (tokenID, tokenSecret string, ok bool) {
+	nameTokenID, ok := parseSecretName(secret.Name)
+	if !ok {
+		glog.V(3).Infof("Invalid secret name: %s. Must be of form %s<secret-id>.", secret.Name, bootstrapapi.BootstrapTokenSecretPrefix)
+		return "", "", false
+	}
+
 	tokenID = getSecretString(secret, bootstrapapi.BootstrapTokenIDKey)
 	if len(tokenID) == 0 {
 		glog.V(3).Infof("No %s key in %s/%s Secret", bootstrapapi.BootstrapTokenIDKey, secret.Namespace, secret.Name)
 		return "", "", false
 	}
 
+	if nameTokenID != tokenID {
+		glog.V(3).Infof("Token ID (%s) doesn't match secret name: %s", tokenID, nameTokenID)
+		return "", "", false
+	}
+
 	tokenSecret = getSecretString(secret, bootstrapapi.BootstrapTokenSecretKey)
 	if len(tokenSecret) == 0 {
 		glog.V(3).Infof("No %s key in %s/%s Secret", bootstrapapi.BootstrapTokenSecretKey, secret.Namespace, secret.Name)

pkg/controller/bootstrap/util_test.go

@@ -20,13 +20,16 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/pkg/api/v1"
 	bootstrapapi "k8s.io/kubernetes/pkg/bootstrap/api"
 )
 
 const (
-	givenTokenID     = "tokenID"
+	givenTokenID     = "abc123"
+	givenTokenID2    = "def456"
 	givenTokenSecret = "tokenSecret"
 )
 
@@ -81,7 +84,7 @@ func TestValidateSecretForSigning(t *testing.T) {
 		secret := &v1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace:       metav1.NamespaceSystem,
-				Name:            "secretName",
+				Name:            bootstrapapi.BootstrapTokenSecretPrefix + givenTokenID,
 				ResourceVersion: "1",
 			},
 			Type: bootstrapapi.SecretTypeBootstrapToken,
@@ -113,7 +116,7 @@ func TestValidateSecret(t *testing.T) {
 	secret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:       metav1.NamespaceSystem,
-			Name:            "secretName",
+			Name:            bootstrapapi.BootstrapTokenSecretPrefix + givenTokenID,
 			ResourceVersion: "1",
 		},
 		Type: bootstrapapi.SecretTypeBootstrapToken,
@@ -135,3 +138,69 @@ func TestValidateSecret(t *testing.T) {
 		t.Errorf("Unexpected Token Secret. Expected %q, got %q", givenTokenSecret, tokenSecret)
 	}
 }
+
+func TestBadSecretName(t *testing.T) {
+	secret := &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:       metav1.NamespaceSystem,
+			Name:            givenTokenID,
+			ResourceVersion: "1",
+		},
+		Type: bootstrapapi.SecretTypeBootstrapToken,
+		Data: map[string][]byte{
+			bootstrapapi.BootstrapTokenIDKey:           []byte(givenTokenID),
+			bootstrapapi.BootstrapTokenSecretKey:       []byte(givenTokenSecret),
+			bootstrapapi.BootstrapTokenUsageSigningKey: []byte("true"),
+		},
+	}
+
+	_, _, ok := validateSecretForSigning(secret)
+	if ok {
+		t.Errorf("Token validation should fail with bad name")
+	}
+}
+
+func TestMismatchSecretName(t *testing.T) {
+	secret := &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:       metav1.NamespaceSystem,
+			Name:            bootstrapapi.BootstrapTokenSecretPrefix + givenTokenID2,
+			ResourceVersion: "1",
+		},
+		Type: bootstrapapi.SecretTypeBootstrapToken,
+		Data: map[string][]byte{
+			bootstrapapi.BootstrapTokenIDKey:           []byte(givenTokenID),
+			bootstrapapi.BootstrapTokenSecretKey:       []byte(givenTokenSecret),
+			bootstrapapi.BootstrapTokenUsageSigningKey: []byte("true"),
+		},
+	}
+
+	_, _, ok := validateSecretForSigning(secret)
+	if ok {
+		t.Errorf("Token validation should fail with mismatched name")
+	}
+}
+
+func TestParseSecretName(t *testing.T) {
+	tokenID, ok := parseSecretName("bootstrap-token-abc123")
+	assert.True(t, ok, "parseSecretName should accept valid name")
+	assert.Equal(t, "abc123", tokenID, "parseSecretName should return token ID")
+
+	_, ok = parseSecretName("")
+	assert.False(t, ok, "parseSecretName should reject blank name")
+
+	_, ok = parseSecretName("abc123")
+	assert.False(t, ok, "parseSecretName should reject with no prefix")
+
+	_, ok = parseSecretName("bootstrap-token-")
+	assert.False(t, ok, "parseSecretName should reject no token ID")
+
+	_, ok = parseSecretName("bootstrap-token-abc")
+	assert.False(t, ok, "parseSecretName should reject short token ID")
+
+	_, ok = parseSecretName("bootstrap-token-abc123ghi")
+	assert.False(t, ok, "parseSecretName should reject long token ID")
+
+	_, ok = parseSecretName("bootstrap-token-ABC123")
+	assert.False(t, ok, "parseSecretName should reject invalid token ID")
+}