From 40d7ebc3180fc9ff102dff3e4e593e39e363eafe Mon Sep 17 00:00:00 2001 From: Ronald Date: Fri, 8 Sep 2023 14:39:09 -0400 Subject: [PATCH] [NET-5330] Support templated policies in Binding rules (#18719) * [NET-5330] Support templated policies in Binding rules * changelog for templated policy support in binding rules --- .changelog/18719.txt | 7 + agent/consul/acl_endpoint.go | 11 +- agent/consul/acl_endpoint_test.go | 67 +++++- agent/consul/auth/binder.go | 89 +++++-- agent/consul/auth/binder_test.go | 69 +++--- agent/consul/auth/login.go | 1 + agent/consul/state/acl.go | 3 +- agent/structs/acl.go | 25 +- api/acl.go | 7 + command/acl/acl_helpers.go | 9 + .../bindingrule/create/bindingrule_create.go | 31 ++- .../create/bindingrule_create_test.go | 53 +++++ command/acl/bindingrule/formatter.go | 7 + .../bindingrule/update/bindingrule_update.go | 35 ++- .../update/bindingrule_update_test.go | 225 +++++++++++++++--- 15 files changed, 550 insertions(+), 89 deletions(-) create mode 100644 .changelog/18719.txt diff --git a/.changelog/18719.txt b/.changelog/18719.txt new file mode 100644 index 0000000000..4da370b91b --- /dev/null +++ b/.changelog/18719.txt @@ -0,0 +1,7 @@ +```release-note:feature +acl: Add BindRule support for templated policies. Add new BindType: templated-policy and BindVar field for templated policy variables. +``` + +```release-note:feature +cli: Add `bind-var` flag to `consul acl binding-rule` for templated policy variables. +``` \ No newline at end of file diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index 7552c6c14c..300d1097b8 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -1719,18 +1719,23 @@ func (a *ACL) BindingRuleSet(args *structs.ACLBindingRuleSetRequest, reply *stru return fmt.Errorf("Invalid Binding Rule: no BindName is set") } + if rule.BindType != structs.BindingRuleBindTypeTemplatedPolicy && rule.BindVars != nil { + return fmt.Errorf("invalid Binding Rule: BindVars cannot be set when bind type is not templated-policy.") + } + switch rule.BindType { case structs.BindingRuleBindTypeService: case structs.BindingRuleBindTypeNode: case structs.BindingRuleBindTypeRole: + case structs.BindingRuleBindTypeTemplatedPolicy: default: return fmt.Errorf("Invalid Binding Rule: unknown BindType %q", rule.BindType) } - if valid, err := auth.IsValidBindName(rule.BindType, rule.BindName, blankID.ProjectedVarNames()); err != nil { - return fmt.Errorf("Invalid Binding Rule: invalid BindName: %v", err) + if valid, err := auth.IsValidBindNameOrBindVars(rule.BindType, rule.BindName, rule.BindVars, blankID.ProjectedVarNames()); err != nil { + return fmt.Errorf("Invalid Binding Rule: invalid BindName or BindVars: %v", err) } else if !valid { - return fmt.Errorf("Invalid Binding Rule: invalid BindName") + return fmt.Errorf("Invalid Binding Rule: invalid BindName or BindVars") } req := &structs.ACLBindingRuleBatchSetRequest{ diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index d844efb778..39840942d1 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -3573,7 +3573,7 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) { } } - requireSetErrors := func(t *testing.T, reqRule structs.ACLBindingRule) { + requireSetErrors := func(t *testing.T, reqRule structs.ACLBindingRule, msg ...string) { req := structs.ACLBindingRuleSetRequest{ Datacenter: "dc1", BindingRule: reqRule, @@ -3583,6 +3583,10 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) { err := aclEp.BindingRuleSet(&req, &resp) require.Error(t, err) + + for _, s := range msg { + require.Contains(t, err.Error(), s) + } } requireOK := func(t *testing.T, reqRule structs.ACLBindingRule) *structs.ACLBindingRule { @@ -3659,6 +3663,40 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) { require.Equal(t, "test-node", rule.BindName) }) + t.Run("templated policy", func(t *testing.T) { + req := structs.ACLBindingRuleSetRequest{ + Datacenter: "dc1", + BindingRule: structs.ACLBindingRule{ + Description: "templated policy binding rule", + AuthMethod: testAuthMethod.Name, + Selector: "serviceaccount.name==abc", + BindType: structs.BindingRuleBindTypeTemplatedPolicy, + BindName: api.ACLTemplatedPolicyNodeName, + BindVars: &structs.ACLTemplatedPolicyVariables{ + Name: "test-node", + }, + }, + WriteRequest: structs.WriteRequest{Token: TestDefaultInitialManagementToken}, + } + var resp structs.ACLBindingRule + + err := aclEp.BindingRuleSet(&req, &resp) + require.NoError(t, err) + require.NotNil(t, resp.ID) + + // Get the rule directly to validate that it exists + ruleResp, err := retrieveTestBindingRule(codec, TestDefaultInitialManagementToken, "dc1", resp.ID) + require.NoError(t, err) + rule := ruleResp.BindingRule + + require.NotEmpty(t, rule.ID) + require.Equal(t, rule.Description, "templated policy binding rule") + require.Equal(t, rule.AuthMethod, testAuthMethod.Name) + require.Equal(t, "serviceaccount.name==abc", rule.Selector) + require.Equal(t, structs.BindingRuleBindTypeTemplatedPolicy, rule.BindType) + require.Equal(t, api.ACLTemplatedPolicyNodeName, rule.BindName) + }) + t.Run("Update fails; cannot change method name", func(t *testing.T) { reqRule := newRule() reqRule.ID = ruleID @@ -3775,10 +3813,35 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) { requireSetErrors(t, reqRule) }) + t.Run("Create fails; when bind vars is set for non templated policy", func(t *testing.T) { + reqRule := newRule() + reqRule.BindVars = &structs.ACLTemplatedPolicyVariables{ + Name: "test", + } + requireSetErrors(t, reqRule, "invalid Binding Rule: BindVars cannot be set when bind type is not templated-policy.") + }) + + t.Run("Create fails; when missing required bindvars", func(t *testing.T) { + reqRule := newRule() + reqRule.BindName = api.ACLTemplatedPolicyServiceName + reqRule.BindType = structs.BindingRuleBindTypeTemplatedPolicy + requireSetErrors(t, reqRule, "Invalid Binding Rule: invalid BindName or BindVars") + }) + + t.Run("Create fails; when bindvars contains unknown vars", func(t *testing.T) { + reqRule := newRule() + reqRule.BindName = api.ACLTemplatedPolicyServiceName + reqRule.BindType = structs.BindingRuleBindTypeTemplatedPolicy + reqRule.BindVars = &structs.ACLTemplatedPolicyVariables{ + Name: "method-${serviceaccount.bizarroname}", + } + requireSetErrors(t, reqRule, "Invalid Binding Rule: invalid BindName or BindVars") + }) + t.Run("Create fails; invalid bind type", func(t *testing.T) { reqRule := newRule() reqRule.BindType = "invalid" - requireSetErrors(t, reqRule) + requireSetErrors(t, reqRule, "Invalid Binding Rule: unknown BindType") }) t.Run("Create fails; bind name with unknown vars", func(t *testing.T) { diff --git a/agent/consul/auth/binder.go b/agent/consul/auth/binder.go index aed1e0f371..3cc56aadd6 100644 --- a/agent/consul/auth/binder.go +++ b/agent/consul/auth/binder.go @@ -43,6 +43,7 @@ type Bindings struct { Roles []structs.ACLTokenRoleLink ServiceIdentities []*structs.ACLServiceIdentity NodeIdentities []*structs.ACLNodeIdentity + TemplatedPolicies structs.ACLTemplatedPolicies EnterpriseMeta acl.EnterpriseMeta } @@ -55,6 +56,7 @@ func (b *Bindings) None() bool { return len(b.ServiceIdentities) == 0 && len(b.NodeIdentities) == 0 && + len(b.TemplatedPolicies) == 0 && len(b.Roles) == 0 } @@ -86,10 +88,10 @@ func (b *Binder) Bind(authMethod *structs.ACLAuthMethod, verifiedIdentity *authm return &bindings, nil } - // Compute role, service identity, or node identity names by interpolating + // Compute role, service identity, node identity or templated policy names by interpolating // the identity's projected variables into the rule BindName templates. for _, rule := range matchingRules { - bindName, valid, err := computeBindName(rule.BindType, rule.BindName, verifiedIdentity.ProjectedVars) + bindName, templatedPolicy, valid, err := computeBindNameAndVars(rule.BindType, rule.BindName, rule.BindVars, verifiedIdentity.ProjectedVars) switch { case err != nil: return nil, fmt.Errorf("cannot compute %q bind name for bind target: %w", rule.BindType, err) @@ -109,6 +111,9 @@ func (b *Binder) Bind(authMethod *structs.ACLAuthMethod, verifiedIdentity *authm Datacenter: b.datacenter, }) + case structs.BindingRuleBindTypeTemplatedPolicy: + bindings.TemplatedPolicies = append(bindings.TemplatedPolicies, templatedPolicy) + case structs.BindingRuleBindTypeRole: _, role, err := b.store.ACLRoleGetByName(nil, bindName, &bindings.EnterpriseMeta) if err != nil { @@ -126,9 +131,9 @@ func (b *Binder) Bind(authMethod *structs.ACLAuthMethod, verifiedIdentity *authm return &bindings, nil } -// IsValidBindName returns whether the given BindName template produces valid +// IsValidBindNameOrBindVars returns whether the given BindName and/or BindVars template produces valid // results when interpolating the auth method's available variables. -func IsValidBindName(bindType, bindName string, availableVariables []string) (bool, error) { +func IsValidBindNameOrBindVars(bindType, bindName string, bindVars *structs.ACLTemplatedPolicyVariables, availableVariables []string) (bool, error) { if bindType == "" || bindName == "" { return false, nil } @@ -138,25 +143,32 @@ func IsValidBindName(bindType, bindName string, availableVariables []string) (bo fakeVarMap[v] = "fake" } - _, valid, err := computeBindName(bindType, bindName, fakeVarMap) + _, _, valid, err := computeBindNameAndVars(bindType, bindName, bindVars, fakeVarMap) if err != nil { return false, err } return valid, nil } -// computeBindName processes the HIL for the provided bind type+name using the -// projected variables. +// computeBindNameAndVars processes the HIL for the provided bind type+name+vars using the +// projected variables. When bindtype is templated-policy, it returns the resulting templated policy +// otherwise, returns nil // -// - If the HIL is invalid ("", false, AN_ERROR) is returned. -// - If the computed name is not valid for the type ("INVALID_NAME", false, nil) is returned. -// - If the computed name is valid for the type ("VALID_NAME", true, nil) is returned. -func computeBindName(bindType, bindName string, projectedVars map[string]string) (string, bool, error) { +// when bindtype is not templated-policy: it evaluates bindName +// - If the HIL is invalid ("", nil, false, AN_ERROR) is returned. +// - If the computed name is not valid for the type ("INVALID_NAME", nil, false, nil) is returned. +// - If the computed name is valid for the type ("VALID_NAME", nil, true, nil) is returned. +// when bindtype is templated-policy: it evalueates both bindName and bindVars +// - If the computed bindvars(failing templated policy schema validation) are invalid ("", nil, false, AN_ERROR) is returned. +// - if the HIL in bindvars is invalid it returns ("", nil, false, AN_ERROR) +// - if the computed bindvars are valid and templated policy validation is successful it returns (bindName, TemplatedPolicy, true, nil) +func computeBindNameAndVars(bindType, bindName string, bindVars *structs.ACLTemplatedPolicyVariables, projectedVars map[string]string) (string, *structs.ACLTemplatedPolicy, bool, error) { bindName, err := template.InterpolateHIL(bindName, projectedVars, true) if err != nil { - return "", false, err + return "", nil, false, err } + var templatedPolicy *structs.ACLTemplatedPolicy var valid bool switch bindType { case structs.BindingRuleBindTypeService: @@ -165,11 +177,60 @@ func computeBindName(bindType, bindName string, projectedVars map[string]string) valid = acl.IsValidNodeIdentityName(bindName) case structs.BindingRuleBindTypeRole: valid = acl.IsValidRoleName(bindName) + case structs.BindingRuleBindTypeTemplatedPolicy: + templatedPolicy, valid, err = generateTemplatedPolicies(bindName, bindVars, projectedVars) + if err != nil { + return "", nil, false, err + } + default: - return "", false, fmt.Errorf("unknown binding rule bind type: %s", bindType) + return "", nil, false, fmt.Errorf("unknown binding rule bind type: %s", bindType) } - return bindName, valid, nil + return bindName, templatedPolicy, valid, nil +} + +func generateTemplatedPolicies(bindName string, bindVars *structs.ACLTemplatedPolicyVariables, projectedVars map[string]string) (*structs.ACLTemplatedPolicy, bool, error) { + computedBindVars, err := computeBindVars(bindVars, projectedVars) + if err != nil { + return nil, false, err + } + + baseTemplate, ok := structs.GetACLTemplatedPolicyBase(bindName) + + if !ok { + return nil, false, fmt.Errorf("Bind name for templated-policy bind type does not match existing template name: %s", bindName) + } + + out := &structs.ACLTemplatedPolicy{ + TemplateName: bindName, + TemplateVariables: computedBindVars, + TemplateID: baseTemplate.TemplateID, + } + + err = out.ValidateTemplatedPolicy(baseTemplate.Schema) + if err != nil { + return nil, false, fmt.Errorf("templated policy failed validation. Error: %v", err) + } + + return out, true, nil +} + +func computeBindVars(bindVars *structs.ACLTemplatedPolicyVariables, projectedVars map[string]string) (*structs.ACLTemplatedPolicyVariables, error) { + if bindVars == nil { + return nil, nil + } + + out := &structs.ACLTemplatedPolicyVariables{} + if bindVars.Name != "" { + nameValue, err := template.InterpolateHIL(bindVars.Name, projectedVars, true) + if err != nil { + return nil, err + } + out.Name = nameValue + } + + return out, nil } // doesSelectorMatch checks that a single selector matches the provided vars. diff --git a/agent/consul/auth/binder_test.go b/agent/consul/auth/binder_test.go index 7eedc89afc..a4b55d1c54 100644 --- a/agent/consul/auth/binder_test.go +++ b/agent/consul/auth/binder_test.go @@ -258,11 +258,12 @@ func TestBinder_NodeIdentities_NameValidation(t *testing.T) { require.Contains(t, err.Error(), "bind name for bind target is invalid") } -func Test_IsValidBindName(t *testing.T) { +func Test_IsValidBindNameOrBindVars(t *testing.T) { type testcase struct { name string bindType string bindName string + bindVars *structs.ACLTemplatedPolicyVariables fields string valid bool // valid HIL, invalid contents err bool // invalid HIL @@ -270,59 +271,72 @@ func Test_IsValidBindName(t *testing.T) { for _, test := range []testcase{ {"no bind type", - "", "", "", false, false}, + "", "", nil, "", false, false}, {"bad bind type", - "invalid", "blah", "", false, true}, + "invalid", "blah", nil, "", false, true}, // valid HIL, invalid name {"empty", - "both", "", "", false, false}, + "both", "", nil, "", false, false}, {"just end", - "both", "}", "", false, false}, + "both", "}", nil, "", false, false}, {"var without start", - "both", " item }", "item", false, false}, + "both", " item }", nil, "item", false, false}, {"two vars missing second start", - "both", "before-${ item }after--more }", "item,more", false, false}, + "both", "before-${ item }after--more }", nil, "item,more", false, false}, // names for the two types are validated differently {"@ is disallowed", - "both", "bad@name", "", false, false}, + "both", "bad@name", nil, "", false, false}, {"leading dash", - "role", "-name", "", true, false}, + "role", "-name", nil, "", true, false}, {"leading dash", - "service", "-name", "", false, false}, + "service", "-name", nil, "", false, false}, {"trailing dash", - "role", "name-", "", true, false}, + "role", "name-", nil, "", true, false}, {"trailing dash", - "service", "name-", "", false, false}, + "service", "name-", nil, "", false, false}, {"inner dash", - "both", "name-end", "", true, false}, + "both", "name-end", nil, "", true, false}, {"upper case", - "role", "NAME", "", true, false}, + "role", "NAME", nil, "", true, false}, {"upper case", - "service", "NAME", "", false, false}, + "service", "NAME", nil, "", false, false}, // valid HIL, valid name {"no vars", - "both", "nothing", "", true, false}, + "both", "nothing", nil, "", true, false}, {"just var", - "both", "${item}", "item", true, false}, + "both", "${item}", nil, "item", true, false}, {"var in middle", - "both", "before-${item}after", "item", true, false}, + "both", "before-${item}after", nil, "item", true, false}, {"two vars", - "both", "before-${item}after-${more}", "item,more", true, false}, + "both", "before-${item}after-${more}", nil, "item,more", true, false}, // bad {"no bind name", - "both", "", "", false, false}, + "both", "", nil, "", false, false}, {"just start", - "both", "${", "", false, true}, + "both", "${", nil, "", false, true}, {"backwards", - "both", "}${", "", false, true}, + "both", "}${", nil, "", false, true}, {"no varname", - "both", "${}", "", false, true}, + "both", "${}", nil, "", false, true}, {"missing map key", - "both", "${item}", "", false, true}, + "both", "${item}", nil, "", false, true}, {"var without end", - "both", "${ item ", "item", false, true}, + "both", "${ item ", nil, "item", false, true}, {"two vars missing first end", - "both", "before-${ item after-${ more }", "item,more", false, true}, + "both", "before-${ item after-${ more }", nil, "item,more", false, true}, + + // bind type: templated policy - bad input + {"templated-policy missing bindvars", "templated-policy", "builtin/service", nil, "", false, true}, + {"templated-policy with unknown templated policy name", + "templated-policy", "builtin/service", &structs.ACLTemplatedPolicyVariables{Name: "before-${item}after-${more}"}, "", false, true}, + {"templated-policy with correct bindvars and unknown vars", + "templated-policy", "builtin/fake", &structs.ACLTemplatedPolicyVariables{Name: "test"}, "", false, true}, + {"templated-policy with correct bindvars but incorrect HIL", + "templated-policy", "builtin/service", &structs.ACLTemplatedPolicyVariables{Name: "before-${ item }after--more }"}, "", false, true}, + + // bind type: templated policy - good input + {"templated-policy with appropriate bindvars", + "templated-policy", "builtin/service", &structs.ACLTemplatedPolicyVariables{Name: "before-${item}after-${more}"}, "item,more", true, false}, } { var cases []testcase if test.bindType == "both" { @@ -339,9 +353,10 @@ func Test_IsValidBindName(t *testing.T) { test := test t.Run(test.bindType+"--"+test.name, func(t *testing.T) { t.Parallel() - valid, err := IsValidBindName( + valid, err := IsValidBindNameOrBindVars( test.bindType, test.bindName, + test.bindVars, strings.Split(test.fields, ","), ) if test.err { diff --git a/agent/consul/auth/login.go b/agent/consul/auth/login.go index 7ca1f70d34..a084f33bf1 100644 --- a/agent/consul/auth/login.go +++ b/agent/consul/auth/login.go @@ -46,6 +46,7 @@ func (l *Login) TokenForVerifiedIdentity(identity *authmethod.Identity, authMeth ExpirationTTL: authMethod.MaxTokenTTL, ServiceIdentities: bindings.ServiceIdentities, NodeIdentities: bindings.NodeIdentities, + TemplatedPolicies: bindings.TemplatedPolicies, Roles: bindings.Roles, EnterpriseMeta: bindings.EnterpriseMeta, } diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 27fdcb799e..34c26c621a 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -526,7 +526,8 @@ func aclTokenSetTxn(tx WriteTxn, idx uint64, token *structs.ACLToken, opts ACLTo } if opts.ProhibitUnprivileged { - if numValidRoles == 0 && numValidPolicies == 0 && len(token.ServiceIdentities) == 0 && len(token.NodeIdentities) == 0 { + if numValidRoles == 0 && numValidPolicies == 0 && len(token.ServiceIdentities) == 0 && + len(token.NodeIdentities) == 0 && len(token.TemplatedPolicies) == 0 { return ErrTokenHasNoPrivileges } } diff --git a/agent/structs/acl.go b/agent/structs/acl.go index fd60624b53..ebf1e72280 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -1068,6 +1068,21 @@ const ( // } // } BindingRuleBindTypeNode = "node" + + // BindingRuleBindTypeTemplatedPolicy is the binding rule bind type that + // assigns a TemplatedPolicy to the token that is created using the value + // of the computed BindVars as template variables and BindName as template name like: + // + // &ACLToken{ + // ...other fields... + // TemplatedPolicies: []*ACLTemplatedPolicy{ + // &ACLTemplatedPolicy{ + // TemplateName: "", + // TemplateVariables: &ACLTemplatedPolicyVariables{} + // }, + // }, + // } + BindingRuleBindTypeTemplatedPolicy = "templated-policy" ) type ACLBindingRule struct { @@ -1087,8 +1102,10 @@ type ACLBindingRule struct { // BindType adjusts how this binding rule is applied at login time. The // valid values are: // - // - BindingRuleBindTypeService = "service" - // - BindingRuleBindTypeRole = "role" + // - BindingRuleBindTypeService = "service" + // - BindingRuleBindTypeNode = "node" + // - BindingRuleBindTypeRole = "role" + // - BindingRuleBindTypeTemplatedPolicy = "templated-policy" BindType string // BindName is the target of the binding. Can be lightly templated using @@ -1096,6 +1113,10 @@ type ACLBindingRule struct { // upon the BindType. BindName string + // BindVars is a the variables used when binding rule type is `templated-policy`. Can be lightly + // templated using HIL ${foo} syntax from available field names. + BindVars *ACLTemplatedPolicyVariables `json:",omitempty"` + // Embedded Enterprise ACL metadata acl.EnterpriseMeta `mapstructure:",squash"` diff --git a/api/acl.go b/api/acl.go index ea9da1efea..31130cafc3 100644 --- a/api/acl.go +++ b/api/acl.go @@ -241,6 +241,12 @@ const ( // BindingRuleBindTypeRole binds to pre-existing roles with the given name. BindingRuleBindTypeRole BindingRuleBindType = "role" + + // BindingRuleBindTypeNode binds to a node identity with given name. + BindingRuleBindTypeNode BindingRuleBindType = "node" + + // BindingRuleBindTypeTemplatedPolicy binds to a templated policy with given template name and variables. + BindingRuleBindTypeTemplatedPolicy BindingRuleBindType = "templated-policy" ) type ACLBindingRule struct { @@ -250,6 +256,7 @@ type ACLBindingRule struct { Selector string BindType BindingRuleBindType BindName string + BindVars *ACLTemplatedPolicyVariables `json:",omitempty"` CreateIndex uint64 ModifyIndex uint64 diff --git a/command/acl/acl_helpers.go b/command/acl/acl_helpers.go index 6766accb28..9dd40e9e0e 100644 --- a/command/acl/acl_helpers.go +++ b/command/acl/acl_helpers.go @@ -265,6 +265,15 @@ func ExtractTemplatedPolicies(templatedPolicy string, templatedPolicyFile string return out, nil } +func ExtractBindVars(bindVars map[string]string) (*api.ACLTemplatedPolicyVariables, error) { + if len(bindVars) == 0 { + return nil, nil + } + out := &api.ACLTemplatedPolicyVariables{} + err := mapstructure.Decode(bindVars, out) + return out, err +} + func getTemplatedPolicyVariables(variables []string) (*api.ACLTemplatedPolicyVariables, error) { if len(variables) == 0 { return nil, nil diff --git a/command/acl/bindingrule/create/bindingrule_create.go b/command/acl/bindingrule/create/bindingrule_create.go index cdedbb744d..ab6a16f3ed 100644 --- a/command/acl/bindingrule/create/bindingrule_create.go +++ b/command/acl/bindingrule/create/bindingrule_create.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/acl" "github.com/hashicorp/consul/command/acl/bindingrule" "github.com/hashicorp/consul/command/flags" "github.com/mitchellh/cli" @@ -31,6 +32,7 @@ type cmd struct { selector string bindType string bindName string + bindVars map[string]string showMeta bool format string @@ -71,7 +73,14 @@ func (c *cmd) init() { &c.bindType, "bind-type", string(api.BindingRuleBindTypeService), - "Type of binding to perform (\"service\" or \"role\").", + "Type of binding to perform (\"service\", \"role\", \"node\" or \"templated-policy\").", + ) + c.flags.Var( + (*flags.FlagMapValue)(&c.bindVars), + "bind-vars", + "Templated policy variables. Can only be used when -bind-type is templated-policy."+ + " May be specified multiple times with different variables. Can use ${var} interpolation."+ + " Format is VariableName=Value", ) c.flags.StringVar( &c.bindName, @@ -100,15 +109,28 @@ func (c *cmd) Run(args []string) int { } if c.authMethodName == "" { - c.UI.Error(fmt.Sprintf("Missing required '-method' flag")) + c.UI.Error("Missing required '-method' flag") c.UI.Error(c.Help()) return 1 } else if c.bindType == "" { - c.UI.Error(fmt.Sprintf("Missing required '-bind-type' flag")) + c.UI.Error("Missing required '-bind-type' flag") c.UI.Error(c.Help()) return 1 } else if c.bindName == "" { - c.UI.Error(fmt.Sprintf("Missing required '-bind-name' flag")) + c.UI.Error("Missing required '-bind-name' flag") + c.UI.Error(c.Help()) + return 1 + } + + if api.BindingRuleBindType(c.bindType) != api.BindingRuleBindTypeTemplatedPolicy && len(c.bindVars) > 0 { + c.UI.Error("Cannot specify -bind-vars when -bind-type is not templated-policy") + c.UI.Error(c.Help()) + return 1 + } + + processBindVars, err := acl.ExtractBindVars(c.bindVars) + if err != nil { + c.UI.Error("Failed to decode '-bind-vars'") c.UI.Error(c.Help()) return 1 } @@ -118,6 +140,7 @@ func (c *cmd) Run(args []string) int { AuthMethod: c.authMethodName, BindType: api.BindingRuleBindType(c.bindType), BindName: c.bindName, + BindVars: processBindVars, Selector: c.selector, } diff --git a/command/acl/bindingrule/create/bindingrule_create_test.go b/command/acl/bindingrule/create/bindingrule_create_test.go index 2985c41abf..07a92851a2 100644 --- a/command/acl/bindingrule/create/bindingrule_create_test.go +++ b/command/acl/bindingrule/create/bindingrule_create_test.go @@ -106,6 +106,24 @@ func TestBindingRuleCreateCommand(t *testing.T) { require.Contains(t, ui.ErrorWriter.String(), "Missing required '-bind-name' flag") }) + t.Run("bind vars specified when by bindtype is not templated-policy", func(t *testing.T) { + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-method=test", + "-bind-name=web", + "-bind-type=service", + "-bind-vars", "name=test", + } + + ui := cli.NewMockUi() + cmd := New(ui) + + code := cmd.Run(args) + require.Equal(t, code, 1) + require.Contains(t, ui.ErrorWriter.String(), "Cannot specify -bind-vars when -bind-type is not templated-policy") + }) + t.Run("must use roughly valid selector", func(t *testing.T) { args := []string{ "-http-addr=" + a.HTTPAddr(), @@ -176,6 +194,41 @@ func TestBindingRuleCreateCommand(t *testing.T) { require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) }) + + t.Run("create it with type templated policy", func(t *testing.T) { + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-method=test", + "-bind-type=templated-policy", + "-bind-name=builtin/service", + "-bind-vars", "name=api", + } + + ui := cli.NewMockUi() + cmd := New(ui) + + code := cmd.Run(args) + require.Equal(t, code, 0) + require.Empty(t, ui.ErrorWriter.String()) + }) + + t.Run("cannot create when missing bind-vars", func(t *testing.T) { + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-method=test", + "-bind-type=templated-policy", + "-bind-name=builtin/service", + } + + ui := cli.NewMockUi() + cmd := New(ui) + + code := cmd.Run(args) + require.Equal(t, code, 1) + require.Contains(t, ui.ErrorWriter.String(), "templated policy failed validation") + }) } func TestBindingRuleCreateCommand_JSON(t *testing.T) { diff --git a/command/acl/bindingrule/formatter.go b/command/acl/bindingrule/formatter.go index 6d96f4b71b..7bf01d9a3d 100644 --- a/command/acl/bindingrule/formatter.go +++ b/command/acl/bindingrule/formatter.go @@ -63,6 +63,10 @@ func (f *prettyFormatter) FormatBindingRule(rule *api.ACLBindingRule) (string, e buffer.WriteString(fmt.Sprintf("Description: %s\n", rule.Description)) buffer.WriteString(fmt.Sprintf("BindType: %s\n", rule.BindType)) buffer.WriteString(fmt.Sprintf("BindName: %s\n", rule.BindName)) + if rule.BindVars != nil && rule.BindVars.Name != "" { + buffer.WriteString(fmt.Sprintf("BindVars: \n - Name: %s\n", rule.BindVars.Name)) + } + buffer.WriteString(fmt.Sprintf("Selector: %s\n", rule.Selector)) if f.showMeta { buffer.WriteString(fmt.Sprintf("Create Index: %d\n", rule.CreateIndex)) @@ -96,6 +100,9 @@ func (f *prettyFormatter) formatBindingRuleListEntry(rule *api.ACLBindingRule) s buffer.WriteString(fmt.Sprintf(" Description: %s\n", rule.Description)) buffer.WriteString(fmt.Sprintf(" BindType: %s\n", rule.BindType)) buffer.WriteString(fmt.Sprintf(" BindName: %s\n", rule.BindName)) + if rule.BindVars != nil && rule.BindVars.Name != "" { + buffer.WriteString(fmt.Sprintf(" BindVars: \n - Name: %s\n", rule.BindVars.Name)) + } buffer.WriteString(fmt.Sprintf(" Selector: %s\n", rule.Selector)) if f.showMeta { buffer.WriteString(fmt.Sprintf(" Create Index: %d\n", rule.CreateIndex)) diff --git a/command/acl/bindingrule/update/bindingrule_update.go b/command/acl/bindingrule/update/bindingrule_update.go index 071e2bdc01..ce70067487 100644 --- a/command/acl/bindingrule/update/bindingrule_update.go +++ b/command/acl/bindingrule/update/bindingrule_update.go @@ -34,6 +34,7 @@ type cmd struct { selector string bindType string bindName string + bindVars map[string]string noMerge bool showMeta bool @@ -102,6 +103,14 @@ func (c *cmd) init() { fmt.Sprintf("Output format {%s}", strings.Join(bindingrule.GetSupportedFormats(), "|")), ) + c.flags.Var( + (*flags.FlagMapValue)(&c.bindVars), + "bind-vars", + "Templated policy variables. Can only be used when -bind-type is templated-policy."+ + " May be specified multiple times with different variables. Can use ${var} interpolation."+ + " Format is VariableName=Value", + ) + c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) flags.Merge(c.flags, c.http.ServerFlags()) @@ -115,7 +124,7 @@ func (c *cmd) Run(args []string) int { } if c.ruleID == "" { - c.UI.Error(fmt.Sprintf("Cannot update a binding rule without specifying the -id parameter")) + c.UI.Error("Cannot update a binding rule without specifying the -id parameter") return 1 } @@ -141,14 +150,25 @@ func (c *cmd) Run(args []string) int { return 1 } + processBindVars, err := acl.ExtractBindVars(c.bindVars) + if err != nil { + c.UI.Error("Failed to decode '-bind-vars'") + c.UI.Error(c.Help()) + return 1 + } + var rule *api.ACLBindingRule if c.noMerge { if c.bindType == "" { - c.UI.Error(fmt.Sprintf("Missing required '-bind-type' flag")) + c.UI.Error("Missing required '-bind-type' flag") c.UI.Error(c.Help()) return 1 } else if c.bindName == "" { - c.UI.Error(fmt.Sprintf("Missing required '-bind-name' flag")) + c.UI.Error("Missing required '-bind-name' flag") + c.UI.Error(c.Help()) + return 1 + } else if len(c.bindVars) > 0 && api.BindingRuleBindType(c.bindType) != api.BindingRuleBindTypeTemplatedPolicy { + c.UI.Error("-bind-vars cannot be specified when -bind-type is not templated-policy") c.UI.Error(c.Help()) return 1 } @@ -158,6 +178,7 @@ func (c *cmd) Run(args []string) int { AuthMethod: currentRule.AuthMethod, // immutable Description: c.description, BindType: api.BindingRuleBindType(c.bindType), + BindVars: processBindVars, BindName: c.bindName, Selector: c.selector, } @@ -174,6 +195,14 @@ func (c *cmd) Run(args []string) int { if c.bindName != "" { rule.BindName = c.bindName } + if len(c.bindVars) > 0 { + rule.BindVars = processBindVars + } + // remove bind vars for non templated-policy binding rules types + if api.BindingRuleBindType(c.bindType) != api.BindingRuleBindTypeTemplatedPolicy { + rule.BindVars = nil + } + if isFlagSet(c.flags, "selector") { rule.Selector = c.selector // empty is valid } diff --git a/command/acl/bindingrule/update/bindingrule_update_test.go b/command/acl/bindingrule/update/bindingrule_update_test.go index 6124e9df2d..1bbcd35cc2 100644 --- a/command/acl/bindingrule/update/bindingrule_update_test.go +++ b/command/acl/bindingrule/update/bindingrule_update_test.go @@ -127,15 +127,24 @@ func TestBindingRuleUpdateCommand(t *testing.T) { require.Contains(t, ui.ErrorWriter.String(), "Binding rule not found with ID") }) - createRule := func(t *testing.T) string { + createRule := func(t *testing.T, isTemplatedPolicy bool) string { + bindingRule := &api.ACLBindingRule{ + AuthMethod: "test", + Description: "test rule", + BindType: api.BindingRuleBindTypeService, + BindName: "test-${serviceaccount.name}", + Selector: "serviceaccount.namespace==default", + } + if isTemplatedPolicy { + bindingRule.BindType = api.BindingRuleBindTypeTemplatedPolicy + bindingRule.BindName = api.ACLTemplatedPolicyServiceName + bindingRule.BindVars = &api.ACLTemplatedPolicyVariables{ + Name: "test-${serviceaccount.name}", + } + } + rule, _, err := client.ACL().BindingRuleCreate( - &api.ACLBindingRule{ - AuthMethod: "test", - Description: "test rule", - BindType: api.BindingRuleBindTypeService, - BindName: "test-${serviceaccount.name}", - Selector: "serviceaccount.namespace==default", - }, + bindingRule, &api.WriteOptions{Token: "root"}, ) require.NoError(t, err) @@ -161,7 +170,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { m[c] = struct{}{} } - _ = createRule(t) + _ = createRule(t, false) } } @@ -183,7 +192,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("must use roughly valid selector", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) args := []string{ "-http-addr=" + a.HTTPAddr(), @@ -201,7 +210,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -233,10 +242,79 @@ func TestBindingRuleUpdateCommand(t *testing.T) { require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) }) + t.Run("update all fields with templated policy", func(t *testing.T) { + id := createRule(t, false) + + ui := cli.NewMockUi() + cmd := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-id", id, + "-description=test rule edited", + "-bind-type", "templated-policy", + "-bind-name=builtin/service", + "-bind-vars", "name=api", + "-selector=serviceaccount.namespace==alt and serviceaccount.name==demo", + } + + code := cmd.Run(args) + require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String()) + require.Empty(t, ui.ErrorWriter.String()) + + rule, _, err := client.ACL().BindingRuleRead( + id, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, rule) + + require.Equal(t, "test rule edited", rule.Description) + require.Equal(t, api.ACLTemplatedPolicyServiceName, rule.BindName) + require.Equal(t, api.BindingRuleBindTypeTemplatedPolicy, rule.BindType) + require.Equal(t, &api.ACLTemplatedPolicyVariables{Name: "api"}, rule.BindVars) + require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) + }) + + t.Run("update bind type to something other than templated-policy unsets bindvars", func(t *testing.T) { + id := createRule(t, true) + + ui := cli.NewMockUi() + cmd := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-id", id, + "-description=test rule edited", + "-bind-type", "role", + "-bind-name=role-updated", + "-selector=serviceaccount.namespace==alt and serviceaccount.name==demo", + } + + code := cmd.Run(args) + require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String()) + require.Empty(t, ui.ErrorWriter.String()) + + rule, _, err := client.ACL().BindingRuleRead( + id, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, rule) + + require.Equal(t, "test rule edited", rule.Description) + require.Equal(t, "role-updated", rule.BindName) + require.Equal(t, api.BindingRuleBindTypeRole, rule.BindType) + require.Empty(t, rule.BindVars) + require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) + }) + t.Run("update all fields - partial", func(t *testing.T) { deleteRules(t) // reset since we created a bunch that might be dupes - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -269,7 +347,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields but description", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -301,7 +379,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields but bind name", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -333,7 +411,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields but must exist", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -365,7 +443,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields but selector", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -397,7 +475,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields clear selector", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -430,7 +508,7 @@ func TestBindingRuleUpdateCommand(t *testing.T) { }) t.Run("update all fields json formatted", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -571,15 +649,25 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { require.Contains(t, ui.ErrorWriter.String(), "Binding rule not found with ID") }) - createRule := func(t *testing.T) string { + createRule := func(t *testing.T, isTemplatedPolicy bool) string { + bindingRule := &api.ACLBindingRule{ + AuthMethod: "test", + Description: "test rule", + BindType: api.BindingRuleBindTypeRole, + BindName: "test-${serviceaccount.name}", + Selector: "serviceaccount.namespace==default", + } + + if isTemplatedPolicy { + bindingRule.BindType = api.BindingRuleBindTypeTemplatedPolicy + bindingRule.BindName = "builtin/service" + bindingRule.BindVars = &api.ACLTemplatedPolicyVariables{ + Name: "test-${serviceaccount.name}", + } + } + rule, _, err := client.ACL().BindingRuleCreate( - &api.ACLBindingRule{ - AuthMethod: "test", - Description: "test rule", - BindType: api.BindingRuleBindTypeRole, - BindName: "test-${serviceaccount.name}", - Selector: "serviceaccount.namespace==default", - }, + bindingRule, &api.WriteOptions{Token: "root"}, ) require.NoError(t, err) @@ -605,7 +693,7 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { m[c] = struct{}{} } - _ = createRule(t) + _ = createRule(t, false) } } @@ -628,7 +716,7 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { }) t.Run("must use roughly valid selector", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -650,7 +738,7 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { }) t.Run("update all fields", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -683,10 +771,81 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) }) + t.Run("update all fields after initial binding rule is templated-policy", func(t *testing.T) { + id := createRule(t, true) + + ui := cli.NewMockUi() + cmd := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-no-merge", + "-id", id, + "-description=test rule edited", + "-bind-type", "service", + "-bind-name=role-updated", + "-selector=serviceaccount.namespace==alt and serviceaccount.name==demo", + } + + code := cmd.Run(args) + require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String()) + require.Empty(t, ui.ErrorWriter.String()) + + rule, _, err := client.ACL().BindingRuleRead( + id, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, rule) + + require.Equal(t, "test rule edited", rule.Description) + require.Equal(t, api.BindingRuleBindTypeService, rule.BindType) + require.Empty(t, rule.BindVars) + require.Equal(t, "role-updated", rule.BindName) + require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) + }) + + t.Run("update all fields with templated-policy bind type", func(t *testing.T) { + id := createRule(t, false) + + ui := cli.NewMockUi() + cmd := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-no-merge", + "-id", id, + "-description=test rule edited", + "-bind-type=templated-policy", + "-bind-name=builtin/service", + "-bind-vars", "name=api", + "-selector=serviceaccount.namespace==alt and serviceaccount.name==demo", + } + + code := cmd.Run(args) + require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String()) + require.Empty(t, ui.ErrorWriter.String()) + + rule, _, err := client.ACL().BindingRuleRead( + id, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, rule) + + require.Equal(t, "test rule edited", rule.Description) + require.Equal(t, api.BindingRuleBindTypeTemplatedPolicy, rule.BindType) + require.Equal(t, api.ACLTemplatedPolicyServiceName, rule.BindName) + require.Equal(t, &api.ACLTemplatedPolicyVariables{Name: "api"}, rule.BindVars) + require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) + }) + t.Run("update all fields - partial", func(t *testing.T) { deleteRules(t) // reset since we created a bunch that might be dupes - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -720,7 +879,7 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { }) t.Run("update all fields but description", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -753,7 +912,7 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { }) t.Run("missing bind name", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui) @@ -773,7 +932,7 @@ func TestBindingRuleUpdateCommand_noMerge(t *testing.T) { }) t.Run("update all fields but selector", func(t *testing.T) { - id := createRule(t) + id := createRule(t, false) ui := cli.NewMockUi() cmd := New(ui)