acl: remove id and revision from Policy constructors

The fields were removed in a previous commit.

Also remove an unused constructor for PolicyMerger
This commit is contained in:
Daniel Nephin 2021-10-15 13:33:31 -04:00
parent 7c679c11e6
commit 8ba760a2fc
16 changed files with 44 additions and 54 deletions

View File

@ -2549,7 +2549,7 @@ func TestACL_ReadAll(t *testing.T) {
body := func(t *testing.T, rules string, defaultPolicy Authorizer, check func(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext)) { body := func(t *testing.T, rules string, defaultPolicy Authorizer, check func(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext)) {
t.Helper() t.Helper()
policy, err := NewPolicyFromSource("", 0, rules, SyntaxCurrent, nil, nil) policy, err := NewPolicyFromSource(rules, SyntaxCurrent, nil, nil)
require.NoError(t, err) require.NoError(t, err)
acl, err := NewPolicyAuthorizerWithDefaults(defaultPolicy, []*Policy{policy}, nil) acl, err := NewPolicyAuthorizerWithDefaults(defaultPolicy, []*Policy{policy}, nil)

View File

@ -267,8 +267,8 @@ func Enforce(authz Authorizer, rsc Resource, segment string, access string, ctx
// NewAuthorizerFromRules is a convenience function to invoke NewPolicyFromSource followed by NewPolicyAuthorizer with // NewAuthorizerFromRules is a convenience function to invoke NewPolicyFromSource followed by NewPolicyAuthorizer with
// the parse policy. // the parse policy.
func NewAuthorizerFromRules(id string, revision uint64, rules string, syntax SyntaxVersion, conf *Config, meta *EnterprisePolicyMeta) (Authorizer, error) { func NewAuthorizerFromRules(rules string, syntax SyntaxVersion, conf *Config, meta *EnterprisePolicyMeta) (Authorizer, error) {
policy, err := NewPolicyFromSource(id, revision, rules, syntax, conf, meta) policy, err := NewPolicyFromSource(rules, syntax, conf, meta)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -427,8 +427,7 @@ func parseLegacy(rules string, conf *Config) (*Policy, error) {
// NewPolicyFromSource is used to parse the specified ACL rules into an // NewPolicyFromSource is used to parse the specified ACL rules into an
// intermediary set of policies, before being compiled into // intermediary set of policies, before being compiled into
// the ACL // the ACL
// TODO: remove id and revision args func NewPolicyFromSource(rules string, syntax SyntaxVersion, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
func NewPolicyFromSource(id string, revision uint64, rules string, syntax SyntaxVersion, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
if rules == "" { if rules == "" {
// Hot path for empty source // Hot path for empty source
return &Policy{}, nil return &Policy{}, nil

View File

@ -313,12 +313,6 @@ type PolicyMerger struct {
enterprisePolicyRulesMergeContext enterprisePolicyRulesMergeContext
} }
func NewPolicyMerger() *PolicyMerger {
merger := &PolicyMerger{}
merger.init()
return merger
}
func (m *PolicyMerger) init() { func (m *PolicyMerger) init() {
m.policyRulesMergeContext.init() m.policyRulesMergeContext.init()
m.enterprisePolicyRulesMergeContext.init() m.enterprisePolicyRulesMergeContext.init()

View File

@ -771,7 +771,7 @@ func TestPolicySourceParse(t *testing.T) {
require.True(t, tc.Rules != "" || tc.RulesJSON != "") require.True(t, tc.Rules != "" || tc.RulesJSON != "")
if tc.Rules != "" { if tc.Rules != "" {
t.Run("hcl", func(t *testing.T) { t.Run("hcl", func(t *testing.T) {
actual, err := NewPolicyFromSource("", 0, tc.Rules, tc.Syntax, nil, nil) actual, err := NewPolicyFromSource(tc.Rules, tc.Syntax, nil, nil)
if tc.Err != "" { if tc.Err != "" {
errStartsWith(t, err, tc.Err) errStartsWith(t, err, tc.Err)
} else { } else {
@ -781,7 +781,7 @@ func TestPolicySourceParse(t *testing.T) {
} }
if tc.RulesJSON != "" { if tc.RulesJSON != "" {
t.Run("json", func(t *testing.T) { t.Run("json", func(t *testing.T) {
actual, err := NewPolicyFromSource("", 0, tc.RulesJSON, tc.Syntax, nil, nil) actual, err := NewPolicyFromSource(tc.RulesJSON, tc.Syntax, nil, nil)
if tc.Err != "" { if tc.Err != "" {
errStartsWith(t, err, tc.Err) errStartsWith(t, err, tc.Err)
} else { } else {

View File

@ -237,7 +237,7 @@ func catalogPolicy(token string) (structs.ACLIdentity, acl.Authorizer, error) {
return nil, nil, acl.ErrNotFound return nil, nil, acl.ErrNotFound
} }
policy, err := acl.NewPolicyFromSource("", 0, tok.rules, acl.SyntaxCurrent, nil, nil) policy, err := acl.NewPolicyFromSource(tok.rules, acl.SyntaxCurrent, nil, nil)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }

View File

@ -1176,7 +1176,7 @@ func (a *ACL) PolicySet(args *structs.ACLPolicySetRequest, reply *structs.ACLPol
} }
// validate the rules // validate the rules
_, err = acl.NewPolicyFromSource("", 0, policy.Rules, policy.Syntax, a.srv.aclConfig, policy.EnterprisePolicyMeta()) _, err = acl.NewPolicyFromSource(policy.Rules, policy.Syntax, a.srv.aclConfig, policy.EnterprisePolicyMeta())
if err != nil { if err != nil {
return err return err
} }

View File

@ -2172,7 +2172,7 @@ func TestACL_filterHealthChecks(t *testing.T) {
} }
// Allowed to see the service but not the node. // Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
service "foo" { service "foo" {
policy = "read" policy = "read"
} }
@ -2195,7 +2195,7 @@ service "foo" {
} }
// Chain on access to the node. // Chain on access to the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node "node1" { node "node1" {
policy = "read" policy = "read"
} }
@ -2253,7 +2253,7 @@ func TestACL_filterIntentions(t *testing.T) {
} }
// Policy to see one // Policy to see one
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
service "foo" { service "foo" {
policy = "read" policy = "read"
} }
@ -2328,7 +2328,7 @@ func TestACL_filterServiceNodes(t *testing.T) {
} }
// Allowed to see the service but not the node. // Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
service "foo" { service "foo" {
policy = "read" policy = "read"
} }
@ -2352,7 +2352,7 @@ service "foo" {
} }
// Chain on access to the node. // Chain on access to the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node "node1" { node "node1" {
policy = "read" policy = "read"
} }
@ -2424,7 +2424,7 @@ func TestACL_filterNodeServices(t *testing.T) {
} }
// Allowed to see the service but not the node. // Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
service "foo" { service "foo" {
policy = "read" policy = "read"
} }
@ -2448,7 +2448,7 @@ service "foo" {
} }
// Chain on access to the node. // Chain on access to the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node "node1" { node "node1" {
policy = "read" policy = "read"
} }
@ -2520,7 +2520,7 @@ func TestACL_filterCheckServiceNodes(t *testing.T) {
} }
// Allowed to see the service but not the node. // Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
service "foo" { service "foo" {
policy = "read" policy = "read"
} }
@ -2543,7 +2543,7 @@ service "foo" {
} }
// Chain on access to the node. // Chain on access to the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node "node1" { node "node1" {
policy = "read" policy = "read"
} }
@ -2646,7 +2646,7 @@ node "node1" {
service "foo" { service "foo" {
policy = "read" policy = "read"
}` }`
policy, err := acl.NewPolicyFromSource("", 0, rules, acl.SyntaxLegacy, nil, nil) policy, err := acl.NewPolicyFromSource(rules, acl.SyntaxLegacy, nil, nil)
if err != nil { if err != nil {
t.Fatalf("err %v", err) t.Fatalf("err %v", err)
} }
@ -2674,7 +2674,7 @@ node "node2" {
service "bar" { service "bar" {
policy = "read" policy = "read"
}` }`
policy, err := acl.NewPolicyFromSource("", 0, rules, acl.SyntaxLegacy, nil, nil) policy, err := acl.NewPolicyFromSource(rules, acl.SyntaxLegacy, nil, nil)
if err != nil { if err != nil {
t.Fatalf("err %v", err) t.Fatalf("err %v", err)
} }
@ -2708,7 +2708,7 @@ node "node2" {
service "bar" { service "bar" {
policy = "read" policy = "read"
}` }`
policy, err := acl.NewPolicyFromSource("", 0, rules, acl.SyntaxLegacy, nil, nil) policy, err := acl.NewPolicyFromSource(rules, acl.SyntaxLegacy, nil, nil)
if err != nil { if err != nil {
t.Fatalf("err %v", err) t.Fatalf("err %v", err)
} }
@ -2837,7 +2837,7 @@ func TestACL_filterNodeDump(t *testing.T) {
} }
// Allowed to see the service but not the node. // Allowed to see the service but not the node.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
service "foo" { service "foo" {
policy = "read" policy = "read"
} }
@ -2861,7 +2861,7 @@ service "foo" {
} }
// Chain on access to the node. // Chain on access to the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node "node1" { node "node1" {
policy = "read" policy = "read"
} }
@ -2970,7 +2970,7 @@ func TestACL_filterDatacenterCheckServiceNodes(t *testing.T) {
perms acl.Authorizer perms acl.Authorizer
) )
// Allowed to see the service but not the node. // Allowed to see the service but not the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
service_prefix "" { policy = "read" } service_prefix "" { policy = "read" }
`, acl.SyntaxCurrent, nil, nil) `, acl.SyntaxCurrent, nil, nil)
require.NoError(t, err) require.NoError(t, err)
@ -2985,7 +2985,7 @@ func TestACL_filterDatacenterCheckServiceNodes(t *testing.T) {
} }
// Allowed to see the node but not the service. // Allowed to see the node but not the service.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node_prefix "" { policy = "read" } node_prefix "" { policy = "read" }
`, acl.SyntaxCurrent, nil, nil) `, acl.SyntaxCurrent, nil, nil)
require.NoError(t, err) require.NoError(t, err)
@ -3000,7 +3000,7 @@ func TestACL_filterDatacenterCheckServiceNodes(t *testing.T) {
} }
// Allowed to see the service AND the node // Allowed to see the service AND the node
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
service_prefix "" { policy = "read" } service_prefix "" { policy = "read" }
node_prefix "" { policy = "read" } node_prefix "" { policy = "read" }
`, acl.SyntaxCurrent, nil, nil) `, acl.SyntaxCurrent, nil, nil)

View File

@ -3437,7 +3437,7 @@ func TestVetRegisterWithACL(t *testing.T) {
} }
// Create a basic node policy. // Create a basic node policy.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
node "node" { node "node" {
policy = "write" policy = "write"
} }
@ -3482,7 +3482,7 @@ node "node" {
} }
// Chain on a basic service policy. // Chain on a basic service policy.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
service "service" { service "service" {
policy = "write" policy = "write"
} }
@ -3512,7 +3512,7 @@ service "service" {
} }
// Chain on a policy that allows them to write to the other service. // Chain on a policy that allows them to write to the other service.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
service "other" { service "other" {
policy = "write" policy = "write"
} }
@ -3586,7 +3586,7 @@ service "other" {
} }
// Chain on a policy that forbids them to write to the other service. // Chain on a policy that forbids them to write to the other service.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
service "other" { service "other" {
policy = "deny" policy = "deny"
} }
@ -3616,7 +3616,7 @@ service "other" {
} }
// Chain on a policy that forbids them to write to the node. // Chain on a policy that forbids them to write to the node.
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
node "node" { node "node" {
policy = "deny" policy = "deny"
} }
@ -3663,7 +3663,7 @@ func TestVetDeregisterWithACL(t *testing.T) {
} }
// Create a basic node policy. // Create a basic node policy.
policy, err := acl.NewPolicyFromSource("", 0, ` policy, err := acl.NewPolicyFromSource(`
node "node" { node "node" {
policy = "write" policy = "write"
} }
@ -3676,7 +3676,7 @@ node "node" {
t.Fatalf("err: %v", err) t.Fatalf("err: %v", err)
} }
policy, err = acl.NewPolicyFromSource("", 0, ` policy, err = acl.NewPolicyFromSource(`
service "my-service" { service "my-service" {
policy = "write" policy = "write"
} }

View File

@ -10,7 +10,7 @@ import (
func TestFilter_DirEnt(t *testing.T) { func TestFilter_DirEnt(t *testing.T) {
t.Parallel() t.Parallel()
policy, _ := acl.NewPolicyFromSource("", 0, testFilterRules, acl.SyntaxLegacy, nil, nil) policy, _ := acl.NewPolicyFromSource(testFilterRules, acl.SyntaxLegacy, nil, nil)
aclR, _ := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil) aclR, _ := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
type tcase struct { type tcase struct {
@ -52,7 +52,7 @@ func TestFilter_DirEnt(t *testing.T) {
func TestFilter_TxnResults(t *testing.T) { func TestFilter_TxnResults(t *testing.T) {
t.Parallel() t.Parallel()
policy, _ := acl.NewPolicyFromSource("", 0, testFilterRules, acl.SyntaxLegacy, nil, nil) policy, _ := acl.NewPolicyFromSource(testFilterRules, acl.SyntaxLegacy, nil, nil)
aclR, _ := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil) aclR, _ := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
type tcase struct { type tcase struct {

View File

@ -612,7 +612,7 @@ node "node1" {
} }
` `
cfg := &acl.Config{WildcardName: structs.WildcardSpecifier} cfg := &acl.Config{WildcardName: structs.WildcardSpecifier}
authorizer, err := acl.NewAuthorizerFromRules("1", 0, rules, acl.SyntaxCurrent, cfg, nil) authorizer, err := acl.NewAuthorizerFromRules(rules, acl.SyntaxCurrent, cfg, nil)
require.NoError(t, err) require.NoError(t, err)
authorizer = acl.NewChainedAuthorizer([]acl.Authorizer{authorizer, acl.DenyAll()}) authorizer = acl.NewChainedAuthorizer([]acl.Authorizer{authorizer, acl.DenyAll()})
require.Equal(t, acl.Deny, authorizer.NodeRead("denied", nil)) require.Equal(t, acl.Deny, authorizer.NodeRead("denied", nil))
@ -808,10 +808,7 @@ node "node1" {
policy = "write" policy = "write"
} }
` `
authorizer, err := acl.NewAuthorizerFromRules( authorizer, err := acl.NewAuthorizerFromRules(rules, acl.SyntaxCurrent, &acl.Config{WildcardName: structs.WildcardSpecifier}, nil)
"1", 0, rules, acl.SyntaxCurrent,
&acl.Config{WildcardName: structs.WildcardSpecifier},
nil)
require.NoError(t, err) require.NoError(t, err)
authorizer = acl.NewChainedAuthorizer([]acl.Authorizer{authorizer, acl.DenyAll()}) authorizer = acl.NewChainedAuthorizer([]acl.Authorizer{authorizer, acl.DenyAll()})
require.Equal(t, acl.Deny, authorizer.NodeRead("denied", nil)) require.Equal(t, acl.Deny, authorizer.NodeRead("denied", nil))

View File

@ -725,7 +725,7 @@ func (policies ACLPolicies) resolveWithCache(cache *ACLCaches, entConf *acl.Conf
continue continue
} }
p, err := acl.NewPolicyFromSource(policy.ID, policy.ModifyIndex, policy.Rules, policy.Syntax, entConf, policy.EnterprisePolicyMeta()) p, err := acl.NewPolicyFromSource(policy.Rules, policy.Syntax, entConf, policy.EnterprisePolicyMeta())
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to parse %q: %v", policy.Name, err) return nil, fmt.Errorf("failed to parse %q: %v", policy.Name, err)
} }

View File

@ -17,7 +17,7 @@ func TestConfigEntries_ListRelatedServices_AndACLs(t *testing.T) {
// This test tests both of these because they are related functions. // This test tests both of these because they are related functions.
newAuthz := func(t *testing.T, src string) acl.Authorizer { newAuthz := func(t *testing.T, src string) acl.Authorizer {
policy, err := acl.NewPolicyFromSource("", 0, src, acl.SyntaxCurrent, nil, nil) policy, err := acl.NewPolicyFromSource(src, acl.SyntaxCurrent, nil, nil)
require.NoError(t, err) require.NoError(t, err)
authorizer, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil) authorizer, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
@ -34,7 +34,7 @@ func TestConfigEntries_ListRelatedServices_AndACLs(t *testing.T) {
buf.WriteString(fmt.Sprintf("service %q { policy = %q }\n", s, "write")) buf.WriteString(fmt.Sprintf("service %q { policy = %q }\n", s, "write"))
} }
policy, err := acl.NewPolicyFromSource("", 0, buf.String(), acl.SyntaxCurrent, nil, nil) policy, err := acl.NewPolicyFromSource(buf.String(), acl.SyntaxCurrent, nil, nil)
require.NoError(t, err) require.NoError(t, err)
authorizer, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil) authorizer, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)

View File

@ -23,7 +23,7 @@ func TestConfigEntries_ACLs(t *testing.T) {
type testcase = configEntryACLTestCase type testcase = configEntryACLTestCase
newAuthz := func(t *testing.T, src string) acl.Authorizer { newAuthz := func(t *testing.T, src string) acl.Authorizer {
policy, err := acl.NewPolicyFromSource("", 0, src, acl.SyntaxCurrent, nil, nil) policy, err := acl.NewPolicyFromSource(src, acl.SyntaxCurrent, nil, nil)
require.NoError(t, err) require.NoError(t, err)
authorizer, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil) authorizer, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)

View File

@ -117,7 +117,7 @@ func TestIntention_ACLs(t *testing.T) {
for name, tcase := range cases { for name, tcase := range cases {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
authz, err := acl.NewAuthorizerFromRules("", 0, tcase.rules, acl.SyntaxCurrent, &config, nil) authz, err := acl.NewAuthorizerFromRules(tcase.rules, acl.SyntaxCurrent, &config, nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, tcase.read, tcase.intention.CanRead(authz)) require.Equal(t, tcase.read, tcase.intention.CanRead(authz))

View File

@ -950,7 +950,7 @@ func TestServer_DeltaAggregatedResources_v3_ACLEnforcement(t *testing.T) {
// Ensure the correct token was passed // Ensure the correct token was passed
require.Equal(t, tt.token, id) require.Equal(t, tt.token, id)
// Parse the ACL and enforce it // Parse the ACL and enforce it
policy, err := acl.NewPolicyFromSource("", 0, tt.acl, acl.SyntaxLegacy, nil, nil) policy, err := acl.NewPolicyFromSource(tt.acl, acl.SyntaxLegacy, nil, nil)
require.NoError(t, err) require.NoError(t, err)
return acl.NewPolicyAuthorizerWithDefaults(acl.RootAuthorizer("deny"), []*acl.Policy{policy}, nil) return acl.NewPolicyAuthorizerWithDefaults(acl.RootAuthorizer("deny"), []*acl.Policy{policy}, nil)
} }
@ -1014,7 +1014,7 @@ func TestServer_DeltaAggregatedResources_v3_ACLTokenDeleted_StreamTerminatedDuri
aclRules := `service "web" { policy = "write" }` aclRules := `service "web" { policy = "write" }`
token := "service-write-on-web" token := "service-write-on-web"
policy, err := acl.NewPolicyFromSource("", 0, aclRules, acl.SyntaxLegacy, nil, nil) policy, err := acl.NewPolicyFromSource(aclRules, acl.SyntaxLegacy, nil, nil)
require.NoError(t, err) require.NoError(t, err)
var validToken atomic.Value var validToken atomic.Value
@ -1112,7 +1112,7 @@ func TestServer_DeltaAggregatedResources_v3_ACLTokenDeleted_StreamTerminatedInBa
aclRules := `service "web" { policy = "write" }` aclRules := `service "web" { policy = "write" }`
token := "service-write-on-web" token := "service-write-on-web"
policy, err := acl.NewPolicyFromSource("", 0, aclRules, acl.SyntaxLegacy, nil, nil) policy, err := acl.NewPolicyFromSource(aclRules, acl.SyntaxLegacy, nil, nil)
require.NoError(t, err) require.NoError(t, err)
var validToken atomic.Value var validToken atomic.Value