From 37836aaae2257ae6df275cf238d1e9740429cc3c Mon Sep 17 00:00:00 2001 From: ahjota Date: Sun, 15 Feb 2026 00:24:57 +0000 Subject: [PATCH] refactor: return EnvCredentials by value in VerifyEnvCredentials - Change VerifyEnvCredentials() signature from (*EnvCredentials, error) to (EnvCredentials, error) - Update test assertions to check struct fields directly instead of pointer non-nil - Document GetEnvCredentials and VerifyEnvCredentials in authentication.md This makes the return type consistent with GetEnvCredentials, which already returns by value. Since EnvCredentials is a small struct (two strings), returning by value is more idiomatic. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- docs/development/authentication.md | 9 ++++++++- internal/auth/auth.go | 6 +++--- internal/auth/auth_test.go | 5 ++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/docs/development/authentication.md b/docs/development/authentication.md index e86788c6..b4a7ad6a 100644 --- a/docs/development/authentication.md +++ b/docs/development/authentication.md @@ -113,8 +113,15 @@ You can still manually run `dr auth login` to refresh credentials or change acco ## Internal APIs -The auth package writes configuration through Viper. +The auth package provides the following functions for authentication management: + +### Configuration management - `WriteConfigFileSilent()`—writes the config file and returns an error. - `WriteConfigFile()`—writes the config file, prints a success message, and returns an error. - `SetURLAction()`—prompts for a DataRobot URL, optionally overwrites an existing value, and returns a boolean indicating whether the URL changed. + +### Credential verification + +- `GetEnvCredentials() EnvCredentials`—reads `DATAROBOT_ENDPOINT` (or `DATAROBOT_API_ENDPOINT` fallback) and `DATAROBOT_API_TOKEN` from environment variables. Returns an `EnvCredentials` struct by value containing the endpoint and token (may be empty strings if not set). +- `VerifyEnvCredentials() (EnvCredentials, error)`—checks if environment variable credentials are valid by calling `GetEnvCredentials()` and verifying the token with the DataRobot API. Returns the credentials by value and an error. Returns `ErrEnvCredentialsNotSet` if either endpoint or token is empty. diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 837af647..ff0c9e67 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -62,15 +62,15 @@ func GetEnvCredentials() EnvCredentials { // VerifyEnvCredentials checks if environment variable credentials are valid. // Returns credentials and nil error if valid, credentials and error otherwise. -func VerifyEnvCredentials() (*EnvCredentials, error) { +func VerifyEnvCredentials() (EnvCredentials, error) { creds := GetEnvCredentials() if creds.Endpoint == "" || creds.Token == "" { - return &creds, ErrEnvCredentialsNotSet + return creds, ErrEnvCredentialsNotSet } err := config.VerifyToken(creds.Endpoint, creds.Token) - return &creds, err + return creds, err } // EnsureAuthenticatedE checks if valid authentication exists, and if not, diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 054d8459..69099a95 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -303,7 +303,8 @@ func TestVerifyEnvCredentials(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, ErrEnvCredentialsNotSet) - assert.NotNil(t, creds) + assert.Empty(t, creds.Endpoint) + assert.Empty(t, creds.Token) }) t.Run("returns error when only endpoint set", func(t *testing.T) { @@ -315,6 +316,7 @@ func TestVerifyEnvCredentials(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, ErrEnvCredentialsNotSet) assert.Equal(t, "https://example.com", creds.Endpoint) + assert.Empty(t, creds.Token) }) t.Run("returns error when only token set", func(t *testing.T) { @@ -326,6 +328,7 @@ func TestVerifyEnvCredentials(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, ErrEnvCredentialsNotSet) + assert.Empty(t, creds.Endpoint) assert.Equal(t, "some-token", creds.Token) })