Allow listing health checks without mount path #19199
Conversation
This allows the bare:
$ vault pki health-check -list
without a corresponding mount path to complete. Otherwise, users would
be greeted with a prompt for the mount, which is less than ideal.
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
added
bug
| // When listing is enabled, we lack an argument here, but do not contact | ||
| // the server at all, so we're safe to use a hard-coded default here. | ||
| pkiPath := "pki" | ||
| if len(args) == 1 { |
There was a problem hiding this comment.
Could we invert this to be specific to c.flagList is set to ignore/use "pki", I think that would be a little more intuitive in the future (basically we ignore the mount arg when c.flagList is set?)
Also there is a call out above around line 138 within the c.flagList's usage that says it still requires a positional mount argument.
There was a problem hiding this comment.
Ah good catch.
I'm not sure I follow how we'd avoid the array-out-of-bounds access?
pkiPath := args[0] // would crash, no?
if c.flagsList {
pkiPath = "pki"
}
I think we're at the equivalent:
pkiPath := "pki" // or <mount> or some other sentinel.
if !c.flagsList { // == len(args) == 1
pkiPath = args[0]
}
There was a problem hiding this comment.
Yeah fair enough, seeing it in code now, it would amount to the same thing, I had been thinking of an if/else scenario but the comment + current code is good.
I'd just request we fix up the usage above then.
There was a problem hiding this comment.
Thank you!! Pushed :-)
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This allows the bare:
without a corresponding mount path to complete. Otherwise, users would be greeted with a prompt for the mount, which is less than ideal.