Implement 'gcp wif-config update' command #667
Conversation
| @@ -545,3 +545,12 @@ func (c *shim) createMemberRoleBindingForProject( | |||
| Role: roleName, | |||
| }) | |||
| } | |||
|
|
|||
| // Checks for WIF config name or id in input | |||
| func wifKeyArgCheck(args []string) error { | |||
There was a problem hiding this comment.
This helper does not belong in this file. I would put it in a helpers.go file.
| @@ -545,3 +545,12 @@ func (c *shim) createMemberRoleBindingForProject( | |||
| Role: roleName, | |||
| }) | |||
| } | |||
|
|
|||
| // Checks for WIF config name or id in input | |||
| func wifKeyArgCheck(args []string) error { | |||
There was a problem hiding this comment.
This helper does not belong in this file as it does not relate to the shim. I would recommend placing it in a 'helpers.go' file.
| // No implementation yet | ||
| ctx := context.Background() | ||
| log := log.Default() | ||
| key := argv[0] |
There was a problem hiding this comment.
I have readability concerns with the way we are deriving the key throughout our wif commands.
We know that the first element of argv exists and will be populated because we run wifKeyArgCheck on argv within the preRunE function that is run prior to RunE; we are safe to access argv[0]. However knowing this requires the reader to be aware of this execution flow, which is defined by the cobra package.
I'm thinking it may be better for the readibility of the code to do away with the preRunE call, and do the flag validation at the start of RunE. We could then have functions that both validate that the data is there and return it:
key, err := wifKeyFromArgs(argv)
There was a problem hiding this comment.
Should we have a manual mode for wif updates?
| return nil | ||
| } | ||
|
|
||
| // findWifConfig finds the WIF configuration by ID or name | ||
| func findWifConfig(client *cmv1.Client, key string) (*cmv1.WifConfig, error) { |
There was a problem hiding this comment.
This function could also go in helpers.go, as it is used by multiple commands.
Related issue: OCM-10615