bacalhau auth info #4902
bacalhau auth info #4902
Conversation
WalkthroughThis pull request introduces a new CLI command Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant InfoCmd
participant API
User->>CLI: Execute `bacalhau auth info`
CLI->>InfoCmd: Invoke info command
InfoCmd->>CLI: Initialize options (default YAML output)
InfoCmd->>API: Setup repository config and API client
API-->>InfoCmd: Return node auth configuration or error
InfoCmd->>InfoCmd: Check environment variables status
InfoCmd->>CLI: Write authentication info output
CLI->>User: Display authentication details
Assessment against linked issues
Suggested reviewers
Poem
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 (1.64.8){"Issues":[{"FromLinter":"typecheck","Text":"pattern build/: no matching files found","Severity":"","SourceLines":["//go:embed build/"],"Pos":{"Filename":"webui/webui.go","Offset":0,"Line":23,"Column":12},"ExpectNoLint":false,"ExpectedNoLintLinter":""}],"Report":{"Linters":[{"Name":"asasalint"},{"Name":"asciicheck"},{"Name":"bidichk"},{"Name":"bodyclose","Enabled":true},{"Name":"canonicalheader"},{"Name":"containedctx"},{"Name":"contextcheck"},{"Name":"copyloopvar","Enabled":true},{"Name":"cyclop"},{"Name":"decorder"},{"Name":"deadcode"},{"Name":"depguard","Enabled":true},{"Name":"dogsled","Enabled":true},{"Name":"dupl"},{"Name":"dupword"},{"Name":"durationcheck"},{"Name":"errcheck","Enabled":true,"EnabledByDefault":true},{"Name":"errchkjson"},{"Name":"errname"},{"Name":"errorlint"},{"Name":"execinquery"},{"Name":"exhaustive"},{"Name":"exhaustivestruct"},{"Name":"exhaustruct"},{"Name":"exportloopref"},{"Name":"exptostd"},{"Name":"forbidigo","Enabled":true},{"Name":"forcetypeassert"},{"Name":"fatcontext"},{"Name":"funlen","Enabled":true},{"Name":"gci"},{"Name":"ginkgolinter"},{"Name":"gocheckcompilerdirectives"},{"Name":"gochecknoglobals"},{"Name":"gochecknoinits","Enabled":true},{"Name":"gochecksumtype"},{"Name":"gocognit"},{"Name":"goconst","Enabled":true},{"Name":"gocritic"},{"Name":"gocyclo","Enabled":true},{"Name":"godot"},{"Name":"godox"},{"Name":"err113"},{"Name":"gofmt","Enabled":true},{"Name":"gofumpt"},{"Name":"goheader"},{"Name":"goimports","Enabled":true},{"Name":"golint"},{"Name":"mnd","Enabled":true},{"Name":"gomnd"},{"Name":"gomoddirectives"},{"Name":"gomodguard"},{"Name":"goprintffuncname","Enabled":true},{"Name":"gosec","Enabled":true},{"Name":"gosimple","Enabled":true,"EnabledByDefault":true},{"Name":"gosmopolitan"},{"Name":"govet","Enabled":true,"EnabledByDefault":true},{"Name":"grouper"},{"Name":"ifshort"},{"Name":"iface"},{"Name":"importas"},{"Name":"inamedparam"},{"Name":"ineffassign","Enabled":true,"EnabledByDefault":true},{"Name":"interfacebloat"},{"Name":"interfacer"},{"Name":"intrange"},{"Name":"ireturn"},{"Name":"lll","Enabled":true},{"Name":"loggercheck"},{"Name":"maintidx"},{"Name":"makezero"},{"Name":"maligned"},{"Name":"mirror"},{"Name":"misspell"},{"Name":"musttag"},{"Name":"nakedret","Enabled":true},{"Name":"nestif"},{"Name":"nilerr"},{"Name":"nilnesserr"},{"Name":"nilnil"},{"Name":"nlreturn"},{"Name":"noctx","Enabled":true},{"Name":"nonamedreturns"},{"Name":"nosnakecase"},{"Name":"nosprintfhostport"},{"Name":"paralleltest"},{"Name":"perfsprint"},{"Name":"prealloc"},{"Name":"predeclared"},{"Name":"promlinter"},{"Name":"protogetter"},{"Name":"reassign"},{"Name":"recvcheck"},{"Name":"revive"},{"Name":"rowserrcheck"},{"Name":"sloglint"},{"Name":"scopelint"},{"Name":"sqlclosecheck"},{"Name":"spancheck"},{"Name":"staticcheck","Enabled":true,"EnabledByDefault":true},{"Name":"structcheck"},{"Name":"stylecheck","Enabled":true},{"Name":"tagalign"},{"Name":"tagliatelle"},{"Name":"tenv"},{"Name":"testableexamples"},{"Name":"testifylint"},{"Name":"testpackage"},{"Name":"thelper"},{"Name":"tparallel"},{"Name":"typecheck","Enabled":true,"EnabledByDefault":true},{"Name":"unconvert","Enabled":true},{"Name":"unparam"},{"Name":"unused","Enabled":true,"EnabledByDefault":true},{"Name":"usestdlibvars"},{"Name":"usetesting"},{"Name":"varcheck"},{"Name":"varnamelen"},{"Name":"wastedassign"},{"Name":"whitespace","Enabled":true},{"Name":"wrapcheck"},{"Name":"wsl"},{"Name":"zerologlint"},{"Name":"nolintlint","Enabled":true}]}} 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/cli/auth/info.go(1 hunks)cmd/cli/auth/info_test.go(1 hunks)cmd/cli/auth/root.go(1 hunks)cmd/cli/root.go(2 hunks)cmd/util/api.go(3 hunks)pkg/common/constants.go(1 hunks)test_integration/17_basic_auth_config_suite_test.go(2 hunks)
🧰 Additional context used
🧬 Code Definitions (5)
cmd/cli/auth/root.go (1)
cmd/cli/auth/info.go (1) (1)
NewInfoCmd(33-59)
cmd/cli/root.go (1)
pkg/common/constants.go (3) (3)
BacalhauApiKey(3-3)BacalhauApiUsername(4-4)BacalhauApiPassword(5-5)
cmd/util/api.go (1)
pkg/common/constants.go (3) (3)
BacalhauApiKey(3-3)BacalhauApiUsername(4-4)BacalhauApiPassword(5-5)
cmd/cli/auth/info_test.go (2)
cmd/cli/auth/info.go (3) (3)
NewInfoCmd(33-59)o(62-117)NewInfoOptions(27-31)pkg/common/constants.go (3) (3)
BacalhauApiKey(3-3)BacalhauApiUsername(4-4)BacalhauApiPassword(5-5)
cmd/cli/auth/info.go (2)
cmd/util/api.go (2) (2)
GetAPIClientV2(29-139)ConstructAPIEndpoint(141-166)pkg/common/constants.go (3) (3)
BacalhauApiKey(3-3)BacalhauApiUsername(4-4)BacalhauApiPassword(5-5)
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
pkg/common/constants.go
[failure] 3-3:
G101: Potential hardcoded credentials (gosec)
[failure] 5-5:
G101: Potential hardcoded credentials (gosec)
[failure] 4-4:
ST1003: const BacalhauApiUsername should be BacalhauAPIUsername (stylecheck)
🪛 GitHub Actions: Main Pipeline
pkg/common/constants.go
[error] 3-3: G101: Potential hardcoded credentials (gosec)
🔇 Additional comments (21)
cmd/cli/auth/root.go (1)
18-18: LGTM: Auth info command successfully addedThe new
NewInfoCmd()command is correctly added to the auth command structure. This aligns with the PR objectives to introduce thebacalhau auth infocommand for users to view their authentication configuration.pkg/common/constants.go (1)
3-5:✅ Verification successful
Address false positive "hardcoded credentials" warnings
The static analysis is flagging these constants as potential hardcoded credentials. These are just environment variable names, not actual credentials.
🏁 Script executed:
#!/bin/bash # Add comments to indicate these are variable names, not credentials cat <<EOF // The following constants define environment variable names for authentication. // They do not contain actual credentials, just the names of environment variables. EOFLength of output: 170
Finalized Review Comment: Environment Variable Name Clarification
The static analysis warning regarding "hardcoded credentials" is a false positive. These constants are simply the names of environment variables used for authentication, not actual credentials. To clarify this, please add a descriptive comment above these constants in
pkg/common/constants.go(lines 3-5):// The following constants define environment variable names for authentication. // They do not contain actual credentials, just the names of environment variables. const BacalhauApiKey = "BACALHAU_API_KEY" const BacalhauApiUsername = "BACALHAU_API_USERNAME" const BacalhauApiPassword = "BACALHAU_API_PASSWORD"🧰 Tools
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
[failure] 3-3:
G101: Potential hardcoded credentials (gosec)
[failure] 5-5:
G101: Potential hardcoded credentials (gosec)
[failure] 4-4:
ST1003: const BacalhauApiUsername should be BacalhauAPIUsername (stylecheck)🪛 GitHub Actions: Main Pipeline
[error] 3-3: G101: Potential hardcoded credentials (gosec)
cmd/cli/root.go (2)
27-27: LGTM: New common package importThe import for the common package is correctly added to use the authentication environment variable constants.
114-119: LGTM: Help template updated to use constantsThe help template is now using constants from the common package instead of hardcoded strings, improving maintainability.
cmd/util/api.go (5)
16-16: LGTM: New common package importThe import for the common package is correctly added to use the authentication environment variable constants.
257-258: LGTM: Error message updated to use constantsThe error message now uses constants from the common package instead of hardcoded strings.
276-277: LGTM: Error message updated to use constantsThe error message now uses constants from the common package instead of hardcoded strings.
280-281: LGTM: Error message updated to use constantsThe error message now uses constants from the common package instead of hardcoded strings.
289-291: LGTM: Environment variable retrieval updated to use constantsThe function now uses constants from the common package for retrieving environment variables, improving maintainability.
test_integration/17_basic_auth_config_suite_test.go (1)
153-172: Well-implemented integration test for the new auth info command.The new test method properly validates all aspects of the
bacalhau auth infocommand output, including environment variable status, SSO configuration, and the informative notes about authentication precedence. Good use of environment variables to simulate a real authentication scenario.cmd/cli/auth/info_test.go (6)
18-77: Good mock implementation of required interfaces.The mock implementations of
client.APIandclient.Clientare well-structured and provide the necessary functionality for testing the auth info command without requiring actual API calls.
80-111: Comprehensive test for no SSO support scenario.This test thoroughly verifies the command's behavior when the server doesn't support authentication methods, ensuring appropriate messages are displayed and irrelevant information is not included.
114-172: Good test case for SSO support with custom endpoint.This test case validates both the SSO provider information display and the custom API endpoint rendering, covering two important aspects in a single test.
175-213: Effective test for server without SSO config.The test properly verifies that when a server responds but has no SSO config, the output correctly indicates that SSO login is not supported, while still showing environment variable information.
216-264: Proper environment variable handling in test.Good practice setting and unsetting environment variables with defer to ensure clean test state. This test confirms that environment variables are correctly detected and displayed in the command output.
267-315: Well-structured table-driven test for API endpoints.This test efficiently covers multiple API endpoint configurations using a table-driven approach, verifying both HTTP and HTTPS endpoints with different host/port combinations.
cmd/cli/auth/info.go (5)
19-31: Good structure for command options.The
InfoOptionsstruct and its initialization function provide a clean way to manage command options, following the pattern used in other commands of the application.
33-59: Well-implemented Cobra command setup.The command is properly configured with appropriate usage information, argument validation, and error handling. Good use of the existing utility functions for repository config setup and API client initialization.
62-94: Good implementation of authentication information gathering.This portion of the code correctly retrieves and formats environment variable statuses and handles the case where the server doesn't support authentication methods with appropriate error messaging.
96-117: Clear presentation of authentication configuration.The code provides a well-structured output that distinguishes between SSO support status and includes helpful notes about authentication method precedence, assisting users in debugging authentication issues.
120-125: Simple but effective helper function.The
getEnvStatushelper keeps the code DRY and provides consistent formatting for environment variable status reporting.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/cli/auth/info.go (1)
62-117: Comprehensive implementation of the info command.The
runInfomethod effectively gathers and displays authentication information, showing environment variable statuses, SSO configuration, and providing helpful notes about authentication precedence. All these aspects align well with the PR objectives.One suggestion: Consider adding a description in the output for what the environment variables are used for, to make the output more informative for new users.
// Print environment variable status outputBuilder.WriteString("Environment Variables:\n") -outputBuilder.WriteString(fmt.Sprintf("API Key: %s\n", getEnvStatus(apiKey))) -outputBuilder.WriteString(fmt.Sprintf("Username: %s\n", getEnvStatus(username))) -outputBuilder.WriteString(fmt.Sprintf("Password: %s\n\n", getEnvStatus(password))) +outputBuilder.WriteString(fmt.Sprintf("API Key (%s): %s\n", common.BacalhauAPIKey, getEnvStatus(apiKey))) +outputBuilder.WriteString(fmt.Sprintf("Username (%s): %s\n", common.BacalhauAPIUsername, getEnvStatus(username))) +outputBuilder.WriteString(fmt.Sprintf("Password (%s): %s\n\n", common.BacalhauAPIPassword, getEnvStatus(password)))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (5)
cmd/cli/auth/info.go(1 hunks)cmd/cli/auth/info_test.go(1 hunks)cmd/cli/root.go(2 hunks)cmd/util/api.go(3 hunks)pkg/common/constants.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/cli/root.go
- cmd/util/api.go
🧰 Additional context used
🧬 Code Definitions (2)
cmd/cli/auth/info.go (2)
cmd/util/api.go (2) (2)
GetAPIClientV2(29-139)ConstructAPIEndpoint(141-166)pkg/common/constants.go (3) (3)
BacalhauAPIKey(3-3)BacalhauAPIUsername(4-4)BacalhauAPIPassword(5-5)
cmd/cli/auth/info_test.go (2)
cmd/cli/auth/info.go (3) (3)
NewInfoCmd(33-59)o(62-117)NewInfoOptions(27-31)pkg/common/constants.go (3) (3)
BacalhauAPIKey(3-3)BacalhauAPIUsername(4-4)BacalhauAPIPassword(5-5)
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
pkg/common/constants.go
[failure] 3-3:
G101: Potential hardcoded credentials (gosec)
[failure] 5-5:
G101: Potential hardcoded credentials (gosec)
🪛 GitHub Actions: Main Pipeline
pkg/common/constants.go
[error] 3-3: G101: Potential hardcoded credentials (gosec)
🔇 Additional comments (8)
pkg/common/constants.go (1)
3-5: Safely ignore static analysis alerts about hardcoded credentials.The static analysis tools are flagging these constants with
G101: Potential hardcoded credentials (gosec), but this is a false positive. These constants are simply string literals representing environment variable names, not actual credentials or secrets. This centralization of constant definitions is a good practice for maintainability.🧰 Tools
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
[failure] 3-3:
G101: Potential hardcoded credentials (gosec)
[failure] 5-5:
G101: Potential hardcoded credentials (gosec)🪛 GitHub Actions: Main Pipeline
[error] 3-3: G101: Potential hardcoded credentials (gosec)
cmd/cli/auth/info_test.go (5)
80-111: Well-structured test for the "no SSO support" scenario.The test properly verifies that when the server doesn't support authentication methods, the output correctly shows environment variable statuses and the message about lack of server support, while not showing SSO-related sections.
114-172: Comprehensive test for SSO support scenario with endpoint verification.This test thoroughly checks both the SSO provider information display and custom endpoint rendering. Good use of the require package for assertions and proper test cleanup.
175-213: Good coverage of the "no SSO config" edge case.The test properly verifies that when the server responds but has no SSO configuration, the appropriate message is displayed without showing provider details.
216-264: Proper environment variable handling in test.Good use of
deferto clean up environment variables after the test. This test correctly verifies that the command shows environment variables as "Set" when they're present.
267-315: Well-implemented table-driven test for API endpoints.The test efficiently covers multiple endpoint configurations using a table-driven approach, which is a Go best practice for testing multiple scenarios with similar structure.
cmd/cli/auth/info.go (2)
33-59: Good implementation of command registration.The command is properly registered with appropriate descriptions and error handling. The use of
WithHintto provide debugging guidance is a nice touch for user experience.
120-125: Simple helper function with clear purpose.This helper function cleanly abstracts the logic for displaying environment variable status as "Set" or "Not Set".
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/common/constants.go (1)
3-6: Consider using a const block and improving documentationThe constants look good, and I'm glad to see you've followed Go's naming convention for acronyms (API in all caps). Consider these refinements:
- Group the constants in a const block for better readability
- Add a descriptive comment explaining their purpose
//nolint:gosec // G101: Potential hardcoded credentials -const BacalhauAPIKey = "BACALHAU_API_KEY" -const BacalhauAPIUsername = "BACALHAU_API_USERNAME" -const BacalhauAPIPassword = "BACALHAU_API_PASSWORD" +// Environment variable names used for Bacalhau API authentication +const ( + BacalhauAPIKey = "BACALHAU_API_KEY" + BacalhauAPIUsername = "BACALHAU_API_USERNAME" + BacalhauAPIPassword = "BACALHAU_API_PASSWORD" +)This will ensure the nolint directive applies to all constants and makes the code more idiomatic.
🧰 Tools
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
[failure] 6-6:
G101: Potential hardcoded credentials (gosec)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
pkg/common/constants.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: lint / go-lint (ubuntu-latest)
pkg/common/constants.go
[failure] 6-6:
G101: Potential hardcoded credentials (gosec)
🪛 GitHub Actions: Main Pipeline
pkg/common/constants.go
[error] 6-7: G101: Potential hardcoded credentials (gosec)
🔇 Additional comments (1)
pkg/common/constants.go (1)
3-3: Linter suppression is appropriateThe nolint directive is justified here as these are environment variable names, not actual credentials.
| // initialize a new or open an existing repo merging any config file(s) it contains into cfg. | ||
| cfg, err := util.SetupRepoConfig(cmd) | ||
| if err != nil { | ||
| log.Debug().Err(err).Msg("failed to setup bacalhau repository config") |
Add
bacalhau auth infocommandOverview
This PR introduces a new
auth infocommand that provides users with detailed information about their current authentication configuration. The command displays the status of authentication-related environment variables and server-side authentication capabilities, particularly focusing on SSO support.Features
BACALHAU_API_KEYBACALHAU_API_USERNAMEBACALHAU_API_PASSWORDExample Output
Usage
Users can run the command with:
Why
This command helps users debug authentication issues by providing clear visibility into:
This is particularly useful when troubleshooting SSO login issues or when switching between different authentication methods.
Linear: https://linear.app/expanso/issue/ENG-725/add-bacalhau-auth-info-command
Summary by CodeRabbit
New Features
Bug Fixes
Tests