Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connect: only look for service account secrets when creating/updating auth method #321

Merged
merged 3 commits into from
Sep 10, 2020
Merged

Connect: only look for service account secrets when creating/updating auth method #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3af44be
Connect: only look for service account secrets when creating/updating…
ishustava Sep 8, 2020
e8f465b
Update subcommand/server-acl-init/connect_inject.go
ishustava Sep 10, 2020
cf17c08
Add a test to check the error is returned when sa secret is not found
ishustava Sep 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions subcommand/server-acl-init/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func TestRun_Defaults(t *testing.T) {
clientset: k8s,
}
args := []string{
"-timeout=1m",
ishustava marked this conversation as resolved.
Show resolved Hide resolved
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
"-server-port", strings.Split(testSvr.HTTPAddr, ":")[1],
Expand Down Expand Up @@ -233,6 +234,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 @@ -389,6 +391,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 @@ -516,6 +519,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 @@ -624,6 +628,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 @@ -718,6 +723,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 @@ -788,6 +794,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 @@ -830,6 +837,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 @@ -954,6 +962,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 @@ -1075,6 +1084,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 @@ -1185,6 +1195,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 @@ -1285,6 +1296,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 @@ -1366,6 +1378,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 @@ -1398,10 +1411,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 @@ -1434,6 +1447,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 @@ -1472,6 +1486,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 @@ -1526,6 +1541,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 @@ -1572,6 +1588,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 @@ -1641,6 +1658,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 @@ -1871,13 +1889,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{
{
ishustava marked this conversation as resolved.
Show resolved Hide resolved
Name: resourcePrefix + "-some-other-secret",
},
{
Name: resourcePrefix + "-connect-injector-authmethod-svc-account",
},
Expand All @@ -1903,17 +1927,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
Open in desktop
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)
ishustava marked this conversation as resolved.
Show resolved Hide resolved
}

kubernetesHost := defaultKubernetesHost

Expand Down
66 changes: 66 additions & 0 deletions subcommand/server-acl-init/connect_inject_test.go
View file
Open in desktop
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")
}