-
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
Merged
Merged
Net 2714/xw cli read command #18462
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
aea9135
save read work
xwa153 bfc7909
renaming
xwa153 4ead439
parse hcl and validate args
xwa153 b2a6275
rename file
xwa153 b1a45af
add unit tests
xwa153 2508d62
update license header
xwa153 befaffc
resolve conflict
xwa153 5f16d90
add more test
xwa153 abe4951
be compatible to peer and peer_name
xwa153 d66dbb5
support consistent flag
xwa153 66cf4f3
Merge branch 'main' into NET-2714/xw-cli-read-command
xwa153 47bdf63
code refactor, add tests
xwa153 1208d20
to lower request string
xwa153 d2ff5dc
refactor and add test
xwa153 cb5fbab
add missing consistent flag
xwa153 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package api | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
| ) | ||
|
|
||
| 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", 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 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 | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,199 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package read | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "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" | ||
| ) | ||
|
|
||
| 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.
Show resolved
Hide resolved
|
||
| var gvk *api.GVK | ||
| 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)) | ||
| return 1 | ||
| } | ||
| } | ||
|
|
||
| if c.flags.Lookup("f").Value.String() != "" { | ||
| 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(), | ||
| RequireConsistent: !c.http.Stale(), | ||
| } | ||
| } else { | ||
| c.UI.Error(fmt.Sprintf("Please provide an input file with resource definition")) | ||
| return 1 | ||
| } | ||
| } else { | ||
| if len(args) < 2 { | ||
| c.UI.Error("Must specify two arguments: resource type and resource name") | ||
| return 1 | ||
| } | ||
| 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.
Show resolved
Hide resolved
|
||
| 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)) | ||
| 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(), | ||
| RequireConsistent: !c.http.Stale(), | ||
| } | ||
| } | ||
|
|
||
| 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 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: You have two options to read the resource specified by the given | ||
| type, name, partition, namespace and peer and outputs its JSON representation. | ||
|
|
||
| consul resource read [type] [name] -partition=<default> -namespace=<default> -peer=<local> | ||
| consul resource read -f [resource_file_path] | ||
|
|
||
| But you could only use one of the approaches. | ||
|
|
||
| Example: | ||
|
|
||
| $ consul resource read catalog.v1alpha1.Service card-processor -partition=billing -namespace=payments -peer=eu | ||
| $ consul resource read -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" | ||
| } | ||
| } | ||
| ` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| // 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{ | ||
| "nil args": { | ||
| args: nil, | ||
| expectedCode: 1, | ||
| expectedErrMsg: "Please provide required arguments", | ||
| }, | ||
| "empty args": { | ||
| args: []string{}, | ||
| expectedCode: 1, | ||
| expectedErrMsg: "Please provide required arguments", | ||
| }, | ||
| "missing file path": { | ||
dhiaayachi marked this conversation as resolved.
Show resolved
Hide 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) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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