Skip to content

Commit

Permalink
Connect: only look for service account secrets when creating/updating…
Browse files Browse the repository at this point in the history 
… auth method (#321)

Kubernetes service account could have multiple secrets associated with it.
Previously, we were only looking at the first secret in the list and using
that secret to create or update the auth method in Consul.
However, on some platforms (Openshift) the service account may contain other types of secrets.
Specifically, in the case of Openshift, there are two secrets: one for the service account token
and the other for docker credentials. This second secret gets injected automatically by Openshift.
This changes our implementation to use the first secret of type kubernetes.io/service-account-token.
  • Loading branch information
@ishustava
ishustava authored Sep 10, 2020
1 parent a7e4a97 commit 9a19fe9
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 14 deletions.
48 changes: 44 additions & 4 deletions subcommand/server-acl-init/command_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func TestRun_Defaults(t *testing.T) {
clientset: k8s,
}
args := []string{
"-timeout=1m",
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
"-server-port", strings.Split(testSvr.HTTPAddr, ":")[1],
Expand Down Expand Up @@ -251,6 +252,7 @@ func TestRun_TokensPrimaryDC(t *testing.T) {
}
cmd.init()
cmdArgs := append([]string{
"-timeout=1m",
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
"-server-port", strings.Split(testSvr.HTTPAddr, ":")[1],
Expand Down Expand Up @@ -407,6 +409,7 @@ func TestRun_TokensReplicatedDC(t *testing.T) {
}
cmd.init()
cmdArgs := append([]string{
"-timeout=1m",
"-k8s-namespace=" + ns,
"-acl-replication-token-file", tokenFile,
"-server-address", strings.Split(secondaryAddr, ":")[0],
Expand Down Expand Up @@ -534,6 +537,7 @@ func TestRun_TokensWithProvidedBootstrapToken(t *testing.T) {
clientset: k8s,
}
cmdArgs := append([]string{
"-timeout=1m",
"-k8s-namespace", ns,
"-bootstrap-token-file", tokenFile,
"-server-address", strings.Split(testAgent.HTTPAddr, ":")[0],
Expand Down Expand Up @@ -642,6 +646,7 @@ func TestRun_AnonymousTokenPolicy(t *testing.T) {
}
cmd.init()
cmdArgs := append([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(consulHTTPAddr, ":")[0],
Expand Down Expand Up @@ -736,6 +741,7 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) {
cmd.init()
bindingRuleSelector := "serviceaccount.name!=default"
cmdArgs := []string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
Expand Down Expand Up @@ -806,6 +812,7 @@ func TestRun_ConnectInjectAuthMethodUpdates(t *testing.T) {

// First, create an auth method using the defaults
responseCode := cmd.Run([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
Expand Down Expand Up @@ -848,6 +855,7 @@ func TestRun_ConnectInjectAuthMethodUpdates(t *testing.T) {

// Run command again
responseCode = cmd.Run([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
Expand Down Expand Up @@ -1049,6 +1057,7 @@ func TestRun_DelayedServers(t *testing.T) {
var responseCode int
go func() {
responseCode = cmd.Run([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address=127.0.0.1",
Expand Down Expand Up @@ -1170,6 +1179,7 @@ func TestRun_NoLeader(t *testing.T) {
var responseCode int
go func() {
responseCode = cmd.Run([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address=" + serverURL.Hostname(),
Expand Down Expand Up @@ -1280,6 +1290,7 @@ func TestRun_ClientTokensRetry(t *testing.T) {
clientset: k8s,
}
responseCode := cmd.Run([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address=" + serverURL.Hostname(),
Expand Down Expand Up @@ -1380,6 +1391,7 @@ func TestRun_AlreadyBootstrapped(t *testing.T) {
}

responseCode := cmd.Run([]string{
"-timeout=500ms",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address=" + serverURL.Hostname(),
Expand Down Expand Up @@ -1461,6 +1473,7 @@ func TestRun_SkipBootstrapping_WhenBootstrapTokenIsProvided(t *testing.T) {
}

responseCode := cmd.Run([]string{
"-timeout=500ms",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address=" + serverURL.Hostname(),
Expand Down Expand Up @@ -1493,10 +1506,10 @@ func TestRun_Timeout(t *testing.T) {
}

responseCode := cmd.Run([]string{
"-timeout=500ms",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address=foo",
"-timeout=500ms",
})
require.Equal(1, responseCode, ui.ErrorWriter.String())
}
Expand Down Expand Up @@ -1529,6 +1542,7 @@ func TestRun_HTTPS(t *testing.T) {
}

responseCode := cmd.Run([]string{
"-timeout=1m",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-use-https",
Expand Down Expand Up @@ -1567,6 +1581,7 @@ func TestRun_ACLReplicationTokenValid(t *testing.T) {
}
secondaryCmd.init()
secondaryCmdArgs := []string{
"-timeout=1m",
"-k8s-namespace=" + ns,
"-server-address", strings.Split(secondaryAddr, ":")[0],
"-server-port", strings.Split(secondaryAddr, ":")[1],
Expand Down Expand Up @@ -1621,6 +1636,7 @@ func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) {
}
cmd.init()
cmdArgs := append([]string{
"-timeout=1m",
"-k8s-namespace=" + ns,
"-acl-replication-token-file", tokenFile,
"-server-address", strings.Split(serverAddr, ":")[0],
Expand Down Expand Up @@ -1667,6 +1683,7 @@ func TestRun_CloudAutoJoin(t *testing.T) {
providers: map[string]discover.Provider{"mock": provider},
}
args := []string{
"-timeout=1m",
"-k8s-namespace=" + ns,
"-resource-prefix=" + resourcePrefix,
"-server-address", "provider=mock",
Expand Down Expand Up @@ -1736,6 +1753,7 @@ func TestRun_GatewayErrors(t *testing.T) {
clientset: k8s,
}
cmdArgs := []string{
"-timeout=500ms",
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
Expand Down Expand Up @@ -1966,13 +1984,19 @@ func setUpK8sServiceAccount(t *testing.T, k8s *fake.Clientset) (string, string)
serviceAccountName := resourcePrefix + "-connect-injector-authmethod-svc-account"
sa, _ := k8s.CoreV1().ServiceAccounts(ns).Get(context.Background(), serviceAccountName, metav1.GetOptions{})
if sa == nil {
// Create a service account that references two secrets.
// The second secret is mimicking the behavior on Openshift,
// where two secrets are injected: one with SA token and one with docker config.
_, err := k8s.CoreV1().ServiceAccounts(ns).Create(
context.Background(),
&v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: serviceAccountName,
},
Secrets: []v1.ObjectReference{
{
Name: resourcePrefix + "-some-other-secret",
},
{
Name: resourcePrefix + "-connect-injector-authmethod-svc-account",
},
Expand All @@ -1998,17 +2022,33 @@ func setUpK8sServiceAccount(t *testing.T, k8s *fake.Clientset) (string, string)
"ca.crt": caCertBytes,
"token": tokenBytes,
},
Type: v1.SecretTypeServiceAccountToken,
}
createOrUpdateSecret(t, k8s, secret)

// Create the second secret of a different type
otherSecret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: resourcePrefix + "-some-other-secret",
},
Data: map[string][]byte{},
Type: v1.SecretTypeDockercfg,
}
existingSecret, _ := k8s.CoreV1().Secrets(ns).Get(context.Background(), secretName, metav1.GetOptions{})
createOrUpdateSecret(t, k8s, otherSecret)

return string(caCertBytes), string(tokenBytes)
}

func createOrUpdateSecret(t *testing.T, k8s *fake.Clientset, secret *v1.Secret) {
existingSecret, _ := k8s.CoreV1().Secrets(ns).Get(context.Background(), secret.Name, metav1.GetOptions{})
var err error
if existingSecret == nil {
_, err = k8s.CoreV1().Secrets(ns).Create(context.Background(), secret, metav1.CreateOptions{})
require.NoError(t, err)
} else {
_, err = k8s.CoreV1().Secrets(ns).Update(context.Background(), secret, metav1.UpdateOptions{})
require.NoError(t, err)
}

return string(caCertBytes), string(tokenBytes)
}

// policyExists asserts that policy with name exists. Returns the policy
Expand Down
32 changes: 22 additions & 10 deletions subcommand/server-acl-init/connect_inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (c *Command) configureConnectInject(consulClient *api.Client) error {
// for this auth method, but none that match the binding
// rule set up here in the bootstrap method.
if abr.ID == "" {
return errors.New("Unable to find a matching ACL binding rule to update")
return errors.New("unable to find a matching ACL binding rule to update")
}

err = c.untilSucceeds(fmt.Sprintf("updating acl binding rule for %s", authMethodName),
Expand Down Expand Up @@ -152,19 +152,31 @@ func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod

// ServiceAccounts always have a secret name. The secret
// contains the JWT token.
saSecretName := authMethodServiceAccount.Secrets[0].Name

// Get the secret that will contain the ServiceAccount JWT token.
// Because there could be multiple secrets attached to the service account,
// we need pick the first one of type "kubernetes.io/service-account-token".
var saSecret *apiv1.Secret
err = c.untilSucceeds(fmt.Sprintf("getting %s Secret", saSecretName),
func() error {
var err error
saSecret, err = c.clientset.CoreV1().Secrets(c.flagK8sNamespace).Get(context.TODO(), saSecretName, metav1.GetOptions{})
return err
})
for _, secretRef := range authMethodServiceAccount.Secrets {
var secret *apiv1.Secret
err = c.untilSucceeds(fmt.Sprintf("getting %s Secret", secretRef.Name),
func() error {
var err error
secret, err = c.clientset.CoreV1().Secrets(c.flagK8sNamespace).Get(context.TODO(), secretRef.Name, metav1.GetOptions{})
return err
})
if secret != nil && secret.Type == apiv1.SecretTypeServiceAccountToken {
saSecret = secret
break
}
}
if err != nil {
return api.ACLAuthMethod{}, err
}
// This is very unlikely to happen because Kubernetes ensure that there is always
// a secret of type ServiceAccountToken.
if saSecret == nil {
return api.ACLAuthMethod{},
fmt.Errorf("found no secret of type 'kubernetes.io/service-account-token' associated with the %s service account", saName)
}

kubernetesHost := defaultKubernetesHost

Expand Down
66 changes: 66 additions & 0 deletions subcommand/server-acl-init/connect_inject_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package serveraclinit

import (
"context"
"testing"

"github.com/hashicorp/go-hclog"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
)

// Test that createAuthMethodTmpl returns an error when
// it cannot find a secret of type kubernetes.io/service-account-token
// associated with the service account.
// Note we are only testing this special error case here because we cannot assert on log output
// from the command.
// Also note that the remainder of this function is tested in the command_test.go.
func TestCommand_createAuthMethodTmpl_SecretNotFound(t *testing.T) {
k8s := fake.NewSimpleClientset()

cmd := &Command{
flagK8sNamespace: ns,
flagResourcePrefix: resourcePrefix,
clientset: k8s,
log: hclog.New(nil),
cmdTimeout: context.TODO(),
}

serviceAccountName := resourcePrefix + "-connect-injector-authmethod-svc-account"
secretName := resourcePrefix + "-connect-injector-authmethod-svc-account"

// Create a service account referencing secretName
sa, _ := k8s.CoreV1().ServiceAccounts(ns).Get(context.Background(), serviceAccountName, metav1.GetOptions{})
if sa == nil {
_, err := k8s.CoreV1().ServiceAccounts(ns).Create(
context.Background(),
&v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: serviceAccountName,
},
Secrets: []v1.ObjectReference{
{
Name: secretName,
},
},
},
metav1.CreateOptions{})
require.NoError(t, err)
}

// Create a secret of non service-account-token type (we're using the opaque type).
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
},
Data: map[string][]byte{},
Type: v1.SecretTypeOpaque,
}
_, err := k8s.CoreV1().Secrets(ns).Create(context.TODO(), secret, metav1.CreateOptions{})
require.NoError(t, err)

_, err = cmd.createAuthMethodTmpl("test")
require.EqualError(t, err, "found no secret of type 'kubernetes.io/service-account-token' associated with the release-name-consul-connect-injector-authmethod-svc-account service account")
}

0 comments on commit 9a19fe9

Please sign in to comment.