feat: add microcks validate command for pre-flight config verification#495
feat: add microcks validate command for pre-flight config verification#495A-d-i-t-y wants to merge 2 commits into
Conversation
|
I've opened a PR for this issue: #495 It adds a 'microcks validate' command that runs pre-flight checks I'm applying for the LFX Mentorship (Term 3) and would really appreciate |
Thanks for the PR. |
| Run: func(cmd *cobra.Command, args []string) { | ||
| ok := true | ||
|
|
||
| // 1. Config file presence and parsing. | ||
| configFile := globalClientOpts.ConfigPath | ||
| localConfig, err := config.ReadLocalConfig(configFile) | ||
| if err != nil { | ||
| fmt.Printf("✗ Config file: invalid — %v\n", err) | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
ReadLocalConfig() can return (nil, nil) for a missing file, but we print ✓ Config file: parsed successfully before checking whether any config/context actually exists. Please restructure this so the success message is only printed when there is a real config to validate.
| ctx, err := localConfig.ResolveContext(globalClientOpts.Context) | ||
| if err != nil { | ||
| fmt.Printf("✗ Context resolution: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| fmt.Printf("✓ Context resolved: %q (server: %s)\n", ctx.Name, ctx.Server.Server) | ||
|
|
||
| // 3. Build a client for this context. NewClient() attempts a token | ||
| // refresh internally, so a failure here reveals an expired or | ||
| // invalid credential. | ||
| mc, err := connectors.NewClient(*globalClientOpts) | ||
| if err != nil { | ||
| fmt.Printf("✗ Authentication: %v\n", err) | ||
| ok = false | ||
| } else { | ||
| fmt.Println("✓ Authentication: token is valid") | ||
| } | ||
|
|
||
| // 4. Server reachability check (also confirms Keycloak config | ||
| // endpoint responds, covering both plain connectivity and |
There was a problem hiding this comment.
This currently only validates the local config/context path.
Please also consider the direct connection mode that other commands support via --microcksURL together with --keycloakClientId / --keycloakClientSecret. For CI/CD pre-flight checks, that mode is at least as important as local config validation.
|
|
||
| var validateCmd = &cobra.Command{ | ||
|
|
||
| Use: "validate", | ||
| Short: "Validate current Microcks CLI configuration and connectivity", | ||
| Long: `Runs pre-flight checks on the active Microcks context: | ||
| - config file is present and parses correctly | ||
| - a context is configured and resolvable | ||
| - the auth token is valid (refreshed if needed) | ||
| - the target Microcks server is reachable | ||
|
|
||
| Exits with code 0 if all checks pass, or 1 if any check fails.`, |
There was a problem hiding this comment.
The command contract in #470 is broader than what is currently implemented.
Right now this checks config/context/auth/server reachability, but it does not implement the proposed service-account permission check or Async Minion connectivity check. Please either extend the implementation to cover those, or narrow the issue/PR scope explicitly.
There was a problem hiding this comment.
These tests do not actually exercise NewValidateCommand() or the command output/exit behavior.
They mostly re-test config.ReadLocalConfig() and ResolveContext(), which are existing helpers. Please add command-level tests for the new behavior itself: success path, missing config/context, and a failing auth/reachability path if possible.
Signed-off-by: A-d-i-t-y <aaaditya1909@gmail.com>
722bd05 to
ea1e04c
Compare
…ection mode, rewrite tests Signed-off-by: A-d-i-t-y <aaaditya1909@gmail.com>
|
Thanks for the detailed review, @Vaishnav88sk! Pushed a new commit (df3dfde) addressing all the points:
Ready for another look whenever you get a chance! |
2 participants
Fixes #470
Summary
Adds a
microcks validatecommand that runs pre-flight checks beforerunning imports or tests, so users catch configuration problems early
instead of mid-execution.
What it checks
Exits with code
0if all checks pass,1if any check fails —suitable for CI/CD pre-flight gating.