From 7c679c11e616a321c1336d9329dbc06a7fd0a08f 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 ed1c74f23c..f19e7a6be4 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 aa52b95396..681772d04f 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 39fb5aefc0..a435d1c57c 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)
 	})
 }