From 426c906ee191840cf97afdc632c1dedeeb532914 Mon Sep 17 00:00:00 2001 From: wangxinyi7 <121973291+wangxinyi7@users.noreply.github.com> Date: Wed, 13 Sep 2023 11:48:40 -0700 Subject: [PATCH] delete command ready (#18679) * delete command for resource management --- api/resource.go | 15 +++ command/registry.go | 2 + command/resource/delete/delete.go | 162 +++++++++++++++++++++++++ command/resource/delete/delete_test.go | 158 ++++++++++++++++++++++++ command/resource/helper.go | 23 ++++ command/resource/read/read.go | 51 ++------ command/resource/read/read_test.go | 158 +++++++++++++++++------- 7 files changed, 483 insertions(+), 86 deletions(-) create mode 100644 command/resource/delete/delete.go create mode 100644 command/resource/delete/delete_test.go diff --git a/api/resource.go b/api/resource.go index 27066039eb..ba9b21e6b7 100644 --- a/api/resource.go +++ b/api/resource.go @@ -5,6 +5,7 @@ package api import ( "fmt" + "net/http" "strings" "github.com/hashicorp/consul/proto-public/pbresource" @@ -65,6 +66,20 @@ func (resource *Resource) Read(gvk *GVK, resourceName string, q *QueryOptions) ( return out, nil } +func (resource *Resource) Delete(gvk *GVK, resourceName string, q *QueryOptions) error { + r := resource.c.newRequest("DELETE", strings.ToLower(fmt.Sprintf("/api/%s/%s/%s/%s", gvk.Group, gvk.Version, gvk.Kind, resourceName))) + r.setQueryOptions(q) + _, resp, err := resource.c.doRequest(r) + if err != nil { + return err + } + defer closeResponseBody(resp) + if err := requireHttpCodes(resp, http.StatusNoContent); err != nil { + return err + } + return nil +} + func (resource *Resource) Apply(gvk *GVK, resourceName string, q *QueryOptions, payload *WriteRequest) (*WriteResponse, *WriteMeta, error) { url := strings.ToLower(fmt.Sprintf("/api/%s/%s/%s/%s", gvk.Group, gvk.Version, gvk.Kind, resourceName)) diff --git a/command/registry.go b/command/registry.go index 047458e6e0..fceb708607 100644 --- a/command/registry.go +++ b/command/registry.go @@ -110,6 +110,7 @@ import ( "github.com/hashicorp/consul/command/reload" "github.com/hashicorp/consul/command/resource" resourceapply "github.com/hashicorp/consul/command/resource/apply" + resourcedelete "github.com/hashicorp/consul/command/resource/delete" resourcelist "github.com/hashicorp/consul/command/resource/list" resourceread "github.com/hashicorp/consul/command/resource/read" "github.com/hashicorp/consul/command/rtt" @@ -244,6 +245,7 @@ func RegisteredCommands(ui cli.Ui) map[string]mcli.CommandFactory { entry{"reload", func(ui cli.Ui) (cli.Command, error) { return reload.New(ui), nil }}, entry{"resource", func(cli.Ui) (cli.Command, error) { return resource.New(), nil }}, entry{"resource read", func(ui cli.Ui) (cli.Command, error) { return resourceread.New(ui), nil }}, + entry{"resource delete", func(ui cli.Ui) (cli.Command, error) { return resourcedelete.New(ui), nil }}, entry{"resource apply", func(ui cli.Ui) (cli.Command, error) { return resourceapply.New(ui), nil }}, entry{"resource list", func(ui cli.Ui) (cli.Command, error) { return resourcelist.New(ui), nil }}, entry{"rtt", func(ui cli.Ui) (cli.Command, error) { return rtt.New(ui), nil }}, diff --git a/command/resource/delete/delete.go b/command/resource/delete/delete.go new file mode 100644 index 0000000000..16904ffcfe --- /dev/null +++ b/command/resource/delete/delete.go @@ -0,0 +1,162 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package delete + +import ( + "errors" + "flag" + "fmt" + + "github.com/mitchellh/cli" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/flags" + "github.com/hashicorp/consul/command/resource" +) + +func New(ui cli.Ui) *cmd { + c := &cmd{UI: ui} + c.init() + return c +} + +type cmd struct { + UI cli.Ui + flags *flag.FlagSet + http *flags.HTTPFlags + help string + + filePath string +} + +func (c *cmd) init() { + c.flags = flag.NewFlagSet("", flag.ContinueOnError) + c.http = &flags.HTTPFlags{} + c.flags.StringVar(&c.filePath, "f", "", "File path with resource definition") + flags.Merge(c.flags, c.http.ClientFlags()) + flags.Merge(c.flags, c.http.ServerFlags()) + flags.Merge(c.flags, c.http.MultiTenancyFlags()) + flags.Merge(c.flags, c.http.AddPeerName()) + c.help = flags.Usage(help, c.flags) +} + +func (c *cmd) Run(args []string) int { + var gvk *api.GVK + var resourceName string + var opts *api.QueryOptions + + if err := c.flags.Parse(args); err != nil { + if !errors.Is(err, flag.ErrHelp) { + c.UI.Error(fmt.Sprintf("Failed to parse args: %v", err)) + return 1 + } + } + + if c.flags.Lookup("f").Value.String() != "" { + if c.filePath != "" { + parsedResource, err := resource.ParseResourceFromFile(c.filePath) + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to decode resource from input file: %v", err)) + return 1 + } + + if parsedResource == nil { + c.UI.Error("Unable to parse the file argument") + return 1 + } + + gvk = &api.GVK{ + Group: parsedResource.Id.Type.GetGroup(), + Version: parsedResource.Id.Type.GetGroupVersion(), + Kind: parsedResource.Id.Type.GetKind(), + } + resourceName = parsedResource.Id.GetName() + opts = &api.QueryOptions{ + Namespace: parsedResource.Id.Tenancy.GetNamespace(), + Partition: parsedResource.Id.Tenancy.GetPartition(), + Peer: parsedResource.Id.Tenancy.GetPeerName(), + Token: c.http.Token(), + } + } else { + c.UI.Error(fmt.Sprintf("Please provide an input file with resource definition")) + return 1 + } + } else { + if len(args) < 2 { + c.UI.Error("Your argument format is incorrect: Must specify two arguments: resource type and resource name") + return 1 + } + var err error + gvk, resourceName, err = resource.GetTypeAndResourceName(args) + if err != nil { + c.UI.Error(fmt.Sprintf("Your argument format is incorrect: %s", err)) + return 1 + } + + inputArgs := args[2:] + err = resource.ParseInputParams(inputArgs, c.flags) + if err != nil { + c.UI.Error(fmt.Sprintf("Error parsing input arguments: %v", err)) + return 1 + } + if c.filePath != "" { + c.UI.Warn("We ignored the -f flag if you provide gvk and resource name") + } + opts = &api.QueryOptions{ + Namespace: c.http.Namespace(), + Partition: c.http.Partition(), + Peer: c.http.PeerName(), + Token: c.http.Token(), + } + } + + client, err := c.http.APIClient() + if err != nil { + c.UI.Error(fmt.Sprintf("Error connect to Consul agent: %s", err)) + return 1 + } + + if err := client.Resource().Delete(gvk, resourceName, opts); err != nil { + c.UI.Error(fmt.Sprintf("Error deleting resource %s.%s.%s/%s: %v", gvk.Group, gvk.Version, gvk.Kind, resourceName, err)) + return 1 + } + + c.UI.Info(fmt.Sprintf("%s.%s.%s/%s deleted", gvk.Group, gvk.Version, gvk.Kind, resourceName)) + return 0 +} + +func (c *cmd) Synopsis() string { + return synopsis +} + +func (c *cmd) Help() string { + return flags.Usage(c.help, nil) +} + +const synopsis = "Delete resource information" +const help = ` +Usage: You have two options to delete the resource specified by the given +type, name, partition, namespace and peer and outputs its JSON representation. + +consul resource delete [type] [name] -partition= -namespace= -peer= +consul resource delete -f [resource_file_path] + +But you could only use one of the approaches. + +Example: + +$ consul resource delete catalog.v1alpha1.Service card-processor -partition=billing -namespace=payments -peer=eu +$ consul resource delete -f resource.hcl + +In resource.hcl, it could be: +ID { + Type = gvk("catalog.v1alpha1.Service") + Name = "card-processor" + Tenancy { + Namespace = "payments" + Partition = "billing" + PeerName = "eu" + } +} +` diff --git a/command/resource/delete/delete_test.go b/command/resource/delete/delete_test.go new file mode 100644 index 0000000000..9eec0225b4 --- /dev/null +++ b/command/resource/delete/delete_test.go @@ -0,0 +1,158 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 +package delete + +import ( + "errors" + "testing" + + "github.com/mitchellh/cli" + "github.com/stretchr/testify/require" + + "github.com/hashicorp/consul/agent" + "github.com/hashicorp/consul/command/resource/apply" + "github.com/hashicorp/consul/testrpc" +) + +func TestResourceDeleteInvalidArgs(t *testing.T) { + t.Parallel() + + type tc struct { + args []string + expectedCode int + expectedErr error + } + + cases := map[string]tc{ + "nil args": { + args: nil, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must specify two arguments: resource type and resource name"), + }, + "empty args": { + args: []string{}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must specify two arguments: resource type and resource name"), + }, + "missing file path": { + args: []string{"-f"}, + expectedCode: 1, + expectedErr: errors.New("Failed to parse args: flag needs an argument: -f"), + }, + "file not found": { + args: []string{"-f=../testdata/test.hcl"}, + expectedCode: 1, + expectedErr: errors.New("Failed to load data: Failed to read file: open ../testdata/test.hcl: no such file or directory"), + }, + "provide type and name": { + args: []string{"a.b.c"}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must specify two arguments: resource type and resource name"), + }, + "provide type and name with -f": { + args: []string{"a.b.c", "name", "-f", "test.hcl"}, + expectedCode: 1, + expectedErr: errors.New("We ignored the -f flag if you provide gvk and resource name"), + }, + "provide type and name with -f and other flags": { + args: []string{"a.b.c", "name", "-f", "test.hcl", "-namespace", "default"}, + expectedCode: 1, + expectedErr: errors.New("We ignored the -f flag if you provide gvk and resource name"), + }, + "does not provide resource name after type": { + args: []string{"a.b.c", "-namespace", "default"}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must provide resource name right after type"), + }, + "invalid resource type format": { + args: []string{"a.", "name", "-namespace", "default"}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must include resource type argument in group.verion.kind format"), + }, + } + + for desc, tc := range cases { + t.Run(desc, func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + + code := c.Run(tc.args) + + require.Equal(t, tc.expectedCode, code) + require.Contains(t, ui.ErrorWriter.String(), tc.expectedErr.Error()) + }) + } +} + +func createResource(t *testing.T, a *agent.TestAgent) { + applyUi := cli.NewMockUi() + applyCmd := apply.New(applyUi) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + } + + args = append(args, []string{"-f=../testdata/demo.hcl"}...) + + code := applyCmd.Run(args) + require.Equal(t, 0, code) + require.Empty(t, applyUi.ErrorWriter.String()) +} + +func TestResourceDelete(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + a := agent.NewTestAgent(t, ``) + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + defaultCmdArgs := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + } + cases := []struct { + name string + args []string + expectedCode int + createResource bool + }{ + { + name: "delete resource in hcl format", + args: []string{"-f=../testdata/demo.hcl"}, + expectedCode: 0, + createResource: true, + }, + { + name: "delete resource in command line format", + args: []string{"demo.v2.Artist", "korn", "-partition=default", "-namespace=default", "-peer=local"}, + expectedCode: 0, + createResource: true, + }, + { + name: "delete resource that doesn't exist in command line format", + args: []string{"demo.v2.Artist", "korn", "-partition=default", "-namespace=default", "-peer=local"}, + expectedCode: 0, + createResource: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + cliArgs := append(tc.args, defaultCmdArgs...) + if tc.createResource { + createResource(t, a) + } + code := c.Run(cliArgs) + require.Empty(t, ui.ErrorWriter.String()) + require.Equal(t, tc.expectedCode, code) + require.Contains(t, ui.OutputWriter.String(), "deleted") + }) + } +} diff --git a/command/resource/helper.go b/command/resource/helper.go index 0bdeb5415d..6299237d2e 100644 --- a/command/resource/helper.go +++ b/command/resource/helper.go @@ -7,8 +7,10 @@ import ( "errors" "flag" "fmt" + "strings" "github.com/hashicorp/consul/agent/consul" + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/helpers" "github.com/hashicorp/consul/internal/resourcehcl" "github.com/hashicorp/consul/proto-public/pbresource" @@ -35,3 +37,24 @@ func ParseInputParams(inputArgs []string, flags *flag.FlagSet) error { } return nil } + +func GetTypeAndResourceName(args []string) (gvk *api.GVK, resourceName string, e error) { + // it has to be resource name after the type + if strings.HasPrefix(args[1], "-") { + return nil, "", fmt.Errorf("Must provide resource name right after type") + } + + s := strings.Split(args[0], ".") + if len(s) != 3 { + return nil, "", fmt.Errorf("Must include resource type argument in group.verion.kind format") + } + + gvk = &api.GVK{ + Group: s[0], + Version: s[1], + Kind: s[2], + } + + resourceName = args[1] + return +} diff --git a/command/resource/read/read.go b/command/resource/read/read.go index 022f64099f..6bb47ac33d 100644 --- a/command/resource/read/read.go +++ b/command/resource/read/read.go @@ -8,15 +8,12 @@ import ( "errors" "flag" "fmt" - "strings" "github.com/mitchellh/cli" - "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" - "github.com/hashicorp/consul/command/helpers" - "github.com/hashicorp/consul/internal/resourcehcl" + "github.com/hashicorp/consul/command/resource" ) func New(ui cli.Ui) *cmd { @@ -50,11 +47,6 @@ func (c *cmd) Run(args []string) int { var resourceName string var opts *api.QueryOptions - if len(args) == 0 { - c.UI.Error("Please provide required arguments") - return 1 - } - if err := c.flags.Parse(args); err != nil { if !errors.Is(err, flag.ErrHelp) { c.UI.Error(fmt.Sprintf("Failed to parse args: %v", err)) @@ -64,14 +56,14 @@ func (c *cmd) Run(args []string) int { if c.flags.Lookup("f").Value.String() != "" { if c.filePath != "" { - data, err := helpers.LoadDataSourceNoRaw(c.filePath, nil) + parsedResource, err := resource.ParseResourceFromFile(c.filePath) if err != nil { - c.UI.Error(fmt.Sprintf("Failed to load data: %v", err)) + c.UI.Error(fmt.Sprintf("Failed to decode resource from input file: %v", err)) return 1 } - parsedResource, err := resourcehcl.Unmarshal([]byte(data), consul.NewTypeRegistry()) - if err != nil { - c.UI.Error(fmt.Sprintf("Failed to decode resource from input file: %v", err)) + + if parsedResource == nil { + c.UI.Error("Unable to parse the file argument") return 1 } @@ -94,27 +86,24 @@ func (c *cmd) Run(args []string) int { } } else { if len(args) < 2 { - c.UI.Error("Must specify two arguments: resource type and resource name") + c.UI.Error("Your argument format is incorrect: Must specify two arguments: resource type and resource name") return 1 } var err error - gvk, resourceName, err = getTypeAndResourceName(args) + gvk, resourceName, err = resource.GetTypeAndResourceName(args) if err != nil { c.UI.Error(fmt.Sprintf("Your argument format is incorrect: %s", err)) return 1 } inputArgs := args[2:] - if err := c.flags.Parse(inputArgs); err != nil { - if errors.Is(err, flag.ErrHelp) { - return 0 - } - c.UI.Error(fmt.Sprintf("Failed to parse args: %v", err)) + err = resource.ParseInputParams(inputArgs, c.flags) + if err != nil { + c.UI.Error(fmt.Sprintf("Error parsing input arguments: %v", err)) return 1 } if c.filePath != "" { - c.UI.Error("You need to provide all information in the HCL file if provide its file path") - return 1 + c.UI.Warn("We ignored the -f flag if you provide gvk and resource name") } opts = &api.QueryOptions{ Namespace: c.http.Namespace(), @@ -147,22 +136,6 @@ func (c *cmd) Run(args []string) int { return 0 } -func getTypeAndResourceName(args []string) (gvk *api.GVK, resourceName string, e error) { - if strings.HasPrefix(args[1], "-") { - return nil, "", fmt.Errorf("Must provide resource name right after type") - } - - s := strings.Split(args[0], ".") - gvk = &api.GVK{ - Group: s[0], - Version: s[1], - Kind: s[2], - } - - resourceName = args[1] - return -} - func (c *cmd) Synopsis() string { return synopsis } diff --git a/command/resource/read/read_test.go b/command/resource/read/read_test.go index 69e9654779..30eecb9802 100644 --- a/command/resource/read/read_test.go +++ b/command/resource/read/read_test.go @@ -3,56 +3,71 @@ package read import ( + "errors" "testing" "github.com/mitchellh/cli" "github.com/stretchr/testify/require" + + "github.com/hashicorp/consul/agent" + "github.com/hashicorp/consul/command/resource/apply" + "github.com/hashicorp/consul/testrpc" ) func TestResourceReadInvalidArgs(t *testing.T) { t.Parallel() type tc struct { - args []string - expectedCode int - expectedErrMsg string + args []string + expectedCode int + expectedErr error } cases := map[string]tc{ "nil args": { - args: nil, - expectedCode: 1, - expectedErrMsg: "Please provide required arguments", + args: nil, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must specify two arguments: resource type and resource name"), }, "empty args": { - args: []string{}, - expectedCode: 1, - expectedErrMsg: "Please provide required arguments", + args: []string{}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must specify two arguments: resource type and resource name"), }, "missing file path": { - args: []string{"-f"}, - expectedCode: 1, - expectedErrMsg: "Please input file path", + args: []string{"-f"}, + expectedCode: 1, + expectedErr: errors.New("Failed to parse args: flag needs an argument: -f"), + }, + "file not found": { + args: []string{"-f=../testdata/test.hcl"}, + expectedCode: 1, + expectedErr: errors.New("Failed to load data: Failed to read file: open ../testdata/test.hcl: no such file or directory"), }, "provide type and name": { - args: []string{"a.b.c"}, - expectedCode: 1, - expectedErrMsg: "Must specify two arguments: resource type and resource name", + args: []string{"a.b.c"}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must specify two arguments: resource type and resource name"), }, "provide type and name with -f": { - args: []string{"a.b.c", "name", "-f", "test.hcl"}, - expectedCode: 1, - expectedErrMsg: "You need to provide all information in the HCL file if provide its file path", + args: []string{"a.b.c", "name", "-f", "test.hcl"}, + expectedCode: 1, + expectedErr: errors.New("We ignored the -f flag if you provide gvk and resource name"), }, "provide type and name with -f and other flags": { - args: []string{"a.b.c", "name", "-f", "test.hcl", "-namespace", "default"}, - expectedCode: 1, - expectedErrMsg: "You need to provide all information in the HCL file if provide its file path", + args: []string{"a.b.c", "name", "-f", "test.hcl", "-namespace", "default"}, + expectedCode: 1, + expectedErr: errors.New("We ignored the -f flag if you provide gvk and resource name"), }, "does not provide resource name after type": { - args: []string{"a.b.c", "-namespace", "default"}, - expectedCode: 1, - expectedErrMsg: "Must provide resource name right after type", + args: []string{"a.b.c", "-namespace", "default"}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must provide resource name right after type"), + }, + "invalid resource type format": { + args: []string{"a.", "name", "-namespace", "default"}, + expectedCode: 1, + expectedErr: errors.New("Your argument format is incorrect: Must include resource type argument in group.verion.kind format"), }, } @@ -61,32 +76,81 @@ func TestResourceReadInvalidArgs(t *testing.T) { ui := cli.NewMockUi() c := New(ui) - require.Equal(t, tc.expectedCode, c.Run(tc.args)) - require.NotEmpty(t, ui.ErrorWriter.String()) + code := c.Run(tc.args) + + require.Equal(t, tc.expectedCode, code) + require.Contains(t, ui.ErrorWriter.String(), tc.expectedErr.Error()) }) } } +func createResource(t *testing.T, a *agent.TestAgent) { + applyUi := cli.NewMockUi() + applyCmd := apply.New(applyUi) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + } + + args = append(args, []string{"-f=../testdata/demo.hcl"}...) + + code := applyCmd.Run(args) + require.Equal(t, 0, code) + require.Empty(t, applyUi.ErrorWriter.String()) +} + func TestResourceRead(t *testing.T) { - // TODO: add read test after apply checked in - //if testing.Short() { - // t.Skip("too slow for testing.Short") - //} - // - //t.Parallel() - // - //a := agent.NewTestAgent(t, ``) - //defer a.Shutdown() - //client := a.Client() - // - //ui := cli.NewMockUi() - //c := New(ui) - - //_, _, err := client.Resource().Apply() - //require.NoError(t, err) - // - //args := []string{} - // - //code := c.Run(args) - //require.Equal(t, 0, code) + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + a := agent.NewTestAgent(t, ``) + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + defaultCmdArgs := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + } + + createResource(t, a) + cases := []struct { + name string + args []string + expectedCode int + errMsg string + }{ + { + name: "read resource in hcl format", + args: []string{"-f=../testdata/demo.hcl"}, + expectedCode: 0, + errMsg: "", + }, + { + name: "read resource in command line format", + args: []string{"demo.v2.Artist", "korn", "-partition=default", "-namespace=default", "-peer=local"}, + expectedCode: 0, + errMsg: "", + }, + { + name: "read resource that doesn't exist", + args: []string{"demo.v2.Artist", "fake-korn", "-partition=default", "-namespace=default", "-peer=local"}, + expectedCode: 1, + errMsg: "Error reading resource &{demo v2 Artist}/fake-korn: Unexpected response code: 404 (rpc error: code = NotFound desc = resource not found)\n", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + cliArgs := append(tc.args, defaultCmdArgs...) + code := c.Run(cliArgs) + require.Equal(t, ui.ErrorWriter.String(), tc.errMsg) + require.Equal(t, tc.expectedCode, code) + }) + } }