Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

server-acl-init: Add -server-address and -server-port #238

Merged
merged 4 commits into from
Apr 9, 2020
Merged

server-acl-init: Add -server-address and -server-port #238

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2581c5
server-acl-init: Add -server-address and -server-port
ishustava Mar 30, 2020
76a2e10
Improvements from code review
ishustava Apr 9, 2020
c702a2e
Remove Pod refs
ishustava Apr 9, 2020
853ae2b
Update comment
ishustava Apr 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 17 additions & 63 deletions subcommand/server-acl-init/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ type Command struct {
flagConsulCACert string
ishustava marked this conversation as resolved.
Show resolved Hide resolved
flagConsulTLSServerName string
flagUseHTTPS bool
flagServerHosts []string
ishustava marked this conversation as resolved.
Show resolved Hide resolved
flagServerPort uint

// Flags to support namespaces
flagEnableNamespaces bool // Use namespacing on all components
Expand All @@ -57,7 +59,7 @@ type Command struct {
flagInjectK8SNSMirroringPrefix string // Prefix added to Consul namespaces created when mirroring injected services

flagLogLevel string
flagTimeout string
flagTimeout time.Duration

clientset kubernetes.Interface
// cmdTimeout is cancelled when the command timeout is reached.
Expand All @@ -73,14 +75,11 @@ type Command struct {

func (c *Command) init() {
c.flags = flag.NewFlagSet("", flag.ContinueOnError)
c.flags.StringVar(&c.flagReleaseName, "release-name", "",
"Name of Consul Helm release. Deprecated: Use -server-label-selector=component=server,app=consul,release=<release-name> instead")
c.flags.StringVar(&c.flagServerLabelSelector, "server-label-selector", "",
"Selector (label query) to select Consul server statefulset pods, supports '=', '==', and '!='. (e.g. -l key1=value1,key2=value2)")
c.flags.StringVar(&c.flagResourcePrefix, "resource-prefix", "",
"Prefix to use for Kubernetes resources. If not set, the \"<release-name>-consul\" prefix is used, where <release-name> is the value set by the -release-name flag.")
c.flags.IntVar(&c.flagReplicas, "expected-replicas", 1,
"Number of expected Consul server replicas")
c.flags.Var((*flags.AppendSliceValue)(&c.flagServerHosts), "server-address",
"The address of the Consul server(s), may be provided multiple times. At least one value is required.")
ishustava marked this conversation as resolved.
Show resolved Hide resolved
c.flags.UintVar(&c.flagServerPort, "server-port", 8500, "The HTTP or HTTPS port of the Consul server.")
ishustava marked this conversation as resolved.
Show resolved Hide resolved
c.flags.StringVar(&c.flagK8sNamespace, "k8s-namespace", "",
"Name of Kubernetes namespace where the servers are deployed")
c.flags.BoolVar(&c.flagAllowDNS, "allow-dns", false,
Expand Down Expand Up @@ -131,7 +130,7 @@ func (c *Command) init() {
"if mirroring is enabled.")
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.flagTimeout, "timeout", "10m",
c.flags.DurationVar(&c.flagTimeout, "timeout", 10*time.Minute,
lkysow marked this conversation as resolved.
Show resolved Hide resolved
"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",
"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
Expand Down Expand Up @@ -164,29 +163,15 @@ func (c *Command) Run(args []string) int {
return 1
}
if len(c.flags.Args()) > 0 {
c.UI.Error(fmt.Sprintf("Should have no non-flag arguments."))
c.UI.Error("Should have no non-flag arguments.")
return 1
}
timeout, err := time.ParseDuration(c.flagTimeout)
if err != nil {
c.UI.Error(fmt.Sprintf("%q is not a valid timeout: %s", c.flagTimeout, err))
if len(c.flagServerHosts) == 0 {
c.UI.Error("-server-address must be set at least once")
return 1
}
if c.flagReleaseName != "" && c.flagServerLabelSelector != "" {
c.UI.Error("-release-name and -server-label-selector cannot both be set")
return 1
}
if c.flagServerLabelSelector != "" && c.flagResourcePrefix == "" {
c.UI.Error("if -server-label-selector is set -resource-prefix must also be set")
return 1
}
if c.flagReleaseName == "" && c.flagServerLabelSelector == "" {
c.UI.Error("-release-name or -server-label-selector must be set")
return 1
}
// If only the -release-name is set, we use it as the label selector.
if c.flagReleaseName != "" {
c.flagServerLabelSelector = fmt.Sprintf("app=consul,component=server,release=%s", c.flagReleaseName)
if c.flagResourcePrefix == "" {
c.UI.Error("-resource-prefix must be set")
ishustava marked this conversation as resolved.
Show resolved Hide resolved
}
var aclReplicationToken string
if c.flagACLReplicationTokenFile != "" {
Expand All @@ -204,7 +189,7 @@ func (c *Command) Run(args []string) int {
}

var cancel context.CancelFunc
c.cmdTimeout, cancel = context.WithTimeout(context.Background(), timeout)
c.cmdTimeout, cancel = context.WithTimeout(context.Background(), c.flagTimeout)
// The context will only ever be intentionally ended by the timeout.
defer cancel()

Expand All @@ -231,27 +216,6 @@ func (c *Command) Run(args []string) int {
if c.flagUseHTTPS {
scheme = "https"
}
// Wait if there's a rollout of servers.
ssName := c.withPrefix("server")
err = c.untilSucceeds(fmt.Sprintf("waiting for rollout of statefulset %s", ssName), func() error {
// Note: We can't use the -server-label-selector flag to find the statefulset
// because in older versions of consul-helm it wasn't labeled with
// component: server. We also can't drop that label because it's required
// for targeting the right server Pods.
statefulset, err := c.clientset.AppsV1().StatefulSets(c.flagK8sNamespace).Get(ssName, metav1.GetOptions{})
if err != nil {
return err
}
if statefulset.Status.CurrentRevision == statefulset.Status.UpdateRevision {
return nil
}
return fmt.Errorf("rollout is in progress (CurrentRevision=%s UpdateRevision=%s)",
statefulset.Status.CurrentRevision, statefulset.Status.UpdateRevision)
})
if err != nil {
c.Log.Error(err.Error())
return 1
}

var updateServerPolicy bool
var bootstrapToken string
Expand All @@ -265,6 +229,7 @@ func (c *Command) Run(args []string) int {
bootstrapToken = aclReplicationToken
} else {
// Check if we've already been bootstrapped.
var err error
bootTokenSecretName := c.withPrefix("bootstrap-acl-token")
bootstrapToken, err = c.getBootstrapToken(bootTokenSecretName)
if err != nil {
Expand All @@ -291,12 +256,7 @@ func (c *Command) Run(args []string) int {
}

// For all of the next operations we'll need a Consul client.
serverPods, err := c.getConsulServers(1, scheme)
if err != nil {
c.Log.Error(err.Error())
return 1
}
serverAddr := serverPods[0].Addr
serverAddr := fmt.Sprintf("%s:%d", c.flagServerHosts[0], c.flagServerPort)
ishustava marked this conversation as resolved.
Show resolved Hide resolved
consulClient, err := api.NewClient(&api.Config{
Address: serverAddr,
Scheme: scheme,
Expand Down Expand Up @@ -544,15 +504,9 @@ func (c *Command) untilSucceeds(opName string, op func() error) error {
}

// withPrefix returns the name of resource with the correct prefix based
// on the -release-name or -resource-prefix flags.
// on the -resource-prefix flags.
ishustava marked this conversation as resolved.
Show resolved Hide resolved
func (c *Command) withPrefix(resource string) string {
if c.flagResourcePrefix != "" {
return fmt.Sprintf("%s-%s", c.flagResourcePrefix, resource)
}
// This is to support an older version of the Helm chart that only specified
// the -release-name flag. We ensure that this is set if -resource-prefix
// is not set when parsing the flags.
return fmt.Sprintf("%s-consul-%s", c.flagReleaseName, resource)
return fmt.Sprintf("%s-%s", c.flagResourcePrefix, resource)
}

const synopsis = "Initialize ACLs on Consul servers and other components."
Expand Down
35 changes: 17 additions & 18 deletions subcommand/server-acl-init/command_ent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package serveraclinit

import (
"strings"
"testing"

"github.com/hashicorp/consul/api"
Expand All @@ -21,7 +22,7 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) {
consulDestNamespaces := []string{"default", "destination"}
for _, consulDestNamespace := range consulDestNamespaces {
t.Run(consulDestNamespace, func(tt *testing.T) {
k8s, testAgent := completeEnterpriseSetup(tt, resourcePrefix, ns)
k8s, testAgent := completeEnterpriseSetup(tt)
defer testAgent.Stop()
setUpK8sServiceAccount(tt, k8s)
require := require.New(tt)
Expand All @@ -33,10 +34,10 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) {
}
cmd.init()
args := []string{
"-server-label-selector=component=server,app=consul,release=" + releaseName,
"-server-address=" + strings.Split(testAgent.HTTPAddr, ":")[0],
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-expected-replicas=1",
"-create-inject-auth-method",
"-enable-namespaces",
"-consul-inject-destination-namespace", consulDestNamespace,
Expand Down Expand Up @@ -141,7 +142,7 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) {

for name, c := range cases {
t.Run(name, func(tt *testing.T) {
k8s, testAgent := completeEnterpriseSetup(t, resourcePrefix, ns)
k8s, testAgent := completeEnterpriseSetup(t)
defer testAgent.Stop()
setUpK8sServiceAccount(tt, k8s)
require := require.New(tt)
Expand All @@ -153,10 +154,10 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) {
}
cmd.init()
args := []string{
"-server-label-selector=component=server,app=consul,release=" + releaseName,
"-server-address=" + strings.Split(testAgent.HTTPAddr, ":")[0],
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-expected-replicas=1",
"-create-inject-auth-method",
"-enable-namespaces",
"-enable-inject-k8s-namespace-mirroring",
Expand Down Expand Up @@ -208,13 +209,14 @@ func TestRun_ACLPolicyUpdates(t *testing.T) {
k8sNamespaceFlags := []string{"default", "other"}
for _, k8sNamespaceFlag := range k8sNamespaceFlags {
t.Run(k8sNamespaceFlag, func(t *testing.T) {
k8s, testAgent := completeEnterpriseSetup(t, resourcePrefix, k8sNamespaceFlag)
k8s, testAgent := completeEnterpriseSetup(t)
defer testAgent.Stop()
require := require.New(t)

ui := cli.NewMockUi()
firstRunArgs := []string{
"-server-label-selector=component=server,app=consul,release=" + releaseName,
"-server-address=" + strings.Split(testAgent.HTTPAddr, ":")[0],
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace", k8sNamespaceFlag,
"-create-client-token",
Expand All @@ -224,7 +226,6 @@ func TestRun_ACLPolicyUpdates(t *testing.T) {
"-create-inject-namespace-token",
"-create-snapshot-agent-token",
"-create-enterprise-license-token",
"-expected-replicas=1",
}
// Our second run, we're going to update from namespaces disabled to
// namespaces enabled with a single destination ns.
Expand Down Expand Up @@ -501,16 +502,16 @@ func TestRun_ConnectInject_Updates(t *testing.T) {
for name, c := range cases {
t.Run(name, func(tt *testing.T) {
require := require.New(tt)
k8s, testAgent := completeEnterpriseSetup(tt, resourcePrefix, ns)
k8s, testAgent := completeEnterpriseSetup(tt)
defer testAgent.Stop()
setUpK8sServiceAccount(tt, k8s)

ui := cli.NewMockUi()
defaultArgs := []string{
"-server-label-selector=component=server,app=consul,release=" + releaseName,
"-server-address=" + strings.Split(testAgent.HTTPAddr, ":")[0],
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-expected-replicas=1",
"-create-inject-auth-method",
}

Expand Down Expand Up @@ -632,7 +633,7 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) {
}
for testName, c := range cases {
t.Run(testName, func(t *testing.T) {
k8s, testSvr := completeEnterpriseSetup(t, resourcePrefix, ns)
k8s, testSvr := completeEnterpriseSetup(t)
defer testSvr.Stop()
require := require.New(t)

Expand All @@ -644,10 +645,10 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) {
}
cmd.init()
cmdArgs := []string{
"-server-label-selector=component=server,app=consul,release=" + releaseName,
"-server-address", strings.Split(testSvr.HTTPAddr, ":")[0],
"-server-port", strings.Split(testSvr.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-expected-replicas=1",
"-enable-namespaces",
c.TokenFlag,
}
Expand Down Expand Up @@ -693,15 +694,13 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) {
}

// Set up test consul agent and kubernetes cluster.
func completeEnterpriseSetup(t *testing.T, prefix string, k8sNamespace string) (*fake.Clientset, *testutil.TestServer) {
func completeEnterpriseSetup(t *testing.T) (*fake.Clientset, *testutil.TestServer) {
k8s := fake.NewSimpleClientset()

svr, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) {
c.ACL.Enabled = true
})
require.NoError(t, err)

createTestK8SResources(t, k8s, svr.HTTPAddr, prefix, "http", k8sNamespace)

return k8s, svr
}
Loading