From d47b7311b8bb996b091aedafe65840f71d6c0290 Mon Sep 17 00:00:00 2001 From: Daniel Upton Date: Mon, 1 Nov 2021 16:42:01 +0000 Subject: [PATCH] Support Check-And-Set deletion of config entries (#11419) Implements #11372 --- .changelog/11419.txt | 6 ++ agent/config_endpoint.go | 19 +++- agent/config_endpoint_test.go | 82 ++++++++++++++++++ agent/consul/config_endpoint.go | 29 ++++++- agent/consul/config_endpoint_test.go | 58 +++++++++++++ agent/consul/config_replication_test.go | 2 +- agent/consul/fsm/commands_oss.go | 8 ++ agent/consul/fsm/commands_oss_test.go | 51 +++++++++++ agent/consul/state/config_entry.go | 35 ++++++++ agent/consul/state/config_entry_test.go | 41 +++++++++ agent/structs/config_entry.go | 5 ++ api/config_entry.go | 33 ++++++- api/config_entry_test.go | 31 +++++++ command/config/delete/config_delete.go | 58 ++++++++++--- command/config/delete/config_delete_test.go | 96 +++++++++++++++++++-- testrpc/wait.go | 2 +- website/content/api-docs/config.mdx | 6 ++ website/content/commands/config/delete.mdx | 8 ++ 18 files changed, 543 insertions(+), 27 deletions(-) create mode 100644 .changelog/11419.txt diff --git a/.changelog/11419.txt b/.changelog/11419.txt new file mode 100644 index 0000000000..2e5248c064 --- /dev/null +++ b/.changelog/11419.txt @@ -0,0 +1,6 @@ +```release-note:improvement +config: Support Check-And-Set (CAS) deletion of config entries +``` +```release-note:improvement +cli: Add `-cas` and `-modify-index` flags to the `consul config delete` command to support Check-And-Set (CAS) deletion of config entries +``` diff --git a/agent/config_endpoint.go b/agent/config_endpoint.go index c930986b41..cc13b27de7 100644 --- a/agent/config_endpoint.go +++ b/agent/config_endpoint.go @@ -101,12 +101,27 @@ func (s *HTTPHandlers) configDelete(resp http.ResponseWriter, req *http.Request) return nil, err } - var reply struct{} + // Check for cas value + if casStr := req.URL.Query().Get("cas"); casStr != "" { + casVal, err := strconv.ParseUint(casStr, 10, 64) + if err != nil { + return nil, err + } + args.Op = structs.ConfigEntryDeleteCAS + args.Entry.GetRaftIndex().ModifyIndex = casVal + } + + var reply structs.ConfigEntryDeleteResponse if err := s.agent.RPC("ConfigEntry.Delete", &args, &reply); err != nil { return nil, err } - return reply, nil + // Return the `deleted` boolean for CAS operations, but not normal deletions + // to maintain backwards-compatibility with existing callers. + if args.Op == structs.ConfigEntryDeleteCAS { + return reply.Deleted, nil + } + return struct{}{}, nil } // ConfigApply applies the given config entry update. diff --git a/agent/config_endpoint_test.go b/agent/config_endpoint_test.go index 8dc01ed8a6..1f2d77c520 100644 --- a/agent/config_endpoint_test.go +++ b/agent/config_endpoint_test.go @@ -196,6 +196,88 @@ func TestConfig_Delete(t *testing.T) { } } +func TestConfig_Delete_CAS(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() + + require := require.New(t) + + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + // Create a config entry. + entry := &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "foo", + } + var created bool + require.NoError(a.RPC("ConfigEntry.Apply", &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: entry, + }, &created)) + require.True(created) + + // Read it back to get its ModifyIndex. + var out structs.ConfigEntryResponse + require.NoError(a.RPC("ConfigEntry.Get", &structs.ConfigEntryQuery{ + Datacenter: "dc1", + Kind: entry.Kind, + Name: entry.Name, + }, &out)) + require.NotNil(out.Entry) + + modifyIndex := out.Entry.GetRaftIndex().ModifyIndex + + t.Run("attempt to delete with an invalid index", func(t *testing.T) { + req := httptest.NewRequest( + "DELETE", + fmt.Sprintf("/v1/config/%s/%s?cas=%d", entry.Kind, entry.Name, modifyIndex-1), + nil, + ) + rawRsp, err := a.srv.Config(httptest.NewRecorder(), req) + require.NoError(err) + + deleted, isBool := rawRsp.(bool) + require.True(isBool, "response should be a boolean") + require.False(deleted, "entry should not have been deleted") + + // Verify it was not deleted. + var out structs.ConfigEntryResponse + require.NoError(a.RPC("ConfigEntry.Get", &structs.ConfigEntryQuery{ + Datacenter: "dc1", + Kind: entry.Kind, + Name: entry.Name, + }, &out)) + require.NotNil(out.Entry) + }) + + t.Run("attempt to delete with a valid index", func(t *testing.T) { + req := httptest.NewRequest( + "DELETE", + fmt.Sprintf("/v1/config/%s/%s?cas=%d", entry.Kind, entry.Name, modifyIndex), + nil, + ) + rawRsp, err := a.srv.Config(httptest.NewRecorder(), req) + require.NoError(err) + + deleted, isBool := rawRsp.(bool) + require.True(isBool, "response should be a boolean") + require.True(deleted, "entry should have been deleted") + + // Verify it was deleted. + var out structs.ConfigEntryResponse + require.NoError(a.RPC("ConfigEntry.Get", &structs.ConfigEntryQuery{ + Datacenter: "dc1", + Kind: entry.Kind, + Name: entry.Name, + }, &out)) + require.Nil(out.Entry) + }) +} + func TestConfig_Apply(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 43e5d3374f..8ae0dba3bd 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -262,7 +262,7 @@ func (c *ConfigEntry) ListAll(args *structs.ConfigEntryListAllRequest, reply *st } // Delete deletes a config entry. -func (c *ConfigEntry) Delete(args *structs.ConfigEntryRequest, reply *struct{}) error { +func (c *ConfigEntry) Delete(args *structs.ConfigEntryRequest, reply *structs.ConfigEntryDeleteResponse) error { if err := c.srv.validateEnterpriseRequest(args.Entry.GetEnterpriseMeta(), true); err != nil { return err } @@ -294,9 +294,30 @@ func (c *ConfigEntry) Delete(args *structs.ConfigEntryRequest, reply *struct{}) return acl.ErrPermissionDenied } - args.Op = structs.ConfigEntryDelete - _, err = c.srv.raftApply(structs.ConfigEntryRequestType, args) - return err + // Only delete and delete-cas ops are supported. If the caller erroneously + // sent something else, we assume they meant delete. + switch args.Op { + case structs.ConfigEntryDelete, structs.ConfigEntryDeleteCAS: + default: + args.Op = structs.ConfigEntryDelete + } + + rsp, err := c.srv.raftApply(structs.ConfigEntryRequestType, args) + if err != nil { + return err + } + + if args.Op == structs.ConfigEntryDeleteCAS { + // In CAS deletions the FSM will return a boolean value indicating whether the + // operation was successful. + deleted, _ := rsp.(bool) + reply.Deleted = deleted + } else { + // For non-CAS deletions any non-error result indicates a successful deletion. + reply.Deleted = true + } + + return nil } // ResolveServiceConfig diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index 1c7bd85dca..59fa2cf174 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -676,6 +676,64 @@ func TestConfigEntry_Delete(t *testing.T) { }) } +func TestConfigEntry_DeleteCAS(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() + + require := require.New(t) + + dir, s := testServer(t) + defer os.RemoveAll(dir) + defer s.Shutdown() + + codec := rpcClient(t, s) + defer codec.Close() + + testrpc.WaitForLeader(t, s.RPC, "dc1") + + // Create a simple config entry. + entry := &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "foo", + } + state := s.fsm.State() + require.NoError(state.EnsureConfigEntry(1, entry)) + + // Verify it's there. + _, existing, err := state.ConfigEntry(nil, entry.Kind, entry.Name, nil) + require.NoError(err) + + // Send a delete CAS request with an invalid index. + args := structs.ConfigEntryRequest{ + Datacenter: "dc1", + Op: structs.ConfigEntryDeleteCAS, + } + args.Entry = entry.Clone() + args.Entry.GetRaftIndex().ModifyIndex = existing.GetRaftIndex().ModifyIndex - 1 + + var rsp structs.ConfigEntryDeleteResponse + require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.Delete", &args, &rsp)) + require.False(rsp.Deleted) + + // Verify the entry was not deleted. + _, existing, err = s.fsm.State().ConfigEntry(nil, structs.ServiceDefaults, "foo", nil) + require.NoError(err) + require.NotNil(existing) + + // Restore the valid index and try again. + args.Entry.GetRaftIndex().ModifyIndex = existing.GetRaftIndex().ModifyIndex + + require.NoError(msgpackrpc.CallWithCodec(codec, "ConfigEntry.Delete", &args, &rsp)) + require.True(rsp.Deleted) + + // Verify the entry was deleted. + _, existing, err = s.fsm.State().ConfigEntry(nil, structs.ServiceDefaults, "foo", nil) + require.NoError(err) + require.Nil(existing) +} + func TestConfigEntry_Delete_ACLDeny(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") diff --git a/agent/consul/config_replication_test.go b/agent/consul/config_replication_test.go index 9a018aedd6..fa10d4efe8 100644 --- a/agent/consul/config_replication_test.go +++ b/agent/consul/config_replication_test.go @@ -237,7 +237,7 @@ func TestReplication_ConfigEntries(t *testing.T) { Entry: entry, } - var out struct{} + var out structs.ConfigEntryDeleteResponse require.NoError(t, s1.RPC("ConfigEntry.Delete", &arg, &out)) } diff --git a/agent/consul/fsm/commands_oss.go b/agent/consul/fsm/commands_oss.go index 1d4d15fdbc..27050113b3 100644 --- a/agent/consul/fsm/commands_oss.go +++ b/agent/consul/fsm/commands_oss.go @@ -543,6 +543,14 @@ func (c *FSM) applyConfigEntryOperation(buf []byte, index uint64) interface{} { return err } return true + case structs.ConfigEntryDeleteCAS: + defer metrics.MeasureSinceWithLabels([]string{"fsm", "config_entry", req.Entry.GetKind()}, time.Now(), + []metrics.Label{{Name: "op", Value: "delete"}}) + deleted, err := c.state.DeleteConfigEntryCAS(index, req.Entry.GetRaftIndex().ModifyIndex, req.Entry) + if err != nil { + return err + } + return deleted case structs.ConfigEntryDelete: defer metrics.MeasureSinceWithLabels([]string{"fsm", "config_entry", req.Entry.GetKind()}, time.Now(), []metrics.Label{{Name: "op", Value: "delete"}}) diff --git a/agent/consul/fsm/commands_oss_test.go b/agent/consul/fsm/commands_oss_test.go index 3763ac4187..380c790c73 100644 --- a/agent/consul/fsm/commands_oss_test.go +++ b/agent/consul/fsm/commands_oss_test.go @@ -1352,6 +1352,57 @@ func TestFSM_ConfigEntry(t *testing.T) { } } +func TestFSM_ConfigEntry_DeleteCAS(t *testing.T) { + t.Parallel() + + require := require.New(t) + + logger := testutil.Logger(t) + fsm, err := New(nil, logger) + require.NoError(err) + + // Create a simple config entry and write it to the state store. + entry := &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "global", + } + require.NoError(fsm.state.EnsureConfigEntry(1, entry)) + + // Raft index is populated by EnsureConfigEntry, hold on to it so that we can + // restore it later. + raftIndex := entry.RaftIndex + require.NotZero(raftIndex.ModifyIndex) + + // Attempt a CAS delete with an invalid index. + entry = entry.Clone() + entry.RaftIndex = structs.RaftIndex{ + ModifyIndex: 99, + } + req := &structs.ConfigEntryRequest{ + Op: structs.ConfigEntryDeleteCAS, + Entry: entry, + } + buf, err := structs.Encode(structs.ConfigEntryRequestType, req) + require.NoError(err) + + // Expect to get boolean false back. + rsp := fsm.Apply(makeLog(buf)) + didDelete, isBool := rsp.(bool) + require.True(isBool) + require.False(didDelete) + + // Attempt a CAS delete with a valid index. + entry.RaftIndex = raftIndex + buf, err = structs.Encode(structs.ConfigEntryRequestType, req) + require.NoError(err) + + // Expect to get boolean true back. + rsp = fsm.Apply(makeLog(buf)) + didDelete, isBool = rsp.(bool) + require.True(isBool) + require.True(didDelete) +} + // This adapts another test by chunking the encoded data and then performing // out-of-order applies of half the logs. It then snapshots, restores to a new // FSM, and applies the rest. The goal is to verify that chunking snapshotting diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index 006e75288e..14d9c49361 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -242,6 +242,41 @@ func (s *Store) EnsureConfigEntryCAS(idx, cidx uint64, conf structs.ConfigEntry) return err == nil, err } +// DeleteConfigEntryCAS performs a check-and-set deletion of a config entry +// with the given raft index. If the index is not specified, or is not equal +// to the entry's current ModifyIndex then the call is a noop, otherwise the +// normal deletion is performed. +func (s *Store) DeleteConfigEntryCAS(idx, cidx uint64, conf structs.ConfigEntry) (bool, error) { + tx := s.db.WriteTxn(idx) + defer tx.Abort() + + existing, err := tx.First(tableConfigEntries, indexID, newConfigEntryQuery(conf)) + if err != nil { + return false, fmt.Errorf("failed config entry lookup: %s", err) + } + + if existing == nil { + return false, nil + } + + if existing.(structs.ConfigEntry).GetRaftIndex().ModifyIndex != cidx { + return false, nil + } + + if err := deleteConfigEntryTxn( + tx, + idx, + conf.GetKind(), + conf.GetName(), + conf.GetEnterpriseMeta(), + ); err != nil { + return false, err + } + + err = tx.Commit() + return err == nil, err +} + func (s *Store) DeleteConfigEntry(idx uint64, kind, name string, entMeta *structs.EnterpriseMeta) error { tx := s.db.WriteTxn(idx) defer tx.Abort() diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go index 8b1627f8f6..b47d9a3565 100644 --- a/agent/consul/state/config_entry_test.go +++ b/agent/consul/state/config_entry_test.go @@ -125,6 +125,47 @@ func TestStore_ConfigEntryCAS(t *testing.T) { require.Equal(updated, config) } +func TestStore_ConfigEntry_DeleteCAS(t *testing.T) { + require := require.New(t) + s := testConfigStateStore(t) + + entry := &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: "global", + Config: map[string]interface{}{ + "DestinationServiceName": "foo", + }, + } + + // Attempt to delete the entry before it exists. + ok, err := s.DeleteConfigEntryCAS(1, 0, entry) + require.NoError(err) + require.False(ok) + + // Create the entry. + require.NoError(s.EnsureConfigEntry(1, entry)) + + // Attempt to delete with an invalid index. + ok, err = s.DeleteConfigEntryCAS(2, 99, entry) + require.NoError(err) + require.False(ok) + + // Entry should not be deleted. + _, config, err := s.ConfigEntry(nil, entry.Kind, entry.Name, nil) + require.NoError(err) + require.NotNil(config) + + // Attempt to delete with a valid index. + ok, err = s.DeleteConfigEntryCAS(2, 1, entry) + require.NoError(err) + require.True(ok) + + // Entry should be deleted. + _, config, err = s.ConfigEntry(nil, entry.Kind, entry.Name, nil) + require.NoError(err) + require.Nil(config) +} + func TestStore_ConfigEntry_UpdateOver(t *testing.T) { // This test uses ServiceIntentions because they are the only // kind that implements UpdateOver() at this time. diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 4b68bec81a..a7703c45a4 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -445,6 +445,7 @@ const ( ConfigEntryUpsert ConfigEntryOp = "upsert" ConfigEntryUpsertCAS ConfigEntryOp = "upsert-cas" ConfigEntryDelete ConfigEntryOp = "delete" + ConfigEntryDeleteCAS ConfigEntryOp = "delete-cas" ) // ConfigEntryRequest is used when creating/updating/deleting a ConfigEntry. @@ -1121,3 +1122,7 @@ func validateConfigEntryMeta(meta map[string]string) error { } return err } + +type ConfigEntryDeleteResponse struct { + Deleted bool +} diff --git a/api/config_entry.go b/api/config_entry.go index e2986e72eb..f5fbbbce45 100644 --- a/api/config_entry.go +++ b/api/config_entry.go @@ -468,20 +468,45 @@ func (conf *ConfigEntries) set(entry ConfigEntry, params map[string]string, w *W } func (conf *ConfigEntries) Delete(kind string, name string, w *WriteOptions) (*WriteMeta, error) { + _, wm, err := conf.delete(kind, name, nil, w) + return wm, err +} + +// DeleteCAS performs a Check-And-Set deletion of the given config entry, and +// returns true if it was successful. If the provided index no longer matches +// the entry's ModifyIndex (i.e. it was modified by another process) then the +// operation will fail and return false. +func (conf *ConfigEntries) DeleteCAS(kind, name string, index uint64, w *WriteOptions) (bool, *WriteMeta, error) { + return conf.delete(kind, name, map[string]string{"cas": strconv.FormatUint(index, 10)}, w) +} + +func (conf *ConfigEntries) delete(kind, name string, params map[string]string, w *WriteOptions) (bool, *WriteMeta, error) { if kind == "" || name == "" { - return nil, fmt.Errorf("Both kind and name parameters must not be empty") + return false, nil, fmt.Errorf("Both kind and name parameters must not be empty") } r := conf.c.newRequest("DELETE", fmt.Sprintf("/v1/config/%s/%s", kind, name)) r.setWriteOptions(w) + for param, value := range params { + r.params.Set(param, value) + } + rtt, resp, err := conf.c.doRequest(r) if err != nil { - return nil, err + return false, nil, err } defer closeResponseBody(resp) + if err := requireOK(resp); err != nil { - return nil, err + return false, nil, err } + + var buf bytes.Buffer + if _, err := io.Copy(&buf, resp.Body); err != nil { + return false, nil, fmt.Errorf("Failed to read response: %v", err) + } + + res := strings.Contains(buf.String(), "true") wm := &WriteMeta{RequestTime: rtt} - return wm, nil + return res, wm, nil } diff --git a/api/config_entry_test.go b/api/config_entry_test.go index a341140f38..699d4ddc72 100644 --- a/api/config_entry_test.go +++ b/api/config_entry_test.go @@ -249,6 +249,37 @@ func TestAPI_ConfigEntries(t *testing.T) { }) }) + t.Run("CAS deletion", func(t *testing.T) { + require := require.New(t) + + entry := &ProxyConfigEntry{ + Kind: ProxyDefaults, + Name: ProxyConfigGlobal, + Config: map[string]interface{}{ + "foo": "bar", + }, + } + + // Create a config entry. + created, _, err := config_entries.Set(entry, nil) + require.NoError(err) + require.True(created, "entry should have been created") + + // Read it back to get the ModifyIndex. + result, _, err := config_entries.Get(entry.Kind, entry.Name, nil) + require.NoError(err) + require.NotNil(entry) + + // Attempt a deletion with an invalid index. + deleted, _, err := config_entries.DeleteCAS(entry.Kind, entry.Name, result.GetModifyIndex()-1, nil) + require.NoError(err) + require.False(deleted, "entry should not have been deleted") + + // Attempt a deletion with a valid index. + deleted, _, err = config_entries.DeleteCAS(entry.Kind, entry.Name, result.GetModifyIndex(), nil) + require.NoError(err) + require.True(deleted, "entry should have been deleted") + }) } func runStep(t *testing.T, name string, fn func(t *testing.T)) { diff --git a/command/config/delete/config_delete.go b/command/config/delete/config_delete.go index 456b068254..913d09e85f 100644 --- a/command/config/delete/config_delete.go +++ b/command/config/delete/config_delete.go @@ -1,6 +1,7 @@ package delete import ( + "errors" "flag" "fmt" @@ -20,14 +21,23 @@ type cmd struct { http *flags.HTTPFlags help string - kind string - name string + kind string + name string + cas bool + modifyIndex uint64 } func (c *cmd) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) c.flags.StringVar(&c.kind, "kind", "", "The kind of configuration to delete.") c.flags.StringVar(&c.name, "name", "", "The name of configuration to delete.") + c.flags.BoolVar(&c.cas, "cas", false, + "Perform a Check-And-Set operation. Specifying this value also "+ + "requires the -modify-index flag to be set. The default value "+ + "is false.") + c.flags.Uint64Var(&c.modifyIndex, "modify-index", 0, + "Unsigned integer representing the ModifyIndex of the config entry. "+ + "This is used in combination with the -cas flag.") c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) flags.Merge(c.flags, c.http.ServerFlags()) @@ -40,13 +50,8 @@ func (c *cmd) Run(args []string) int { return 1 } - if c.kind == "" { - c.UI.Error("Must specify the -kind parameter") - return 1 - } - - if c.name == "" { - c.UI.Error("Must specify the -name parameter") + if err := c.validateArgs(); err != nil { + c.UI.Error(err.Error()) return 1 } @@ -55,17 +60,50 @@ func (c *cmd) Run(args []string) int { c.UI.Error(fmt.Sprintf("Error connect to Consul agent: %s", err)) return 1 } + entries := client.ConfigEntries() + + var deleted bool + if c.cas { + deleted, _, err = entries.DeleteCAS(c.kind, c.name, c.modifyIndex, nil) + } else { + _, err = entries.Delete(c.kind, c.name, nil) + deleted = err == nil + } - _, err = client.ConfigEntries().Delete(c.kind, c.name, nil) if err != nil { c.UI.Error(fmt.Sprintf("Error deleting config entry %s/%s: %v", c.kind, c.name, err)) return 1 } + if !deleted { + c.UI.Error(fmt.Sprintf("Config entry not deleted: %s/%s", c.kind, c.name)) + return 1 + } + c.UI.Info(fmt.Sprintf("Config entry deleted: %s/%s", c.kind, c.name)) return 0 } +func (c *cmd) validateArgs() error { + if c.kind == "" { + return errors.New("Must specify the -kind parameter") + } + + if c.name == "" { + return errors.New("Must specify the -name parameter") + } + + if c.cas && c.modifyIndex == 0 { + return errors.New("Must specify a -modify-index greater than 0 with -cas") + } + + if c.modifyIndex != 0 && !c.cas { + return errors.New("Cannot specify -modify-index without -cas") + } + + return nil +} + func (c *cmd) Synopsis() string { return synopsis } diff --git a/command/config/delete/config_delete_test.go b/command/config/delete/config_delete_test.go index 6cef704918..01c00b549b 100644 --- a/command/config/delete/config_delete_test.go +++ b/command/config/delete/config_delete_test.go @@ -1,6 +1,7 @@ package delete import ( + "strconv" "testing" "github.com/hashicorp/consul/agent" @@ -53,12 +54,97 @@ func TestConfigDelete(t *testing.T) { require.Nil(t, entry) } +func TestConfigDelete_CAS(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + a := agent.NewTestAgent(t, ``) + defer a.Shutdown() + client := a.Client() + + _, _, err := client.ConfigEntries().Set(&api.ServiceConfigEntry{ + Kind: api.ServiceDefaults, + Name: "web", + Protocol: "tcp", + }, nil) + require.NoError(t, err) + + entry, _, err := client.ConfigEntries().Get(api.ServiceDefaults, "web", nil) + require.NoError(t, err) + + t.Run("with an invalid modify index", func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-kind=" + api.ServiceDefaults, + "-name=web", + "-cas", + "-modify-index=" + strconv.FormatUint(entry.GetModifyIndex()-1, 10), + } + + code := c.Run(args) + require.Equal(t, 1, code) + require.Contains(t, ui.ErrorWriter.String(), + "Config entry not deleted: service-defaults/web") + require.Empty(t, ui.OutputWriter.String()) + }) + + t.Run("with a valid modify index", func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-kind=" + api.ServiceDefaults, + "-name=web", + "-cas", + "-modify-index=" + strconv.FormatUint(entry.GetModifyIndex(), 10), + } + + code := c.Run(args) + require.Equal(t, 0, code) + require.Contains(t, ui.OutputWriter.String(), + "Config entry deleted: service-defaults/web") + require.Empty(t, ui.ErrorWriter.String()) + + entry, _, err := client.ConfigEntries().Get(api.ServiceDefaults, "web", nil) + require.Error(t, err) + require.Nil(t, entry) + }) +} + func TestConfigDelete_InvalidArgs(t *testing.T) { t.Parallel() - cases := map[string][]string{ - "no kind": {}, - "no name": {"-kind", "service-defaults"}, + cases := map[string]struct { + args []string + err string + }{ + "no kind": { + args: []string{}, + err: "Must specify the -kind parameter", + }, + "no name": { + args: []string{"-kind", api.ServiceDefaults}, + err: "Must specify the -name parameter", + }, + "cas but no modify-index": { + args: []string{"-kind", api.ServiceDefaults, "-name", "web", "-cas"}, + err: "Must specify a -modify-index greater than 0 with -cas", + }, + "cas but no zero modify-index": { + args: []string{"-kind", api.ServiceDefaults, "-name", "web", "-cas", "-modify-index", "0"}, + err: "Must specify a -modify-index greater than 0 with -cas", + }, + "modify-index but no cas": { + args: []string{"-kind", api.ServiceDefaults, "-name", "web", "-modify-index", "1"}, + err: "Cannot specify -modify-index without -cas", + }, } for name, tcase := range cases { @@ -66,8 +152,8 @@ func TestConfigDelete_InvalidArgs(t *testing.T) { ui := cli.NewMockUi() c := New(ui) - require.NotEqual(t, 0, c.Run(tcase)) - require.NotEmpty(t, ui.ErrorWriter.String()) + require.NotEqual(t, 0, c.Run(tcase.args)) + require.Contains(t, ui.ErrorWriter.String(), tcase.err) }) } } diff --git a/testrpc/wait.go b/testrpc/wait.go index c593f66062..63f6b603ea 100644 --- a/testrpc/wait.go +++ b/testrpc/wait.go @@ -182,7 +182,7 @@ func WaitForServiceIntentions(t *testing.T, rpc rpcFn, dc string) { Name: fakeConfigName, }, } - var ignored struct{} + var ignored structs.ConfigEntryDeleteResponse if err := rpc("ConfigEntry.Delete", args, &ignored); err != nil { r.Fatalf("err: %v", err) } diff --git a/website/content/api-docs/config.mdx b/website/content/api-docs/config.mdx index 5cf8d4a2d4..7587e14b3d 100644 --- a/website/content/api-docs/config.mdx +++ b/website/content/api-docs/config.mdx @@ -276,6 +276,12 @@ The table below shows this endpoint's support for `X-Consul-Namespace` header. If not provided, the namespace will be inherited from the request's ACL token or will default to the `default` namespace. Added in Consul 1.7.0. +- `cas` `(int: 0)` - Specifies to use a Check-And-Set operation. Unlike `PUT`, + the index must be greater than 0 for Consul to take any action: a 0 index will + not delete the config entry. If the index is non-zero, the config entry is only + deleted if the index matches the `ModifyIndex` of that config entry. This is + specified as part of the URL as a query parameter. + ### Sample Request ```shell-session diff --git a/website/content/commands/config/delete.mdx b/website/content/commands/config/delete.mdx index e338f31a72..986ebeb3c0 100644 --- a/website/content/commands/config/delete.mdx +++ b/website/content/commands/config/delete.mdx @@ -31,6 +31,14 @@ Usage: `consul config delete [options]` `proxy-defaults` config entry must be `global`, and the name of the `mesh` config entry must be `mesh`. +- `-cas` - Perform a Check-And-Set operation. Specifying this value also + requires the -modify-index flag to be set. The default value is false. + +- `-modify-index=` - Unsigned integer representing the ModifyIndex of the + config entry. This is used in combination with the -cas flag. + ## Examples $ consul config delete -kind service-defaults -name web + + $ consul config delete -kind service-defaults -name web -cas -modify-index 26