Skip to content

Commit

Permalink
Always update auth method (#282)
Browse files Browse the repository at this point in the history 
* Always update auth method.

Previously, we would only update auth method in Consul
if Consul namespaces are enabled. This was necessary to make sure
that if namespace configuration changes, it is reflected in the auth method.

However, running Consul servers externally is another use case of why it's important
to update the auth method. That is because if you run a Helm install, then uninstall
and install again, the auth method saved in Consul will be out-of-date: it will have
the service account JWT token saved in the auth method refers to the service account token
that has been deleted from Kubernetes when you ran helm uninstall.

This change proposes to always update the auth method to solve for the
external servers use case.
  • Loading branch information
@ishustava
ishustava authored Jun 24, 2020
1 parent 241e3b4 commit df99da1
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 99 deletions.
2 changes: 1 addition & 1 deletion subcommand/server-acl-init/command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (c *Command) init() {
"default namespace, specify the value in the form <GatewayName>.<ConsulNamespace>.")

c.flags.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address",
"The IP, DNS name or the cloud auto-join string of the Consul server(s). If providing IPs or DNS names, may be specified multiple times."+
"The IP, DNS name or the cloud auto-join string of the Consul server(s). If providing IPs or DNS names, may be specified multiple times. "+
"At least one value is required.")
c.flags.UintVar(&c.flagServerPort, "server-port", 8500, "The HTTP or HTTPS port of the Consul server. Defaults to 8500.")
c.flags.StringVar(&c.flagConsulCACert, "consul-ca-cert", "",
Expand Down
150 changes: 118 additions & 32 deletions subcommand/server-acl-init/command_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,8 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) {
t.Parallel()

cases := map[string]struct {
flags []string
expectedHost string
expectedCACert string
flags []string
expectedHost string
}{
"-create-inject-token flag": {
flags: []string{"-create-inject-token"},
Expand All @@ -707,9 +706,6 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) {
defer testSvr.Stop()
caCert, jwtToken := setUpK8sServiceAccount(tt, k8s)
require := require.New(tt)
if c.expectedCACert != "" {
caCert = c.expectedCACert
}

// Run the command.
ui := cli.NewMockUi()
Expand Down Expand Up @@ -770,6 +766,91 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) {
}
}

// Test that when we provide a different k8s auth method parameters,
// the auth method is updated.
func TestRun_ConnectInjectAuthMethodUpdates(t *testing.T) {
t.Parallel()

k8s, testSvr := completeSetup(t)
defer testSvr.Stop()
caCert, jwtToken := setUpK8sServiceAccount(t, k8s)
require := require.New(t)

ui := cli.NewMockUi()
cmd := Command{
UI: ui,
clientset: k8s,
}

bindingRuleSelector := "serviceaccount.name!=default"

// First, create an auth method using the defaults
responseCode := cmd.Run([]string{
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
"-server-port", strings.Split(testSvr.HTTPAddr, ":")[1],
"-create-inject-auth-method",
"-acl-binding-rule-selector=" + bindingRuleSelector,
})
require.Equal(0, responseCode, ui.ErrorWriter.String())

// Check that the auth method was created.
bootToken := getBootToken(t, k8s, resourcePrefix, ns)
consul, err := api.NewClient(&api.Config{
Address: testSvr.HTTPAddr,
})
require.NoError(err)
authMethodName := resourcePrefix + "-k8s-auth-method"
authMethod, _, err := consul.ACL().AuthMethodRead(authMethodName,
&api.QueryOptions{Token: bootToken})
require.NoError(err)
require.NotNil(authMethod)
require.Contains(authMethod.Config, "Host")
require.Equal(authMethod.Config["Host"], defaultKubernetesHost)
require.Contains(authMethod.Config, "CACert")
require.Equal(authMethod.Config["CACert"], caCert)
require.Contains(authMethod.Config, "ServiceAccountJWT")
require.Equal(authMethod.Config["ServiceAccountJWT"], jwtToken)

// Generate a new CA certificate
_, _, caCertPem, _, err := cert.GenerateCA("kubernetes")
require.NoError(err)

// Overwrite the default kubernetes api, service account token and CA cert
kubernetesHost := "https://kubernetes.example.com"
// This token is the base64 encoded example token from jwt.io
serviceAccountToken = "ZXlKaGJHY2lPaUpJVXpJMU5pSXNJblI1Y0NJNklrcFhWQ0o5LmV5SnpkV0lpT2lJeE1qTTBOVFkzT0Rrd0lpd2libUZ0WlNJNklrcHZhRzRnUkc5bElpd2lhV0YwSWpveE5URTJNak01TURJeWZRLlNmbEt4d1JKU01lS0tGMlFUNGZ3cE1lSmYzNlBPazZ5SlZfYWRRc3N3NWM="
serviceAccountCACert = base64.StdEncoding.EncodeToString([]byte(caCertPem))

// Create a new service account
updatedCACert, updatedJWTToken := setUpK8sServiceAccount(t, k8s)

// Run command again
responseCode = cmd.Run([]string{
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
"-server-port", strings.Split(testSvr.HTTPAddr, ":")[1],
"-acl-binding-rule-selector=" + bindingRuleSelector,
"-create-inject-auth-method",
"-inject-auth-method-host=" + kubernetesHost,
})
require.Equal(0, responseCode, ui.ErrorWriter.String())

// Check that the auth method has been updated
authMethod, _, err = consul.ACL().AuthMethodRead(authMethodName,
&api.QueryOptions{Token: bootToken})
require.NoError(err)
require.NotNil(authMethod)
require.Contains(authMethod.Config, "Host")
require.Equal(authMethod.Config["Host"], kubernetesHost)
require.Contains(authMethod.Config, "CACert")
require.Equal(authMethod.Config["CACert"], updatedCACert)
require.Contains(authMethod.Config, "ServiceAccountJWT")
require.Equal(authMethod.Config["ServiceAccountJWT"], updatedJWTToken)
}

// Test that ACL binding rules are updated if the rule selector changes.
func TestRun_BindingRuleUpdates(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -1766,45 +1847,50 @@ func generateServerCerts(t *testing.T) (string, string, string, func()) {
// when the injector deployment is created. It returns the Service Account
// CA Cert and JWT token.
func setUpK8sServiceAccount(t *testing.T, k8s *fake.Clientset) (string, string) {
// Create Kubernetes Service.
_, err := k8s.CoreV1().Services(ns).Create(&v1.Service{
Spec: v1.ServiceSpec{
ClusterIP: "1.2.3.4",
},
ObjectMeta: metav1.ObjectMeta{
Name: "kubernetes",
},
})
require.NoError(t, err)

// Create ServiceAccount for the injector that the helm chart creates.
_, err = k8s.CoreV1().ServiceAccounts(ns).Create(&v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: resourcePrefix + "-connect-injector-authmethod-svc-account",
},
Secrets: []v1.ObjectReference{
{
Name: resourcePrefix + "-connect-injector-authmethod-svc-account",
// Create ServiceAccount for the kubernetes auth method if it doesn't exist,
// otherwise, do nothing.
serviceAccountName := resourcePrefix + "-connect-injector-authmethod-svc-account"
sa, _ := k8s.CoreV1().ServiceAccounts(ns).Get(serviceAccountName, metav1.GetOptions{})
if sa == nil {
_, err := k8s.CoreV1().ServiceAccounts(ns).Create(&v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: serviceAccountName,
},
},
})
require.NoError(t, err)
Secrets: []v1.ObjectReference{
{
Name: resourcePrefix + "-connect-injector-authmethod-svc-account",
},
},
})
require.NoError(t, err)
}

// Create the ServiceAccount Secret.
caCertBytes, err := base64.StdEncoding.DecodeString(serviceAccountCACert)
require.NoError(t, err)
tokenBytes, err := base64.StdEncoding.DecodeString(serviceAccountToken)
require.NoError(t, err)
_, err = k8s.CoreV1().Secrets(ns).Create(&v1.Secret{

// Create a Kubernetes secret if it doesn't exist, otherwise update it
secretName := resourcePrefix + "-connect-injector-authmethod-svc-account"
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: resourcePrefix + "-connect-injector-authmethod-svc-account",
Name: secretName,
},
Data: map[string][]byte{
"ca.crt": caCertBytes,
"token": tokenBytes,
},
})
require.NoError(t, err)
}
existingSecret, _ := k8s.CoreV1().Secrets(ns).Get(secretName, metav1.GetOptions{})
if existingSecret == nil {
_, err = k8s.CoreV1().Secrets(ns).Create(secret)
require.NoError(t, err)
} else {
_, err = k8s.CoreV1().Secrets(ns).Update(secret)
require.NoError(t, err)
}

return string(caCertBytes), string(tokenBytes)
}

Expand Down
111 changes: 45 additions & 66 deletions subcommand/server-acl-init/connect_inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,83 +9,62 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// We use the default Kubernetes service as the default host
// for the auth method created in Consul.
// This is recommended as described here:
// https://kubernetes.io/docs/tasks/access-application-cluster/access-cluster/#accessing-the-api-from-a-pod
const defaultKubernetesHost = "https://kubernetes.default.svc"

// configureConnectInject sets up auth methods so that connect injection will
// work.
func (c *Command) configureConnectInject(consulClient *api.Client) error {

authMethodName := c.withPrefix("k8s-auth-method")

// If not running namespaces, check if there's already an auth method.
// This means no changes need to be made to it. Binding rules should
// still be checked in case a user has updated their config.
var createAuthMethod bool
if !c.flagEnableNamespaces {
// Check if an auth method exists with the given name
err := c.untilSucceeds(fmt.Sprintf("checking if %s auth method exists", authMethodName),
func() error {
am, _, err := consulClient.ACL().AuthMethodRead(authMethodName, &api.QueryOptions{})
// This call returns nil if an AuthMethod does
// not exist with that name. This means we will
// need to create one.
if err == nil && am == nil {
createAuthMethod = true
}
return err
})
if err != nil {
return err
}
// Create the auth method template. This requires calls to the
// kubernetes environment.
authMethodTmpl, err := c.createAuthMethodTmpl(authMethodName)
if err != nil {
return err
}

// If namespaces are enabled, a namespace configuration change may need
// the auth method to be updated (as with a different mirroring prefix)
// or a new auth method created (if a new destination namespace is specified).
if c.flagEnableNamespaces || createAuthMethod {
// Create the auth method template. This requires calls to the
// kubernetes environment.
authMethodTmpl, err := c.createAuthMethodTmpl(authMethodName)
if err != nil {
return err
}

// Set up the auth method in the specific namespace if not mirroring
// If namespaces and mirroring are enabled, this is not necessary because
// the auth method will fall back to being created in the Consul `default`
// namespace automatically, as is necessary for mirroring.
// Note: if the config changes, an auth method will be created in the
// correct namespace, but the old auth method will not be removed.
writeOptions := api.WriteOptions{}
if c.flagEnableNamespaces && !c.flagEnableInjectK8SNSMirroring {
writeOptions.Namespace = c.flagConsulInjectDestinationNamespace

if c.flagConsulInjectDestinationNamespace != consulDefaultNamespace {
// If not the default namespace, check if it exists, creating it
// if necessary. The Consul namespace must exist for the AuthMethod
// to be created there.
err = c.untilSucceeds(fmt.Sprintf("checking or creating namespace %s",
c.flagConsulInjectDestinationNamespace),
func() error {
err := c.checkAndCreateNamespace(c.flagConsulInjectDestinationNamespace, consulClient)
return err
})
if err != nil {
// Set up the auth method in the specific namespace if not mirroring.
// If namespaces and mirroring are enabled, this is not necessary because
// the auth method will fall back to being created in the Consul `default`
// namespace automatically, as is necessary for mirroring.
// Note: if the config changes, an auth method will be created in the
// correct namespace, but the old auth method will not be removed.
writeOptions := api.WriteOptions{}
if c.flagEnableNamespaces && !c.flagEnableInjectK8SNSMirroring {
writeOptions.Namespace = c.flagConsulInjectDestinationNamespace

if c.flagConsulInjectDestinationNamespace != consulDefaultNamespace {
// If not the default namespace, check if it exists, creating it
// if necessary. The Consul namespace must exist for the AuthMethod
// to be created there.
err = c.untilSucceeds(fmt.Sprintf("checking or creating namespace %s",
c.flagConsulInjectDestinationNamespace),
func() error {
err := c.checkAndCreateNamespace(c.flagConsulInjectDestinationNamespace, consulClient)
return err
}
})
if err != nil {
return err
}
}
}

err = c.untilSucceeds(fmt.Sprintf("creating auth method %s", authMethodTmpl.Name),
func() error {
var err error
// `AuthMethodCreate` will also be able to update an existing
// AuthMethod based on the name provided. This means that any namespace
// configuration changes will correctly update the AuthMethod.
_, _, err = consulClient.ACL().AuthMethodCreate(&authMethodTmpl, &writeOptions)
return err
})
if err != nil {
err = c.untilSucceeds(fmt.Sprintf("creating auth method %s", authMethodTmpl.Name),
func() error {
var err error
// `AuthMethodCreate` will also be able to update an existing
// AuthMethod based on the name provided. This means that any
// configuration changes will correctly update the AuthMethod.
_, _, err = consulClient.ACL().AuthMethodCreate(&authMethodTmpl, &writeOptions)
return err
}
})
if err != nil {
return err
}

// Create the binding rule.
Expand All @@ -110,7 +89,7 @@ func (c *Command) configureConnectInject(consulClient *api.Client) error {
}

var existingRules []*api.ACLBindingRule
err := c.untilSucceeds(fmt.Sprintf("listing binding rules for auth method %s", authMethodName),
err = c.untilSucceeds(fmt.Sprintf("listing binding rules for auth method %s", authMethodName),
func() error {
var err error
existingRules, _, err = consulClient.ACL().BindingRuleList(authMethodName, &queryOptions)
Expand Down Expand Up @@ -186,7 +165,7 @@ func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod
return api.ACLAuthMethod{}, err
}

kubernetesHost := "https://kubernetes.default.svc"
kubernetesHost := defaultKubernetesHost

// Check if custom auth method Host and CACert are provided
if c.flagInjectAuthMethodHost != "" {
Expand Down

0 comments on commit df99da1

Please sign in to comment.