Check if Namespace Exists for Task List Command #349
Conversation
tekton-robot
added
the
needs-ok-to-test
|
Hi @danielhelfand. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
tekton-robot
added
the
size/M
tekton-robot
added
ok-to-test
|
The following is the coverage report on pkg/.
|
pkg/cmd/task/list.go
Outdated
| // Check if namespace exists. Return error if namespace specified with -n doesn't exist. | ||
| _, err = cs.Kube.CoreV1().Namespaces().Get(p.Namespace(), metav1.GetOptions{}) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error: Namespace \"%s\" does not exist. Please enter a valid namespace.\n", p.Namespace()) |
There was a problem hiding this comment.
I think we need to handle the case of if the user doesn't have access to that namespace. In that case, this error message may be misleading.
There was a problem hiding this comment.
It may be better to just return err here as what ever is returned will be displayed to the user. I ran into issues with testing by returning err originally since it appears to display the usage as part of the output. I'll take another shot at that approach.
There was a problem hiding this comment.
This should now display RBAC errors as well with most recent push.
|
The following is the coverage report on pkg/.
|
| @@ -33,6 +33,7 @@ func ExecuteCommandC(c *cobra.Command, args ...string) (*cobra.Command, string, | |||
| buf := new(bytes.Buffer) | |||
| c.SetOutput(buf) | |||
| c.SetArgs(args) | |||
| c.SilenceUsage = true | |||
There was a problem hiding this comment.
Adding this since tkn already has SilenceUsage set to true in root.go. Tests should be the same output.
|
The following is the coverage report on pkg/.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
This pull request is a proposal to help deal with #311. It adds an error message to the CLI to check whether the namespace specified by the user exists on his or her cluster.
Assuming this is a valid proposal, I can begin implementing this same change for other locations in the CLI that use
-n.Changes
Adds a check to
task/list.goto see whether a namespace exists. If it does not, an error message is returned:"Error: namespaces \"foo\" not found\n". The use ofnamespacesversus non-pluralnamespacein the error message is not considered an issue for client-go.Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make docsandmake manif needed.make checkSee the contribution guide
for more details.
Release Notes