fix: configure profile handling, global-flag validation & client connect timeout#21
Merged
Merged
Conversation
LoadConfig returns (nil, err) for a profile not present in the credentials file. configure set and configure list discarded that error and dereferenced the nil *Config, crashing with SIGSEGV (e.g. `grn configure set region HCM-3 --profile prod-hcm-qc3`). Fall back to empty defaults — matching the fix already applied to interactive `configure` — so a value can be set or listed for a brand-new profile. Adds regression tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A profile present only in the config file — e.g. just after `configure set region --profile X` on a profile that has no credentials yet — previously failed: LoadConfig returned early with an error when the credentials section was missing, so region/output/project_id (which live in the config file) were never read, and set/list saw a nil *Config. LoadConfig now treats the two files independently: a profile "exists" if it has a section in either file (or credentials via env vars). It only returns a "profile does not exist" error when config files are present but the profile is in neither — matching `aws configure`. Adds LoadConfig tests for the config-only, neither-file, creds-only, and no-files cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
configure list now exits non-zero with "profile does not exist" when the profile is absent from existing config files, instead of printing empty values. A fresh machine with no config files still lists unset defaults, matching `aws configure list`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An unrecognized --output value (e.g. the typo 'tabel') previously fell through formatter's default case to JSON and exited 0, so users got a different format than intended with no warning. Add a root PersistentPreRunE that validates the --output flag up front and fails fast with a non-zero exit, a "maybe you meant" suggestion (Levenshtein-nearest valid format), and the list of valid formats — matching `aws`/`gcloud`. Only validates the flag when explicitly set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend up-front global-flag validation to --color (on/off/auto) using the same "maybe you meant" suggestion as --output. Set SilenceErrors/SilenceUsage on the root command and print a single "Error: ..." line in Execute, so failures no longer emit cobra's duplicated error plus a full usage block — matching aws/gcloud. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The flag was registered but never read, and the HTTP transport used a bare http.Transport with no dial timeout — so the connect/TLS phase was unbounded and an unreachable endpoint hung for ~127s (4 attempts x the 30s read timeout) regardless of --cli-connect-timeout. NewGreenodeClient now takes connectTimeout and wires it into the transport's net.Dialer (DialContext) and TLSHandshakeTimeout; both the VKS and vServer client builders read --cli-connect-timeout and pass it through. A connect to an unreachable host with --cli-connect-timeout 1 now fails per-attempt in ~1s (dial i/o timeout) instead of 30s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
config-auto-healing's --unhealthy-range help said only "Unhealthy range" with
no hint at the expected syntax. State the format ("[min-max]", e.g. "[2-5]")
matching the API spec, like --max-unhealthy already shows "30%".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vks-team
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A batch of correctness and AWS-parity fixes found while exercising
grn. Started as aconfigurenil-panic fix and grew to cover global-flag validation, error output, and the connect-timeout flag.Changes
configure — profile handling
configure set/configure listfor a profile not yet in the credentials file (e.g.grn configure set region HCM-3 --profile prod-hcm-qc3). LoadConfig returned(nil, err)and the nil*Configwas dereferenced.LoadConfignow reads credentials and config files independently: a profile created viaconfigure set(config file only, no credentials yet) loads its region/output/project_id correctly. A profile is considered to exist if present in either file (or via env credentials).get/listreport a missing profile with a clearprofile '...' does not existerror and non-zero exit (likeaws configure), instead of crashing or printing empty values. A fresh machine with no config files still lists unset defaults.Global flags — validation & error output
--output/--colorup front (PersistentPreRunE) with a "maybe you meant" suggestion (Levenshtein-nearest) and a non-zero exit, instead of silently falling back to JSON / default.SilenceErrors/SilenceUsageand print a singleError: ...line, instead of cobra's duplicated error plus a full usage dump.Client
--cli-connect-timeout: the flag was registered but never read, and the HTTP transport had no dial timeout, so an unreachable endpoint hung ~127s regardless. Now wired into the transport'snet.Dialer(DialContext) +TLSHandshakeTimeoutin both the VKS and vServer clients. A connect to an unreachable host with--cli-connect-timeout 1now fails per-attempt in ~1s.Docs
--unhealthy-rangeformat inconfig-auto-healinghelp ("[min-max]", e.g."[2-5]").Testing
--output/--colorvalidation + suggestion, client connect-timeout wiring.go vet ./...clean; fullgo test ./...passes; verified end-to-end via the binary (incl. a real connect-timeout timing check: ~127s → ~11s with 4 retries, ~1s per connect).Notes / follow-ups (not done, awaiting decision)
--cli-connect-timeout 1fails in ~11s total because the client retries 3× with backoff (per-connect is correctly ~1s). Could make connect errors non-retryable or add a total request deadline if a faster hard fail is wanted.--unhealthy-rangeis not validated client-side (server rejects malformed values).🤖 Generated with Claude Code