rpk cloud get namespaces/clusters #1322
Conversation
|
wow this is fire! |
src/go/rpk/pkg/vcloud/yak/client.go
Outdated
| func (yc *YakClient) get(token, url string) (*http.Response, error) { | ||
| log.Debugf("Calling yak api on url %s", url) | ||
| req, err := http.NewRequest(http.MethodGet, url, bytes.NewBuffer([]byte{})) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error creating new request. %w", err) | ||
| } | ||
| req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) | ||
| return http.DefaultClient.Do(req) | ||
| } | ||
|
|
||
| func (yc *YakClient) handleErrResponseCodes( | ||
| resp *http.Response, body []byte, | ||
| ) error { | ||
| // targetting HTTP codes 401, 403 which are used by yak | ||
| if resp.StatusCode > 400 && resp.StatusCode < 404 { | ||
| log.Debug(string(body)) | ||
| return ErrNotAuthorized | ||
| } | ||
| if resp.StatusCode != 200 { | ||
| return fmt.Errorf("error retrieving resource, http code %d. %s", resp.StatusCode, body) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
These changes should go in the 1st commit, where you introduced the YakClient.
There was a problem hiding this comment.
Agree I was waiting until the second method to extract the code meaningfully and to be honest solving the merge conflicts makes me not want to do it. Do you feel strongly about doing that? :D
alenkacz
force-pushed
the
av/rpk-cloud-get
branch
2 times, most recently
from
8438b8f
to
4d80984
Compare
|
|
||
| func NewNamespacesCommand(fs afero.Fs, out io.Writer) *cobra.Command { | ||
| return &cobra.Command{ | ||
| Use: "namespaces", |
There was a problem hiding this comment.
I meant the command. An alias for rpk cloud get namespaces, following the kubectl conventions would be rpk cloud get ns.
kubernetes/kubernetes#42873
| @@ -45,33 +45,53 @@ func (yc *YakClient) GetNamespaces() ([]*Namespace, error) { | |||
| return nil, ErrLoginTokenMissing{err} | |||
There was a problem hiding this comment.
This commit could be squashed onto the 1st one, right?
There was a problem hiding this comment.
mm maybe, can we keep it like this thought? 🙏
|
@alenkacz please rebase and let's merge this! 🎉 |
This is to keep vcloud independent of rpk. This just duplicates the current rpk table.
Cover letter
Adds
cloud get namespacesandcloud get clusterscommands.Release notes
If the PR changes the user experience, write a short summary of the changes. See the CONTRIBUTING guidelines for details.
Release note: [1-2 sentences of what this PR changes]