Skip to content

Fix datumctl version crashing on unparseable client/server versions - #279

Open
smerwin wants to merge 1 commit into
datum-cloud:mainfrom
smerwin:fix/version-skew-parse-error
Open

Fix datumctl version crashing on unparseable client/server versions#279
smerwin wants to merge 1 commit into
datum-cloud:mainfrom
smerwin:fix/version-skew-parse-error

Conversation

@smerwin

@smerwin smerwin commented Sep 6, 2026

Copy link
Copy Markdown

Summary

  • datumctl version delegates to kubectl's version command, which after printing the client/server versions runs a skew check that strictly parses both GitVersion strings as semver.
  • When a server (or a locally built client) reports a version containing an unresolved git archive export-subst placeholder like v0.0.0-master+$Format:%H$, that parse fails and kubectl treats it as fatal — the process exits non-zero even though both versions were already printed.
  • Since the buggy behavior lives in the vendored k8s.io/kubectl/pkg/cmd/version package, this overrides versionCmd's RunE to drive kubectl's own exported version.Options type directly (same pattern already used in WrapGetCommand for kubectl get). The parse failure is detected via the two stable error prefixes kubectl's skew-check uses ("client version error:" / "server version error:") and downgraded to a Warning: on stderr instead of a fatal error. Real connectivity/discovery errors still propagate normally.

Fixes #264

Test plan

  • go build ./...
  • go test ./internal/cmd/... (new TestIsVersionSkewParseError passes)
  • Built the binary and confirmed datumctl version --client (plain, -o json, -o yaml) still works
  • Reproduced the exact issue scenario with a throwaway fake-discovery-client harness: before the fix, the command prints both versions then returns the ... version error: could not parse pre-release/metadata ... error; after the fix, the same input now returns success (exit 0) with a warning on stderr instead of crashing

🤖 Generated with Claude Code

@cla-assistant

cla-assistant Bot commented Sep 6, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Sep 6, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

kubectl's version command treats a failure to parse either the client or
server GitVersion as semver during its skew check as fatal, even though
both versions were already printed. This surfaces as a hard crash when a
server (or a locally built client) reports a version string containing
an unresolved `git archive` export-subst placeholder like
`v0.0.0-master+$Format:%H$`, which isn't valid semver.

Override the version command's RunE to drive kubectl's own exported
Options type directly, and downgrade that specific parse failure to a
warning instead of a fatal error, while still surfacing real
connectivity/discovery errors normally.

Fixes datum-cloud#264

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@smerwin
smerwin force-pushed the fix/version-skew-parse-error branch from 3cdb906 to 579904c Compare September 10, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

datumctl version: fails to parse server version, breaks on unresolved git archive placeholder

1 participant