fix(nvcf-cli): report which auth credentials are missing

[mesutoezdil]

TL;DR

LoadConfig collects the unset credential variables into missing, then returns a fixed "set NVCF_API_KEY" message and drops the list. An incomplete OAuth2 setup is told to set an API key. Now the error names the unset variables.

QA

go build ./... and go test ./internal/... clean from src/clis/nvcf-cli; gofmt -l clean on both changed files. The new test fails against the old message.

TestLoadConfigAuthCredentials is a table covering both directions: the failure paths (nothing set, partial OAuth2) assert the error names exactly the unset variables and omits the set ones, and the success paths (NVCF_API_KEY, NVCF_TOKEN, complete OAuth2) assert LoadConfig returns no error and picks the right AuthType. The success cases guard the valid-credential branches next to the error branch this PR changes. Renamed from TestLoadConfigReportsMissingCredentials since it no longer only covers the missing case.

Note: internal/client/accounts.go and internal/client/clusters.go are unformatted on main already and are left untouched here.

Issues

Closes #538

Summary by CodeRabbit

• Bug Fixes
  • Improved authentication configuration errors to clearly list every missing credential field.
  • Removed misleading guidance that previously referenced only a single credential source.
• Tests
  • Added test coverage for authentication credential selection via environment variables, including success and detailed failure cases.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 725a7064-2f70-4974-b7ba-b817dcef8f11

📥 Commits

Reviewing files that changed from the base of the PR and between 5ae1955 and 06f1c6a.

📒 Files selected for processing (2)

• src/clis/nvcf-cli/internal/client/client.go
• src/clis/nvcf-cli/internal/client/client_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• src/clis/nvcf-cli/internal/client/client_test.go
• src/clis/nvcf-cli/internal/client/client.go

---

📝 Walkthrough

Walkthrough

LoadConfig now reports specific unset authentication credentials instead of a fixed API-key message. A table-driven test covers missing, partial, and complete API, token, and OAuth2 credential configurations.

Changes

Credential reporting

Layer / File(s)	Summary
Missing credential error and coverage src/clis/nvcf-cli/internal/client/client.go, src/clis/nvcf-cli/internal/client/client_test.go	LoadConfig includes unset credential names in its error. Tests cover missing fields and configured API, token, and OAuth2 credentials.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: kferris-nvidia

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title uses the required fix(nvcf-cli): format and accurately describes the authentication error correction.
Linked Issues check	✅ Passed	The code reports the specific unset credential variables and adds tests for incomplete and valid authentication configurations required by issue #538.
Out of Scope Changes check	✅ Passed	The implementation and tests remain within the scope of correcting nvcf-cli authentication error reporting for issue #538.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/clis/nvcf-cli/internal/client/client_test.go (1)

1259-1311: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover successful credential configurations as well.

The current cases only expect failures. Add table entries for NVCF_API_KEY, NVCF_TOKEN, and complete OAuth2 credentials, asserting LoadConfig succeeds; otherwise, regressions in the valid-credential branches could pass unnoticed. This aligns with the PR objective to cover both bearer and OAuth2 credential combinations.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/clis/nvcf-cli/internal/client/client_test.go` around lines 1259 - 1311,
The TestLoadConfigReportsMissingCredentials table only covers failure paths; add
successful cases for NVCF_API_KEY, NVCF_TOKEN, and complete OAuth2 credentials.
For each valid configuration, assert LoadConfig returns no error while
preserving the existing missing-credential assertions for incomplete setups.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/clis/nvcf-cli/internal/client/client_test.go`:
- Around line 1259-1311: The TestLoadConfigReportsMissingCredentials table only
covers failure paths; add successful cases for NVCF_API_KEY, NVCF_TOKEN, and
complete OAuth2 credentials. For each valid configuration, assert LoadConfig
returns no error while preserving the existing missing-credential assertions for
incomplete setups.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 73792324-98f3-4401-b9ca-fcb1ff198bfe

📥 Commits

Reviewing files that changed from the base of the PR and between 64d8c48 and 99275f0.

📒 Files selected for processing (2)

• src/clis/nvcf-cli/internal/client/client.go
• src/clis/nvcf-cli/internal/client/client_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/clis/nvcf-cli/internal/client/client_test.go`:
- Around line 1268-1280: Update the partial OAuth2 cases in the
credential-validation test table to include both missing credential
requirements: “NVCF_API_KEY or NVCF_TOKEN” and “NVCF_OAUTH2_TOKEN_ENDPOINT”. Add
separate cases omitting NVCF_OAUTH2_CLIENT_ID and NVCF_OAUTH2_CLIENT_SECRET
individually, covering each production validation branch while preserving the
existing complete-configuration case.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 07e6b5a7-cc2d-45de-8570-523468194739

📥 Commits

Reviewing files that changed from the base of the PR and between 99275f0 and 127185b.

📒 Files selected for processing (2)

• src/clis/nvcf-cli/internal/client/client.go
• src/clis/nvcf-cli/internal/client/client_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• src/clis/nvcf-cli/internal/client/client.go