Skip to content

Commit

Permalink
Fix bug looking for boot token in StringData field
Browse files Browse the repository at this point in the history
Previously, we would re-use an existing bootstrap token if the Secret
already existed, by looking for the token in the StringData field of the
Secret.

Unfortunately, the StringData field of a Kubernetes Secret is only used as a
convenience when writing a Kubernetes Secret. When reading the secret it
is not populated and is converted to a []byte and moved to the Data
field. This conversion happens in a real Kubernetes cluster but not with
the testing 'fakeclient', hence not being caught by our tests.

This change removes all use of StringData to avoid confusion.
  • Loading branch information
lkysow committed Oct 10, 2019
1 parent b94fe58 commit 4208b33
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 22 deletions.
24 changes: 12 additions & 12 deletions subcommand/server-acl-init/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@ func (c *Command) Run(args []string) int {
consulClient, err = api.NewClient(&api.Config{
Address: firstServerAddr,
Scheme: "http",
Token: bootstrapToken,
Token: string(bootstrapToken),
})
if err != nil {
c.UI.Error(fmt.Sprintf("Error creating Consul client for addr %q: %s", firstServerAddr, err))
return 1
}

// Create new tokens for each server and apply them.
if err := c.setServerTokens(logger, consulClient, serverPods, serverPodAddrs, bootstrapToken); err != nil {
if err := c.setServerTokens(logger, consulClient, serverPods, serverPodAddrs, string(bootstrapToken)); err != nil {
c.UI.Error(err.Error())
return 1
}
Expand Down Expand Up @@ -247,15 +247,15 @@ func (c *Command) getConsulServers(logger hclog.Logger) *apiv1.PodList {
return serverPods
}

func (c *Command) bootstrapACLs(logger hclog.Logger, consulClient *api.Client) (string, error) {
func (c *Command) bootstrapACLs(logger hclog.Logger, consulClient *api.Client) ([]byte, error) {
// Bootstrap the ACLs unless already bootstrapped.
alreadyBootstrapped := false
var bootstrapToken string
var bootstrapToken []byte
c.untilSucceeds("bootstrapping ACLs - PUT /v1/acl/bootstrap",
func() error {
bootstrapResp, _, err := consulClient.ACL().Bootstrap()
if err == nil {
bootstrapToken = bootstrapResp.SecretID
bootstrapToken = []byte(bootstrapResp.SecretID)
return nil
}

Expand All @@ -280,16 +280,16 @@ func (c *Command) bootstrapACLs(logger hclog.Logger, consulClient *api.Client) (
secret, err := c.clientset.CoreV1().Secrets(c.flagNamespace).Get(bootTokenSecretName, metav1.GetOptions{})
if err != nil {
if k8serrors.IsNotFound(err) {
return "", fmt.Errorf("Bootstrap token secret %q was not found."+
return nil, fmt.Errorf("Bootstrap token secret %q was not found."+
" We can't proceed because the bootstrap token is lost."+
" You must reset ACLs.", bootTokenSecretName)
}
return "", fmt.Errorf("Error getting bootstrap token Secret %q: %s", bootTokenSecretName, err)
return nil, fmt.Errorf("Error getting bootstrap token Secret %q: %s", bootTokenSecretName, err)
}
var ok bool
bootstrapToken, ok = secret.StringData["token"]
bootstrapToken, ok = secret.Data["token"]
if !ok {
return "", fmt.Errorf("Secret %q does not have data key 'token'", bootTokenSecretName)
return nil, fmt.Errorf("Secret %q does not have data key 'token'", bootTokenSecretName)
}
logger.Info(fmt.Sprintf("Got bootstrap token from Secret %q", bootTokenSecretName))
} else {
Expand All @@ -300,7 +300,7 @@ func (c *Command) bootstrapACLs(logger hclog.Logger, consulClient *api.Client) (
ObjectMeta: metav1.ObjectMeta{
Name: bootTokenSecretName,
},
StringData: map[string]string{
Data: map[string][]byte{
"token": bootstrapToken,
},
}
Expand Down Expand Up @@ -417,8 +417,8 @@ func (c *Command) createACL(name, rules string, consulClient *api.Client, logger
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-consul-%s-acl-token", c.flagReleaseName, name),
},
StringData: map[string]string{
"token": token,
Data: map[string][]byte{
"token": []byte(token),
},
}
_, err := c.clientset.CoreV1().Secrets(c.flagNamespace).Create(secret)
Expand Down
20 changes: 10 additions & 10 deletions subcommand/server-acl-init/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ func TestRun_Tokens(t *testing.T) {
tokenSecret, err := k8s.CoreV1().Secrets(ns).Get(fmt.Sprintf("%s-consul-%s-acl-token", releaseName, c.TokenName), metav1.GetOptions{})
require.NoError(err)
require.NotNil(tokenSecret)
token, ok := tokenSecret.StringData["token"]
token, ok := tokenSecret.Data["token"]
require.True(ok)

// Test that the token has the expected policies in Consul.
tokenData, _, err := consul.ACL().TokenReadSelf(&api.QueryOptions{Token: token})
tokenData, _, err := consul.ACL().TokenReadSelf(&api.QueryOptions{Token: string(token)})
require.NoError(err)
require.Equal(c.TokenName+"-token", tokenData.Policies[0].Name)
})
Expand Down Expand Up @@ -659,8 +659,8 @@ func TestRun_AlreadyBootstrapped(t *testing.T) {
ObjectMeta: v12.ObjectMeta{
Name: releaseName + "-consul-bootstrap-acl-token",
},
StringData: map[string]string{
"token": "bootstrap-token",
Data: map[string][]byte{
"token": []byte("bootstrap-token"),
},
})
require.NoError(err)
Expand Down Expand Up @@ -894,8 +894,8 @@ func TestRun_BootstrapTokenExists(t *testing.T) {
ObjectMeta: v12.ObjectMeta{
Name: releaseName + "-consul-bootstrap-acl-token",
},
StringData: map[string]string{
"token": "old-token",
Data: map[string][]byte{
"token": []byte("old-token"),
},
})
require.NoError(err)
Expand All @@ -917,8 +917,8 @@ func TestRun_BootstrapTokenExists(t *testing.T) {
// Test that the Secret was updated.
secret, err := k8s.CoreV1().Secrets(ns).Get(releaseName+"-consul-bootstrap-acl-token", metav1.GetOptions{})
require.NoError(err)
require.Contains(secret.StringData, "token")
require.NotEqual("old-token", secret.StringData["token"])
require.Contains(secret.Data, "token")
require.NotEqual("old-token", string(secret.Data["token"]))

// Test that the expected API calls were made.
require.Equal([]APICall{
Expand Down Expand Up @@ -955,9 +955,9 @@ func getBootToken(t *testing.T, k8s *fake.Clientset, releaseName string) string
bootstrapSecret, err := k8s.CoreV1().Secrets(ns).Get(fmt.Sprintf("%s-consul-bootstrap-acl-token", releaseName), metav1.GetOptions{})
require.NoError(t, err)
require.NotNil(t, bootstrapSecret)
bootToken, ok := bootstrapSecret.StringData["token"]
bootToken, ok := bootstrapSecret.Data["token"]
require.True(t, ok)
return bootToken
return string(bootToken)
}

var serviceAccountCACert = "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURDekNDQWZPZ0F3SUJBZ0lRS3pzN05qbDlIczZYYzhFWG91MjVoekFOQmdrcWhraUc5dzBCQVFzRkFEQXYKTVMwd0t3WURWUVFERXlRMU9XVTJaR00wTVMweU1EaG1MVFF3T1RVdFlUSTRPUzB4Wm1NM01EQmhZekZqWXpndwpIaGNOTVRrd05qQTNNVEF4TnpNeFdoY05NalF3TmpBMU1URXhOek14V2pBdk1TMHdLd1lEVlFRREV5UTFPV1UyClpHTTBNUzB5TURobUxUUXdPVFV0WVRJNE9TMHhabU0zTURCaFl6RmpZemd3Z2dFaU1BMEdDU3FHU0liM0RRRUIKQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURaakh6d3FvZnpUcEdwYzBNZElDUzdldXZmdWpVS0UzUEMvYXBmREFnQgo0anpFRktBNzgvOStLVUd3L2MvMFNIZVNRaE4rYThnd2xIUm5BejFOSmNmT0lYeTRkd2VVdU9rQWlGeEg4cGh0CkVDd2tlTk83ejhEb1Y4Y2VtaW5DUkhHamFSbW9NeHBaN2cycFpBSk5aZVB4aTN5MWFOa0ZBWGU5Z1NVU2RqUloKUlhZa2E3d2gyQU85azJkbEdGQVlCK3Qzdld3SjZ0d2pHMFR0S1FyaFlNOU9kMS9vTjBFMDFMekJjWnV4a04xawo4Z2ZJSHk3Yk9GQ0JNMldURURXLzBhQXZjQVByTzhETHFESis2TWpjM3I3K3psemw4YVFzcGIwUzA4cFZ6a2k1CkR6Ly84M2t5dTBwaEp1aWo1ZUI4OFY3VWZQWHhYRi9FdFY2ZnZyTDdNTjRmQWdNQkFBR2pJekFoTUE0R0ExVWQKRHdFQi93UUVBd0lDQkRBUEJnTlZIUk1CQWY4RUJUQURBUUgvTUEwR0NTcUdTSWIzRFFFQkN3VUFBNElCQVFCdgpRc2FHNnFsY2FSa3RKMHpHaHh4SjUyTm5SVjJHY0lZUGVOM1p2MlZYZTNNTDNWZDZHMzJQVjdsSU9oangzS21BCi91TWg2TmhxQnpzZWtrVHowUHVDM3dKeU0yT0dvblZRaXNGbHF4OXNGUTNmVTJtSUdYQ2Ezd0M4ZS9xUDhCSFMKdzcvVmVBN2x6bWozVFFSRS9XMFUwWkdlb0F4bjliNkp0VDBpTXVjWXZQMGhYS1RQQldsbnpJaWphbVU1MHIyWQo3aWEwNjVVZzJ4VU41RkxYL3Z4T0EzeTRyanBraldvVlFjdTFwOFRaclZvTTNkc0dGV3AxMGZETVJpQUhUdk9ICloyM2pHdWs2cm45RFVIQzJ4UGozd0NUbWQ4U0dFSm9WMzFub0pWNWRWZVE5MHd1c1h6M3ZURzdmaWNLbnZIRlMKeHRyNVBTd0gxRHVzWWZWYUdIMk8KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo="
Expand Down

0 comments on commit 4208b33

Please sign in to comment.