From 617b11302f880c2179f46d320a86ff547e3a3358 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Fri, 15 Oct 2021 13:31:04 -0400
Subject: [PATCH] acl: remove Policy.ID and Policy.Revision

These two fields do not appear to be used anywhere. We use the structs.ACLPolicy ID in the
ACLResolver cache, but the acl.Policy ID and revision are not used.
---
 acl/policy.go             | 10 ++--------
 acl/policy_merger.go      | 24 +-----------------------
 agent/structs/acl_test.go | 20 +++++++++-----------
 3 files changed, 12 insertions(+), 42 deletions(-)

diff --git a/acl/policy.go b/acl/policy.go
index ed1c74f23..f19e7a6be 100644
--- a/acl/policy.go
+++ b/acl/policy.go
@@ -89,8 +89,6 @@ type PolicyRules struct {
 
 // Policy is used to represent the policy specified by an ACL configuration.
 type Policy struct {
-	ID                    string `hcl:"id"`
-	Revision              uint64 `hcl:"revision"`
 	PolicyRules           `hcl:",squash"`
 	EnterprisePolicyRules `hcl:",squash"`
 }
@@ -429,10 +427,11 @@ func parseLegacy(rules string, conf *Config) (*Policy, error) {
 // NewPolicyFromSource is used to parse the specified ACL rules into an
 // intermediary set of policies, before being compiled into
 // the ACL
+// TODO: remove id and revision args
 func NewPolicyFromSource(id string, revision uint64, rules string, syntax SyntaxVersion, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
 	if rules == "" {
 		// Hot path for empty source
-		return &Policy{ID: id, Revision: revision}, nil
+		return &Policy{}, nil
 	}
 
 	var policy *Policy
@@ -445,11 +444,6 @@ func NewPolicyFromSource(id string, revision uint64, rules string, syntax Syntax
 	default:
 		return nil, fmt.Errorf("Invalid rules version: %d", syntax)
 	}
-
-	if err == nil {
-		policy.ID = id
-		policy.Revision = revision
-	}
 	return policy, err
 }
 
diff --git a/acl/policy_merger.go b/acl/policy_merger.go
index aa52b9539..681772d04 100644
--- a/acl/policy_merger.go
+++ b/acl/policy_merger.go
@@ -1,13 +1,5 @@
 package acl
 
-import (
-	"encoding/binary"
-	"fmt"
-	"hash"
-
-	"golang.org/x/crypto/blake2b"
-)
-
 type policyRulesMergeContext struct {
 	aclRule                  string
 	agentRules               map[string]*AgentRule
@@ -317,7 +309,6 @@ func (p *policyRulesMergeContext) fill(merged *PolicyRules) {
 }
 
 type PolicyMerger struct {
-	idHasher hash.Hash
 	policyRulesMergeContext
 	enterprisePolicyRulesMergeContext
 }
@@ -329,31 +320,18 @@ func NewPolicyMerger() *PolicyMerger {
 }
 
 func (m *PolicyMerger) init() {
-	var err error
-	m.idHasher, err = blake2b.New256(nil)
-	if err != nil {
-		panic(err)
-	}
-
 	m.policyRulesMergeContext.init()
 	m.enterprisePolicyRulesMergeContext.init()
 }
 
 func (m *PolicyMerger) Merge(policy *Policy) {
-	// This is part of calculating the merged policies ID
-	m.idHasher.Write([]byte(policy.ID))
-	binary.Write(m.idHasher, binary.BigEndian, policy.Revision)
-
 	m.policyRulesMergeContext.merge(&policy.PolicyRules)
 	m.enterprisePolicyRulesMergeContext.merge(&policy.EnterprisePolicyRules)
 }
 
 // Policy outputs the merged policy
 func (m *PolicyMerger) Policy() *Policy {
-	merged := &Policy{
-		ID: fmt.Sprintf("%x", m.idHasher.Sum(nil)),
-	}
-
+	merged := &Policy{}
 	m.policyRulesMergeContext.fill(&merged.PolicyRules)
 	m.enterprisePolicyRulesMergeContext.fill(&merged.EnterprisePolicyRules)
 
diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go
index 39fb5aefc..a435d1c57 100644
--- a/agent/structs/acl_test.go
+++ b/agent/structs/acl_test.go
@@ -418,21 +418,19 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 		policies, err := testPolicies.resolveWithCache(cache, nil)
 		require.NoError(t, err)
 		require.Len(t, policies, 4)
-		for i := range testPolicies {
-			require.Equal(t, testPolicies[i].ID, policies[i].ID)
-			require.Equal(t, testPolicies[i].ModifyIndex, policies[i].Revision)
-		}
+		require.Len(t, policies[0].NodePrefixes, 1)
+		require.Len(t, policies[1].AgentPrefixes, 1)
+		require.Len(t, policies[2].KeyPrefixes, 1)
+		require.Len(t, policies[3].ServicePrefixes, 1)
 	})
 
 	t.Run("Check Cache", func(t *testing.T) {
 		for i := range testPolicies {
 			entry := cache.GetParsedPolicy(fmt.Sprintf("%x", testPolicies[i].Hash))
 			require.NotNil(t, entry)
-			require.Equal(t, testPolicies[i].ID, entry.Policy.ID)
-			require.Equal(t, testPolicies[i].ModifyIndex, entry.Policy.Revision)
 
 			// set this to detect using from the cache next time
-			entry.Policy.Revision = 9999
+			testPolicies[i].Rules = "invalid"
 		}
 	})
 
@@ -440,10 +438,10 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 		policies, err := testPolicies.resolveWithCache(cache, nil)
 		require.NoError(t, err)
 		require.Len(t, policies, 4)
-		for i := range testPolicies {
-			require.Equal(t, testPolicies[i].ID, policies[i].ID)
-			require.Equal(t, uint64(9999), policies[i].Revision)
-		}
+		require.Len(t, policies[0].NodePrefixes, 1)
+		require.Len(t, policies[1].AgentPrefixes, 1)
+		require.Len(t, policies[2].KeyPrefixes, 1)
+		require.Len(t, policies[3].ServicePrefixes, 1)
 	})
 }