-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Net 2714/xw cli read command #18462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Net 2714/xw cli read command #18462
Changes from 8 commits
aea9135
bfc7909
4ead439
b2a6275
b1a45af
2508d62
befaffc
5f16d90
abe4951
d66dbb5
66cf4f3
47bdf63
1208d20
d2ff5dc
cb5fbab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package api | ||
|
|
||
| import ( | ||
| "fmt" | ||
| ) | ||
|
|
||
| type Resource struct { | ||
| c *Client | ||
| } | ||
|
|
||
| type GVK struct { | ||
| Group string | ||
| Version string | ||
| Kind string | ||
| } | ||
|
|
||
| // Config returns a handle to the Config endpoints | ||
| func (c *Client) Resource() *Resource { | ||
| return &Resource{c} | ||
| } | ||
|
|
||
| func (resource *Resource) Read(gvk *GVK, resourceName string, q *QueryOptions) (map[string]interface{}, error) { | ||
| r := resource.c.newRequest("GET", 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 nil, err | ||
| } | ||
| defer closeResponseBody(resp) | ||
| if err := requireOK(resp); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var out map[string]interface{} | ||
| if err := decodeBody(resp, &out); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return out, nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package read | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "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" | ||
| ) | ||
|
|
||
| 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 { | ||
|
dhiaayachi marked this conversation as resolved.
|
||
| var gvk *api.GVK | ||
| var resourceName string | ||
| var opts *api.QueryOptions | ||
|
|
||
| if args[0] == "-f" { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we check of len(args)>0 in here to avoid a panic?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! I'll add the sanity check here |
||
| if err := c.flags.Parse(args); err != nil { | ||
| if err == flag.ErrHelp { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is copied from elsewhere, but a better check here would be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @dhiaayachi |
||
| return 0 | ||
| } | ||
| c.UI.Error(fmt.Sprintf("Failed to parse args: %v", err)) | ||
| return 1 | ||
| } | ||
| if c.filePath != "" { | ||
| data, err := helpers.LoadDataSourceNoRaw(c.filePath, nil) | ||
| if err != nil { | ||
| c.UI.Error(fmt.Sprintf("Failed to load data: %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)) | ||
| 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(), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing consistent flag here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! updated |
||
| } | ||
| } else { | ||
| c.UI.Error("Please input file path") | ||
| return 1 | ||
| } | ||
| } else { | ||
| var err error | ||
| gvk, resourceName, err = getTypeAndResourceName(args) | ||
| if err != nil { | ||
| c.UI.Error(fmt.Sprintf("Your argument format is incorrect: %s", err)) | ||
| return 1 | ||
| } | ||
|
|
||
| inputArgs := args[2:] | ||
|
dhiaayachi marked this conversation as resolved.
|
||
| if err := c.flags.Parse(inputArgs); err != nil { | ||
| if err == flag.ErrHelp { | ||
| return 0 | ||
| } | ||
| c.UI.Error(fmt.Sprintf("Failed to parse args: %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 | ||
| } | ||
| 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 | ||
| } | ||
|
|
||
| entry, err := client.Resource().Read(gvk, resourceName, opts) | ||
| if err != nil { | ||
| c.UI.Error(fmt.Sprintf("Error reading resource %s/%s: %v", gvk, resourceName, err)) | ||
| return 1 | ||
| } | ||
|
|
||
| b, err := json.MarshalIndent(entry, "", " ") | ||
| if err != nil { | ||
| c.UI.Error("Failed to encode output data") | ||
| return 1 | ||
| } | ||
|
|
||
| c.UI.Info(string(b)) | ||
| return 0 | ||
| } | ||
|
|
||
| func getTypeAndResourceName(args []string) (gvk *api.GVK, resourceName string, e error) { | ||
| if len(args) < 2 { | ||
| return nil, "", fmt.Errorf("Must specify two arguments: resource type and resource name") | ||
| } | ||
| 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 | ||
| } | ||
|
|
||
| func (c *cmd) Help() string { | ||
| return flags.Usage(c.help, nil) | ||
| } | ||
|
|
||
| const synopsis = "Read resource information" | ||
| const help = ` | ||
| Usage: consul resource read [type] [name] -partition=<default> -namespace=<default> -peer=<local> -consistent=<false> -json | ||
|
|
||
| Reads the resource specified by the given type, name, partition, namespace, peer and reading mode | ||
| and outputs its JSON representation. | ||
|
|
||
| Example: | ||
|
|
||
| $ consul resource read catalog.v1alpha1.Service card-processor -partition=billing -namespace=payments -peer=eu | ||
| ` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
| package read | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/mitchellh/cli" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestResourceReadInvalidArgs(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| type tc struct { | ||
| args []string | ||
| expectedCode int | ||
| expectedErrMsg string | ||
| } | ||
|
|
||
| cases := map[string]tc{ | ||
| "missing file path": { | ||
|
dhiaayachi marked this conversation as resolved.
|
||
| args: []string{"-f"}, | ||
| expectedCode: 1, | ||
| expectedErrMsg: "Please input file path", | ||
| }, | ||
| "provide type and name": { | ||
| args: []string{"a.b.c"}, | ||
| expectedCode: 1, | ||
| expectedErrMsg: "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", | ||
| }, | ||
| "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", | ||
| }, | ||
| "does not provide resource name after type": { | ||
| args: []string{"a.b.c", "-namespace", "default"}, | ||
| expectedCode: 1, | ||
| expectedErrMsg: "Must provide resource name right after type", | ||
| }, | ||
| } | ||
|
|
||
| for desc, tc := range cases { | ||
| t.Run(desc, func(t *testing.T) { | ||
| ui := cli.NewMockUi() | ||
| c := New(ui) | ||
|
|
||
| require.Equal(t, tc.expectedCode, c.Run(tc.args)) | ||
| require.NotEmpty(t, ui.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) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package resource | ||
|
|
||
| import ( | ||
| "github.com/mitchellh/cli" | ||
|
|
||
| "github.com/hashicorp/consul/command/flags" | ||
| ) | ||
|
|
||
| func New() *cmd { | ||
| return &cmd{} | ||
| } | ||
|
|
||
| type cmd struct{} | ||
|
|
||
| func (c *cmd) Run(args []string) int { | ||
| return cli.RunResultHelp | ||
| } | ||
|
|
||
| func (c *cmd) Synopsis() string { | ||
| return synopsis | ||
| } | ||
|
|
||
| func (c *cmd) Help() string { | ||
| return flags.Usage(help, nil) | ||
| } | ||
|
|
||
| const synopsis = "Interact with Consul's resources" | ||
| const help = ` | ||
| Usage: consul resource <subcommand> [options] | ||
|
|
||
| This command has subcommands for interacting with Consul's resources. | ||
| Here are some simple examples, and more detailed examples are available | ||
| in the subcommands or the documentation. | ||
|
|
||
| Read a resource: | ||
|
|
||
| $ consul resource read [type] [name] -partition=<default> -namespace=<default> -peer=<local> -consistent=<false> -json | ||
|
xwa153 marked this conversation as resolved.
|
||
|
|
||
| Run | ||
|
|
||
| consul resource <subcommand> -h | ||
|
|
||
| for help on that subcommand. | ||
| ` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this added specifically for the resource API? I'm wondering if we should hold on for peering as it's not clear how we will implement the api? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhiaayachi
I'm not sure how the
peerflag is used by the backend, I added this flag here based on the AC of the ticket, it basically requires thepeerinformation, that's why I added it here.Please let me know if the information here is redundant or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhiaayachi The
peerfield is part of the resource gRPC API hence it's included in both the HTTP side and the CLI side.The pending discussion/work is about whether peer should be moved outside the tenancy block but I am not sure what you mean by "we are not clear how we will implement the api" can you elaborate?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO peering tenancy should be different fields, as they represent different values/concepts but I think the backend discussion could happen independently from this