[CIS-1.5] Move profile flag to common flags #387
Conversation
|
Pinged you about this, but going to post here for transparency: |
Move profile flag to common flags for server/agent so that it can be loaded from config YAML. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
| @@ -54,8 +54,16 @@ func NewAgentCommand() cli.Command { | |||
| } | |||
|
|
|||
| func AgentRun(clx *cli.Context) error { | |||
| if profile == "" { | |||
| switch profile { | |||
There was a problem hiding this comment.
This would probably make sense to have in a function in root.go that takes the profile string as an arg and does this logic so it's not needed to be maintained in 2 different places. More easily extensible too if/when we move to 1.6, etc. Not blocking however it'd be cool to have an issue that captures this.
There was a problem hiding this comment.
I agree 100%. I had originally moved this into the validateCISreqs function, but Craig asked me to reduce the amount of change so I went for a more minimal approach at the cost of some repetition.
There was a problem hiding this comment.
+1. He basically had that, but I asked him to minimize the pr to the smallest change possible for the purpose of reviewing and merging as quickly as possible.
There was a problem hiding this comment.
Cool. Do we have an issue for this for after GA?
brandond
changed the title
Stop ranging over CLI flags; the type checks are tedious and error-prone. Fixes errors turned up in QA validation of rancher#387. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Stop ranging over CLI flags; the type checks are tedious and error-prone. Fixes errors turned up in QA validation of #387. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Proposed Changes
Move profile flag to common flags for server/agent so that it can be loaded from config YAML. Also simplified flag use a bit to get rid of unnecessary iteration over flag lists.
Types of Changes
Verification
profile: cis-1.5in config.yamlLinked Issues
Related to #351
Further Comments