refactor(cli): remove the legacy exit shim and enforce it in CI - #491
refactor(cli): remove the legacy exit shim and enforce it in CI#491Caesarsage wants to merge 4 commits into
Conversation
| return "", errors.Wrap(errors.KindAPI, fmt.Errorf("parsing Keycloak config response: %w", err)) | ||
| } | ||
|
|
||
| // Retrieve auth server url and realm name. | ||
| enabled := configResp["enabled"].(bool) | ||
| authServerURL := configResp["auth-server-url"].(string) | ||
| realmName := configResp["realm"].(string) | ||
| // Return 'null' if Keycloak is disabled. | ||
| if enabled, _ := configResp["enabled"].(bool); !enabled { | ||
| return "null", nil | ||
| } | ||
|
|
||
| // Return a proper URL or 'null' if Keycloak is disables. | ||
| if enabled { | ||
| return authServerURL + "/realms/" + realmName + "/", nil | ||
| authServerURL, _ := configResp["auth-server-url"].(string) |
There was a problem hiding this comment.
Please check this as the same issue as previous PRs. This silently discards the malformed-response case again. Check all such cases to ensure code consistency.
Rebase
There was a problem hiding this comment.
The empty-string check catches most malformed cases, but explicit ok is cleaner and I will implement that. Thanks
There was a problem hiding this comment.
Needs a rebase, so the previous copyright block will be there
Signed-off-by: caesarsage <destinyerhabor6@gmail.com>
Signed-off-by: caesarsage <destinyerhabor6@gmail.com>
Signed-off-by: caesarsage <destinyerhabor6@gmail.com>
481a7b8 to
884714b
Compare
Signed-off-by: caesarsage <destinyerhabor6@gmail.com>
|
Hello @Vaishnav88sk and @Harsh4902 I have attended to all the issues here and from the other PR (merged) here instead of raising a separate PR. And i also added a CI guard to check for missing policies, like missing copyright header, ignored-error and stray-exit |
| if grep -rnE '(^|[^[:alnum:]_])_ =|, _ :=|fmt\.Println\(err\)' --include='*.go' cmd pkg; then | ||
| echo "::error::Potential silently ignored error found. Handle it explicitly or return errors.Wrap(kind, err)." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The silently-ignored-error grep pattern _ =|, _ :=|fmt\.Println\(err\) is a nice guard, but it's a blunt regex; it'll false-positive on any deliberate _ = x (e.g. explicitly discarding a value that's genuinely fine to ignore, if that ever comes up) with no allowlist mechanism. Not blocking, just flag it as something that may need a // nolint-style escape hatch down the line if a legitimate case shows up.
|
LGTM 👍🏻 |
2 participants
Part of the Microcks CLI v2 work (#255). Stacked on PR #490.
CheckError/CheckConfigNil/Fatal+ErrorGenericshim frompkg/errors; converts the standalonewatcher/binary to handle errors inline.go test ./...tobuild-verify.yml(the pipeline previously only built) and a guard step that fails ifos.Exit/log.Fatal/panicreappears outsidecmd/exit.go.CONTRIBUTING.md.Scope
Pure cleanup: nothing referenced the shim after PR #490.
The only process exits left in the tree are the two
mainentrypoints andcmd.Handle.