From 724a5323a2bd02f3ddd93b7c7793f43db3fd9201 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Mon, 30 Mar 2020 15:57:39 -0700 Subject: [PATCH 01/11] server-acl-init: Add -server-address and -server-port * Require -server-address to be provided instead of discovering the server IPs from Kubernetes pods. This allows us to eventually to run this command against external servers or servers deployed on Kube in the same way. On Kubernetes, instead of discovering Pod IPs, we can use server's stateful set DNS names. * [Breaking change] Remove -expected-replicas, -release-name, and -server-label-selector flags because we no longer need them. --- subcommand/server-acl-init/command_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index 4be4bd87d1..0f98dd7b1b 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -1153,6 +1153,8 @@ func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) { "-server-address", strings.Split(serverAddr, ":")[0], "-server-port", strings.Split(serverAddr, ":")[1], "-resource-prefix=" + resourcePrefix, + "-server-address", strings.Split(serverAddr, ":")[0], + "-server-port", strings.Split(serverAddr, ":")[1], }, flag) responseCode := cmd.Run(cmdArgs) require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) From 4a33ffad06723462c52525084f80cecb5041a3bc Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 7 Apr 2020 23:51:08 -0700 Subject: [PATCH 02/11] conflicts --- subcommand/server-acl-init/command.go | 60 +++++++++++++--------- subcommand/server-acl-init/command_test.go | 58 +++++++++++++++++++++ 2 files changed, 95 insertions(+), 23 deletions(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 92f54038c7..9cd5d51ef2 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -15,6 +15,7 @@ import ( k8sflags "github.com/hashicorp/consul-k8s/subcommand/flags" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" + "github.com/hashicorp/go-discover" "github.com/hashicorp/go-hclog" "github.com/mitchellh/cli" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -25,26 +26,32 @@ import ( type Command struct { UI cli.Ui - flags *flag.FlagSet - k8s *k8sflags.K8SFlags - flagResourcePrefix string - flagK8sNamespace string - flagAllowDNS bool - flagCreateClientToken bool - flagCreateSyncToken bool - flagCreateInjectToken bool - flagCreateInjectAuthMethod bool - flagBindingRuleSelector string - flagCreateEntLicenseToken bool - flagCreateSnapshotAgentToken bool - flagCreateMeshGatewayToken bool + flags *flag.FlagSet + k8s *k8sflags.K8SFlags + + flagResourcePrefix string + flagK8sNamespace string + + flagAllowDNS bool + flagCreateClientToken bool + flagCreateSyncToken bool + flagCreateInjectToken bool + flagCreateInjectAuthMethod bool + flagBindingRuleSelector string + flagCreateEntLicenseToken bool + flagCreateSnapshotAgentToken bool + flagCreateMeshGatewayToken bool + + // Flags to configure Consul client + flagServerAddresses []string + flagServerPort uint + flagConsulCACert string + flagConsulTLSServerName string + flagUseHTTPS bool + + // Flags for ACL replication flagCreateACLReplicationToken bool flagACLReplicationTokenFile string - flagConsulCACert string - flagConsulTLSServerName string - flagUseHTTPS bool - flagServerAddresses []string - flagServerPort uint // Flags to support namespaces flagEnableNamespaces bool // Use namespacing on all components @@ -68,15 +75,14 @@ type Command struct { once sync.Once help string + + providers map[string]discover.Provider } func (c *Command) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) c.flags.StringVar(&c.flagResourcePrefix, "resource-prefix", "", "Prefix to use for Kubernetes resources. If not set, the \"-consul\" prefix is used, where is the value set by the -release-name flag.") - c.flags.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address", - "The IP or DNS name of the Consul server(s), may be provided 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.flagK8sNamespace, "k8s-namespace", "", "Name of Kubernetes namespace where the servers are deployed") c.flags.BoolVar(&c.flagAllowDNS, "allow-dns", false, @@ -99,14 +105,18 @@ func (c *Command) init() { "Toggle for creating a token for the Consul snapshot agent deployment (enterprise only)") c.flags.BoolVar(&c.flagCreateMeshGatewayToken, "create-mesh-gateway-token", false, "Toggle for creating a token for a Connect mesh gateway") - c.flags.BoolVar(&c.flagCreateACLReplicationToken, "create-acl-replication-token", false, - "Toggle for creating a token for ACL replication between datacenters") + + c.flags.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address", + "The IP, DNS name or cloud auto-join string of the Consul server(s), may be provided 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", "", "Path to the PEM-encoded CA certificate of the Consul cluster.") c.flags.StringVar(&c.flagConsulTLSServerName, "consul-tls-server-name", "", "The server name to set as the SNI header when sending HTTPS requests to Consul.") c.flags.BoolVar(&c.flagUseHTTPS, "use-https", false, "Toggle for using HTTPS for all API calls to Consul.") + c.flags.BoolVar(&c.flagEnableNamespaces, "enable-namespaces", false, "[Enterprise Only] Enables namespaces, in either a single Consul namespace or mirrored [Enterprise only feature]") c.flags.StringVar(&c.flagConsulSyncDestinationNamespace, "consul-sync-destination-namespace", "default", @@ -125,6 +135,9 @@ func (c *Command) init() { c.flags.StringVar(&c.flagInjectK8SNSMirroringPrefix, "inject-k8s-namespace-mirroring-prefix", "", "[Enterprise Only] Prefix that will be added to all k8s namespaces mirrored into Consul by Connect inject "+ "if mirroring is enabled.") + + c.flags.BoolVar(&c.flagCreateACLReplicationToken, "create-acl-replication-token", false, + "Toggle for creating a token for ACL replication between datacenters") c.flags.StringVar(&c.flagACLReplicationTokenFile, "acl-replication-token-file", "", "Path to file containing ACL token to be used for ACL replication. If set, ACL replication is enabled.") c.flags.DurationVar(&c.flagTimeout, "timeout", 10*time.Minute, @@ -171,6 +184,7 @@ func (c *Command) Run(args []string) int { c.UI.Error("-resource-prefix must be set") return 1 } + var aclReplicationToken string if c.flagACLReplicationTokenFile != "" { // Load the ACL replication token from file. diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index 0f98dd7b1b..907b88e890 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "fmt" "io/ioutil" + "log" "math/rand" "net/http" "net/http/httptest" @@ -19,6 +20,7 @@ import ( "github.com/hashicorp/consul/sdk/freeport" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/hashicorp/go-discover" "github.com/mitchellh/cli" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" @@ -1171,6 +1173,49 @@ func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) { } } +func TestRun_DefaultWithCloudAutoJoin(t *testing.T) { + t.Parallel() + + k8s, testSvr := completeSetup(t) + defer testSvr.Stop() + require := require.New(t) + + provider := &fakeProvider{} + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + providers: map[string]discover.Provider{"fake": provider}, + } + args := []string{ + "-k8s-namespace=" + ns, + "-resource-prefix=" + resourcePrefix, + "-server-address", "provider=fake address=127.0.0.1", + "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], + } + responseCode := cmd.Run(args) + require.Equal(0, responseCode, ui.ErrorWriter.String()) + + // Test that the bootstrap kube secret is created. + bootToken := getBootToken(t, k8s, resourcePrefix, ns) + + // Check that it has the right policies. + consul, err := api.NewClient(&api.Config{ + Address: testSvr.HTTPAddr, + Token: bootToken, + }) + require.NoError(err) + tokenData, _, err := consul.ACL().TokenReadSelf(nil) + require.NoError(err) + require.Equal("global-management", tokenData.Policies[0].Name) + + // Check that the agent policy was created. + agentPolicy := policyExists(t, "agent-token", consul) + // Should be a global policy. + require.Len(agentPolicy.Datacenters, 0) +} + // Set up test consul agent and kubernetes cluster. func completeSetup(t *testing.T) (*fake.Clientset, *testutil.TestServer) { k8s := fake.NewSimpleClientset() @@ -1425,5 +1470,18 @@ func writeTempFile(t *testing.T, contents string) (string, func()) { } } +type fakeProvider struct { + addrsNumCalls int +} + +func (p *fakeProvider) Addrs(args map[string]string, l *log.Logger) ([]string, error) { + p.addrsNumCalls++ + return []string{args["address"]}, nil +} + +func (p *fakeProvider) Help() string { + return "fake-provider help" +} + var serviceAccountCACert = "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURDekNDQWZPZ0F3SUJBZ0lRS3pzN05qbDlIczZYYzhFWG91MjVoekFOQmdrcWhraUc5dzBCQVFzRkFEQXYKTVMwd0t3WURWUVFERXlRMU9XVTJaR00wTVMweU1EaG1MVFF3T1RVdFlUSTRPUzB4Wm1NM01EQmhZekZqWXpndwpIaGNOTVRrd05qQTNNVEF4TnpNeFdoY05NalF3TmpBMU1URXhOek14V2pBdk1TMHdLd1lEVlFRREV5UTFPV1UyClpHTTBNUzB5TURobUxUUXdPVFV0WVRJNE9TMHhabU0zTURCaFl6RmpZemd3Z2dFaU1BMEdDU3FHU0liM0RRRUIKQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURaakh6d3FvZnpUcEdwYzBNZElDUzdldXZmdWpVS0UzUEMvYXBmREFnQgo0anpFRktBNzgvOStLVUd3L2MvMFNIZVNRaE4rYThnd2xIUm5BejFOSmNmT0lYeTRkd2VVdU9rQWlGeEg4cGh0CkVDd2tlTk83ejhEb1Y4Y2VtaW5DUkhHamFSbW9NeHBaN2cycFpBSk5aZVB4aTN5MWFOa0ZBWGU5Z1NVU2RqUloKUlhZa2E3d2gyQU85azJkbEdGQVlCK3Qzdld3SjZ0d2pHMFR0S1FyaFlNOU9kMS9vTjBFMDFMekJjWnV4a04xawo4Z2ZJSHk3Yk9GQ0JNMldURURXLzBhQXZjQVByTzhETHFESis2TWpjM3I3K3psemw4YVFzcGIwUzA4cFZ6a2k1CkR6Ly84M2t5dTBwaEp1aWo1ZUI4OFY3VWZQWHhYRi9FdFY2ZnZyTDdNTjRmQWdNQkFBR2pJekFoTUE0R0ExVWQKRHdFQi93UUVBd0lDQkRBUEJnTlZIUk1CQWY4RUJUQURBUUgvTUEwR0NTcUdTSWIzRFFFQkN3VUFBNElCQVFCdgpRc2FHNnFsY2FSa3RKMHpHaHh4SjUyTm5SVjJHY0lZUGVOM1p2MlZYZTNNTDNWZDZHMzJQVjdsSU9oangzS21BCi91TWg2TmhxQnpzZWtrVHowUHVDM3dKeU0yT0dvblZRaXNGbHF4OXNGUTNmVTJtSUdYQ2Ezd0M4ZS9xUDhCSFMKdzcvVmVBN2x6bWozVFFSRS9XMFUwWkdlb0F4bjliNkp0VDBpTXVjWXZQMGhYS1RQQldsbnpJaWphbVU1MHIyWQo3aWEwNjVVZzJ4VU41RkxYL3Z4T0EzeTRyanBraldvVlFjdTFwOFRaclZvTTNkc0dGV3AxMGZETVJpQUhUdk9ICloyM2pHdWs2cm45RFVIQzJ4UGozd0NUbWQ4U0dFSm9WMzFub0pWNWRWZVE5MHd1c1h6M3ZURzdmaWNLbnZIRlMKeHRyNVBTd0gxRHVzWWZWYUdIMk8KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=" var serviceAccountToken = "ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklpSjkuZXlKcGMzTWlPaUpyZFdKbGNtNWxkR1Z6TDNObGNuWnBZMlZoWTJOdmRXNTBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5dVlXMWxjM0JoWTJVaU9pSmtaV1poZFd4MElpd2lhM1ZpWlhKdVpYUmxjeTVwYnk5elpYSjJhV05sWVdOamIzVnVkQzl6WldOeVpYUXVibUZ0WlNJNkltdG9ZV3RwTFdGeVlXTm9ibWxrTFdOdmJuTjFiQzFqYjI1dVpXTjBMV2x1YW1WamRHOXlMV0YxZEdodFpYUm9iMlF0YzNaakxXRmpZMjlvYm1SaWRpSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMbTVoYldVaU9pSnJhR0ZyYVMxaGNtRmphRzVwWkMxamIyNXpkV3d0WTI5dWJtVmpkQzFwYm1wbFkzUnZjaTFoZFhSb2JXVjBhRzlrTFhOMll5MWhZMk52ZFc1MElpd2lhM1ZpWlhKdVpYUmxjeTVwYnk5elpYSjJhV05sWVdOamIzVnVkQzl6WlhKMmFXTmxMV0ZqWTI5MWJuUXVkV2xrSWpvaU4yVTVOV1V4TWprdFpUUTNNeTB4TVdVNUxUaG1ZV0V0TkRJd01UQmhPREF3TVRJeUlpd2ljM1ZpSWpvaWMzbHpkR1Z0T25ObGNuWnBZMlZoWTJOdmRXNTBPbVJsWm1GMWJIUTZhMmhoYTJrdFlYSmhZMmh1YVdRdFkyOXVjM1ZzTFdOdmJtNWxZM1F0YVc1cVpXTjBiM0l0WVhWMGFHMWxkR2h2WkMxemRtTXRZV05qYjNWdWRDSjkuWWk2M01NdHpoNU1CV0tLZDNhN2R6Q0pqVElURTE1aWtGeV9UbnBka19Bd2R3QTlKNEFNU0dFZUhONXZXdEN1dUZqb19sTUpxQkJQSGtLMkFxYm5vRlVqOW01Q29wV3lxSUNKUWx2RU9QNGZVUS1SYzBXMVBfSmpVMXJaRVJIRzM5YjVUTUxnS1BRZ3V5aGFpWkVKNkNqVnRtOXdVVGFncmdpdXFZVjJpVXFMdUY2U1lObTZTckt0a1BTLWxxSU8tdTdDMDZ3Vms1bTV1cXdJVlFOcFpTSUNfNUxzNWFMbXlaVTNuSHZILVY3RTNIbUJoVnlaQUI3NmpnS0IwVHlWWDFJT3NrdDlQREZhck50VTNzdVp5Q2p2cUMtVUpBNnNZZXlTZTRkQk5Lc0tsU1o2WXV4VVVtbjFSZ3YzMllNZEltbnNXZzhraGYtekp2cWdXazdCNUVB" From b8f385ba0c77fcb721ba2f1e44d7e2bdccc37a98 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 7 Apr 2020 23:51:25 -0700 Subject: [PATCH 03/11] Support external servers * Add -bootstrap-token-file to provide your own bootstrap token. If provided, server-acl-init will skip ACL bootstrapping of the servers and will not update server policies and set tokens. * The -server-address flag now can also be a cloud auto-join string. This enables us to re-use the same string you would use for retry-join. --- helper/go-discover/discover.go | 61 ++++++ subcommand/get-consul-client-ca/command.go | 44 +---- subcommand/server-acl-init/anonymous_token.go | 2 +- subcommand/server-acl-init/command.go | 135 ++++++++----- subcommand/server-acl-init/command_test.go | 185 +++++++++++++++++- .../server-acl-init/create_or_update.go | 8 +- subcommand/server-acl-init/servers.go | 12 +- subcommand/sync-catalog/command_ent_test.go | 2 - 8 files changed, 347 insertions(+), 102 deletions(-) create mode 100644 helper/go-discover/discover.go diff --git a/helper/go-discover/discover.go b/helper/go-discover/discover.go new file mode 100644 index 0000000000..f94621158a --- /dev/null +++ b/helper/go-discover/discover.go @@ -0,0 +1,61 @@ +package godiscover + +import ( + "fmt" + "strings" + + "github.com/hashicorp/consul-k8s/version" + "github.com/hashicorp/go-discover" + discoverk8s "github.com/hashicorp/go-discover/provider/k8s" + "github.com/hashicorp/go-hclog" +) + +// ConsulServerAddresses uses go-discover to discover Consul servers +// provided by the 'discoverString' and returns them. +func ConsulServerAddresses(discoverString string, providers map[string]discover.Provider, logger hclog.Logger) ([]string, error) { + // If it's a cloud-auto join string, discover server addresses through the cloud provider. + // This code was adapted from + // https://github.com/hashicorp/consul/blob/c5fe112e59f6e8b03159ec8f2dbe7f4a026ce823/agent/retry_join.go#L55-L89. + disco, err := newDiscover(providers) + if err != nil { + return nil, err + } + logger.Debug("using cloud auto-join", "server-addr", discoverString) + servers, err := disco.Addrs(discoverString, logger.StandardLogger(&hclog.StandardLoggerOptions{ + InferLevels: true, + })) + if err != nil { + return nil, err + } + + // check if we discovered any servers + if len(servers) == 0 { + return nil, fmt.Errorf("could not discover any Consul servers with %q", discoverString) + } + + logger.Debug("discovered servers", "servers", strings.Join(servers, " ")) + + return servers, nil +} + +// newDiscover initializes the new Discover object +// set up with all predefined providers, as well as +// the k8s provider. +// This code was adapted from +// https://github.com/hashicorp/consul/blob/c5fe112e59f6e8b03159ec8f2dbe7f4a026ce823/agent/retry_join.go#L42-L53 +func newDiscover(providers map[string]discover.Provider) (*discover.Discover, error) { + if providers == nil { + providers = make(map[string]discover.Provider) + } + + for k, v := range discover.Providers { + providers[k] = v + } + providers["k8s"] = &discoverk8s.Provider{} + + userAgent := fmt.Sprintf("consul-k8s/%s (https://www.consul.io/)", version.GetHumanVersion()) + return discover.New( + discover.WithUserAgent(userAgent), + discover.WithProviders(providers), + ) +} diff --git a/subcommand/get-consul-client-ca/command.go b/subcommand/get-consul-client-ca/command.go index 36c1012c47..4453f48a8e 100644 --- a/subcommand/get-consul-client-ca/command.go +++ b/subcommand/get-consul-client-ca/command.go @@ -10,11 +10,10 @@ import ( "time" "github.com/cenkalti/backoff" - "github.com/hashicorp/consul-k8s/version" + godiscover "github.com/hashicorp/consul-k8s/helper/go-discover" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" "github.com/hashicorp/go-discover" - discoverk8s "github.com/hashicorp/go-discover/provider/k8s" "github.com/hashicorp/go-hclog" "github.com/mitchellh/cli" ) @@ -175,27 +174,10 @@ func (c *Command) consulServerAddr(logger hclog.Logger) (string, error) { return fmt.Sprintf("%s:%s", c.flagServerAddr, c.flagServerPort), nil } - // If it's a cloud-auto join string, discover server addresses through the cloud provider. - // This code was adapted from - // https://github.com/hashicorp/consul/blob/c5fe112e59f6e8b03159ec8f2dbe7f4a026ce823/agent/retry_join.go#L55-L89. - disco, err := c.newDiscover() + servers, err := godiscover.ConsulServerAddresses(c.flagServerAddr, c.providers, logger) if err != nil { return "", err } - logger.Debug("using cloud auto-join", "server-addr", c.flagServerAddr) - servers, err := disco.Addrs(c.flagServerAddr, logger.StandardLogger(&hclog.StandardLoggerOptions{ - InferLevels: true, - })) - if err != nil { - return "", err - } - - // check if we discovered any servers - if len(servers) == 0 { - return "", fmt.Errorf("could not discover any Consul servers with %q", c.flagServerAddr) - } - - logger.Debug("discovered servers", "servers", strings.Join(servers, " ")) // Pick the first server from the list, // ignoring the port since we need to use HTTP API @@ -204,28 +186,6 @@ func (c *Command) consulServerAddr(logger hclog.Logger) (string, error) { return fmt.Sprintf("%s:%s", firstServer, c.flagServerPort), nil } -// newDiscover initializes the new Discover object -// set up with all predefined providers, as well as -// the k8s provider. -// This code was adapted from -// https://github.com/hashicorp/consul/blob/c5fe112e59f6e8b03159ec8f2dbe7f4a026ce823/agent/retry_join.go#L42-L53 -func (c *Command) newDiscover() (*discover.Discover, error) { - if c.providers == nil { - c.providers = make(map[string]discover.Provider) - } - - for k, v := range discover.Providers { - c.providers[k] = v - } - c.providers["k8s"] = &discoverk8s.Provider{} - - userAgent := fmt.Sprintf("consul-k8s/%s (https://www.consul.io/)", version.GetHumanVersion()) - return discover.New( - discover.WithUserAgent(userAgent), - discover.WithProviders(c.providers), - ) -} - // getActiveRoot returns the currently active root // from the roots list, otherwise returns error. func getActiveRoot(roots *api.CARootList) (string, error) { diff --git a/subcommand/server-acl-init/anonymous_token.go b/subcommand/server-acl-init/anonymous_token.go index 43d8e4e456..3423ee78da 100644 --- a/subcommand/server-acl-init/anonymous_token.go +++ b/subcommand/server-acl-init/anonymous_token.go @@ -9,7 +9,7 @@ import ( func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { anonRules, err := c.anonymousTokenRules() if err != nil { - c.Log.Error("Error templating anonymous token rules", "err", err) + c.log.Error("Error templating anonymous token rules", "err", err) return err } diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 9cd5d51ef2..ceac59b08a 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -11,6 +11,7 @@ import ( "sync" "time" + godiscover "github.com/hashicorp/consul-k8s/helper/go-discover" "github.com/hashicorp/consul-k8s/subcommand" k8sflags "github.com/hashicorp/consul-k8s/subcommand/flags" "github.com/hashicorp/consul/api" @@ -26,8 +27,8 @@ import ( type Command struct { UI cli.Ui - flags *flag.FlagSet - k8s *k8sflags.K8SFlags + flags *flag.FlagSet + k8s *k8sflags.K8SFlags flagResourcePrefix string flagK8sNamespace string @@ -43,7 +44,7 @@ type Command struct { flagCreateMeshGatewayToken bool // Flags to configure Consul client - flagServerAddresses []string + flagServerAddresses []string flagServerPort uint flagConsulCACert string flagConsulTLSServerName string @@ -62,16 +63,20 @@ type Command struct { flagEnableInjectK8SNSMirroring bool // Enables mirroring of k8s namespaces into Consul for Connect inject flagInjectK8SNSMirroringPrefix string // Prefix added to Consul namespaces created when mirroring injected services + // Flag to support a custom bootstrap token + flagBootstrapTokenFile string + flagLogLevel string flagTimeout time.Duration clientset kubernetes.Interface + // cmdTimeout is cancelled when the command timeout is reached. cmdTimeout context.Context retryDuration time.Duration - // Log - Log hclog.Logger + // log + log hclog.Logger once sync.Once help string @@ -84,13 +89,14 @@ func (c *Command) init() { c.flags.StringVar(&c.flagResourcePrefix, "resource-prefix", "", "Prefix to use for Kubernetes resources. If not set, the \"-consul\" prefix is used, where is the value set by the -release-name flag.") c.flags.StringVar(&c.flagK8sNamespace, "k8s-namespace", "", - "Name of Kubernetes namespace where the servers are deployed") + "Name of Kubernetes namespace where Consul and consul-k8s components are deployed.") + c.flags.BoolVar(&c.flagAllowDNS, "allow-dns", false, "Toggle for updating the anonymous token to allow DNS queries to work") c.flags.BoolVar(&c.flagCreateClientToken, "create-client-token", true, - "Toggle for creating a client agent token") + "Toggle for creating a client agent token. Default is true.") c.flags.BoolVar(&c.flagCreateSyncToken, "create-sync-token", false, - "Toggle for creating a catalog sync token") + "Toggle for creating a catalog sync token.") c.flags.BoolVar(&c.flagCreateInjectToken, "create-inject-namespace-token", false, "Toggle for creating a connect injector token. Only required when namespaces are enabled.") c.flags.BoolVar(&c.flagCreateInjectAuthMethod, "create-inject-auth-method", false, @@ -100,15 +106,15 @@ func (c *Command) init() { c.flags.StringVar(&c.flagBindingRuleSelector, "acl-binding-rule-selector", "", "Selector string for connectInject ACL Binding Rule") c.flags.BoolVar(&c.flagCreateEntLicenseToken, "create-enterprise-license-token", false, - "Toggle for creating a token for the enterprise license job") + "Toggle for creating a token for the enterprise license job.") c.flags.BoolVar(&c.flagCreateSnapshotAgentToken, "create-snapshot-agent-token", false, - "Toggle for creating a token for the Consul snapshot agent deployment (enterprise only)") + "[Enterprise Only] Toggle for creating a token for the Consul snapshot agent deployment.") c.flags.BoolVar(&c.flagCreateMeshGatewayToken, "create-mesh-gateway-token", false, "Toggle for creating a token for a Connect mesh gateway") c.flags.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address", - "The IP, DNS name or cloud auto-join string of the Consul server(s), may be provided multiple times." + - "At least one value is required.") + "The IP, DNS name or the cloud auto-join string of the Consul server(s), may be provided 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", "", "Path to the PEM-encoded CA certificate of the Consul cluster.") @@ -140,6 +146,11 @@ func (c *Command) init() { "Toggle for creating a token for ACL replication between datacenters") c.flags.StringVar(&c.flagACLReplicationTokenFile, "acl-replication-token-file", "", "Path to file containing ACL token to be used for ACL replication. If set, ACL replication is enabled.") + + c.flags.StringVar(&c.flagBootstrapTokenFile, "bootstrap-token-file", "", + "Path to file containing ACL token for creating policies and tokens. This token must have 'acl:write' permissions."+ + "When provided, servers will not be bootstrapped and their policies and tokens will not be updated.") + c.flags.DurationVar(&c.flagTimeout, "timeout", 10*time.Minute, "How long we'll try to bootstrap ACLs for before timing out, e.g. 1ms, 2s, 3m") c.flags.StringVar(&c.flagLogLevel, "log-level", "info", @@ -200,6 +211,21 @@ func (c *Command) Run(args []string) int { aclReplicationToken = strings.TrimSpace(string(tokenBytes)) } + var providedBootstrapToken string + if c.flagBootstrapTokenFile != "" { + // Load the bootstrap token from file. + tokenBytes, err := ioutil.ReadFile(c.flagBootstrapTokenFile) + if err != nil { + c.UI.Error(fmt.Sprintf("Unable to read bootstrap token from file %q: %s", c.flagBootstrapTokenFile, err)) + return 1 + } + if len(tokenBytes) == 0 { + c.UI.Error(fmt.Sprintf("Bootstrap token file %q is empty", c.flagBootstrapTokenFile)) + return 1 + } + providedBootstrapToken = strings.TrimSpace(string(tokenBytes)) + } + var cancel context.CancelFunc c.cmdTimeout, cancel = context.WithTimeout(context.Background(), c.flagTimeout) // The context will only ever be intentionally ended by the timeout. @@ -211,15 +237,27 @@ func (c *Command) Run(args []string) int { c.UI.Error(fmt.Sprintf("Unknown log level: %s", c.flagLogLevel)) return 1 } - c.Log = hclog.New(&hclog.LoggerOptions{ + c.log = hclog.New(&hclog.LoggerOptions{ Level: level, Output: os.Stderr, }) + serverAddresses := c.flagServerAddresses + // Check if the provided addresses contain a cloud-auto join string. + // If yes, call go-discover to discover addresses of the Consul servers. + if len(c.flagServerAddresses) == 1 && strings.Contains(c.flagServerAddresses[0], "provider=") { + var err error + serverAddresses, err = godiscover.ConsulServerAddresses(c.flagServerAddresses[0], c.providers, c.log) + if err != nil { + c.UI.Error(fmt.Sprintf("Unable to discover any Consul addresses from %q: %s", c.flagServerAddresses[0], err)) + return 1 + } + } + // The ClientSet might already be set if we're in a test. if c.clientset == nil { if err := c.configureKubeClient(); err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -232,12 +270,17 @@ func (c *Command) Run(args []string) int { var updateServerPolicy bool var bootstrapToken string - if c.flagACLReplicationTokenFile != "" { + if c.flagBootstrapTokenFile != "" { + // If bootstrap token is provided, we skip server bootstrapping and use + // the provided token to create policies and tokens for the rest of the components. + c.log.Info("Bootstrap token is provided so skipping ACL bootstrapping") + bootstrapToken = providedBootstrapToken + } else if c.flagACLReplicationTokenFile != "" { // If ACL replication is enabled, we don't need to ACL bootstrap the servers // since they will be performing replication. // We can use the replication token as our bootstrap token because it // has permissions to create policies and tokens. - c.Log.Info("ACL replication is enabled so skipping ACL bootstrapping") + c.log.Info("ACL replication is enabled so skipping ACL bootstrapping") bootstrapToken = aclReplicationToken } else { // Check if we've already been bootstrapped. @@ -245,12 +288,12 @@ func (c *Command) Run(args []string) int { bootTokenSecretName := c.withPrefix("bootstrap-acl-token") bootstrapToken, err = c.getBootstrapToken(bootTokenSecretName) if err != nil { - c.Log.Error(fmt.Sprintf("Unexpected error looking for preexisting bootstrap Secret: %s", err)) + c.log.Error(fmt.Sprintf("Unexpected error looking for preexisting bootstrap Secret: %s", err)) return 1 } if bootstrapToken != "" { - c.Log.Info(fmt.Sprintf("ACLs already bootstrapped - retrieved bootstrap token from Secret %q", bootTokenSecretName)) + c.log.Info(fmt.Sprintf("ACLs already bootstrapped - retrieved bootstrap token from Secret %q", bootTokenSecretName)) // Mark that we should update the server ACL policy in case // there are namespace related config changes. Because of the @@ -258,17 +301,17 @@ func (c *Command) Run(args []string) int { // otherwise won't be updated. updateServerPolicy = true } else { - c.Log.Info("No bootstrap token from previous installation found, continuing on to bootstrapping") - bootstrapToken, err = c.bootstrapServers(bootTokenSecretName, scheme) + c.log.Info("No bootstrap token from previous installation found, continuing on to bootstrapping") + bootstrapToken, err = c.bootstrapServers(serverAddresses, bootTokenSecretName, scheme) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } } // For all of the next operations we'll need a Consul client. - serverAddr := fmt.Sprintf("%s:%d", c.flagServerAddresses[0], c.flagServerPort) + serverAddr := fmt.Sprintf("%s:%d", serverAddresses[0], c.flagServerPort) consulClient, err := api.NewClient(&api.Config{ Address: serverAddr, Scheme: scheme, @@ -279,16 +322,16 @@ func (c *Command) Run(args []string) int { }, }) if err != nil { - c.Log.Error(fmt.Sprintf("Error creating Consul client for addr %q: %s", serverAddr, err)) + c.log.Error(fmt.Sprintf("Error creating Consul client for addr %q: %s", serverAddr, err)) return 1 } consulDC, err := c.consulDatacenter(consulClient) if err != nil { - c.Log.Error("Error getting datacenter name", "err", err) + c.log.Error("Error getting datacenter name", "err", err) return 1 } - c.Log.Info("Current datacenter", "datacenter", consulDC) + c.log.Info("Current datacenter", "datacenter", consulDC) // With the addition of namespaces, the ACL policies associated // with the server tokens may need to be updated if Enterprise Consul @@ -297,7 +340,7 @@ func (c *Command) Run(args []string) int { if updateServerPolicy { _, err = c.setServerPolicy(consulClient) if err != nil { - c.Log.Error("Error updating the server ACL policy", "err", err) + c.log.Error("Error updating the server ACL policy", "err", err) return 1 } } @@ -319,7 +362,7 @@ func (c *Command) Run(args []string) int { return c.createOrUpdateACLPolicy(policyTmpl, consulClient) }) if err != nil { - c.Log.Error("Error creating or updating the cross namespace policy", "err", err) + c.log.Error("Error creating or updating the cross namespace policy", "err", err) return 1 } @@ -335,7 +378,7 @@ func (c *Command) Run(args []string) int { } _, _, err = consulClient.Namespaces().Update(&consulNamespace, &api.WriteOptions{}) if err != nil { - c.Log.Error("Error updating the default namespace to include the cross namespace policy", "err", err) + c.log.Error("Error updating the default namespace to include the cross namespace policy", "err", err) return 1 } } @@ -343,13 +386,13 @@ func (c *Command) Run(args []string) int { if c.flagCreateClientToken { agentRules, err := c.agentRules() if err != nil { - c.Log.Error("Error templating client agent rules", "err", err) + c.log.Error("Error templating client agent rules", "err", err) return 1 } err = c.createLocalACL("client", agentRules, consulDC, consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -357,7 +400,7 @@ func (c *Command) Run(args []string) int { if c.createAnonymousPolicy() { err := c.configureAnonymousPolicy(consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -365,7 +408,7 @@ func (c *Command) Run(args []string) int { if c.flagCreateSyncToken { syncRules, err := c.syncRules() if err != nil { - c.Log.Error("Error templating sync rules", "err", err) + c.log.Error("Error templating sync rules", "err", err) return 1 } @@ -377,7 +420,7 @@ func (c *Command) Run(args []string) int { err = c.createLocalACL("catalog-sync", syncRules, consulDC, consulClient) } if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -385,7 +428,7 @@ func (c *Command) Run(args []string) int { if c.flagCreateInjectToken { injectRules, err := c.injectRules() if err != nil { - c.Log.Error("Error templating inject rules", "err", err) + c.log.Error("Error templating inject rules", "err", err) return 1 } @@ -398,7 +441,7 @@ func (c *Command) Run(args []string) int { } if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -406,7 +449,7 @@ func (c *Command) Run(args []string) int { if c.flagCreateEntLicenseToken { err := c.createLocalACL("enterprise-license", entLicenseRules, consulDC, consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -414,7 +457,7 @@ func (c *Command) Run(args []string) int { if c.flagCreateSnapshotAgentToken { err := c.createLocalACL("client-snapshot-agent", snapshotAgentRules, consulDC, consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -422,7 +465,7 @@ func (c *Command) Run(args []string) int { if c.flagCreateMeshGatewayToken { meshGatewayRules, err := c.meshGatewayRules() if err != nil { - c.Log.Error("Error templating dns rules", "err", err) + c.log.Error("Error templating dns rules", "err", err) return 1 } @@ -430,7 +473,7 @@ func (c *Command) Run(args []string) int { // discover services in other datacenters. err = c.createGlobalACL("mesh-gateway", meshGatewayRules, consulDC, consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -438,7 +481,7 @@ func (c *Command) Run(args []string) int { if c.flagCreateInjectAuthMethod { err := c.configureConnectInject(consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } @@ -446,19 +489,19 @@ func (c *Command) Run(args []string) int { if c.flagCreateACLReplicationToken { rules, err := c.aclReplicationRules() if err != nil { - c.Log.Error("Error templating acl replication token rules", "err", err) + c.log.Error("Error templating acl replication token rules", "err", err) return 1 } // Policy must be global because it replicates from the primary DC // and so the primary DC needs to be able to accept the token. err = c.createGlobalACL("acl-replication", rules, consulDC, consulClient) if err != nil { - c.Log.Error(err.Error()) + c.log.Error(err.Error()) return 1 } } - c.Log.Info("server-acl-init completed successfully") + c.log.Info("server-acl-init completed successfully") return 0 } @@ -498,11 +541,11 @@ func (c *Command) untilSucceeds(opName string, op func() error) error { for { err := op() if err == nil { - c.Log.Info(fmt.Sprintf("Success: %s", opName)) + c.log.Info(fmt.Sprintf("Success: %s", opName)) break } - c.Log.Error(fmt.Sprintf("Failure: %s", opName), "err", err) - c.Log.Info("Retrying in " + c.retryDuration.String()) + c.log.Error(fmt.Sprintf("Failure: %s", opName), "err", err) + c.log.Info("Retrying in " + c.retryDuration.String()) // Wait on either the retry duration (in which case we continue) or the // overall command timeout. select { diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index 907b88e890..bbaf6840e9 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -48,6 +48,10 @@ func TestRun_FlagValidation(t *testing.T) { Flags: []string{"-acl-replication-token-file=/notexist", "-server-address=localhost", "-resource-prefix=prefix"}, ExpErr: "Unable to read ACL replication token from file \"/notexist\": open /notexist: no such file or directory", }, + { + Flags: []string{"-bootstrap-token-file=/notexist", "-server-address=localhost", "-resource-prefix=prefix"}, + ExpErr: "Unable to read bootstrap token from file \"/notexist\": open /notexist: no such file or directory", + }, } for _, c := range cases { @@ -341,6 +345,104 @@ func TestRun_TokensReplicatedDC(t *testing.T) { } } +// Test creating each token type when the bootstrap token is provided. +func TestRun_TokensWithProvidedBootstrapToken(t *testing.T) { + t.Parallel() + + cases := []struct { + TokenFlag string + PolicyName string + SecretName string + }{ + { + TokenFlag: "-create-client-token", + PolicyName: "client-token", + SecretName: resourcePrefix + "-client-acl-token", + }, + { + TokenFlag: "-create-sync-token", + PolicyName: "catalog-sync-token", + SecretName: resourcePrefix + "-catalog-sync-acl-token", + }, + { + TokenFlag: "-create-inject-namespace-token", + PolicyName: "connect-inject-token", + SecretName: resourcePrefix + "-connect-inject-acl-token", + }, + { + TokenFlag: "-create-enterprise-license-token", + PolicyName: "enterprise-license-token", + SecretName: resourcePrefix + "-enterprise-license-acl-token", + }, + { + TokenFlag: "-create-snapshot-agent-token", + PolicyName: "client-snapshot-agent-token", + SecretName: resourcePrefix + "-client-snapshot-agent-acl-token", + }, + { + TokenFlag: "-create-mesh-gateway-token", + PolicyName: "mesh-gateway-token", + SecretName: resourcePrefix + "-mesh-gateway-acl-token", + }, + { + TokenFlag: "-create-acl-replication-token", + PolicyName: "acl-replication-token", + SecretName: resourcePrefix + "-acl-replication-acl-token", + }, + } + for _, c := range cases { + t.Run(c.TokenFlag, func(t *testing.T) { + bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + tokenFile, fileCleanup := writeTempFile(t, bootToken) + defer fileCleanup() + + k8s, testAgent := completeBootstrappedSetup(t, bootToken) + + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + cmdArgs := []string{ + "-k8s-namespace", ns, + "-bootstrap-token-file", tokenFile, + "-server-address", strings.Split(testAgent.HTTPAddr, ":")[0], + "-server-port", strings.Split(testAgent.HTTPAddr, ":")[1], + "-resource-prefix", resourcePrefix, + c.TokenFlag, + } + responseCode := cmd.Run(cmdArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + + consul, err := api.NewClient(&api.Config{ + Address: testAgent.HTTPAddr, + Token: bootToken, + }) + require.NoError(t, err) + + // Check that the expected policy was created. + retry.Run(t, func(r *retry.R) { + policyExists(r, c.PolicyName, consul) + }) + + retry.Run(t, func(r *retry.R) { + // Test that the token was created as a Kubernetes Secret. + tokenSecret, err := k8s.CoreV1().Secrets(ns).Get(c.SecretName, metav1.GetOptions{}) + require.NoError(r, err) + require.NotNil(r, tokenSecret) + token, ok := tokenSecret.Data["token"] + require.True(r, ok) + + // Test that the token has the expected policies in Consul. + tokenData, _, err := consul.ACL().TokenReadSelf(&api.QueryOptions{Token: string(token)}) + require.NoError(r, err) + require.Equal(r, c.PolicyName, tokenData.Policies[0].Name) + }) + }) + } +} + // Test the conditions under which we should create the anonymous token // policy. func TestRun_AnonymousTokenPolicy(t *testing.T) { @@ -1010,6 +1112,71 @@ func TestRun_AlreadyBootstrapped(t *testing.T) { }, consulAPICalls) } +// Test if there is a provided bootstrap we skip bootstrapping of the servers +// and continue on to the next step. +func TestRun_SkipBootstrapping_WhenBootstrapTokenIsProvided(t *testing.T) { + t.Parallel() + require := require.New(t) + k8s := fake.NewSimpleClientset() + + bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + tokenFile, fileCleanup := writeTempFile(t, bootToken) + defer fileCleanup() + + type APICall struct { + Method string + Path string + } + var consulAPICalls []APICall + + // Start the Consul server. + consulServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Record all the API calls made. + consulAPICalls = append(consulAPICalls, APICall{ + Method: r.Method, + Path: r.URL.Path, + }) + switch r.URL.Path { + case "/v1/agent/self": + fmt.Fprintln(w, `{"Config": {"Datacenter": "dc1"}}`) + default: + // Send an empty JSON response with code 200 to all calls. + fmt.Fprintln(w, "{}") + } + })) + defer consulServer.Close() + + // Create the Server Pods. + serverURL, err := url.Parse(consulServer.URL) + require.NoError(err) + + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + + responseCode := cmd.Run([]string{ + "-resource-prefix=" + resourcePrefix, + "-k8s-namespace=" + ns, + "-server-address=" + serverURL.Hostname(), + "-server-port=" + serverURL.Port(), + "-bootstrap-token-file=" + tokenFile, + "-create-client-token=false", // disable client token, so there are less calls + }) + require.Equal(0, responseCode, ui.ErrorWriter.String()) + + // Test that the expected API calls were made. + require.Equal([]APICall{ + // We only expect the calls to get the datacenter + { + "GET", + "/v1/agent/self", + }, + }, consulAPICalls) +} + // Test that we exit after timeout. func TestRun_Timeout(t *testing.T) { t.Parallel() @@ -1173,7 +1340,9 @@ func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) { } } -func TestRun_DefaultWithCloudAutoJoin(t *testing.T) { +// Test that when the -server-address contains a cloud-auto join string, +// we are still able to bootstrap ACLs. +func TestRun_CloudAutoJoin(t *testing.T) { t.Parallel() k8s, testSvr := completeSetup(t) @@ -1228,6 +1397,20 @@ func completeSetup(t *testing.T) (*fake.Clientset, *testutil.TestServer) { return k8s, svr } +// Set up test consul agent and kubernetes cluster. +// The consul agent is bootstrapped with the master token. +func completeBootstrappedSetup(t *testing.T, masterToken string) (*fake.Clientset, *testutil.TestServer) { + k8s := fake.NewSimpleClientset() + + svr, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + c.ACL.Enabled = true + c.ACL.Tokens.Master = masterToken + }) + require.NoError(t, err) + + return k8s, svr +} + // completeReplicatedSetup sets up two Consul servers with ACL replication // using the server-acl-init command to start the replication. // Returns the Kubernetes client for the secondary DC, diff --git a/subcommand/server-acl-init/create_or_update.go b/subcommand/server-acl-init/create_or_update.go index 65f55dfc6f..ae890c7b07 100644 --- a/subcommand/server-acl-init/create_or_update.go +++ b/subcommand/server-acl-init/create_or_update.go @@ -58,7 +58,7 @@ func (c *Command) createACL(name, rules string, localToken bool, dc string, cons secretName := c.withPrefix(name + "-acl-token") _, err = c.clientset.CoreV1().Secrets(c.flagK8sNamespace).Get(secretName, metav1.GetOptions{}) if err == nil { - c.Log.Info(fmt.Sprintf("Secret %q already exists", secretName)) + c.log.Info(fmt.Sprintf("Secret %q already exists", secretName)) return nil } @@ -107,7 +107,7 @@ func (c *Command) createOrUpdateACLPolicy(policy api.ACLPolicy, consulClient *ap // updated to be namespace aware. if isPolicyExistsErr(err, policy.Name) { if c.flagEnableNamespaces { - c.Log.Info(fmt.Sprintf("Policy %q already exists, updating", policy.Name)) + c.log.Info(fmt.Sprintf("Policy %q already exists, updating", policy.Name)) // The policy ID is required in any PolicyUpdate call, so first we need to // get the existing policy to extract its ID. @@ -134,7 +134,7 @@ func (c *Command) createOrUpdateACLPolicy(policy api.ACLPolicy, consulClient *ap _, _, err = consulClient.ACL().PolicyUpdate(&policy, &api.WriteOptions{}) return err } else { - c.Log.Info(fmt.Sprintf("Policy %q already exists, skipping update", policy.Name)) + c.log.Info(fmt.Sprintf("Policy %q already exists, skipping update", policy.Name)) return nil } } @@ -169,7 +169,7 @@ func (c *Command) checkAndCreateNamespace(ns string, consulClient *api.Client) e if err != nil { return err } - c.Log.Info("created consul namespace", "name", consulNamespace.Name) + c.log.Info("created consul namespace", "name", consulNamespace.Name) } return nil diff --git a/subcommand/server-acl-init/servers.go b/subcommand/server-acl-init/servers.go index 14f81fd7e8..c280225a4c 100644 --- a/subcommand/server-acl-init/servers.go +++ b/subcommand/server-acl-init/servers.go @@ -11,9 +11,9 @@ import ( ) // bootstrapServers bootstraps ACLs and ensures each server has an ACL token. -func (c *Command) bootstrapServers(bootTokenSecretName, scheme string) (string, error) { +func (c *Command) bootstrapServers(serverAddresses []string, bootTokenSecretName, scheme string) (string, error) { // Pick the first server address to connect to for bootstrapping and set up connection. - firstServerAddr := fmt.Sprintf("%s:%d", c.flagServerAddresses[0], c.flagServerPort) + firstServerAddr := fmt.Sprintf("%s:%d", serverAddresses[0], c.flagServerPort) consulClient, err := api.NewClient(&api.Config{ Address: firstServerAddr, Scheme: scheme, @@ -93,7 +93,7 @@ func (c *Command) bootstrapServers(bootTokenSecretName, scheme string) (string, } // Create new tokens for each server and apply them. - if err := c.setServerTokens(consulClient, string(bootstrapToken), scheme); err != nil { + if err := c.setServerTokens(consulClient, serverAddresses, string(bootstrapToken), scheme); err != nil { return "", err } return string(bootstrapToken), nil @@ -101,14 +101,14 @@ func (c *Command) bootstrapServers(bootTokenSecretName, scheme string) (string, // setServerTokens creates policies and associated ACL token for each server // and then provides the token to the server. -func (c *Command) setServerTokens(consulClient *api.Client, bootstrapToken, scheme string) error { +func (c *Command) setServerTokens(consulClient *api.Client, serverAddresses []string, bootstrapToken, scheme string) error { agentPolicy, err := c.setServerPolicy(consulClient) if err != nil { return err } // Create agent token for each server agent. - for _, host := range c.flagServerAddresses { + for _, host := range serverAddresses { var token *api.ACLToken // We create a new client for each server because we need to call each @@ -156,7 +156,7 @@ func (c *Command) setServerTokens(consulClient *api.Client, bootstrapToken, sche func (c *Command) setServerPolicy(consulClient *api.Client) (api.ACLPolicy, error) { agentRules, err := c.agentRules() if err != nil { - c.Log.Error("Error templating server agent rules", "err", err) + c.log.Error("Error templating server agent rules", "err", err) return api.ACLPolicy{}, err } diff --git a/subcommand/sync-catalog/command_ent_test.go b/subcommand/sync-catalog/command_ent_test.go index bbd09e6b2b..fe696e294c 100644 --- a/subcommand/sync-catalog/command_ent_test.go +++ b/subcommand/sync-catalog/command_ent_test.go @@ -723,8 +723,6 @@ func TestRun_ToConsulNamespacesACLs(t *testing.T) { } // Set up test consul agent and fake kubernetes cluster client -// todo: use this setup method everywhere. The old one (completeSetup) uses -// the test agent instead of the testserver. func completeSetupEnterprise(t *testing.T) (*fake.Clientset, *testutil.TestServer) { k8s := fake.NewSimpleClientset() svr, err := testutil.NewTestServerT(t) From 703f97809878b88e61715de460c5832d738232cc Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 15 Apr 2020 16:51:19 -0700 Subject: [PATCH 04/11] Update subcommand/server-acl-init/command.go Co-Authored-By: Rebecca Zanzig --- subcommand/server-acl-init/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index ceac59b08a..026496707e 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -43,7 +43,7 @@ type Command struct { flagCreateSnapshotAgentToken bool flagCreateMeshGatewayToken bool - // Flags to configure Consul client + // Flags to configure Consul connection flagServerAddresses []string flagServerPort uint flagConsulCACert string From becb9d56f36d68303499ec26bb84a124983c5975 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 15 Apr 2020 22:19:06 -0700 Subject: [PATCH 05/11] Add go-discover tests --- helper/go-discover/discover_test.go | 70 +++++++++++++++++++ helper/go-discover/mocks/mock_provider.go | 24 +++++++ .../get-consul-client-ca/command_test.go | 30 +++----- subcommand/server-acl-init/command.go | 10 +-- subcommand/server-acl-init/command_test.go | 37 +++++----- 5 files changed, 127 insertions(+), 44 deletions(-) create mode 100644 helper/go-discover/discover_test.go create mode 100644 helper/go-discover/mocks/mock_provider.go diff --git a/helper/go-discover/discover_test.go b/helper/go-discover/discover_test.go new file mode 100644 index 0000000000..1ebd30af6b --- /dev/null +++ b/helper/go-discover/discover_test.go @@ -0,0 +1,70 @@ +package godiscover + +import ( + "errors" + "testing" + + "github.com/hashicorp/consul-k8s/helper/go-discover/mocks" + "github.com/hashicorp/go-discover" + "github.com/hashicorp/go-hclog" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestConsulServerAddresses(t *testing.T) { + logger := hclog.New(nil) + + tests := []struct { + name string + discoverString string + want []string + wantErr bool + errMessage string + wantProviderErr bool + }{ + { + "Gets addresses from the provider", + "provider=mock", + []string{"1.1.1.1", "2.2.2.2"}, + false, + "", + false, + }, + { + "Errors when no addresses were discovered", + "provider=mock", + nil, + true, + "could not discover any Consul servers with \"provider=mock\"", + false, + }, + { + "Errors when the the provider errors", + "provider=mock", + nil, + true, + "provider error", + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + provider := mocks.MockProvider{} + providers := map[string]discover.Provider{ + "mock": &provider, + } + if tt.wantProviderErr { + provider.On("Addrs", mock.Anything, mock.Anything).Return(nil, errors.New(tt.errMessage)) + } else { + provider.On("Addrs", mock.Anything, mock.Anything).Return(tt.want, nil) + } + got, err := ConsulServerAddresses(tt.discoverString, providers, logger) + if !tt.wantErr { + require.Equal(t, tt.want, got) + } else { + require.Error(t, err) + require.EqualError(t, err, tt.errMessage) + } + }) + } +} diff --git a/helper/go-discover/mocks/mock_provider.go b/helper/go-discover/mocks/mock_provider.go new file mode 100644 index 0000000000..51afb86a84 --- /dev/null +++ b/helper/go-discover/mocks/mock_provider.go @@ -0,0 +1,24 @@ +package mocks + +import ( + "log" + + "github.com/stretchr/testify/mock" +) + +type MockProvider struct { + mock.Mock +} + +func (m *MockProvider) Addrs(args map[string]string, l *log.Logger) ([]string, error) { + retVal := m.Called(args, l) + addresses := retVal.Get(0) + if addresses != nil { + return addresses.([]string), nil + } + return nil, retVal.Error(1) +} + +func (m *MockProvider) Help() string { + return "mock-provider help" +} diff --git a/subcommand/get-consul-client-ca/command_test.go b/subcommand/get-consul-client-ca/command_test.go index 45725d5a5d..86681fe891 100644 --- a/subcommand/get-consul-client-ca/command_test.go +++ b/subcommand/get-consul-client-ca/command_test.go @@ -3,7 +3,6 @@ package getconsulclientca import ( "fmt" "io/ioutil" - "log" "os" "strings" "sync" @@ -11,12 +10,14 @@ import ( "time" "github.com/hashicorp/consul-k8s/helper/cert" + "github.com/hashicorp/consul-k8s/helper/go-discover/mocks" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/freeport" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/go-discover" "github.com/mitchellh/cli" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -303,13 +304,17 @@ func TestRun_WithProvider(t *testing.T) { ui := cli.NewMockUi() - // create a fake provider + // create a mock provider // that always returns the server address // provided through the cloud-auto join string - provider := &fakeProvider{} + provider := new(mocks.MockProvider) + // create stubs for our MockProvider so that it returns + // the address of the test agent + provider.On("Addrs", mock.Anything, mock.Anything).Return([]string{"127.0.0.1"}, nil) + cmd := Command{ UI: ui, - providers: map[string]discover.Provider{"fake": provider}, + providers: map[string]discover.Provider{"mock": provider}, } caFile, certFile, keyFile, cleanup := generateServerCerts(t) @@ -329,7 +334,7 @@ func TestRun_WithProvider(t *testing.T) { // run the command exitCode := cmd.Run([]string{ - "-server-addr", "provider=fake address=127.0.0.1", + "-server-addr", "provider=mock", "-server-port", strings.Split(a.HTTPSAddr, ":")[1], "-output-file", outputFile.Name(), "-ca-file", caFile, @@ -337,7 +342,7 @@ func TestRun_WithProvider(t *testing.T) { require.Equal(t, 0, exitCode, ui.ErrorWriter.String()) // check that the provider has been called - require.Equal(t, 1, provider.addrsNumCalls, "provider's Addrs method was not called") + provider.AssertNumberOfCalls(t, "Addrs", 1) client, err := api.NewClient(&api.Config{ Address: a.HTTPSAddr, @@ -417,16 +422,3 @@ func generateServerCerts(t *testing.T) (string, string, string, func()) { } return caFile.Name(), certFile.Name(), certKeyFile.Name(), cleanupFunc } - -type fakeProvider struct { - addrsNumCalls int -} - -func (p *fakeProvider) Addrs(args map[string]string, l *log.Logger) ([]string, error) { - p.addrsNumCalls++ - return []string{args["address"]}, nil -} - -func (p *fakeProvider) Help() string { - return "fake-provider help" -} diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 026496707e..90432159ab 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -104,16 +104,16 @@ func (c *Command) init() { c.flags.BoolVar(&c.flagCreateInjectAuthMethod, "create-inject-token", false, "Toggle for creating a connect inject auth method. Deprecated: use -create-inject-auth-method instead.") c.flags.StringVar(&c.flagBindingRuleSelector, "acl-binding-rule-selector", "", - "Selector string for connectInject ACL Binding Rule") + "Selector string for connectInject ACL Binding Rule.") c.flags.BoolVar(&c.flagCreateEntLicenseToken, "create-enterprise-license-token", false, "Toggle for creating a token for the enterprise license job.") c.flags.BoolVar(&c.flagCreateSnapshotAgentToken, "create-snapshot-agent-token", false, "[Enterprise Only] Toggle for creating a token for the Consul snapshot agent deployment.") c.flags.BoolVar(&c.flagCreateMeshGatewayToken, "create-mesh-gateway-token", false, - "Toggle for creating a token for a Connect mesh gateway") + "Toggle for creating a token for a Connect mesh gateway.") c.flags.Var((*flags.AppendSliceValue)(&c.flagServerAddresses), "server-address", - "The IP, DNS name or the cloud auto-join string of the Consul server(s), may be provided 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", "", @@ -143,7 +143,7 @@ func (c *Command) init() { "if mirroring is enabled.") c.flags.BoolVar(&c.flagCreateACLReplicationToken, "create-acl-replication-token", false, - "Toggle for creating a token for ACL replication between datacenters") + "Toggle for creating a token for ACL replication between datacenters.") c.flags.StringVar(&c.flagACLReplicationTokenFile, "acl-replication-token-file", "", "Path to file containing ACL token to be used for ACL replication. If set, ACL replication is enabled.") @@ -244,7 +244,7 @@ func (c *Command) Run(args []string) int { serverAddresses := c.flagServerAddresses // Check if the provided addresses contain a cloud-auto join string. - // If yes, call go-discover to discover addresses of the Consul servers. + // If yes, call godiscover to discover addresses of the Consul servers. if len(c.flagServerAddresses) == 1 && strings.Contains(c.flagServerAddresses[0], "provider=") { var err error serverAddresses, err = godiscover.ConsulServerAddresses(c.flagServerAddresses[0], c.providers, c.log) diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index bbaf6840e9..1bb28a2a99 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -4,7 +4,6 @@ import ( "encoding/base64" "fmt" "io/ioutil" - "log" "math/rand" "net/http" "net/http/httptest" @@ -16,12 +15,14 @@ import ( "time" "github.com/hashicorp/consul-k8s/helper/cert" + "github.com/hashicorp/consul-k8s/helper/go-discover/mocks" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/sdk/freeport" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/go-discover" "github.com/mitchellh/cli" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -759,7 +760,7 @@ func TestRun_DelayedServers(t *testing.T) { testServerReady := make(chan bool) var srv *testutil.TestServer go func() { - // Create the Pods after a delay between 100 and 500ms. + // Start the servers after a delay between 100 and 500ms. // It's randomized to ensure we're not relying on specific timing. delay := 100 + rand.Intn(400) time.Sleep(time.Duration(delay) * time.Millisecond) @@ -854,7 +855,6 @@ func TestRun_NoLeader(t *testing.T) { })) defer consulServer.Close() - // Create the Server Pods. serverURL, err := url.Parse(consulServer.URL) require.NoError(err) @@ -1146,7 +1146,6 @@ func TestRun_SkipBootstrapping_WhenBootstrapTokenIsProvided(t *testing.T) { })) defer consulServer.Close() - // Create the Server Pods. serverURL, err := url.Parse(consulServer.URL) require.NoError(err) @@ -1168,6 +1167,7 @@ func TestRun_SkipBootstrapping_WhenBootstrapTokenIsProvided(t *testing.T) { require.Equal(0, responseCode, ui.ErrorWriter.String()) // Test that the expected API calls were made. + // We expect not to see the call to /v1/acl/bootstrap. require.Equal([]APICall{ // We only expect the calls to get the datacenter { @@ -1349,23 +1349,33 @@ func TestRun_CloudAutoJoin(t *testing.T) { defer testSvr.Stop() require := require.New(t) - provider := &fakeProvider{} + // create a mock provider + // that always returns the server address + // provided through the cloud-auto join string + provider := new(mocks.MockProvider) + // create stubs for our MockProvider so that it returns + // the address of the test agent + provider.On("Addrs", mock.Anything, mock.Anything).Return([]string{"127.0.0.1"}, nil) + // Run the command. ui := cli.NewMockUi() cmd := Command{ UI: ui, clientset: k8s, - providers: map[string]discover.Provider{"fake": provider}, + providers: map[string]discover.Provider{"mock": provider}, } args := []string{ "-k8s-namespace=" + ns, "-resource-prefix=" + resourcePrefix, - "-server-address", "provider=fake address=127.0.0.1", + "-server-address", "provider=mock", "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], } responseCode := cmd.Run(args) require.Equal(0, responseCode, ui.ErrorWriter.String()) + // check that the provider has been called + provider.AssertNumberOfCalls(t, "Addrs", 1) + // Test that the bootstrap kube secret is created. bootToken := getBootToken(t, k8s, resourcePrefix, ns) @@ -1653,18 +1663,5 @@ func writeTempFile(t *testing.T, contents string) (string, func()) { } } -type fakeProvider struct { - addrsNumCalls int -} - -func (p *fakeProvider) Addrs(args map[string]string, l *log.Logger) ([]string, error) { - p.addrsNumCalls++ - return []string{args["address"]}, nil -} - -func (p *fakeProvider) Help() string { - return "fake-provider help" -} - var serviceAccountCACert = "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURDekNDQWZPZ0F3SUJBZ0lRS3pzN05qbDlIczZYYzhFWG91MjVoekFOQmdrcWhraUc5dzBCQVFzRkFEQXYKTVMwd0t3WURWUVFERXlRMU9XVTJaR00wTVMweU1EaG1MVFF3T1RVdFlUSTRPUzB4Wm1NM01EQmhZekZqWXpndwpIaGNOTVRrd05qQTNNVEF4TnpNeFdoY05NalF3TmpBMU1URXhOek14V2pBdk1TMHdLd1lEVlFRREV5UTFPV1UyClpHTTBNUzB5TURobUxUUXdPVFV0WVRJNE9TMHhabU0zTURCaFl6RmpZemd3Z2dFaU1BMEdDU3FHU0liM0RRRUIKQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURaakh6d3FvZnpUcEdwYzBNZElDUzdldXZmdWpVS0UzUEMvYXBmREFnQgo0anpFRktBNzgvOStLVUd3L2MvMFNIZVNRaE4rYThnd2xIUm5BejFOSmNmT0lYeTRkd2VVdU9rQWlGeEg4cGh0CkVDd2tlTk83ejhEb1Y4Y2VtaW5DUkhHamFSbW9NeHBaN2cycFpBSk5aZVB4aTN5MWFOa0ZBWGU5Z1NVU2RqUloKUlhZa2E3d2gyQU85azJkbEdGQVlCK3Qzdld3SjZ0d2pHMFR0S1FyaFlNOU9kMS9vTjBFMDFMekJjWnV4a04xawo4Z2ZJSHk3Yk9GQ0JNMldURURXLzBhQXZjQVByTzhETHFESis2TWpjM3I3K3psemw4YVFzcGIwUzA4cFZ6a2k1CkR6Ly84M2t5dTBwaEp1aWo1ZUI4OFY3VWZQWHhYRi9FdFY2ZnZyTDdNTjRmQWdNQkFBR2pJekFoTUE0R0ExVWQKRHdFQi93UUVBd0lDQkRBUEJnTlZIUk1CQWY4RUJUQURBUUgvTUEwR0NTcUdTSWIzRFFFQkN3VUFBNElCQVFCdgpRc2FHNnFsY2FSa3RKMHpHaHh4SjUyTm5SVjJHY0lZUGVOM1p2MlZYZTNNTDNWZDZHMzJQVjdsSU9oangzS21BCi91TWg2TmhxQnpzZWtrVHowUHVDM3dKeU0yT0dvblZRaXNGbHF4OXNGUTNmVTJtSUdYQ2Ezd0M4ZS9xUDhCSFMKdzcvVmVBN2x6bWozVFFSRS9XMFUwWkdlb0F4bjliNkp0VDBpTXVjWXZQMGhYS1RQQldsbnpJaWphbVU1MHIyWQo3aWEwNjVVZzJ4VU41RkxYL3Z4T0EzeTRyanBraldvVlFjdTFwOFRaclZvTTNkc0dGV3AxMGZETVJpQUhUdk9ICloyM2pHdWs2cm45RFVIQzJ4UGozd0NUbWQ4U0dFSm9WMzFub0pWNWRWZVE5MHd1c1h6M3ZURzdmaWNLbnZIRlMKeHRyNVBTd0gxRHVzWWZWYUdIMk8KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=" var serviceAccountToken = "ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNklpSjkuZXlKcGMzTWlPaUpyZFdKbGNtNWxkR1Z6TDNObGNuWnBZMlZoWTJOdmRXNTBJaXdpYTNWaVpYSnVaWFJsY3k1cGJ5OXpaWEoyYVdObFlXTmpiM1Z1ZEM5dVlXMWxjM0JoWTJVaU9pSmtaV1poZFd4MElpd2lhM1ZpWlhKdVpYUmxjeTVwYnk5elpYSjJhV05sWVdOamIzVnVkQzl6WldOeVpYUXVibUZ0WlNJNkltdG9ZV3RwTFdGeVlXTm9ibWxrTFdOdmJuTjFiQzFqYjI1dVpXTjBMV2x1YW1WamRHOXlMV0YxZEdodFpYUm9iMlF0YzNaakxXRmpZMjlvYm1SaWRpSXNJbXQxWW1WeWJtVjBaWE11YVc4dmMyVnlkbWxqWldGalkyOTFiblF2YzJWeWRtbGpaUzFoWTJOdmRXNTBMbTVoYldVaU9pSnJhR0ZyYVMxaGNtRmphRzVwWkMxamIyNXpkV3d0WTI5dWJtVmpkQzFwYm1wbFkzUnZjaTFoZFhSb2JXVjBhRzlrTFhOMll5MWhZMk52ZFc1MElpd2lhM1ZpWlhKdVpYUmxjeTVwYnk5elpYSjJhV05sWVdOamIzVnVkQzl6WlhKMmFXTmxMV0ZqWTI5MWJuUXVkV2xrSWpvaU4yVTVOV1V4TWprdFpUUTNNeTB4TVdVNUxUaG1ZV0V0TkRJd01UQmhPREF3TVRJeUlpd2ljM1ZpSWpvaWMzbHpkR1Z0T25ObGNuWnBZMlZoWTJOdmRXNTBPbVJsWm1GMWJIUTZhMmhoYTJrdFlYSmhZMmh1YVdRdFkyOXVjM1ZzTFdOdmJtNWxZM1F0YVc1cVpXTjBiM0l0WVhWMGFHMWxkR2h2WkMxemRtTXRZV05qYjNWdWRDSjkuWWk2M01NdHpoNU1CV0tLZDNhN2R6Q0pqVElURTE1aWtGeV9UbnBka19Bd2R3QTlKNEFNU0dFZUhONXZXdEN1dUZqb19sTUpxQkJQSGtLMkFxYm5vRlVqOW01Q29wV3lxSUNKUWx2RU9QNGZVUS1SYzBXMVBfSmpVMXJaRVJIRzM5YjVUTUxnS1BRZ3V5aGFpWkVKNkNqVnRtOXdVVGFncmdpdXFZVjJpVXFMdUY2U1lObTZTckt0a1BTLWxxSU8tdTdDMDZ3Vms1bTV1cXdJVlFOcFpTSUNfNUxzNWFMbXlaVTNuSHZILVY3RTNIbUJoVnlaQUI3NmpnS0IwVHlWWDFJT3NrdDlQREZhck50VTNzdVp5Q2p2cUMtVUpBNnNZZXlTZTRkQk5Lc0tsU1o2WXV4VVVtbjFSZ3YzMllNZEltbnNXZzhraGYtekp2cWdXazdCNUVB" From ae132bca9ccdfc3c23bbfacf12d9d942dac9bbec Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Wed, 15 Apr 2020 23:55:58 -0700 Subject: [PATCH 06/11] Use Kubernetes service DNS name We don't need to read the service from Kube since we can just use Kube DNS directly --- subcommand/server-acl-init/command_test.go | 2 +- subcommand/server-acl-init/connect_inject.go | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index 1bb28a2a99..c26b1f11e5 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -618,7 +618,7 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) { &api.QueryOptions{Token: bootToken}) require.NoError(err) require.Contains(authMethod.Config, "Host") - require.Equal(authMethod.Config["Host"], "https://1.2.3.4:443") + require.Equal(authMethod.Config["Host"], "https://kubernetes.default.svc") require.Contains(authMethod.Config, "CACert") require.Equal(authMethod.Config["CACert"], caCert) require.Contains(authMethod.Config, "ServiceAccountJWT") diff --git a/subcommand/server-acl-init/connect_inject.go b/subcommand/server-acl-init/connect_inject.go index 08c009540a..409d555876 100644 --- a/subcommand/server-acl-init/connect_inject.go +++ b/subcommand/server-acl-init/connect_inject.go @@ -157,21 +157,10 @@ func (c *Command) configureConnectInject(consulClient *api.Client) error { } func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod, error) { - var kubeSvc *apiv1.Service - err := c.untilSucceeds("getting kubernetes service IP", - func() error { - var err error - kubeSvc, err = c.clientset.CoreV1().Services("default").Get("kubernetes", metav1.GetOptions{}) - return err - }) - if err != nil { - return api.ACLAuthMethod{}, err - } - // Get the Secret name for the auth method ServiceAccount. var authMethodServiceAccount *apiv1.ServiceAccount saName := c.withPrefix("connect-injector-authmethod-svc-account") - err = c.untilSucceeds(fmt.Sprintf("getting %s ServiceAccount", saName), + err := c.untilSucceeds(fmt.Sprintf("getting %s ServiceAccount", saName), func() error { var err error authMethodServiceAccount, err = c.clientset.CoreV1().ServiceAccounts(c.flagK8sNamespace).Get(saName, metav1.GetOptions{}) @@ -203,7 +192,7 @@ func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod Description: "Kubernetes AuthMethod", Type: "kubernetes", Config: map[string]interface{}{ - "Host": fmt.Sprintf("https://%s:443", kubeSvc.Spec.ClusterIP), + "Host": "https://kubernetes.default.svc", "CACert": string(saSecret.Data["ca.crt"]), "ServiceAccountJWT": string(saSecret.Data["token"]), }, From d037062b1eeae2ca0306b51f0b3bc78cd4a018d1 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Thu, 16 Apr 2020 10:31:08 -0700 Subject: [PATCH 07/11] Add inject-auth-method-host and inject-auth-method-ca-cert flags These flags are added so that the auth method can be configured with custom credentials for the Kubernetes API. This is useful when Consul servers are running externally because in that case we can't assume that the Kubernetes API location and CA cert we have access to in the cluster is routable/has correct SANs from the Consul servers running outside the cluster. --- subcommand/server-acl-init/command.go | 32 +++++++++---- .../server-acl-init/command_ent_test.go | 4 +- subcommand/server-acl-init/command_test.go | 48 +++++++++++++++++-- subcommand/server-acl-init/connect_inject.go | 22 +++++++-- 4 files changed, 88 insertions(+), 18 deletions(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 90432159ab..288ae7dee9 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -33,15 +33,23 @@ type Command struct { flagResourcePrefix string flagK8sNamespace string - flagAllowDNS bool - flagCreateClientToken bool - flagCreateSyncToken bool - flagCreateInjectToken bool - flagCreateInjectAuthMethod bool - flagBindingRuleSelector string - flagCreateEntLicenseToken bool + flagAllowDNS bool + + flagCreateClientToken bool + + flagCreateSyncToken bool + + flagCreateInjectToken bool + flagCreateInjectAuthMethod bool + flagInjectAuthMethodHost string + flagInjectAuthMethodCACert string + flagBindingRuleSelector string + + flagCreateEntLicenseToken bool + flagCreateSnapshotAgentToken bool - flagCreateMeshGatewayToken bool + + flagCreateMeshGatewayToken bool // Flags to configure Consul connection flagServerAddresses []string @@ -97,14 +105,22 @@ func (c *Command) init() { "Toggle for creating a client agent token. Default is true.") c.flags.BoolVar(&c.flagCreateSyncToken, "create-sync-token", false, "Toggle for creating a catalog sync token.") + c.flags.BoolVar(&c.flagCreateInjectToken, "create-inject-namespace-token", false, "Toggle for creating a connect injector token. Only required when namespaces are enabled.") c.flags.BoolVar(&c.flagCreateInjectAuthMethod, "create-inject-auth-method", false, "Toggle for creating a connect inject auth method.") c.flags.BoolVar(&c.flagCreateInjectAuthMethod, "create-inject-token", false, "Toggle for creating a connect inject auth method. Deprecated: use -create-inject-auth-method instead.") + c.flags.StringVar(&c.flagInjectAuthMethodHost, "inject-auth-method-host", "", + "Kubernetes Host config parameter for the auth method."+ + "If not provided, the default cluster Kuberentes service will be used.") + c.flags.StringVar(&c.flagInjectAuthMethodCACert, "inject-auth-method-ca-cert", "", + "Base64-encoded PEM-encoded CA Certificate for the auth method."+ + "If not provided, the CA cert from the service account for this auth method will be used.") c.flags.StringVar(&c.flagBindingRuleSelector, "acl-binding-rule-selector", "", "Selector string for connectInject ACL Binding Rule.") + c.flags.BoolVar(&c.flagCreateEntLicenseToken, "create-enterprise-license-token", false, "Toggle for creating a token for the enterprise license job.") c.flags.BoolVar(&c.flagCreateSnapshotAgentToken, "create-snapshot-agent-token", false, diff --git a/subcommand/server-acl-init/command_ent_test.go b/subcommand/server-acl-init/command_ent_test.go index 7acc9f9007..7ec5715b44 100644 --- a/subcommand/server-acl-init/command_ent_test.go +++ b/subcommand/server-acl-init/command_ent_test.go @@ -68,7 +68,7 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) { require.NoError(err) require.NotNil(actMethod) require.Equal("kubernetes", actMethod.Type) - require.Equal("Kubernetes AuthMethod", actMethod.Description) + require.Equal("Kubernetes Auth Method", actMethod.Description) require.NotContains(actMethod.Config, "MapNamespaces") require.NotContains(actMethod.Config, "ConsulNamespacePrefix") @@ -181,7 +181,7 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) { require.NoError(err) require.NotNil(method, authMethodName+" not found") require.Equal("kubernetes", method.Type) - require.Equal("Kubernetes AuthMethod", method.Description) + require.Equal("Kubernetes Auth Method", method.Description) require.Contains(method.Config, "MapNamespaces") require.Contains(method.Config, "ConsulNamespacePrefix") require.Equal(true, method.Config["MapNamespaces"]) diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index c26b1f11e5..2d98c3a489 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -33,6 +33,8 @@ var ns = "default" var resourcePrefix = "release-name-consul" func TestRun_FlagValidation(t *testing.T) { + t.Parallel() + cases := []struct { Flags []string ExpErr string @@ -570,14 +572,47 @@ func TestRun_AnonymousTokenPolicy(t *testing.T) { func TestRun_ConnectInjectAuthMethod(t *testing.T) { t.Parallel() + + // Generate CA + _, _, caCertPem, _, err := cert.GenerateCA("Kubernetes CA - Test") + require.NoError(t, err) + cases := map[string]struct { - AuthMethodFlag string + flags []string + expectedHost string + expectedCACert string }{ "-create-inject-token flag": { - AuthMethodFlag: "-create-inject-token", + flags: []string{"-create-inject-token"}, + expectedHost: "https://kubernetes.default.svc", }, "-create-inject-auth-method flag": { - AuthMethodFlag: "-create-inject-auth-method", + flags: []string{"-create-inject-auth-method"}, + expectedHost: "https://kubernetes.default.svc", + }, + "-inject-auth-method-host flag": { + flags: []string{ + "-create-inject-auth-method", + "-inject-auth-method-host=https://my-kube.com", + }, + expectedHost: "https://my-kube.com", + }, + "-inject-auth-method-ca-cert flag": { + flags: []string{ + "-create-inject-auth-method", + "-inject-auth-method-ca-cert=" + base64.StdEncoding.EncodeToString([]byte(caCertPem)), + }, + expectedHost: "https://kubernetes.default.svc", + expectedCACert: caCertPem, + }, + "-inject-auth-method-host and -inject-auth-method-ca-cert flags": { + flags: []string{ + "-create-inject-auth-method", + "-inject-auth-method-host=https://my-kube.com", + "-inject-auth-method-ca-cert=" + base64.StdEncoding.EncodeToString([]byte(caCertPem)), + }, + expectedHost: "https://my-kube.com", + expectedCACert: caCertPem, }, } for testName, c := range cases { @@ -587,6 +622,9 @@ 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() @@ -603,7 +641,7 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) { "-server-port", strings.Split(testSvr.HTTPAddr, ":")[1], "-acl-binding-rule-selector=" + bindingRuleSelector, } - cmdArgs = append(cmdArgs, c.AuthMethodFlag) + cmdArgs = append(cmdArgs, c.flags...) responseCode := cmd.Run(cmdArgs) require.Equal(0, responseCode, ui.ErrorWriter.String()) @@ -618,7 +656,7 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) { &api.QueryOptions{Token: bootToken}) require.NoError(err) require.Contains(authMethod.Config, "Host") - require.Equal(authMethod.Config["Host"], "https://kubernetes.default.svc") + require.Equal(authMethod.Config["Host"], c.expectedHost) require.Contains(authMethod.Config, "CACert") require.Equal(authMethod.Config["CACert"], caCert) require.Contains(authMethod.Config, "ServiceAccountJWT") diff --git a/subcommand/server-acl-init/connect_inject.go b/subcommand/server-acl-init/connect_inject.go index 409d555876..4602949400 100644 --- a/subcommand/server-acl-init/connect_inject.go +++ b/subcommand/server-acl-init/connect_inject.go @@ -1,6 +1,7 @@ package serveraclinit import ( + "encoding/base64" "errors" "fmt" @@ -186,14 +187,29 @@ func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod return api.ACLAuthMethod{}, err } + kubernetesHost := "https://kubernetes.default.svc" + kubernetesCACert := string(saSecret.Data["ca.crt"]) + + // Check if custom auth method Host and CACert are provided + if c.flagInjectAuthMethodHost != "" { + kubernetesHost = c.flagInjectAuthMethodHost + } + if c.flagInjectAuthMethodCACert != "" { + kubernetesCACertBytes, err := base64.StdEncoding.DecodeString(c.flagInjectAuthMethodCACert) + if err != nil { + return api.ACLAuthMethod{}, err + } + kubernetesCACert = string(kubernetesCACertBytes) + } + // Now we're ready to set up Consul's auth method. authMethodTmpl := api.ACLAuthMethod{ Name: authMethodName, - Description: "Kubernetes AuthMethod", + Description: "Kubernetes Auth Method", Type: "kubernetes", Config: map[string]interface{}{ - "Host": "https://kubernetes.default.svc", - "CACert": string(saSecret.Data["ca.crt"]), + "Host": kubernetesHost, + "CACert": kubernetesCACert, "ServiceAccountJWT": string(saSecret.Data["token"]), }, } From 072c5a4796be22b97e388aef0db06c10b537892d Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 21 Apr 2020 10:43:48 -0700 Subject: [PATCH 08/11] Update subcommand/server-acl-init/command.go Co-Authored-By: Rebecca Zanzig --- subcommand/server-acl-init/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 288ae7dee9..53d48c1e95 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -114,7 +114,7 @@ func (c *Command) init() { "Toggle for creating a connect inject auth method. Deprecated: use -create-inject-auth-method instead.") c.flags.StringVar(&c.flagInjectAuthMethodHost, "inject-auth-method-host", "", "Kubernetes Host config parameter for the auth method."+ - "If not provided, the default cluster Kuberentes service will be used.") + "If not provided, the default cluster Kubernetes service will be used.") c.flags.StringVar(&c.flagInjectAuthMethodCACert, "inject-auth-method-ca-cert", "", "Base64-encoded PEM-encoded CA Certificate for the auth method."+ "If not provided, the CA cert from the service account for this auth method will be used.") From de820f044678db154d26e7626e845020e35ea16a Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 21 Apr 2020 10:44:14 -0700 Subject: [PATCH 09/11] Update subcommand/server-acl-init/command.go Co-Authored-By: Rebecca Zanzig --- subcommand/server-acl-init/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 53d48c1e95..c65f458545 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -289,7 +289,7 @@ func (c *Command) Run(args []string) int { if c.flagBootstrapTokenFile != "" { // If bootstrap token is provided, we skip server bootstrapping and use // the provided token to create policies and tokens for the rest of the components. - c.log.Info("Bootstrap token is provided so skipping ACL bootstrapping") + c.log.Info("Bootstrap token is provided so skipping Consul server ACL bootstrapping") bootstrapToken = providedBootstrapToken } else if c.flagACLReplicationTokenFile != "" { // If ACL replication is enabled, we don't need to ACL bootstrap the servers From 2d44151e861392e3f4cac1e54c5655aff3ba99c2 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 21 Apr 2020 10:44:22 -0700 Subject: [PATCH 10/11] Update subcommand/server-acl-init/command.go Co-Authored-By: Rebecca Zanzig --- subcommand/server-acl-init/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index c65f458545..fa80c2b1ea 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -296,7 +296,7 @@ func (c *Command) Run(args []string) int { // since they will be performing replication. // We can use the replication token as our bootstrap token because it // has permissions to create policies and tokens. - c.log.Info("ACL replication is enabled so skipping ACL bootstrapping") + c.log.Info("ACL replication is enabled so skipping Consul server ACL bootstrapping") bootstrapToken = aclReplicationToken } else { // Check if we've already been bootstrapped. From b34867c6318e6d7583e7c37fd9b009ff47cb7141 Mon Sep 17 00:00:00 2001 From: Iryna Shustava Date: Tue, 21 Apr 2020 15:11:12 -0700 Subject: [PATCH 11/11] Remove -inject-auth-method-ca-cert flag We think that using a custom CA that would be different from the one included in the service account is pretty uncommon. --- subcommand/server-acl-init/command.go | 4 ---- subcommand/server-acl-init/command_test.go | 23 -------------------- subcommand/server-acl-init/connect_inject.go | 11 +--------- 3 files changed, 1 insertion(+), 37 deletions(-) diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index fa80c2b1ea..13ff58e5f3 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -42,7 +42,6 @@ type Command struct { flagCreateInjectToken bool flagCreateInjectAuthMethod bool flagInjectAuthMethodHost string - flagInjectAuthMethodCACert string flagBindingRuleSelector string flagCreateEntLicenseToken bool @@ -115,9 +114,6 @@ func (c *Command) init() { c.flags.StringVar(&c.flagInjectAuthMethodHost, "inject-auth-method-host", "", "Kubernetes Host config parameter for the auth method."+ "If not provided, the default cluster Kubernetes service will be used.") - c.flags.StringVar(&c.flagInjectAuthMethodCACert, "inject-auth-method-ca-cert", "", - "Base64-encoded PEM-encoded CA Certificate for the auth method."+ - "If not provided, the CA cert from the service account for this auth method will be used.") c.flags.StringVar(&c.flagBindingRuleSelector, "acl-binding-rule-selector", "", "Selector string for connectInject ACL Binding Rule.") diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index 2d98c3a489..6f1aeab776 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -573,10 +573,6 @@ func TestRun_AnonymousTokenPolicy(t *testing.T) { func TestRun_ConnectInjectAuthMethod(t *testing.T) { t.Parallel() - // Generate CA - _, _, caCertPem, _, err := cert.GenerateCA("Kubernetes CA - Test") - require.NoError(t, err) - cases := map[string]struct { flags []string expectedHost string @@ -597,23 +593,6 @@ func TestRun_ConnectInjectAuthMethod(t *testing.T) { }, expectedHost: "https://my-kube.com", }, - "-inject-auth-method-ca-cert flag": { - flags: []string{ - "-create-inject-auth-method", - "-inject-auth-method-ca-cert=" + base64.StdEncoding.EncodeToString([]byte(caCertPem)), - }, - expectedHost: "https://kubernetes.default.svc", - expectedCACert: caCertPem, - }, - "-inject-auth-method-host and -inject-auth-method-ca-cert flags": { - flags: []string{ - "-create-inject-auth-method", - "-inject-auth-method-host=https://my-kube.com", - "-inject-auth-method-ca-cert=" + base64.StdEncoding.EncodeToString([]byte(caCertPem)), - }, - expectedHost: "https://my-kube.com", - expectedCACert: caCertPem, - }, } for testName, c := range cases { t.Run(testName, func(tt *testing.T) { @@ -1360,8 +1339,6 @@ func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) { "-server-address", strings.Split(serverAddr, ":")[0], "-server-port", strings.Split(serverAddr, ":")[1], "-resource-prefix=" + resourcePrefix, - "-server-address", strings.Split(serverAddr, ":")[0], - "-server-port", strings.Split(serverAddr, ":")[1], }, flag) responseCode := cmd.Run(cmdArgs) require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) diff --git a/subcommand/server-acl-init/connect_inject.go b/subcommand/server-acl-init/connect_inject.go index 4602949400..eb8461e98e 100644 --- a/subcommand/server-acl-init/connect_inject.go +++ b/subcommand/server-acl-init/connect_inject.go @@ -1,7 +1,6 @@ package serveraclinit import ( - "encoding/base64" "errors" "fmt" @@ -188,19 +187,11 @@ func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod } kubernetesHost := "https://kubernetes.default.svc" - kubernetesCACert := string(saSecret.Data["ca.crt"]) // Check if custom auth method Host and CACert are provided if c.flagInjectAuthMethodHost != "" { kubernetesHost = c.flagInjectAuthMethodHost } - if c.flagInjectAuthMethodCACert != "" { - kubernetesCACertBytes, err := base64.StdEncoding.DecodeString(c.flagInjectAuthMethodCACert) - if err != nil { - return api.ACLAuthMethod{}, err - } - kubernetesCACert = string(kubernetesCACertBytes) - } // Now we're ready to set up Consul's auth method. authMethodTmpl := api.ACLAuthMethod{ @@ -209,7 +200,7 @@ func (c *Command) createAuthMethodTmpl(authMethodName string) (api.ACLAuthMethod Type: "kubernetes", Config: map[string]interface{}{ "Host": kubernetesHost, - "CACert": kubernetesCACert, + "CACert": string(saSecret.Data["ca.crt"]), "ServiceAccountJWT": string(saSecret.Data["token"]), }, }