From 6e1ebd3df7dea78ab4181b28ddee8fe4f1b95b30 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 22 Sep 2021 11:43:13 -0400 Subject: [PATCH] acl: remove the last of the legacy FSM Replace it with an implementation that returns an error, and rename some symbols to use a Deprecated suffix to make it clear. Also remove the ACLRequest struct, which is no longer referenced. --- agent/consul/acl_endpoint_legacy.go | 4 +++- agent/consul/fsm/commands_oss.go | 27 +++--------------------- agent/consul/fsm/snapshot_oss.go | 2 +- agent/consul/fsm/snapshot_oss_test.go | 2 +- agent/structs/acl.go | 9 -------- agent/structs/acl_legacy.go | 15 ------------- agent/structs/structs.go | 4 ++-- website/content/docs/agent/telemetry.mdx | 1 - 8 files changed, 10 insertions(+), 54 deletions(-) diff --git a/agent/consul/acl_endpoint_legacy.go b/agent/consul/acl_endpoint_legacy.go index e851631479..069fb1db2a 100644 --- a/agent/consul/acl_endpoint_legacy.go +++ b/agent/consul/acl_endpoint_legacy.go @@ -10,7 +10,9 @@ func (a *ACL) Bootstrap(*structs.DCSpecificRequest, *structs.ACL) error { return fmt.Errorf("ACL.Bootstrap: the legacy ACL system has been removed") } -func (a *ACL) Apply(*structs.ACLRequest, *string) error { +type LegacyACLRequest struct{} + +func (a *ACL) Apply(*LegacyACLRequest, *string) error { return fmt.Errorf("ACL.Apply: the legacy ACL system has been removed") } diff --git a/agent/consul/fsm/commands_oss.go b/agent/consul/fsm/commands_oss.go index 43063ef273..1d4d15fdbc 100644 --- a/agent/consul/fsm/commands_oss.go +++ b/agent/consul/fsm/commands_oss.go @@ -108,7 +108,7 @@ func init() { registerCommand(structs.KVSRequestType, (*FSM).applyKVSOperation) registerCommand(structs.SessionRequestType, (*FSM).applySessionOperation) // DEPRECATED (ACL-Legacy-Compat) - Only needed for v1 ACL compat - registerCommand(structs.ACLRequestType, (*FSM).applyACLOperation) + registerCommand(structs.DeprecatedACLRequestType, (*FSM).deprecatedApplyACLOperation) registerCommand(structs.TombstoneRequestType, (*FSM).applyTombstoneOperation) registerCommand(structs.CoordinateBatchUpdateType, (*FSM).applyCoordinateBatchUpdate) registerCommand(structs.PreparedQueryRequestType, (*FSM).applyPreparedQueryOperation) @@ -243,29 +243,8 @@ func (c *FSM) applySessionOperation(buf []byte, index uint64) interface{} { } } -// DEPRECATED (ACL-Legacy-Compat) - Only needed for legacy compat -func (c *FSM) applyACLOperation(buf []byte, index uint64) interface{} { - // TODO (ACL-Legacy-Compat) - Should we warn here somehow about using deprecated features - // maybe emit a second metric? - var req structs.ACLRequest - if err := structs.Decode(buf, &req); err != nil { - panic(fmt.Errorf("failed to decode request: %v", err)) - } - defer metrics.MeasureSinceWithLabels([]string{"fsm", "acl"}, time.Now(), - []metrics.Label{{Name: "op", Value: string(req.Op)}}) - switch req.Op { - case structs.ACLSet: - if err := c.state.ACLTokenSet(index, req.ACL.Convert(), true); err != nil { - return err - } - return req.ACL.ID - // Legacy commands that have been removed - case "bootstrap-now", "bootstrap-init", "force-set", "delete": - return fmt.Errorf("command %v has been removed with the legacy ACL system", req.Op) - default: - c.logger.Warn("Invalid ACL operation", "operation", req.Op) - return fmt.Errorf("Invalid ACL operation '%s'", req.Op) - } +func (c *FSM) deprecatedApplyACLOperation(_ []byte, _ uint64) interface{} { + return fmt.Errorf("legacy ACL command has been removed with the legacy ACL system") } func (c *FSM) applyTombstoneOperation(buf []byte, index uint64) interface{} { diff --git a/agent/consul/fsm/snapshot_oss.go b/agent/consul/fsm/snapshot_oss.go index 57b3e81d8f..ec9a638d17 100644 --- a/agent/consul/fsm/snapshot_oss.go +++ b/agent/consul/fsm/snapshot_oss.go @@ -15,7 +15,7 @@ func init() { registerRestorer(structs.KVSRequestType, restoreKV) registerRestorer(structs.TombstoneRequestType, restoreTombstone) registerRestorer(structs.SessionRequestType, restoreSession) - registerRestorer(structs.ACLRequestType, restoreACL) + registerRestorer(structs.DeprecatedACLRequestType, restoreACL) registerRestorer(structs.ACLBootstrapRequestType, restoreACLBootstrap) registerRestorer(structs.CoordinateBatchUpdateType, restoreCoordinates) registerRestorer(structs.PreparedQueryRequestType, restorePreparedQuery) diff --git a/agent/consul/fsm/snapshot_oss_test.go b/agent/consul/fsm/snapshot_oss_test.go index e2f8c6e03d..86b9a3d9b1 100644 --- a/agent/consul/fsm/snapshot_oss_test.go +++ b/agent/consul/fsm/snapshot_oss_test.go @@ -452,7 +452,7 @@ func TestFSM_SnapshotRestore_OSS(t *testing.T) { // Persist a legacy ACL token - this is not done in newer code // but we want to ensure that restoring legacy tokens works as // expected so we must inject one here manually - _, err = sink.Write([]byte{byte(structs.ACLRequestType)}) + _, err = sink.Write([]byte{byte(structs.DeprecatedACLRequestType)}) require.NoError(t, err) acl := structs.ACL{ diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 4b6815dbd2..4394356bdb 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -32,9 +32,6 @@ const ( ACLModeUnknown ACLMode = "3" ) -// ACLOp is used in RPCs to encode ACL operations. -type ACLOp string - type ACLTokenIDType string const ( @@ -89,12 +86,6 @@ func ACLIDReserved(id string) bool { return strings.HasPrefix(id, ACLReservedPrefix) } -const ( - // ACLSet creates or updates a token. - // TODO(ACL-Legacy-Compat): remove - ACLSet ACLOp = "set" -) - // ACLBootstrapNotAllowedErr is returned once we know that a bootstrap can no // longer be done since the cluster was bootstrapped var ACLBootstrapNotAllowedErr = errors.New("ACL bootstrap no longer allowed") diff --git a/agent/structs/acl_legacy.go b/agent/structs/acl_legacy.go index a8e6a98347..330213321f 100644 --- a/agent/structs/acl_legacy.go +++ b/agent/structs/acl_legacy.go @@ -87,21 +87,6 @@ func (tok *ACLToken) Convert() (*ACL, error) { return compat, nil } -// ACLRequest is used to create, update or delete an ACL -type ACLRequest struct { - Datacenter string - Op ACLOp - ACL ACL - WriteRequest -} - -func (r *ACLRequest) RequestDatacenter() string { - return r.Datacenter -} - -// ACLRequests is a list of ACL change requests. -type ACLRequests []*ACLRequest - // ACLSpecificRequest is used to request an ACL by ID type ACLSpecificRequest struct { Datacenter string diff --git a/agent/structs/structs.go b/agent/structs/structs.go index cb47bff63a..83ae63e798 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -41,7 +41,7 @@ const ( DeregisterRequestType = 1 KVSRequestType = 2 SessionRequestType = 3 - ACLRequestType = 4 // DEPRECATED (ACL-Legacy-Compat) + DeprecatedACLRequestType = 4 // Removed with the legacy ACL system TombstoneRequestType = 5 CoordinateBatchUpdateType = 6 PreparedQueryRequestType = 7 @@ -81,7 +81,7 @@ var requestTypeStrings = map[MessageType]string{ DeregisterRequestType: "Deregister", KVSRequestType: "KVS", SessionRequestType: "Session", - ACLRequestType: "ACL", // DEPRECATED (ACL-Legacy-Compat) + DeprecatedACLRequestType: "ACL", // DEPRECATED (ACL-Legacy-Compat) TombstoneRequestType: "Tombstone", CoordinateBatchUpdateType: "CoordinateBatchUpdate", PreparedQueryRequestType: "PreparedQuery", diff --git a/website/content/docs/agent/telemetry.mdx b/website/content/docs/agent/telemetry.mdx index 509987582c..2902d43553 100644 --- a/website/content/docs/agent/telemetry.mdx +++ b/website/content/docs/agent/telemetry.mdx @@ -380,7 +380,6 @@ These metrics are used to monitor the health of the Consul servers. | `consul.catalog.deregister` | Measures the time it takes to complete a catalog deregister operation. | ms | timer | | `consul.fsm.register` | Measures the time it takes to apply a catalog register operation to the FSM. | ms | timer | | `consul.fsm.deregister` | Measures the time it takes to apply a catalog deregister operation to the FSM. | ms | timer | -| `consul.fsm.acl.` | Measures the time it takes to apply the given ACL operation to the FSM. | ms | timer | | `consul.fsm.session.` | Measures the time it takes to apply the given session operation to the FSM. | ms | timer | | `consul.fsm.kvs.` | Measures the time it takes to apply the given KV operation to the FSM. | ms | timer | | `consul.fsm.tombstone.` | Measures the time it takes to apply the given tombstone operation to the FSM. | ms | timer |