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

add ACL support for health checks controller #368

Merged
merged 13 commits into from
Nov 4, 2020
Merged

add ACL support for health checks controller #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e708e13
add ACL support for health checks
kschoche Oct 26, 2020
f0bb511
update name of variables and flags and remove namespace_prefix rule
kschoche Oct 27, 2020
ccd3231
undeprecate create-inject-token flag and deprecate others and update …
kschoche Oct 29, 2020
7f5b97f
update flag name
kschoche Oct 29, 2020
004af05
update consul-ent tests for new flags
kschoche Oct 29, 2020
cf9e83f
change variable name
kschoche Oct 29, 2020
dfb42b5
Update tests for health checks acls (and some refactoring) (#371)
lkysow Oct 30, 2020
582b817
Revert t => tt changes (#374)
lkysow Oct 30, 2020
8a93c1e
add deprecated test
kschoche Oct 30, 2020
eea97aa
Apply suggestions from code review
kschoche Nov 2, 2020
3d3439b
Apply suggestions from code review
kschoche Nov 2, 2020
c549329
update function name
kschoche Nov 2, 2020
307ba52
address review comments
kschoche Nov 4, 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
2 changes: 1 addition & 1 deletion subcommand/inject-connect/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func TestRun_FlagValidation(t *testing.T) {
expErr: "request must be <= limit: -lifecycle-sidecar-cpu-request value of \"50m\" is greater than the -lifecycle-sidecar-cpu-limit value of \"25m\"",
},
{
flags: []string{"-consul-k8s-image", "kschoche/consul-k8s-dev",
flags: []string{"-consul-k8s-image", "hashicorpdev/consul-k8s:latest",
kschoche marked this conversation as resolved.
Show resolved Hide resolved
"-enable-health-checks-controller=true"},
expErr: "CONSUL_HTTP_ADDR is not specified",
},
Expand Down
71 changes: 43 additions & 28 deletions subcommand/server-acl-init/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ type Command struct {
// Flag to support a custom bootstrap token
flagBootstrapTokenFile string

// Flag to indicate that the health checks controller is enabled.
flagEnableHealthChecks bool

flagLogLevel string
flagTimeout time.Duration

Expand Down Expand Up @@ -116,12 +119,19 @@ func (c *Command) init() {
"The Consul node name to register for catalog sync. Defaults to k8s-sync. To be discoverable "+
"via DNS, the name should only contain alpha-numerics and dashes.")

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.")
// Previously when this flag was set, -enable-namespaces and -create-inject-auth-method
// were always passed, so now we just look at those flags and ignore
// this flag. We keep the flag here though so there's no error if it's
// passed.
var unused bool
c.flags.BoolVar(&unused, "create-inject-namespace-token", false,
"Toggle for creating a connect injector token. Only required when namespaces are enabled. "+
"Deprecated: set -enable-namespaces and -create-inject-token instead.")

c.flags.BoolVar(&c.flagCreateInjectToken, "create-inject-auth-method", false,
"Toggle for creating a connect inject auth method. Deprecated: use -create-inject-token instead.")
c.flags.BoolVar(&c.flagCreateInjectToken, "create-inject-token", false,
"Toggle for creating a connect inject auth method and an ACL token. The ACL token will only be created if either of the -enable-namespaces or -enable-health-checks flags is set.")
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.")
Expand Down Expand Up @@ -185,6 +195,9 @@ func (c *Command) init() {
"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.BoolVar(&c.flagEnableHealthChecks, "enable-health-checks", false,
"Toggle for adding ACL rules for the health check controller to the connect ACL token. Requires -create-inject-token to be also be set.")

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",
Expand Down Expand Up @@ -458,23 +471,33 @@ func (c *Command) Run(args []string) int {
}

if c.flagCreateInjectToken {
injectRules, err := c.injectRules()
err := c.configureConnectInjectAuthMethod(consulClient)
if err != nil {
c.log.Error("Error templating inject rules", "err", err)
c.log.Error(err.Error())
return 1
}

// If namespaces are enabled, the policy and token needs to be global
// to be allowed to create namespaces.
if c.flagEnableNamespaces {
err = c.createGlobalACL("connect-inject", injectRules, consulDC, consulClient)
} else {
err = c.createLocalACL("connect-inject", injectRules, consulDC, consulClient)
}
// If health checks or namespaces are enabled,
// then the connect injector needs an ACL token.
if c.flagEnableNamespaces || c.flagEnableHealthChecks {
injectRules, err := c.injectRules()
if err != nil {
c.log.Error("Error templating inject rules", "err", err)
return 1
}

if err != nil {
c.log.Error(err.Error())
return 1
// If namespaces are enabled, the policy and token need to be global
// to be allowed to create namespaces.
if c.flagEnableNamespaces {
err = c.createGlobalACL("connect-inject", injectRules, consulDC, consulClient)
} else {
err = c.createLocalACL("connect-inject", injectRules, consulDC, consulClient)
}

if err != nil {
c.log.Error(err.Error())
return 1
}
}
}

Expand Down Expand Up @@ -622,14 +645,6 @@ func (c *Command) Run(args []string) int {
}
}

if c.flagCreateInjectAuthMethod {
err := c.configureConnectInject(consulClient)
if err != nil {
c.log.Error(err.Error())
return 1
}
}

if c.flagCreateACLReplicationToken {
rules, err := c.aclReplicationRules()
if err != nil {
Expand Down Expand Up @@ -763,14 +778,14 @@ func (c *Command) createAnonymousPolicy() bool {
// Consul DNS requires the anonymous policy because DNS queries don't
// have ACL tokens.
(c.flagAllowDNS ||
// If the connect auth method and ACL replication token are being
// If connect is enabled and the ACL replication token is being
// created then we know we're using multi-dc Connect.
// In this case the anonymous policy is required because Connect
// services in Kubernetes have local tokens which are stripped
// on cross-dc API calls. The cross-dc API calls thus use the anonymous
// token. Cross-dc API calls are needed by the Connect proxies to talk
// cross-dc.
(c.flagCreateInjectAuthMethod && c.flagCreateACLReplicationToken))
(c.flagCreateInjectToken && c.flagCreateACLReplicationToken))
}

func (c *Command) validateFlags() error {
Expand Down
42 changes: 28 additions & 14 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 @@ -25,7 +25,7 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) {
t.Run(consulDestNamespace, func(tt *testing.T) {
k8s, testAgent := completeEnterpriseSetup(tt)
defer testAgent.Stop()
setUpK8sServiceAccount(tt, k8s)
setUpK8sServiceAccount(tt, k8s, ns)
require := require.New(tt)

ui := cli.NewMockUi()
Expand All @@ -39,7 +39,7 @@ func TestRun_ConnectInject_SingleDestinationNamespace(t *testing.T) {
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-create-inject-auth-method",
"-create-inject-token",
"-enable-namespaces",
"-consul-inject-destination-namespace", consulDestNamespace,
"-acl-binding-rule-selector=serviceaccount.name!=default",
Expand Down Expand Up @@ -143,9 +143,9 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) {

for name, c := range cases {
t.Run(name, func(tt *testing.T) {
k8s, testAgent := completeEnterpriseSetup(t)
k8s, testAgent := completeEnterpriseSetup(tt)
defer testAgent.Stop()
setUpK8sServiceAccount(tt, k8s)
setUpK8sServiceAccount(tt, k8s, ns)
require := require.New(tt)

ui := cli.NewMockUi()
Expand All @@ -159,7 +159,7 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) {
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-create-inject-auth-method",
"-create-inject-token",
"-enable-namespaces",
"-enable-inject-k8s-namespace-mirroring",
"-inject-k8s-namespace-mirroring-prefix", c.MirroringPrefix,
Expand All @@ -169,7 +169,7 @@ func TestRun_ConnectInject_NamespaceMirroring(t *testing.T) {
responseCode := cmd.Run(args)
require.Equal(0, responseCode, ui.ErrorWriter.String())

bootToken := getBootToken(t, k8s, resourcePrefix, ns)
bootToken := getBootToken(tt, k8s, resourcePrefix, ns)
consul, err := api.NewClient(&api.Config{
Address: testAgent.HTTPAddr,
Token: bootToken,
Expand Down Expand Up @@ -211,6 +211,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) {
for _, k8sNamespaceFlag := range k8sNamespaceFlags {
t.Run(k8sNamespaceFlag, func(t *testing.T) {
k8s, testAgent := completeEnterpriseSetup(t)
setUpK8sServiceAccount(t, k8s, k8sNamespaceFlag)
defer testAgent.Stop()
require := require.New(t)

Expand All @@ -224,7 +225,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) {
"-allow-dns",
"-create-mesh-gateway-token",
"-create-sync-token",
"-create-inject-namespace-token",
"-create-inject-token",
"-create-snapshot-agent-token",
"-create-enterprise-license-token",
"-ingress-gateway-name=gw",
Expand Down Expand Up @@ -259,7 +260,6 @@ func TestRun_ACLPolicyUpdates(t *testing.T) {
"anonymous-token-policy",
"client-token",
"catalog-sync-token",
"connect-inject-token",
"mesh-gateway-token",
"client-snapshot-agent-token",
"enterprise-license-token",
Expand Down Expand Up @@ -407,7 +407,6 @@ func TestRun_ConnectInject_Updates(t *testing.T) {
"no ns => single dest ns": {
FirstRunArgs: nil,
SecondRunArgs: []string{
"-create-inject-auth-method",
"-enable-namespaces",
"-consul-inject-destination-namespace=dest",
},
Expand Down Expand Up @@ -517,15 +516,15 @@ func TestRun_ConnectInject_Updates(t *testing.T) {
require := require.New(tt)
k8s, testAgent := completeEnterpriseSetup(tt)
defer testAgent.Stop()
setUpK8sServiceAccount(tt, k8s)
setUpK8sServiceAccount(tt, k8s, ns)

ui := cli.NewMockUi()
defaultArgs := []string{
"-server-address=" + strings.Split(testAgent.HTTPAddr, ":")[0],
"-server-port=" + strings.Split(testAgent.HTTPAddr, ":")[1],
"-resource-prefix=" + resourcePrefix,
"-k8s-namespace=" + ns,
"-create-inject-auth-method",
"-create-inject-token",
}

// First run. NOTE: we don't assert anything here since we've
Expand All @@ -549,7 +548,7 @@ func TestRun_ConnectInject_Updates(t *testing.T) {
require.Equal(0, responseCode, ui.ErrorWriter.String())

// Now check that everything is as expected.
bootToken := getBootToken(t, k8s, resourcePrefix, ns)
bootToken := getBootToken(tt, k8s, resourcePrefix, ns)
consul, err := api.NewClient(&api.Config{
Address: testAgent.HTTPAddr,
Token: bootToken,
Expand Down Expand Up @@ -608,8 +607,8 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) {
SecretNames: []string{resourcePrefix + "-catalog-sync-acl-token"},
LocalToken: false,
},
"connect-inject-namespace token": {
TokenFlags: []string{"-create-inject-namespace-token"},
"connect-inject-token": {
TokenFlags: []string{"-create-inject-token", "-enable-namespaces"},
kschoche marked this conversation as resolved.
Show resolved Hide resolved
PolicyNames: []string{"connect-inject-token"},
PolicyDCs: nil,
SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"},
Expand Down Expand Up @@ -669,10 +668,25 @@ func TestRun_TokensWithNamespacesEnabled(t *testing.T) {
SecretNames: []string{resourcePrefix + "-acl-replication-acl-token"},
LocalToken: false,
},
"inject token with namespaces (deprecated)": {
TokenFlags: []string{"-create-inject-auth-method", "-enable-namespaces", "-create-inject-namespace-token"},
PolicyNames: []string{"connect-inject-token"},
PolicyDCs: nil,
SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"},
LocalToken: false,
},
"inject token with health checks and namespaces": {
TokenFlags: []string{"-create-inject-token", "-enable-namespaces", "-enable-health-checks"},
PolicyNames: []string{"connect-inject-token"},
PolicyDCs: nil,
SecretNames: []string{resourcePrefix + "-connect-inject-acl-token"},
LocalToken: false,
},
}
for testName, c := range cases {
t.Run(testName, func(t *testing.T) {
k8s, testSvr := completeEnterpriseSetup(t)
setUpK8sServiceAccount(t, k8s, ns)
defer testSvr.Stop()
require := require.New(t)

Expand Down
Loading