From 3166e1762273bd96ce4d44063b8e0aa06abfed51 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Tue, 1 Sep 2026 18:08:28 -0600 Subject: [PATCH 1/3] Added support for multiple site configurations --- cmd/auth/check/cmd.go | 2 +- cmd/auth/cmd.go | 9 +- cmd/auth/login/cmd.go | 4 + cmd/auth/profile/cmd.go | 51 ++++++ cmd/auth/profile/list/cmd.go | 128 ++++++++++++++ cmd/auth/profile/list/cmd_test.go | 173 ++++++++++++++++++ cmd/auth/profile/show/cmd.go | 217 +++++++++++++++++++++++ cmd/auth/profile/show/cmd_test.go | 171 ++++++++++++++++++ cmd/auth/seturl/cmd.go | 4 + cmd/root_factory.go | 29 +++- cmd/root_factory_test.go | 128 ++++++++++++++ docs/commands/auth.md | 110 ++++++++++-- docs/development/authentication.md | 2 +- docs/development/configuration.md | 71 ++++++-- docs/development/flags.md | 2 + docs/user-guide/configuration.md | 70 +++++++- docs/user-guide/quick-reference.md | 7 + internal/auth/auth.go | 39 +++-- internal/auth/auth_test.go | 18 +- internal/auth/writeConfig_test.go | 115 ++++++++++++ internal/config/config.go | 159 +++++++++++++---- internal/config/config_test.go | 151 ++++++++++++++++ internal/config/constants.go | 11 ++ internal/config/profile.go | 270 +++++++++++++++++++++++++++++ internal/config/profile_test.go | 194 +++++++++++++++++++++ internal/config/write.go | 71 ++++++-- internal/config/write_test.go | 185 ++++++++++++++++++++ internal/plugin/exec_test.go | 32 ++++ 28 files changed, 2330 insertions(+), 93 deletions(-) create mode 100644 cmd/auth/profile/cmd.go create mode 100644 cmd/auth/profile/list/cmd.go create mode 100644 cmd/auth/profile/list/cmd_test.go create mode 100644 cmd/auth/profile/show/cmd.go create mode 100644 cmd/auth/profile/show/cmd_test.go create mode 100644 internal/config/profile.go create mode 100644 internal/config/profile_test.go create mode 100644 internal/config/write_test.go diff --git a/cmd/auth/check/cmd.go b/cmd/auth/check/cmd.go index 22749201b..04f57ef21 100644 --- a/cmd/auth/check/cmd.go +++ b/cmd/auth/check/cmd.go @@ -45,7 +45,7 @@ func checkCLICredentials(w io.Writer) bool { // If env vars were set but invalid, report the error. Unlike // EnsureAuthenticated this deliberately skips the "not falling back to - // the stored profile" line: check evaluates the stored profile itself + // the stored credentials" line: check evaluates the stored credentials itself // right below and reports that result on its own. if !errors.Is(err, auth.ErrEnvCredentialsNotSet) { auth.ReportEnvCredentialsError(w, creds, err) diff --git a/cmd/auth/cmd.go b/cmd/auth/cmd.go index 29f1acd87..ccd59d333 100644 --- a/cmd/auth/cmd.go +++ b/cmd/auth/cmd.go @@ -19,6 +19,7 @@ import ( "github.com/datarobot/cli/cmd/auth/export" "github.com/datarobot/cli/cmd/auth/login" "github.com/datarobot/cli/cmd/auth/logout" + "github.com/datarobot/cli/cmd/auth/profile" "github.com/datarobot/cli/cmd/auth/seturl" "github.com/datarobot/cli/internal/version" "github.com/spf13/cobra" @@ -36,8 +37,13 @@ Manage your DataRobot credentials and connection settings: • Log in using OAuth authentication • Log out and clear stored credentials • Export your credentials as environment variables + • Inspect named profiles for working with multiple DataRobot installations -🚀 Quick start: dr auth set-url && dr auth login`, +🚀 Quick start: dr auth set-url && dr auth login + +💡 Multiple installations? Use --profile (or DATAROBOT_CLI_PROFILE) on +any command, e.g. dr --profile eu auth set-url && dr --profile eu auth login. +See 'dr auth profile list'.`, } cmd.AddCommand( @@ -45,6 +51,7 @@ Manage your DataRobot credentials and connection settings: export.Cmd(), login.Cmd(), logout.Cmd(), + profile.Cmd(), seturl.Cmd(), ) diff --git a/cmd/auth/login/cmd.go b/cmd/auth/login/cmd.go index 156cde747..39a73de86 100644 --- a/cmd/auth/login/cmd.go +++ b/cmd/auth/login/cmd.go @@ -123,6 +123,10 @@ If the browser cannot be opened, the CLI prints a link to open yourself. Pass SilenceErrors: true, SilenceUsage: true, RunE: RunE, + Annotations: map[string]string{ + // login is expected to create a not-yet-existing --profile. + config.ProfileCreateAnnotationKey: "true", + }, } // Read directly from cobra rather than binding to viper: this is a transient diff --git a/cmd/auth/profile/cmd.go b/cmd/auth/profile/cmd.go new file mode 100644 index 000000000..284e2157e --- /dev/null +++ b/cmd/auth/profile/cmd.go @@ -0,0 +1,51 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package profile provides read-only inspection of the named profiles +// stored in drconfig.yaml. Profiles are created by pointing --profile (or +// DATAROBOT_CLI_PROFILE) at a name that doesn't exist yet and running +// `dr auth login` or `dr auth set-url`; there is no `create` here. +package profile + +import ( + "github.com/datarobot/cli/cmd/auth/profile/list" + "github.com/datarobot/cli/cmd/auth/profile/show" + "github.com/spf13/cobra" +) + +func Cmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "profile", + Aliases: []string{"profiles"}, + Short: "📇 Inspect named DataRobot profiles", + Long: `Inspect the named profiles stored in drconfig.yaml. + +A profile is a named set of credentials (endpoint + token, optionally +ca-cert and ssl_verify) alongside the default one, so you can work against +several DataRobot installations without re-authenticating each time. +Select one with --profile or DATAROBOT_CLI_PROFILE= on any +command. + +There is no 'create' or 'delete' subcommand: a profile is created the first +time you run 'dr --profile auth login' (or 'auth set-url') for a name +that doesn't exist yet, and removed by editing drconfig.yaml directly.`, + } + + cmd.AddCommand( + list.Cmd(), + show.Cmd(), + ) + + return cmd +} diff --git a/cmd/auth/profile/list/cmd.go b/cmd/auth/profile/list/cmd.go new file mode 100644 index 000000000..48ab9dcbc --- /dev/null +++ b/cmd/auth/profile/list/cmd.go @@ -0,0 +1,128 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package list + +import ( + "fmt" + "os" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" + "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/outputformat" + "github.com/datarobot/cli/tui" + "github.com/spf13/cobra" +) + +// defaultProfileLabel is how the top-level (unnamed) profile is displayed +// and how it is addressed in JSON output and by `dr auth profile show`. +const defaultProfileLabel = config.DefaultProfileLabel + +// profileOutput is the JSON representation of one profile for +// --output-format json. +type profileOutput struct { + Name string `json:"name"` + Endpoint string `json:"endpoint"` + HasToken bool `json:"has_token"` + Active bool `json:"active"` +} + +func Cmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "list", + Short: "📋 List named profiles from drconfig.yaml", + Long: "List the default profile and every named profile stored in drconfig.yaml, marking the active one.", + RunE: runList, + } + + return cmd +} + +func runList(cmd *cobra.Command, _ []string) error { + def, profiles, err := config.LoadProfiles() + if err != nil { + return fmt.Errorf("failed to read profiles: %w", err) + } + + active := config.ActiveProfile() + + outputs := make([]profileOutput, 0, len(profiles)+1) + outputs = append(outputs, toProfileOutput(defaultProfileLabel, def, active == "")) + + for _, p := range profiles { + outputs = append(outputs, toProfileOutput(p.Name, p, p.Name == active)) + } + + format := outputformat.GetFormat(cmd) + if format == outputformat.OutputFormatJSON { + return outputformat.PrintJSONEnvelope(os.Stdout, "profiles", outputs) + } + + printProfilesTable(outputs) + + return nil +} + +func toProfileOutput(name string, info config.ProfileInfo, active bool) profileOutput { + return profileOutput{ + Name: name, + Endpoint: info.Endpoint, + HasToken: info.HasToken, + Active: active, + } +} + +func printProfilesTable(outputs []profileOutput) { + fmt.Println(tui.SubTitleStyle.Render("DataRobot Profiles")) + + nameStyle := tui.BaseTextStyle. + Foreground(tui.GetAdaptiveColor(tui.DrPurple, tui.DrPurpleDark)). + Padding(0, 1) + + dimStyle := tui.DimStyle.Padding(0, 1) + + t := table.New(). + Border(lipgloss.RoundedBorder()). + BorderStyle(tui.TableBorderStyle). + StyleFunc(func(_, col int) lipgloss.Style { + if col == 0 { + return nameStyle + } + + return dimStyle + }). + Headers("NAME", "ENDPOINT", "TOKEN", "ACTIVE") + + for _, o := range outputs { + endpoint := o.Endpoint + if endpoint == "" { + endpoint = "-" + } + + token := "not set" + if o.HasToken { + token = "set" + } + + active := "" + if o.Active { + active = "✓" + } + + t.Row(o.Name, endpoint, token, active) + } + + _, _ = fmt.Fprintln(os.Stdout, t.Render()) +} diff --git a/cmd/auth/profile/list/cmd_test.go b/cmd/auth/profile/list/cmd_test.go new file mode 100644 index 000000000..9f8264590 --- /dev/null +++ b/cmd/auth/profile/list/cmd_test.go @@ -0,0 +1,173 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package list + +import ( + "encoding/json" + "io" + "os" + "path/filepath" + "testing" + + "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/config/viperx" + "github.com/datarobot/cli/internal/outputformat" + "github.com/datarobot/cli/internal/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeTestConfig(t *testing.T) { + t.Helper() + + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viperx.Reset() + t.Cleanup(viperx.Reset) + + configDir := filepath.Join(tempDir, ".config", "datarobot") + require.NoError(t, os.MkdirAll(configDir, 0o755)) + + raw := `endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token + no-token: + endpoint: https://notoken.example.com/api/v2 +` + require.NoError(t, os.WriteFile(filepath.Join(configDir, "drconfig.yaml"), []byte(raw), 0o600)) + require.NoError(t, config.ReadConfigFile("")) +} + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + orig := os.Stdout + + t.Cleanup(func() { os.Stdout = orig }) + + r, w, err := os.Pipe() + require.NoError(t, err) + + os.Stdout = w + + fn() + + os.Stdout = orig + + require.NoError(t, w.Close()) + + out, err := io.ReadAll(r) + require.NoError(t, err) + + return string(out) +} + +func TestList_JSON(t *testing.T) { + writeTestConfig(t) + + root := &cobra.Command{Use: "test"} + + var format outputformat.OutputFormat + + outputformat.AddPersistentFlag(root, &format) + root.AddCommand(Cmd()) + root.SetArgs([]string{"list", "--output-format", "json"}) + + out := captureStdout(t, func() { + require.NoError(t, root.Execute()) + }) + + var envelope struct { + Profiles []profileOutput `json:"profiles"` + } + + require.NoError(t, json.Unmarshal([]byte(out), &envelope)) + require.Len(t, envelope.Profiles, 3) + + byName := map[string]profileOutput{} + for _, p := range envelope.Profiles { + byName[p.Name] = p + } + + def := byName[defaultProfileLabel] + assert.Equal(t, "https://app.datarobot.com/api/v2", def.Endpoint) + assert.True(t, def.HasToken) + assert.True(t, def.Active, "the default profile is active when no --profile is given") + + eu := byName["eu-mtsaas"] + assert.Equal(t, "https://app.eu.datarobot.com/api/v2", eu.Endpoint) + assert.True(t, eu.HasToken) + assert.False(t, eu.Active) + + noToken := byName["no-token"] + assert.False(t, noToken.HasToken) + + assert.NotContains(t, out, "default-token", "the token value itself must never be printed") + assert.NotContains(t, out, "eu-token") +} + +func TestList_Text(t *testing.T) { + writeTestConfig(t) + + root := &cobra.Command{Use: "test"} + + var format outputformat.OutputFormat + + outputformat.AddPersistentFlag(root, &format) + root.AddCommand(Cmd()) + root.SetArgs([]string{"list"}) + + out := captureStdout(t, func() { + require.NoError(t, root.Execute()) + }) + + assert.Contains(t, out, defaultProfileLabel) + assert.Contains(t, out, "eu-mtsaas") + assert.Contains(t, out, "no-token") + assert.NotContains(t, out, "default-token") + assert.NotContains(t, out, "eu-token") +} + +func TestList_ActiveProfileMarked(t *testing.T) { + writeTestConfig(t) + + viperx.Set(config.ProfileKey, "eu-mtsaas") + + root := &cobra.Command{Use: "test"} + + var format outputformat.OutputFormat + + outputformat.AddPersistentFlag(root, &format) + root.AddCommand(Cmd()) + root.SetArgs([]string{"list", "--output-format", "json"}) + + out := captureStdout(t, func() { + require.NoError(t, root.Execute()) + }) + + var envelope struct { + Profiles []profileOutput `json:"profiles"` + } + + require.NoError(t, json.Unmarshal([]byte(out), &envelope)) + + for _, p := range envelope.Profiles { + assert.Equal(t, p.Name == "eu-mtsaas", p.Active, "profile %q active flag", p.Name) + } +} diff --git a/cmd/auth/profile/show/cmd.go b/cmd/auth/profile/show/cmd.go new file mode 100644 index 000000000..626eab470 --- /dev/null +++ b/cmd/auth/profile/show/cmd.go @@ -0,0 +1,217 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package show + +import ( + "fmt" + "os" + + "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/outputformat" + "github.com/datarobot/cli/tui" + "github.com/spf13/cobra" +) + +// defaultProfileLabel is how the top-level (unnamed) profile is addressed +// on the command line and in output. +const defaultProfileLabel = config.DefaultProfileLabel + +// profileDetail is the JSON representation of a single profile's resolved +// settings for --output-format json. +type profileDetail struct { + Name string `json:"name"` + Active bool `json:"active"` + Endpoint string `json:"endpoint"` + EndpointOwn bool `json:"endpoint_own"` + HasToken bool `json:"has_token"` + HasTokenOwn bool `json:"has_token_own"` + CACert string `json:"ca_cert,omitempty"` + CACertOwn bool `json:"ca_cert_own"` + SSLVerify *bool `json:"ssl_verify,omitempty"` + SSLVerifyOwn bool `json:"ssl_verify_own"` +} + +func Cmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "show [name]", + Short: "🔍 Show a profile's resolved settings", + Long: `Show a profile's resolved settings: its own endpoint/token/ca-cert/ssl_verify, +falling back to the default profile's values for anything it doesn't define. + +Defaults to the active profile (selected via --profile or +DATAROBOT_CLI_PROFILE) when no name is given. Pass "default" to see the +top-level profile explicitly. The token itself is never printed.`, + Args: cobra.MaximumNArgs(1), + RunE: runShow, + } + + return cmd +} + +func runShow(cmd *cobra.Command, args []string) error { + activeProfile := config.ActiveProfile() + + // name is empty-string-means-default throughout, matching + // config.ActiveProfile()'s convention; only displayName is user-facing. + name := activeProfile + if len(args) > 0 { + name = config.NormalizeProfileName(args[0]) + } + + if name == defaultProfileLabel { + name = "" + } + + def, profiles, err := config.LoadProfiles() + if err != nil { + return fmt.Errorf("failed to read profiles: %w", err) + } + + own, err := resolveOwnSection(name, def, profiles) + if err != nil { + return err + } + + displayName := name + if displayName == "" { + displayName = defaultProfileLabel + } + + detail := resolveDetail(displayName, own, def, name == activeProfile) + + format := outputformat.GetFormat(cmd) + if format == outputformat.OutputFormatJSON { + return outputformat.PrintJSONEnvelope(os.Stdout, "profile", detail) + } + + printDetail(detail) + + return nil +} + +// resolveOwnSection returns the named profile's own section, or def when +// name is "" (the default profile). +func resolveOwnSection(name string, def config.ProfileInfo, profiles []config.ProfileInfo) (config.ProfileInfo, error) { + if name == "" { + return def, nil + } + + for _, p := range profiles { + if p.Name == name { + return p, nil + } + } + + return config.ProfileInfo{}, &config.UnknownProfileError{Name: name, Known: profileNames(profiles)} +} + +func profileNames(profiles []config.ProfileInfo) []string { + names := make([]string, len(profiles)) + for i, p := range profiles { + names[i] = p.Name + } + + return names +} + +// resolveDetail merges own (the named profile's own settings) with def (the +// default profile's settings) so callers can see both the effective value +// and whether it came from the profile itself or was inherited. The default +// profile is its own fallback, so nothing is ever "inherited" for it. +func resolveDetail(name string, own, def config.ProfileInfo, active bool) profileDetail { + detail := profileDetail{Name: name, Active: active} + isDefault := name == defaultProfileLabel + + detail.EndpointOwn = isDefault || own.Endpoint != "" + detail.Endpoint = own.Endpoint + + if !detail.EndpointOwn { + detail.Endpoint = def.Endpoint + } + + detail.HasTokenOwn = isDefault || own.HasToken + detail.HasToken = own.HasToken + + if !detail.HasTokenOwn { + detail.HasToken = def.HasToken + } + + detail.CACertOwn = isDefault || own.CACert != "" + detail.CACert = own.CACert + + if !detail.CACertOwn { + detail.CACert = def.CACert + } + + detail.SSLVerifyOwn = isDefault || own.SSLVerify != nil + detail.SSLVerify = own.SSLVerify + + if !detail.SSLVerifyOwn { + detail.SSLVerify = def.SSLVerify + } + + return detail +} + +func printDetail(d profileDetail) { + title := d.Name + if d.Active { + title += " (active)" + } + + fmt.Println(tui.SubTitleStyle.Render(title)) + + printField("endpoint", valueOrDash(d.Endpoint), d.EndpointOwn) + printField("token", tokenStatus(d.HasToken), d.HasTokenOwn) + printField("ca-cert", valueOrDash(d.CACert), d.CACertOwn) + printField("ssl_verify", sslVerifyStatus(d.SSLVerify), d.SSLVerifyOwn) +} + +func printField(label, value string, own bool) { + source := "" + if !own { + source = tui.DimStyle.Render(" (inherited from default)") + } + + fmt.Printf(" %s: %s%s\n", tui.InfoStyle.Render(label), value, source) +} + +func valueOrDash(s string) string { + if s == "" { + return "-" + } + + return s +} + +func tokenStatus(hasToken bool) string { + if hasToken { + return "set" + } + + return "not set" +} + +func sslVerifyStatus(v *bool) string { + if v == nil { + return "-" + } + + if *v { + return "true" + } + + return "false" +} diff --git a/cmd/auth/profile/show/cmd_test.go b/cmd/auth/profile/show/cmd_test.go new file mode 100644 index 000000000..3186a62b6 --- /dev/null +++ b/cmd/auth/profile/show/cmd_test.go @@ -0,0 +1,171 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package show + +import ( + "encoding/json" + "io" + "os" + "path/filepath" + "testing" + + "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/config/viperx" + "github.com/datarobot/cli/internal/outputformat" + "github.com/datarobot/cli/internal/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeTestConfig(t *testing.T) { + t.Helper() + + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viperx.Reset() + t.Cleanup(viperx.Reset) + + configDir := filepath.Join(tempDir, ".config", "datarobot") + require.NoError(t, os.MkdirAll(configDir, 0o755)) + + raw := `endpoint: https://app.datarobot.com/api/v2 +token: default-token +ca-cert: /etc/ssl/corp.pem +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token + onprem: + endpoint: https://onprem.example.com/api/v2 + token: onprem-token + ca-cert: /etc/ssl/onprem.pem +` + require.NoError(t, os.WriteFile(filepath.Join(configDir, "drconfig.yaml"), []byte(raw), 0o600)) + require.NoError(t, config.ReadConfigFile("")) +} + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + orig := os.Stdout + + t.Cleanup(func() { os.Stdout = orig }) + + r, w, err := os.Pipe() + require.NoError(t, err) + + os.Stdout = w + + fn() + + os.Stdout = orig + + require.NoError(t, w.Close()) + + out, err := io.ReadAll(r) + require.NoError(t, err) + + return string(out) +} + +func runShowJSON(t *testing.T, args ...string) (profileDetail, string) { + t.Helper() + + root := &cobra.Command{Use: "test"} + + var format outputformat.OutputFormat + + outputformat.AddPersistentFlag(root, &format) + root.AddCommand(Cmd()) + root.SetArgs(append([]string{"show", "--output-format", "json"}, args...)) + + out := captureStdout(t, func() { + require.NoError(t, root.Execute()) + }) + + var envelope struct { + Profile profileDetail `json:"profile"` + } + + require.NoError(t, json.Unmarshal([]byte(out), &envelope)) + + return envelope.Profile, out +} + +func TestShow_NamedProfile_OwnCACertNotInherited(t *testing.T) { + writeTestConfig(t) + + detail, out := runShowJSON(t, "onprem") + + assert.Equal(t, "onprem", detail.Name) + assert.Equal(t, "https://onprem.example.com/api/v2", detail.Endpoint) + assert.True(t, detail.EndpointOwn) + assert.True(t, detail.HasToken) + assert.Equal(t, "/etc/ssl/onprem.pem", detail.CACert) + assert.True(t, detail.CACertOwn) + assert.NotContains(t, out, "onprem-token", "the token value itself must never be printed") +} + +func TestShow_NamedProfile_InheritsCACertFromDefault(t *testing.T) { + writeTestConfig(t) + + detail, _ := runShowJSON(t, "eu-mtsaas") + + assert.Equal(t, "eu-mtsaas", detail.Name) + assert.Equal(t, "/etc/ssl/corp.pem", detail.CACert, "ca-cert should fall back to the default profile's value") + assert.False(t, detail.CACertOwn) +} + +func TestShow_NoArgsDefaultsToActiveProfile(t *testing.T) { + writeTestConfig(t) + + viperx.Set(config.ProfileKey, "eu-mtsaas") + require.NoError(t, config.ReadConfigFile("")) + + detail, _ := runShowJSON(t) + + assert.Equal(t, "eu-mtsaas", detail.Name) + assert.True(t, detail.Active) +} + +func TestShow_ExplicitDefaultLabel(t *testing.T) { + writeTestConfig(t) + + detail, _ := runShowJSON(t, defaultProfileLabel) + + assert.Equal(t, defaultProfileLabel, detail.Name) + assert.Equal(t, "https://app.datarobot.com/api/v2", detail.Endpoint) + assert.True(t, detail.Active) +} + +func TestShow_UnknownProfileErrors(t *testing.T) { + writeTestConfig(t) + + root := &cobra.Command{Use: "test"} + + var format outputformat.OutputFormat + + outputformat.AddPersistentFlag(root, &format) + root.AddCommand(Cmd()) + root.SetArgs([]string{"show", "does-not-exist"}) + root.SilenceErrors = true + root.SilenceUsage = true + + err := root.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "does-not-exist") + assert.Contains(t, err.Error(), "eu-mtsaas") +} diff --git a/cmd/auth/seturl/cmd.go b/cmd/auth/seturl/cmd.go index b43022661..a155a789a 100644 --- a/cmd/auth/seturl/cmd.go +++ b/cmd/auth/seturl/cmd.go @@ -34,6 +34,10 @@ This command helps you choose the correct DataRobot environment: • Custom/On-Premise: Your organization's DataRobot URL 💡 If you're unsure, check the URL you use to log in to DataRobot in your browser.`, + Annotations: map[string]string{ + // set-url is expected to create a not-yet-existing --profile. + config.ProfileCreateAnnotationKey: "true", + }, Run: func(cmd *cobra.Command, args []string) { var url string if len(args) > 0 { diff --git a/cmd/root_factory.go b/cmd/root_factory.go index 98deaa936..55139040b 100644 --- a/cmd/root_factory.go +++ b/cmd/root_factory.go @@ -27,6 +27,7 @@ package cmd import ( "context" + "errors" "fmt" "strings" "sync/atomic" @@ -511,6 +512,7 @@ func (f *RootFactory) registerFlags(adder *cli.CommandAdder, outputFormat *outpu flags.String("config", "", "path to config file (default location: $HOME/.config/datarobot/drconfig.yaml)") + flags.String(config.ProfileKey, "", "named profile from drconfig.yaml to use") flags.BoolP("version", "V", false, "display the version") flags.BoolP("verbose", "v", false, "verbose output") flags.Bool("debug", false, "debug output") @@ -560,6 +562,7 @@ func bindViperFlags(adder *cli.CommandAdder) { bindUniversalOn("verbose") bindUniversalOn("skip-certificate-check") bindUniversalOn("ca-cert") + bindUniversalOn(config.ProfileKey) // Non-universal flags: bound to viper only (not forwarded to plugins). pflags := adder.PersistentFlags() @@ -663,11 +666,33 @@ func defaultConfigInitializer(cmd *cobra.Command) error { resolved = viperx.GetString("config") } - if err := config.ReadConfigFile(resolved); err != nil { + // --profile and DATAROBOT_CLI_PROFILE are already resolved through the + // standard flag/env viper bindings (see bindViperFlags), so + // config.ReadConfigFile picks up the active profile via + // config.ActiveProfile() with no extra wiring here. + err := config.ReadConfigFile(resolved) + if err == nil { + return nil + } + + var unknownProfile *config.UnknownProfileError + if !errors.As(err, &unknownProfile) { return fmt.Errorf("failed to read config file: %w", err) } - return nil + // Commands that create a profile (auth login, auth set-url) are allowed + // to name one that doesn't exist yet. Clear any inherited credentials so + // the new profile doesn't silently authenticate against the default + // profile's instance; the profile's section is created on first write. + if cmd.Annotations[config.ProfileCreateAnnotationKey] != "" { + log.Debugf("profile %q does not exist yet; %s is expected to create it", unknownProfile.Name, cmd.CommandPath()) + viperx.Set(config.DataRobotURL, "") + viperx.Set(config.DataRobotAPIKey, "") + + return nil + } + + return unknownProfile } // --------------------------------------------------------------------------- diff --git a/cmd/root_factory_test.go b/cmd/root_factory_test.go index 50578c1dd..940982f09 100644 --- a/cmd/root_factory_test.go +++ b/cmd/root_factory_test.go @@ -15,12 +15,17 @@ package cmd import ( + "os" + "path/filepath" "sync" "testing" "github.com/amplitude/analytics-go/amplitude" "github.com/datarobot/cli/internal/cli" + "github.com/datarobot/cli/internal/config" + "github.com/datarobot/cli/internal/config/viperx" "github.com/datarobot/cli/internal/telemetry" + "github.com/datarobot/cli/internal/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -127,3 +132,126 @@ func TestPersistentPreRunStampsInteractionMode(t *testing.T) { assert.True(t, props.NonInteractive, "persistentPreRun must stamp non_interactive for --yes invocations") } + +// buildProfileAwareTree builds a tree with the real ConfigInitializer and +// ViperBinder (so --profile/--config are actually bound to viper and +// drconfig.yaml is actually read), while keeping every other dependency +// isolated the way NewIsolatedRootFactory does. +func buildProfileAwareTree() *cli.CommandAdder { + return NewRootFactory( + WithConfigInitializer(defaultConfigInitializer), + WithViperBinder(bindViperFlags), + WithTLSSetup(func(_ *cobra.Command) error { return nil }), + WithTelemetryProps(func() *telemetry.CommonProperties { return nil }), + WithTelemetryClient(func(_ *telemetry.CommonProperties) *telemetry.Client { + return telemetry.NewTestClient(nil, nil) + }), + WithAnimation(func() {}), + WithPluginRegistrar(func(_ *cobra.Command) {}), + ).Build() +} + +// writeProfileConfig writes a drconfig.yaml under a fresh temp home dir with +// a default profile plus one named "eu-mtsaas", and points HOME/XDG at it. +func writeProfileConfig(t *testing.T) { + t.Helper() + + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + + configDir := filepath.Join(tempDir, ".config", "datarobot") + require.NoError(t, os.MkdirAll(configDir, 0o755)) + + raw := `endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token +` + require.NoError(t, os.WriteFile(filepath.Join(configDir, "drconfig.yaml"), []byte(raw), 0o600)) +} + +func addNoopStub(root *cli.CommandAdder) { + stub := &cobra.Command{ + Use: "stub", + RunE: func(_ *cobra.Command, _ []string) error { return nil }, + } + + root.AddCommand(stub) +} + +func TestProfileFlag_RegisteredAndUniversal(t *testing.T) { + viperx.Reset() + t.Cleanup(viperx.Reset) + + root := buildProfileAwareTree() + + flag := root.PersistentFlags().Lookup(config.ProfileKey) + require.NotNil(t, flag, "--profile must be registered as a persistent flag") + + suffixes, ok := flag.Annotations[config.UniversalAnnotationKey] + require.True(t, ok, "--profile must carry the universal annotation so it forwards to plugins") + assert.Equal(t, []string{"PROFILE"}, suffixes) +} + +func TestProfileFlag_FlagBeatsEnv(t *testing.T) { + writeProfileConfig(t) + viperx.Reset() + t.Cleanup(viperx.Reset) + + t.Setenv("DATAROBOT_CLI_PROFILE", "does-not-exist") + + root := buildProfileAwareTree() + addNoopStub(root) + + root.SetArgs([]string{"--profile", "eu-mtsaas", "stub"}) + require.NoError(t, root.Execute(), "the explicit --profile flag must win over DATAROBOT_CLI_PROFILE") + + assert.Equal(t, "https://app.eu.datarobot.com/api/v2", viperx.GetString(config.DataRobotURL)) +} + +func TestProfileFlag_UnknownProfileFailsWithCandidateList(t *testing.T) { + writeProfileConfig(t) + viperx.Reset() + t.Cleanup(viperx.Reset) + + root := buildProfileAwareTree() + addNoopStub(root) + + root.SetArgs([]string{"--profile", "nope", "stub"}) + err := root.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), `"nope"`) + assert.Contains(t, err.Error(), "eu-mtsaas") +} + +func TestProfileFlag_CreateAnnotationSurvivesUnknownProfile(t *testing.T) { + writeProfileConfig(t) + viperx.Reset() + t.Cleanup(viperx.Reset) + + root := buildProfileAwareTree() + + var sawEndpoint, sawToken string + + creator := &cobra.Command{ + Use: "creator", + Annotations: map[string]string{ + config.ProfileCreateAnnotationKey: "true", + }, + RunE: func(_ *cobra.Command, _ []string) error { + sawEndpoint = viperx.GetString(config.DataRobotURL) + sawToken = viperx.GetString(config.DataRobotAPIKey) + + return nil + }, + } + root.AddCommand(creator) + + root.SetArgs([]string{"--profile", "brand-new", "creator"}) + require.NoError(t, root.Execute(), "a profile-creating command must survive an unknown --profile") + + assert.Empty(t, sawEndpoint, "credentials must be cleared so a new profile doesn't inherit the default's endpoint") + assert.Empty(t, sawToken, "credentials must be cleared so a new profile doesn't inherit the default's token") +} diff --git a/docs/commands/auth.md b/docs/commands/auth.md index c9bcac6eb..e42f58765 100644 --- a/docs/commands/auth.md +++ b/docs/commands/auth.md @@ -363,15 +363,70 @@ Error: Invalid URL format > - Malformed domain names > - For self-managed instances, ensure the URL includes the full domain (e.g., `https://datarobot.company.com`) +### `profile` + +Inspect the named profiles stored in `drconfig.yaml`. Read-only: there is no `create` or +`delete` subcommand. A profile is created the first time `dr --profile auth login` +(or `auth set-url`) is run for a name that doesn't exist yet; remove one by editing +`drconfig.yaml` directly. + +```bash +dr auth profile list +dr auth profile show [name] +``` + +**`profile list`** shows the default profile plus every named profile, marking the active +one (selected via `--profile` or `DATAROBOT_CLI_PROFILE`): + +```bash +$ dr auth profile list +DataRobot Profiles +────────────────── +╭───────────┬───────────────────────────────────────┬───────┬────────╮ +│ NAME │ ENDPOINT │ TOKEN │ ACTIVE │ +├───────────┼───────────────────────────────────────┼───────┼────────┤ +│ default │ https://app.datarobot.com/api/v2 │ set │ ✓ │ +│ eu-mtsaas │ https://app.eu.datarobot.com/api/v2 │ set │ │ +╰───────────┴───────────────────────────────────────┴───────┴────────╯ +``` + +**`profile show [name]`** shows one profile's resolved settings — its own values, falling +back to the default profile's for anything it doesn't define. Defaults to the active +profile when no name is given; pass `default` to see the top-level profile explicitly. The +token value itself is never printed, only whether one is set. + +```bash +$ dr auth profile show eu-mtsaas +eu-mtsaas +───────── + endpoint: https://app.eu.datarobot.com/api/v2 + token: set + ca-cert: - (inherited from default) + ssl_verify: - (inherited from default) +``` + +Both support `--output-format json`. To work against a specific profile with any command, +not just `auth profile`, use `--profile ` or `DATAROBOT_CLI_PROFILE=`: + +```bash +dr --profile eu-mtsaas auth login +dr --profile eu-mtsaas templates list +DATAROBOT_CLI_PROFILE=eu-mtsaas dr templates list +``` + +See [Named profiles](../user-guide/configuration.md#named-profiles) for the full +`drconfig.yaml` shape and precedence rules. + ## Global options These options work with all `auth` commands: ```bash - -v, --verbose Enable verbose output - --debug Enable debug output - --skip-auth Skip authentication checks (for advanced users) - -h, --help Show help for command + -v, --verbose Enable verbose output + --debug Enable debug output + --skip-auth Skip authentication checks (for advanced users) + --profile string Named profile from drconfig.yaml to use + -h, --help Show help for command ``` > [!WARNING] @@ -453,6 +508,18 @@ $ dr auth set-url https://staging.datarobot.com $ dr auth login ``` +If you switch back and forth between the same instances often, a +[named profile](#profile) avoids re-authenticating each time: + +```bash +$ dr --profile staging auth set-url https://staging.datarobot.com +$ dr --profile staging auth login +$ dr --profile staging templates list + +# Later, back to the default instance — no re-login needed: +$ dr templates list +``` + ### Debug authentication issues ```bash @@ -535,12 +602,19 @@ After authentication, credentials are stored in: **Format:** -Keys are flat and top-level; there is no `datarobot:` or `preferences:` nesting: +Keys are flat and top-level (the default profile); there is no `datarobot:` or +`preferences:` nesting. The only nested key is `profiles:`, one section per +[named profile](#profile): ```yaml endpoint: https://app.datarobot.com/api/v2 token: api-consumer-tracking-enabled: true + +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: ``` Only allowlisted keys are ever written back (see `config.PersistableKeys`), so transient @@ -581,15 +655,28 @@ chmod 600 ~/.config/datarobot/drconfig.yaml ### Use per-environment authentication +Prefer a [named profile](#profile) for multiple DataRobot environments — one +`drconfig.yaml`, no re-authenticating when you switch back: + ```bash # Development -export DATAROBOT_CLI_CONFIG=~/.config/datarobot/dev-config.yaml -dr auth set-url https://dev.datarobot.com --config $DATAROBOT_CLI_CONFIG -dr auth login +dr --profile dev auth set-url https://dev.datarobot.com +dr --profile dev auth login # Production -export DATAROBOT_CLI_CONFIG=~/.config/datarobot/prod-config.yaml -dr auth set-url https://prod.datarobot.com --config $DATAROBOT_CLI_CONFIG +dr --profile prod auth set-url https://prod.datarobot.com +dr --profile prod auth login + +dr --profile dev templates list +dr --profile prod templates list +``` + +Separate config files remain available via `--config`/`DATAROBOT_CLI_CONFIG` for cases +that genuinely need a different file, such as a self-contained CI config: + +```bash +export DATAROBOT_CLI_CONFIG=~/.config/datarobot/ci-config.yaml +dr auth set-url https://app.datarobot.com --config $DATAROBOT_CLI_CONFIG dr auth login ``` @@ -616,6 +703,9 @@ export DATAROBOT_API_TOKEN=your-api-token # Custom config file location export DATAROBOT_CLI_CONFIG=~/.config/datarobot/custom-config.yaml + +# Named profile to use (see 'profile' above) +export DATAROBOT_CLI_PROFILE=eu-mtsaas ``` To go the other way — take the credentials the CLI already has and put them in your shell environment for the DataRobot SDKs and other tools — use [`dr auth export`](#export): diff --git a/docs/development/authentication.md b/docs/development/authentication.md index 52def4ea0..6b3e53c36 100644 --- a/docs/development/authentication.md +++ b/docs/development/authentication.md @@ -29,7 +29,7 @@ var MyCmd = &cobra.Command{ The hook functions are outlined below. -1. **Checks environment credentials first**: A complete `DATAROBOT_ENDPOINT` (or `DATAROBOT_API_ENDPOINT`) and `DATAROBOT_API_TOKEN` pair takes precedence over the config file. If the pair fails verification, the command fails with the reason (timeout, malformed endpoint, unreachable endpoint, a non-2xx status from the instance, or an invalid token; only a 401 or 403 blames the token). It never falls back to the stored profile and never starts the login flow, because that would silently run the command against a different DataRobot instance than the one requested. +1. **Checks environment credentials first**: A complete `DATAROBOT_ENDPOINT` (or `DATAROBOT_API_ENDPOINT`) and `DATAROBOT_API_TOKEN` pair takes precedence over the config file, **including over an active named profile** (`--profile`/`DATAROBOT_CLI_PROFILE`) — the same rule, applied consistently. If the pair fails verification, the command fails with the reason (timeout, malformed endpoint, unreachable endpoint, a non-2xx status from the instance, or an invalid token; only a 401 or 403 blames the token). It never falls back to the stored credentials and never starts the login flow, because that would silently run the command against a different DataRobot instance than the one requested. 2. **Checks for valid credentials**: With no complete environment pair, checks if a valid API key already exists in the config file. 3. **Auto-configures URL if missing**: If no DataRobot URL is configured, prompts you to set it up. 4. **Retrieves new credentials**: If the stored credentials are missing, or DataRobot rejected them with a 401 or 403, the hook automatically triggers the browser-based login flow. A timeout, an unreachable host, or any other status means DataRobot never judged the credentials, so the login flow does not start and the stored token is left intact. diff --git a/docs/development/configuration.md b/docs/development/configuration.md index 6efdfa63b..abbaa923c 100644 --- a/docs/development/configuration.md +++ b/docs/development/configuration.md @@ -67,16 +67,21 @@ Viper resolves a key from these sources in priority order: today — see below) 3. Environment variable bound via `viperx.BindEnv(key, "DATAROBOT_…")` or auto-mapped via `viperx.SetEnvPrefix("DATAROBOT_CLI")` -4. Value loaded from `drconfig.yaml` -5. Default registered via `viperx.SetDefault` +4. The active named profile's own keys, merged into this layer by + `config.ReadConfigFile` via `viper.MergeConfigMap` (see + [Named profiles](#named-profiles) below) — shadows the default + profile's values from the same layer +5. Value loaded from `drconfig.yaml` (the default/top-level profile) +6. Default registered via `viperx.SetDefault` ### Persistent root flags bound to viper -Only the persistent root flags listed in `cmd/root.go::init()` are bound -explicitly with `viperx.BindPFlag`. We do **not** bulk-bind subcommand -flags (and `viperx` does not even expose a `BindPFlags` function), because -that would slurp every subcommand flag (such as `--yes`, `--if-needed`) -into `viper.AllSettings()` and risk leaking transient flag state into +Only the persistent root flags registered in `cmd/root_factory.go`'s +`registerFlags` are bound explicitly with `viperx.BindPFlag`, in +`bindViperFlags`. We do **not** bulk-bind subcommand flags (and `viperx` +does not even expose a `BindPFlags` function), because that would slurp +every subcommand flag (such as `--yes`, `--if-needed`) into +`viper.AllSettings()` and risk leaking transient flag state into `drconfig.yaml`. `--output-format` is one of these root-bound flags. It is global and supports @@ -129,6 +134,39 @@ viper state — including transient flags such as `--yes`, `--verbose`, The wrappers in the auth package (`auth.WriteConfigFileSilent`, `auth.WriteConfigFile`) call this writer under the hood. +### Named profiles + +`--profile ` / `DATAROBOT_CLI_PROFILE` select a section under +`profiles:` in `drconfig.yaml` (see the +[user-facing docs](../user-guide/configuration.md#named-profiles)). The +mechanism lives in `internal/config/profile.go`: + +- `config.ReadConfigFile` calls `applyProfile(name)`, which + `viper.MergeConfigMap`s the profile's own keys into viper's **config** + layer — the same layer `viper.ReadInConfig` populates from the file. + Deliberately **not** `viperx.Set`: `Set` writes the override layer, which + outranks flags and env, so a profile's `endpoint` would beat an explicit + `DATAROBOT_CLI_ENDPOINT` instead of losing to it. +- Only `config.ProfileScopedKeys` (`endpoint`, `token`, `ca-cert`, + `ssl_verify`) may live under a profile; every other persistable key stays + global at the top level, shared by all profiles. +- `endpoint`/`token` merge atomically: if a profile defines either one, both + are merged (substituting `""` for the one it omits), so a profile can + never end up pairing its own endpoint with the default profile's token. + +On the write side, `UpdateConfigFile`'s `PersistableKeys` allowlist is +unchanged — `profileDestPath(key)` in `internal/config/write.go` maps an +allowlisted key to `profiles..` instead of `` when a +profile is active. A **bare** sweep (`UpdateConfigFile()` with no explicit +keys) additionally restricts profile-scoped candidates to keys the +profile's own section already defines (`candidateKeys`) — otherwise it +would copy every value the profile currently inherits from the default +profile (including its endpoint/token) permanently into the profile's +section. Call sites that already pass explicit keys (e.g. +`auth.WriteConfigFileSilent` passing `endpoint`, `token`) are unaffected by +that restriction, which is what lets a brand-new profile's section be +created on its first write. + ## Rules for new flags When adding a new flag, decide which category it falls into: @@ -137,7 +175,7 @@ When adding a new flag, decide which category it falls into: | --------------------------------------------------------- | --------------- | ------------------------------------ | | Transient subcommand flag (e.g. `--yes`, `--all`) | No | No | | Global transient root flag (e.g. `--output-format`) | Yes (root only) | No | -| Sticky preference (e.g. `--external-editor`) | Yes (root only) | Yes — add to `PersistableKeys` | +| Sticky preference (e.g. `--ca-cert`) | Yes (root only) | Yes — add to `PersistableKeys` | | Connection credential (e.g. `--token`) | Yes | Yes | For transient subcommand flags: @@ -155,8 +193,9 @@ For global transient root flags: ## Rules for new env vars `viperx.AutomaticEnv()` with prefix `DATAROBOT_CLI` is enabled in -`initializeConfig`, so any key you `viperx.Get` will already check -`DATAROBOT_CLI_` (with `-` replaced by `_`). +`defaultConfigInitializer` (`cmd/root_factory.go`), so any key you +`viperx.Get` will already check `DATAROBOT_CLI_` (with `-` replaced by +`_`). For env vars that should map to a different name (e.g. `DATAROBOT_CLI_NON_INTERACTIVE` → key `yes`), use `viperx.BindEnv` and @@ -169,7 +208,12 @@ To make a key writable to `drconfig.yaml`: 1. Add the key to `PersistableKeys` in `internal/config/write.go` 2. Update its production write call sites to pass the key explicitly: `config.UpdateConfigFile("my-new-key")` -3. Add a regression test under `internal/auth/writeConfig_test.go` (or a +3. Decide whether the key is per-profile or global (see + [Named profiles](#named-profiles) above). Per-profile-with-fallback is + the default for anything that can legitimately differ between DataRobot + installations (e.g. TLS settings); add it to `ProfileScopedKeys` in + `internal/config/profile.go` if so. Leave it out (global) otherwise. +4. Add a regression test under `internal/auth/writeConfig_test.go` (or a dedicated test file) verifying the key round-trips correctly and that transient flags still do not leak. @@ -193,6 +237,11 @@ to prevent accidental exposure of secrets in logs. To mark a key as sensitive: 2. When `--debug` is enabled, the key will be redacted as `****` in console output from `DebugViperConfig()`. +Redaction is depth-recursive (`redactSettings`/`redactValue` in +`internal/config/config.go`): a key match is redacted no matter how deeply +nested, which is what keeps e.g. `profiles.eu-mtsaas.token` redacted the +same as top-level `token`. + ## Common pitfalls - **Don't import `github.com/spf13/viper` outside `internal/config/`.** diff --git a/docs/development/flags.md b/docs/development/flags.md index 5d38a3abd..d1ec44ee8 100644 --- a/docs/development/flags.md +++ b/docs/development/flags.md @@ -158,6 +158,8 @@ Concretely: Some root flags must be forwarded to plugin subprocesses as `DATAROBOT_CLI_*` environment variables so plugins can honour them (e.g. `--debug` → `DATAROBOT_CLI_DEBUG=1`). These are called **universal flags**. +Current universal flags: `debug`, `disable-telemetry`, `verbose`, `skip-certificate-check`, `ca-cert`, `profile` (bound in `bindViperFlags`, `cmd/root_factory.go`). + ### How it works Separation of concerns is strict: diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index 5c57e6e33..55fad2425 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -75,10 +75,63 @@ yq -i 'del(.plugin_update_checks.assist)' ~/.config/datarobot/state.yaml The state file respects `XDG_CONFIG_HOME`. If that variable is set, the file is written to `$XDG_CONFIG_HOME/datarobot/state.yaml` rather than `~/.config/datarobot/state.yaml`. +### Named profiles + +If you work with multiple DataRobot installations (EU/US/JP cloud, an on-premise +instance, a second account), store each one as a named profile inside the same +`drconfig.yaml` rather than juggling separate config files: + +```yaml +# Default profile (used when no --profile is given) +endpoint: https://app.datarobot.com/api/v2 +token: default-api-key + +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-api-key + onprem: + endpoint: https://dr.corp.example.com/api/v2 + token: onprem-api-key + ca-cert: /etc/ssl/onprem-ca.pem +``` + +Select a profile with `--profile ` on any command, or set it for a whole +shell session with `DATAROBOT_CLI_PROFILE`: + +```bash +# Per-invocation +dr --profile eu-mtsaas templates list + +# Per-session +export DATAROBOT_CLI_PROFILE=eu-mtsaas +dr templates list +``` + +Only `endpoint`, `token`, `ca-cert`, and `ssl_verify` are profile-scoped. A +profile that doesn't define `ca-cert` or `ssl_verify` inherits the default +profile's value. Every other setting (e.g. `default-llm-id`) is global and +shared by all profiles. + +There's no `create` or `delete` subcommand: a profile is created the first +time you run `dr --profile auth login` (or `auth set-url`) for a name +that doesn't exist yet. Inspect what's configured with: + +```bash +dr auth profile list +dr auth profile show eu-mtsaas +``` + +An explicit `DATAROBOT_ENDPOINT`/`DATAROBOT_API_TOKEN` environment variable +pair still takes precedence over any profile, exactly as it does over the +default profile — see [Environment variables](#environment-variables) below. + ### Environment-specific configs > [!TIP] -> If you work with multiple DataRobot environments (development, staging, production), you can maintain separate configuration files for each. For example: +> Named profiles (above) are usually the better fit for multiple DataRobot +> environments. `--config` selects an entirely different *file* instead, +> which is still useful for e.g. a self-contained CI config: > > ```bash > # Development @@ -98,6 +151,9 @@ export DATAROBOT_CLI_CONFIG=~/.config/datarobot/dev-config.yaml dr templates list ``` +`--config` and `--profile` compose: `--config` picks the file, `--profile` +picks a section within it. + ## Configuration options ### Connection settings @@ -130,6 +186,9 @@ export DATAROBOT_API_TOKEN=your_api_token # Custom config file path export DATAROBOT_CLI_CONFIG=~/.config/datarobot/custom-config.yaml +# Named profile to use (see Named profiles above) +export DATAROBOT_CLI_PROFILE=eu-mtsaas + # Editor for text editing export EDITOR=nano @@ -190,10 +249,11 @@ dr --plugin-discovery-timeout 2s --help When the CLI needs configuration settings, it looks for them in this order (highest to lowest priority): -1. **Command-line flags** (e.g., `--config `)—overrides everything. -2. **Environment variables** (e.g., `DATAROBOT_CLI_CONFIG`)—overrides config files. -3. **Config files** (e.g., `~/.config/datarobot/drconfig.yaml`)—default location. -4. **Built-in defaults**—fallback values. +1. **Command-line flags** (e.g., `--config `, `--profile `)—overrides everything. +2. **Environment variables** (e.g., `DATAROBOT_CLI_CONFIG`, `DATAROBOT_CLI_PROFILE`)—overrides config files. +3. **The active named profile's own settings**—shadows the default profile's `endpoint`/`token`/`ca-cert`/`ssl_verify`. +4. **Config files** (e.g., `~/.config/datarobot/drconfig.yaml`)—default (top-level) profile. +5. **Built-in defaults**—fallback values. This means if you set an environment variable, it will take precedence over what's in your config file. This is useful for temporarily overriding settings without editing files. diff --git a/docs/user-guide/quick-reference.md b/docs/user-guide/quick-reference.md index b91bf9d72..e7de1e21f 100644 --- a/docs/user-guide/quick-reference.md +++ b/docs/user-guide/quick-reference.md @@ -24,6 +24,10 @@ dr auth check # Export credentials into the current shell session eval "$(dr auth export)" + +# Work against another DataRobot installation without re-authenticating +dr --profile eu-mtsaas templates list +dr auth profile list ``` ## Templates @@ -174,6 +178,9 @@ dr run --parallel [task1] [task2] # Custom config file dr --config /path/to/config.yaml [command] + +# Named profile (multiple DataRobot installations, one config file) +dr --profile eu-mtsaas [command] ``` ## File locations diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 0ed525fd2..96248d505 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -171,7 +171,7 @@ func ReportEnvCredentialsError(w io.Writer, creds *EnvCredentials, err error) { FprintUnsetTokenInstructions(w) } -// StoredEndpointName names the endpoint in messages about the stored profile. +// StoredEndpointName names the endpoint in messages about the stored credentials. // It avoids naming dr auth set-url, since DATAROBOT_CLI_ENDPOINT can override the file. const StoredEndpointName = "the configured DataRobot endpoint" @@ -275,11 +275,11 @@ func hostOrEndpoint(endpoint string) string { return endpoint } -// reportStoredProfileNotUsed names both sides of the substitution this CLI +// reportStoredCredentialsNotUsed names both sides of the substitution this CLI // refuses to make: the endpoint the environment asked for and the stored -// profile it will NOT fall back to. Printed only when a stored profile exists, -// since otherwise there is nothing to substitute. -func reportStoredProfileNotUsed(w io.Writer, creds *EnvCredentials) { +// credentials it will NOT fall back to. Printed only when stored credentials +// exist, since otherwise there is nothing to substitute. +func reportStoredCredentialsNotUsed(w io.Writer, creds *EnvCredentials) { storedHost := config.GetBaseURL() if storedHost == "" { return @@ -289,7 +289,7 @@ func reportStoredProfileNotUsed(w io.Writer, creds *EnvCredentials) { fmt.Fprint(w, base.Render("Environment credentials for ")) fmt.Fprint(w, info.Render(hostOrEndpoint(creds.Endpoint))) - fmt.Fprint(w, base.Render(" failed to verify; not falling back to the stored profile for ")) + fmt.Fprint(w, base.Render(" failed to verify; not falling back to the stored credentials for ")) fmt.Fprint(w, info.Render(storedHost)) fmt.Fprintln(w, base.Render(".")) } @@ -324,7 +324,7 @@ func EnsureAuthenticatedE(cmd *cobra.Command, _ []string) error { // is valid or was successfully obtained. // // A complete DATAROBOT_ENDPOINT/DATAROBOT_API_TOKEN pair that fails -// verification returns false without falling back to the stored profile and +// verification returns false without falling back to the stored credentials and // without starting the login flow: environment credentials are an explicit // instance request, and substituting the profile would silently run against // the wrong instance. @@ -350,11 +350,11 @@ func EnsureAuthenticated(ctx context.Context) bool { //nolint: cyclop // A complete pair of environment credentials is an explicit request for // that instance (the same precedence `dr auth export` documents). Falling - // back to the stored profile here would silently swap instances, so fail + // back to the stored credentials here would silently swap instances, so fail // loudly instead and never touch the profile. if !errors.Is(envErr, ErrEnvCredentialsNotSet) { ReportEnvCredentialsError(os.Stderr, creds, envErr) - reportStoredProfileNotUsed(os.Stderr, creds) + reportStoredCredentialsNotUsed(os.Stderr, creds) return false } @@ -425,10 +425,23 @@ func EnsureAuthenticated(ctx context.Context) bool { //nolint: cyclop } func WriteConfigFileSilent() error { - // Only persist allowlisted keys (see config.PersistableKeys). This avoids - // writing transient flags such as --yes back to drconfig.yaml. - err := config.UpdateConfigFile() - if err != nil { + // Persist endpoint/token explicitly first: a bare sweep alone would skip + // them for a brand-new profile, since config.candidateKeys restricts a + // bare sweep's profile-scoped keys to ones the profile's own section + // already owns (to avoid forking an inherited value into it), and a + // brand-new profile owns nothing yet. + if err := config.UpdateConfigFile(config.DataRobotURL, config.DataRobotAPIKey); err != nil { + log.Error(err) + return err + } + + // Also sweep the rest of config.PersistableKeys (e.g. ca-cert), same as + // before named profiles existed, so a flag like --ca-cert still persists + // across invocations. The bare sweep still restricts any OTHER + // profile-scoped key (ca-cert, ssl_verify) to ones the active profile's + // section already owns, so this can't fork an inherited value into a + // brand-new profile the way passing them explicitly here would. + if err := config.UpdateConfigFile(); err != nil { log.Error(err) return err } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index ffe3e7109..5803fdd13 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -191,7 +191,7 @@ func TestEnsureAuthenticated_EnvInvalidStoredValid(t *testing.T) { result := EnsureAuthenticated(context.Background()) assert.False(t, result, - "Expected EnsureAuthenticated to fail instead of falling back to the valid stored profile") + "Expected EnsureAuthenticated to fail instead of falling back to the valid stored credentials") } // TestEnsureAuthenticated_EnvMalformedEndpointStoredValid covers the malformed @@ -207,7 +207,7 @@ func TestEnsureAuthenticated_EnvMalformedEndpointStoredValid(t *testing.T) { result := EnsureAuthenticated(context.Background()) assert.False(t, result, - "Expected EnsureAuthenticated to fail on a malformed endpoint instead of using the stored profile") + "Expected EnsureAuthenticated to fail on a malformed endpoint instead of using the stored credentials") } func TestReportEnvCredentialsError(t *testing.T) { @@ -451,7 +451,7 @@ func TestEnsureAuthenticated_StoredProfileServerError(t *testing.T) { result := EnsureAuthenticated(context.Background()) - assert.False(t, result, "Expected EnsureAuthenticated to fail on a 503 from the stored profile") + assert.False(t, result, "Expected EnsureAuthenticated to fail on a 503 from the stored credentials") assert.Equal(t, "valid-token", viperx.GetString(config.DataRobotAPIKey), "Expected the stored token to survive a status the instance never judged it with") } @@ -518,7 +518,7 @@ func TestEnsureAuthenticated_EnvUnreachableStoredValid(t *testing.T) { result := EnsureAuthenticated(context.Background()) assert.False(t, result, - "Expected EnsureAuthenticated to fail on an unreachable endpoint instead of using the stored profile") + "Expected EnsureAuthenticated to fail on an unreachable endpoint instead of using the stored credentials") } func TestReportStoredProfileNotUsed(t *testing.T) { @@ -527,20 +527,20 @@ func TestReportStoredProfileNotUsed(t *testing.T) { creds := &EnvCredentials{Endpoint: "https://requested.example.com/api/v2", Token: "bad-token"} - t.Run("names both endpoints when a stored profile exists", func(t *testing.T) { + t.Run("names both endpoints when stored credentials exist", func(t *testing.T) { var buf bytes.Buffer - reportStoredProfileNotUsed(&buf, creds) + reportStoredCredentialsNotUsed(&buf, creds) assert.Contains(t, buf.String(), "https://requested.example.com") - assert.Contains(t, buf.String(), "not falling back to the stored profile") + assert.Contains(t, buf.String(), "not falling back to the stored credentials") }) - t.Run("silent without a stored profile", func(t *testing.T) { + t.Run("silent without a stored credentials", func(t *testing.T) { var buf bytes.Buffer viperx.Set(config.DataRobotURL, "") - reportStoredProfileNotUsed(&buf, creds) + reportStoredCredentialsNotUsed(&buf, creds) assert.Empty(t, buf.String()) }) diff --git a/internal/auth/writeConfig_test.go b/internal/auth/writeConfig_test.go index 295a77b4b..ec8374606 100644 --- a/internal/auth/writeConfig_test.go +++ b/internal/auth/writeConfig_test.go @@ -231,6 +231,58 @@ func TestWriteConfigFileSilent_OnlyAllowlistedFieldsWritten(t *testing.T) { "Non-allowlisted fields must not leak into drconfig.yaml") } +// TestWriteConfigFileSilent_CACertStillPersisted guards against a regression +// where narrowing WriteConfigFileSilent to explicit endpoint/token keys (so a +// brand-new profile's section gets created, see the named-profiles change) +// silently stopped persisting other allowlisted keys such as --ca-cert on +// every `auth login`/`auth set-url`. +func TestWriteConfigFileSilent_CACertStillPersisted(t *testing.T) { + tempDir, err := os.MkdirTemp("", "auth-test-*") + require.NoError(t, err) + + defer os.RemoveAll(tempDir) + + testutil.SetTestHomeDir(t, tempDir) + + viperx.Reset() + + defer viperx.Reset() + + err = config.CreateConfigFileDirIfNotExists() + require.NoError(t, err) + + configDir := filepath.Join(tempDir, ".config", "datarobot") + configFile := filepath.Join(configDir, "drconfig.yaml") + + initialConfig := map[string]interface{}{ + "endpoint": "https://onprem.example.com/api/v2", + "token": "original-token", + } + + initialYaml, err := yaml.Marshal(initialConfig) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(configFile, initialYaml, 0o644)) + require.NoError(t, config.ReadConfigFile("")) + + // Simulate `dr --ca-cert /etc/ssl/corp.pem auth login`. + viperx.Set("ca-cert", "/etc/ssl/corp.pem") + viperx.Set("token", "new-token") + + require.NoError(t, WriteConfigFileSilent()) + + rawYaml, err := os.ReadFile(configFile) + require.NoError(t, err) + + var configMap map[string]interface{} + + require.NoError(t, yaml.Unmarshal(rawYaml, &configMap)) + + assert.Equal(t, "/etc/ssl/corp.pem", configMap["ca-cert"], + "ca-cert set via flag must still be persisted by login/logout, not just endpoint/token") + assert.Equal(t, "new-token", configMap["token"]) +} + func TestWriteConfigFileSilent_TransientFlagsNotPersisted(t *testing.T) { tempDir, err := os.MkdirTemp("", "auth-test-*") require.NoError(t, err) @@ -288,3 +340,66 @@ func TestWriteConfigFileSilent_TransientFlagsNotPersisted(t *testing.T) { assert.NotContains(t, configMap, "force-interactive") assert.NotContains(t, configMap, "debug") } + +func TestWriteConfigFileSilent_WithActiveProfile_WritesUnderProfileAndLeavesDefaultUntouched(t *testing.T) { + tempDir, err := os.MkdirTemp("", "auth-test-*") + require.NoError(t, err) + + defer os.RemoveAll(tempDir) + + testutil.SetTestHomeDir(t, tempDir) + + viperx.Reset() + + defer viperx.Reset() + + err = config.CreateConfigFileDirIfNotExists() + require.NoError(t, err) + + configDir := filepath.Join(tempDir, ".config", "datarobot") + configFile := filepath.Join(configDir, "drconfig.yaml") + + initialConfig := map[string]interface{}{ + "endpoint": "https://app.datarobot.com/api/v2", + "token": "default-token", + } + + initialYaml, err := yaml.Marshal(initialConfig) + require.NoError(t, err) + + err = os.WriteFile(configFile, initialYaml, 0o644) + require.NoError(t, err) + + err = config.ReadConfigFile("") + require.NoError(t, err) + + // Simulate `dr --profile eu-mtsaas auth login` on a not-yet-existing + // profile: the root factory clears endpoint/token before RunE, then the + // login flow sets the newly obtained ones. + viperx.Set(config.ProfileKey, "eu-mtsaas") + viperx.Set(config.DataRobotURL, "https://app.eu.datarobot.com/api/v2") + viperx.Set(config.DataRobotAPIKey, "eu-token") + + _ = WriteConfigFileSilent() + + rawYaml, err := os.ReadFile(configFile) + require.NoError(t, err) + + var configMap map[string]interface{} + + err = yaml.Unmarshal(rawYaml, &configMap) + require.NoError(t, err) + + // The default profile's top-level credentials must be untouched. + assert.Equal(t, "https://app.datarobot.com/api/v2", configMap["endpoint"]) + assert.Equal(t, "default-token", configMap["token"]) + + profiles, ok := configMap["profiles"].(map[string]interface{}) + require.True(t, ok, "expected a profiles: block to be created") + + eu, ok := profiles["eu-mtsaas"].(map[string]interface{}) + require.True(t, ok, "expected profiles.eu-mtsaas to be created") + + assert.Equal(t, "https://app.eu.datarobot.com/api/v2", eu["endpoint"]) + assert.Equal(t, "eu-token", eu["token"]) +} diff --git a/internal/config/config.go b/internal/config/config.go index 0ffff6e7a..4a84f59f4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -135,20 +135,8 @@ func ReadConfigFile(filePath string) error { return err } - viper.SetConfigType("yaml") - - if filePath != "" { - if !strings.HasSuffix(filePath, ".yaml") && !strings.HasSuffix(filePath, ".yml") { - return fmt.Errorf("Config file must have .yaml or .yml extension: %s.", filePath) - } - - dir := filepath.Dir(filePath) - filename := filepath.Base(filePath) - viper.SetConfigName(filename) - viper.AddConfigPath(dir) - } else { - viper.SetConfigName(configFileName) - viper.AddConfigPath(defaultConfigFileDir) + if err := configureViperSearchPath(filePath, defaultConfigFileDir); err != nil { + return err } // Read in the config file @@ -163,15 +151,51 @@ func ReadConfigFile(filePath string) error { } } - if viper.GetBool("debug") { - output, err := DebugViperConfig() - if err != nil { - return fmt.Errorf("Failed to generate debug config output: %w", err) + if name := ActiveProfile(); name != "" { + if err := applyProfile(name); err != nil { + return err } + } + + return printDebugConfigIfEnabled() +} + +// configureViperSearchPath points viper at filePath, or at +// defaultConfigFileDir/configFileName when filePath is empty. +func configureViperSearchPath(filePath, defaultConfigFileDir string) error { + viper.SetConfigType("yaml") + + if filePath == "" { + viper.SetConfigName(configFileName) + viper.AddConfigPath(defaultConfigFileDir) + + return nil + } - fmt.Print(output) + if !strings.HasSuffix(filePath, ".yaml") && !strings.HasSuffix(filePath, ".yml") { + return fmt.Errorf("Config file must have .yaml or .yml extension: %s.", filePath) } + viper.SetConfigName(filepath.Base(filePath)) + viper.AddConfigPath(filepath.Dir(filePath)) + + return nil +} + +// printDebugConfigIfEnabled prints the effective viper configuration when +// --debug is set. +func printDebugConfigIfEnabled() error { + if !viper.GetBool("debug") { + return nil + } + + output, err := DebugViperConfig() + if err != nil { + return fmt.Errorf("Failed to generate debug config output: %w", err) + } + + fmt.Print(output) + return nil } @@ -196,27 +220,104 @@ func DebugViperConfig() (string, error) { sb.WriteString("Configuration initialized. Using config file: ") sb.WriteString(configFile) + sb.WriteString("\n") + + activeProfile := ActiveProfile() + if activeProfile == "" { + activeProfile = "default" + } + + sb.WriteString("Active profile: ") + sb.WriteString(activeProfile) sb.WriteString("\n\n") - // Print out the viper configuration for debugging - // Alphabetically, and redacting sensitive information - keys := make([]string, 0, len(viper.AllSettings())) - for key := range viper.AllSettings() { + // Print out the viper configuration for debugging, alphabetically, and + // redacting sensitive information at any nesting depth: a named + // profile's endpoint/token/etc. live under "profiles..*", below + // the top level, so a top-level-only redaction check would print a + // non-default profile's token in the clear. + settings := redactSettings(viper.AllSettings()) + + keys := make([]string, 0, len(settings)) + for key := range settings { keys = append(keys, key) } sort.Strings(keys) for _, key := range keys { - value := viper.Get(key) + fmt.Fprintf(&sb, " %s: %v\n", key, settings[key]) + } + + return sb.String(), nil +} - // Redact sensitive keys +// redactSettings returns a deep copy of m with the value of every key in +// sensitiveDebugKeys replaced by "****", at any nesting depth. Nested named +// profiles put credentials below the top level, so a shallow check would +// print them in the clear. +func redactSettings(m map[string]any) map[string]any { + out := make(map[string]any, len(m)) + + for key, value := range m { if _, sensitive := sensitiveDebugKeys[key]; sensitive { - fmt.Fprintf(&sb, " %s: %s\n", key, "****") - } else { - fmt.Fprintf(&sb, " %s: %v\n", key, value) + out[key] = "****" + continue } + + out[key] = redactValue(value) } - return sb.String(), nil + return out +} + +// redactValue recurses into nested maps and slices so redactSettings can +// redact sensitive keys regardless of depth. Viper's YAML decoding path can +// produce either map[string]any or map[any]any for nested maps. +func redactValue(value any) any { + switch v := value.(type) { + case map[string]any: + return redactSettings(v) + + case map[any]any: + asStringMap, _ := toStringKeyedMap(v) + + return redactSettings(asStringMap) + + case []any: + redacted := make([]any, len(v)) + + for i, item := range v { + redacted[i] = redactValue(item) + } + + return redacted + + default: + return value + } +} + +// toStringKeyedMap normalizes a value that is either map[string]any or +// map[any]any into map[string]any, or reports false for anything else. +// Viper's YAML decoding path can produce either shape for a nested map +// depending on how deeply it is nested, so both redactValue and profile.go's +// normalizeSectionMap need this same conversion. +func toStringKeyedMap(v any) (map[string]any, bool) { + if asMap, ok := v.(map[string]any); ok { + return asMap, true + } + + rawMap, ok := v.(map[any]any) + if !ok { + return nil, false + } + + normalized := make(map[string]any, len(rawMap)) + + for key, val := range rawMap { + normalized[fmt.Sprintf("%v", key)] = val + } + + return normalized, true } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 862f82226..4440fb720 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -109,6 +109,157 @@ func (suite *ConfigTestSuite) TestCreateConfigFileDirWithXDGConfigHome() { suite.FileExists(filepath.Join(expectedDir, expectedFileName), "Expected config file to be created in XDG_CONFIG_HOME") } +// writeConfigFile writes yamlContent to the suite's default drconfig.yaml +// path and returns that path. +func (suite *ConfigTestSuite) writeConfigFile(yamlContent string) string { + suite.Require().NoError(CreateConfigFileDirIfNotExists()) + + configFile := filepath.Join(suite.tempDir, ".config", "datarobot", "drconfig.yaml") + suite.Require().NoError(os.WriteFile(configFile, []byte(yamlContent), 0o600)) + + return configFile +} + +func (suite *ConfigTestSuite) TestReadConfigFile_FlatConfigNoProfilesBlock_BackwardCompat() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: flat-token +`) + + suite.Require().NoError(ReadConfigFile("")) + + suite.Equal("https://app.datarobot.com/api/v2", viper.GetString(DataRobotURL)) + suite.Equal("flat-token", viper.GetString(DataRobotAPIKey)) + suite.Empty(ActiveProfile()) +} + +func (suite *ConfigTestSuite) TestReadConfigFile_ProfilesBlockPresentButNoneSelected() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token +`) + + suite.Require().NoError(ReadConfigFile("")) + + suite.Equal("https://app.datarobot.com/api/v2", viper.GetString(DataRobotURL)) + suite.Equal("default-token", viper.GetString(DataRobotAPIKey)) +} + +func (suite *ConfigTestSuite) TestReadConfigFile_ActiveProfileShadowsEndpointAndToken() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token +`) + + viper.Set(ProfileKey, "eu-mtsaas") + + suite.Require().NoError(ReadConfigFile("")) + + suite.Equal("https://app.eu.datarobot.com/api/v2", viper.GetString(DataRobotURL)) + suite.Equal("eu-token", viper.GetString(DataRobotAPIKey)) +} + +func (suite *ConfigTestSuite) TestReadConfigFile_ProfileOmittingCACertInheritsTopLevel() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: default-token +ca-cert: /etc/ssl/corp.pem +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token +`) + + viper.Set(ProfileKey, "eu-mtsaas") + + suite.Require().NoError(ReadConfigFile("")) + + suite.Equal("/etc/ssl/corp.pem", viper.GetString("ca-cert"), "ca-cert should be inherited from the top level") +} + +func (suite *ConfigTestSuite) TestReadConfigFile_EnvVarPairStillWinsOverActiveProfile() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + eu-mtsaas: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token +`) + + viper.Set(ProfileKey, "eu-mtsaas") + + // DATAROBOT_CLI_ENDPOINT / DATAROBOT_CLI_TOKEN are automatically mapped + // by AutomaticEnv + SetEnvPrefix once bindViperFlags-equivalent setup has + // run; reproduce that here since this test reads ReadConfigFile in + // isolation from cmd/root_factory.go. + viper.SetEnvPrefix("DATAROBOT_CLI") + viper.AutomaticEnv() + suite.T().Setenv("DATAROBOT_CLI_ENDPOINT", "https://override.example.com/api/v2") + + suite.Require().NoError(ReadConfigFile("")) + + suite.Equal("https://override.example.com/api/v2", viper.GetString(DataRobotURL), + "an explicit env var must still beat the active profile's merged value") +} + +func (suite *ConfigTestSuite) TestReadConfigFile_ProfileNameIsCaseInsensitive() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + EU-MTSaaS: + endpoint: https://app.eu.datarobot.com/api/v2 + token: eu-token +`) + + viper.Set(ProfileKey, "eu-mtsaas") + + suite.Require().NoError(ReadConfigFile("")) + + suite.Equal("https://app.eu.datarobot.com/api/v2", viper.GetString(DataRobotURL)) +} + +func (suite *ConfigTestSuite) TestReadConfigFile_UnknownProfileReturnsTypedError() { + suite.writeConfigFile(`endpoint: https://app.datarobot.com/api/v2 +token: default-token +profiles: + staging: + endpoint: https://staging.example.com/api/v2 + token: staging-token +`) + + viper.Set(ProfileKey, "eu-mtsaas") + + err := ReadConfigFile("") + suite.Require().Error(err) + + var unknown *UnknownProfileError + + suite.Require().ErrorAs(err, &unknown) + suite.Equal("eu-mtsaas", unknown.Name) + suite.Equal([]string{"staging"}, unknown.Known) +} + +func TestDebugViperConfig_RedactsNestedProfileToken(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + viper.Set("profiles", map[string]any{ + "eu-mtsaas": map[string]any{ + "endpoint": "https://app.eu.datarobot.com/api/v2", + "token": "SUPER_SECRET_EU_TOKEN", + }, + }) + + output, err := DebugViperConfig() + require.NoError(t, err) + assert.Contains(t, output, "****") + assert.NotContains(t, output, "SUPER_SECRET_EU_TOKEN") +} + func TestDebugViperConfig_RedactsPulumiConfigPassphrase(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) diff --git a/internal/config/constants.go b/internal/config/constants.go index e703cfdd0..98d0eb35f 100644 --- a/internal/config/constants.go +++ b/internal/config/constants.go @@ -32,6 +32,17 @@ const ( // either an LLM Gateway model id or a DataRobot deployment id. DefaultLLMID = "default-llm-id" + // ProfileKey is the viper key behind the --profile persistent flag and the + // DATAROBOT_CLI_PROFILE env var. It must match the flag name exactly (see + // the SkipAuthKey comment above for why). Empty means the default + // (top-level) profile. + ProfileKey = "profile" + + // ProfileCreateAnnotationKey marks a command permitted to name a profile + // that does not exist yet in drconfig.yaml, because the command is about + // to create it (e.g. auth login, auth set-url). + ProfileCreateAnnotationKey = "profile-may-create" + // EnvPrefix is the canonical prefix for all DATAROBOT_CLI_* environment // variables. Use this constant instead of hard-coding the string literal. EnvPrefix = "DATAROBOT_CLI_" diff --git a/internal/config/profile.go b/internal/config/profile.go new file mode 100644 index 000000000..bc4e80ae6 --- /dev/null +++ b/internal/config/profile.go @@ -0,0 +1,270 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "errors" + "fmt" + "regexp" + "sort" + "strings" + + "github.com/spf13/viper" +) + +// profilesKey is the top-level drconfig.yaml key holding the named-profile +// sections. +const profilesKey = "profiles" + +// DefaultProfileLabel is how the default (top-level, unnamed) profile is +// addressed on the command line and in `dr auth profile` output. It is +// reserved: ValidateProfileName rejects it as a named-profile name, since +// drconfig.yaml would otherwise have no way to tell a literal +// "profiles.default" section apart from the true default profile. +const DefaultProfileLabel = "default" + +// ProfileScopedKeys are the persistable keys a named profile may override. +// Every other persistable key (e.g. default-llm-id) is global: it lives only +// at the top level and is shared by every profile. +var ProfileScopedKeys = map[string]struct{}{ + DataRobotURL: {}, + DataRobotAPIKey: {}, + "ca-cert": {}, + "ssl_verify": {}, +} + +// profileNamePattern is what viper can address as a dotted-path key segment: +// "." is viper's key delimiter, so a profile name cannot contain one. +var profileNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]*$`) + +// UnknownProfileError is returned when --profile (or DATAROBOT_CLI_PROFILE) +// names a section that does not exist in the config file. +type UnknownProfileError struct { + Name string + Known []string +} + +func (e *UnknownProfileError) Error() string { + if len(e.Known) == 0 { + return fmt.Sprintf("unknown profile %q; no profiles are configured in drconfig.yaml", e.Name) + } + + return fmt.Sprintf("unknown profile %q; known profiles: %s", e.Name, strings.Join(e.Known, ", ")) +} + +// NormalizeProfileName lowercases and trims a profile name to match viper's +// case-insensitive config keys. +func NormalizeProfileName(name string) string { + return strings.ToLower(strings.TrimSpace(name)) +} + +// ValidateProfileName rejects names viper cannot address as a config +// section: empty, or containing characters outside [A-Za-z0-9_-]. +func ValidateProfileName(name string) error { + if name == "" { + return errors.New("profile name must not be empty") + } + + if NormalizeProfileName(name) == DefaultProfileLabel { + return fmt.Errorf("profile name %q is reserved for the default profile; omit --profile to use it", name) + } + + if !profileNamePattern.MatchString(name) { + return fmt.Errorf("profile name %q must start with a letter or digit and contain only letters, digits, '-', or '_'", name) + } + + return nil +} + +// ActiveProfile returns the normalized name of the active profile, or "" for +// the default (top-level) profile. +func ActiveProfile() string { + return NormalizeProfileName(viper.GetString(ProfileKey)) +} + +// profileSection returns the raw sub-map for a profile as loaded from the +// config file, and whether it exists. name must already be normalized. +func profileSection(name string) (map[string]any, bool) { + profiles := viper.GetStringMap(profilesKey) + + section, ok := profiles[name] + if !ok { + return nil, false + } + + return toStringKeyedMap(section) +} + +// ProfileNames returns the sorted profile names present in the config file. +func ProfileNames() []string { + profiles := viper.GetStringMap(profilesKey) + names := make([]string, 0, len(profiles)) + + for name := range profiles { + names = append(names, name) + } + + sort.Strings(names) + + return names +} + +// applyProfile merges the named profile's scoped keys into viper's config +// layer, so they shadow the top-level keys while still losing to explicit +// flags and environment variables (both of which sit above the config layer +// in viper's own precedence). name must already be normalized. +// +// endpoint and token are merged atomically: if the profile section defines +// either one, both are merged (substituting "" for the one it omits), so a +// profile can never end up pairing its own endpoint with the default +// profile's token or vice versa. +func applyProfile(name string) error { + if err := ValidateProfileName(name); err != nil { + return err + } + + section, ok := profileSection(name) + if !ok { + return &UnknownProfileError{Name: name, Known: ProfileNames()} + } + + overlay := map[string]any{} + + _, hasEndpoint := section[DataRobotURL] + _, hasToken := section[DataRobotAPIKey] + + if hasEndpoint || hasToken { + overlay[DataRobotURL] = stringOrEmpty(section[DataRobotURL]) + overlay[DataRobotAPIKey] = stringOrEmpty(section[DataRobotAPIKey]) + } + + for key := range ProfileScopedKeys { + if key == DataRobotURL || key == DataRobotAPIKey { + continue + } + + if value, ok := section[key]; ok { + overlay[key] = value + } + } + + if len(overlay) == 0 { + return nil + } + + return viper.MergeConfigMap(overlay) +} + +// stringOrEmpty returns v as a string, or "" if v is nil or not a string. +func stringOrEmpty(v any) string { + s, ok := v.(string) + if !ok { + return "" + } + + return s +} + +// ProfileInfo is a read-only snapshot of one profile's own settings, exactly +// as stored in drconfig.yaml. Name is "" for the default (top-level) +// profile. A field is the zero value when the profile does not define it +// (it inherits from the default profile at merge time; see applyProfile). +type ProfileInfo struct { + Name string + Endpoint string + HasToken bool + CACert string + SSLVerify *bool +} + +// LoadProfiles re-reads the config file drconfig.yaml is currently pointed +// at into an isolated viper instance and returns the default profile's own +// settings plus every named profile's own settings, sorted by name. +// +// This is deliberately independent of the process-global viper instance: +// once a named profile is active, applyProfile has overwritten that +// instance's top-level endpoint/token with the profile's values, so it can +// no longer answer "what does the default profile's own endpoint say" -- +// exactly the question `dr auth profile list`/`show` need to answer for a +// profile other than the active one. +func LoadProfiles() (ProfileInfo, []ProfileInfo, error) { + configFile := viper.ConfigFileUsed() + if configFile == "" { + return ProfileInfo{}, nil, nil + } + + v := viper.New() + v.SetConfigFile(configFile) + v.SetConfigType("yaml") + + err := v.ReadInConfig() + if err != nil && errors.As(err, &viper.ConfigFileNotFoundError{}) { + return ProfileInfo{}, nil, nil + } + + if err != nil { + return ProfileInfo{}, nil, err + } + + def := profileInfoFromSection("", v.AllSettings()) + + // GetStringMap, not AllSettings()[profilesKey]: AllSettings silently + // drops a profile section that defines nothing of its own (an empty + // YAML mapping), which would make a bare `profiles.: {}` section + // vanish from `dr auth profile list` while still resolving correctly + // via applyProfile (which uses the same GetStringMap access pattern). + rawProfiles := v.GetStringMap(profilesKey) + + names := make([]string, 0, len(rawProfiles)) + for name := range rawProfiles { + names = append(names, name) + } + + sort.Strings(names) + + profiles := make([]ProfileInfo, 0, len(names)) + + for _, name := range names { + section, _ := toStringKeyedMap(rawProfiles[name]) + profiles = append(profiles, profileInfoFromSection(name, section)) + } + + return def, profiles, nil +} + +// profileInfoFromSection builds a ProfileInfo from a profile's own raw +// section (or, for the default profile, the config file's top-level map). +func profileInfoFromSection(name string, section map[string]any) ProfileInfo { + info := ProfileInfo{ + Name: name, + Endpoint: stringOrEmpty(section[DataRobotURL]), + HasToken: stringOrEmpty(section[DataRobotAPIKey]) != "", + CACert: stringOrEmpty(section["ca-cert"]), + } + + raw, ok := section["ssl_verify"] + if !ok { + return info + } + + b, ok := raw.(bool) + if !ok { + return info + } + + info.SSLVerify = &b + + return info +} diff --git a/internal/config/profile_test.go b/internal/config/profile_test.go new file mode 100644 index 000000000..a85c01af1 --- /dev/null +++ b/internal/config/profile_test.go @@ -0,0 +1,194 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/datarobot/cli/internal/testutil" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestProfileScopedKeys_SubsetOfPersistableKeys guards the invariant that +// candidateKeys (write.go) and profileDestPath (write.go) rely on: every +// profile-scoped key must also be persistable, or a bare sweep would never +// consider it a write candidate in the first place, silently making it +// impossible to ever persist through the profile-aware path. +func TestProfileScopedKeys_SubsetOfPersistableKeys(t *testing.T) { + for key := range ProfileScopedKeys { + _, ok := PersistableKeys[key] + assert.True(t, ok, "profile-scoped key %q must also be in PersistableKeys", key) + } +} + +func TestNormalizeProfileName(t *testing.T) { + assert.Equal(t, "eu-mtsaas", NormalizeProfileName("EU-MTSaaS")) + assert.Equal(t, "eu-mtsaas", NormalizeProfileName(" eu-mtsaas ")) + assert.Empty(t, NormalizeProfileName("")) + assert.Empty(t, NormalizeProfileName(" ")) +} + +func TestValidateProfileName(t *testing.T) { + valid := []string{"eu", "eu-mtsaas", "eu_mtsaas", "prod2"} + for _, name := range valid { + require.NoError(t, ValidateProfileName(name), "expected %q to be valid", name) + } + + invalid := []string{"", "eu.mtsaas", "eu mtsaas", "-eu", "eu/prod", "default", "Default", "DEFAULT"} + for _, name := range invalid { + assert.Error(t, ValidateProfileName(name), "expected %q to be invalid", name) + } +} + +func TestUnknownProfileError(t *testing.T) { + withCandidates := &UnknownProfileError{Name: "eu-mtsaa", Known: []string{"eu-mtsaas", "staging"}} + assert.Contains(t, withCandidates.Error(), `"eu-mtsaa"`) + assert.Contains(t, withCandidates.Error(), "eu-mtsaas, staging") + + noCandidates := &UnknownProfileError{Name: "eu"} + assert.Contains(t, noCandidates.Error(), `"eu"`) + assert.Contains(t, noCandidates.Error(), "no profiles are configured") +} + +func TestProfileNames_Sorted(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + viper.Set("profiles", map[string]any{ + "staging": map[string]any{"endpoint": "https://staging.example.com"}, + "eu-mtsaas": map[string]any{"endpoint": "https://eu.example.com"}, + }) + + assert.Equal(t, []string{"eu-mtsaas", "staging"}, ProfileNames()) +} + +func TestProfileNames_Empty(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + assert.Empty(t, ProfileNames()) +} + +func TestApplyProfile_UnknownProfile(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + viper.Set("profiles", map[string]any{ + "staging": map[string]any{"endpoint": "https://staging.example.com"}, + }) + + err := applyProfile("eu-mtsaas") + require.Error(t, err) + + var unknown *UnknownProfileError + + require.ErrorAs(t, err, &unknown) + assert.Equal(t, "eu-mtsaas", unknown.Name) + assert.Equal(t, []string{"staging"}, unknown.Known) +} + +func TestApplyProfile_InvalidName(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + err := applyProfile("has.dots") + require.Error(t, err) + + var unknown *UnknownProfileError + + assert.NotErrorAs(t, err, &unknown, "an invalid name should fail validation, not the not-found lookup") +} + +// TestApplyProfile_DefaultNameReserved guards against a "profiles.default" +// section that would be indistinguishable from, yet different than, the +// true top-level default profile in `dr auth profile list`/`show`. +func TestApplyProfile_DefaultNameReserved(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + err := applyProfile("default") + require.Error(t, err) + + var unknown *UnknownProfileError + + assert.NotErrorAs(t, err, &unknown, "the reserved name should fail validation, not the not-found lookup") +} + +func TestLoadProfiles_EmptySectionStillListed(t *testing.T) { + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, CreateConfigFileDirIfNotExists()) + + configFile := filepath.Join(tempDir, ".config", "datarobot", "drconfig.yaml") + rawYAML := `endpoint: https://default.example.com/api/v2 +token: default-token +profiles: + eu-mtsaas: + endpoint: https://eu.example.com/api/v2 + empty-profile: {} +` + require.NoError(t, os.WriteFile(configFile, []byte(rawYAML), 0o600)) + require.NoError(t, ReadConfigFile("")) + + def, profiles, err := LoadProfiles() + require.NoError(t, err) + + assert.Equal(t, "https://default.example.com/api/v2", def.Endpoint) + + names := make([]string, len(profiles)) + for i, p := range profiles { + names[i] = p.Name + } + + // AllSettings() silently drops an empty nested map, unlike + // GetStringMap; empty-profile must still surface here. + assert.Equal(t, []string{"empty-profile", "eu-mtsaas"}, names) +} + +func TestApplyProfile_EndpointTokenAtomicity(t *testing.T) { + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, CreateConfigFileDirIfNotExists()) + + configFile := filepath.Join(tempDir, ".config", "datarobot", "drconfig.yaml") + rawYAML := `endpoint: https://default.example.com/api/v2 +token: default-token +profiles: + partial: + endpoint: https://partial.example.com/api/v2 +` + require.NoError(t, os.WriteFile(configFile, []byte(rawYAML), 0o600)) + + // Load the same way ReadConfigFile does: raw file into viper's config + // layer, so this test exercises the same layer applyProfile merges into + // (unlike viper.Set, which writes the override layer above it). + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + + require.NoError(t, applyProfile("partial")) + + assert.Equal(t, "https://partial.example.com/api/v2", viper.GetString(DataRobotURL)) + assert.Empty(t, viper.GetString(DataRobotAPIKey), "token must not inherit the default profile's token") +} diff --git a/internal/config/write.go b/internal/config/write.go index e40a53334..85d0d2619 100644 --- a/internal/config/write.go +++ b/internal/config/write.go @@ -152,16 +152,7 @@ func readYAMLNode(path string) (*yaml.Node, error) { // and non-allowlisted keys. It navigates to nested keys using dotted notation // (e.g. "foo.bar.baz"). func applyAllowedKeysToNode(node *yaml.Node, keys []string) { - candidates := keys - if len(candidates) == 0 { - candidates = make([]string, 0, len(PersistableKeys)) - - for k := range PersistableKeys { - candidates = append(candidates, k) - } - } - - for _, key := range candidates { + for _, key := range candidateKeys(keys) { if _, ok := PersistableKeys[key]; !ok { continue } @@ -170,8 +161,66 @@ func applyAllowedKeysToNode(node *yaml.Node, keys []string) { continue } - setNestedKeyInNode(node, key, viper.Get(key)) + setNestedKeyInNode(node, profileDestPath(key), viper.Get(key)) + } +} + +// candidateKeys expands the keys UpdateConfigFile should consider writing. +// Explicit keys always pass through unchanged. +// +// For a bare sweep (keys is empty) with no profile active, every allowlisted +// key is a candidate, as before. +// +// For a bare sweep with a profile active, profile-scoped candidates are +// restricted to keys the profile's own section already defines. Without +// this, the sweep would copy every inherited top-level value (including +// endpoint and token belonging to a *different* instance) down into the +// profile and permanently fork it. Global (non-profile-scoped) keys sweep +// as usual. +func candidateKeys(keys []string) []string { + if len(keys) > 0 { + return keys + } + + candidates := make([]string, 0, len(PersistableKeys)) + + activeProfile := ActiveProfile() + + var owned map[string]any + + if activeProfile != "" { + owned, _ = profileSection(activeProfile) } + + for key := range PersistableKeys { + _, scoped := ProfileScopedKeys[key] + _, owns := owned[key] + + if scoped && activeProfile != "" && !owns { + continue + } + + candidates = append(candidates, key) + } + + return candidates +} + +// profileDestPath returns the YAML path a persistable key should be written +// to: the key itself for the default profile, or profiles.. when +// a named profile is active and the key is profile-scoped. PersistableKeys +// is always matched against the logical key, never against this path. +func profileDestPath(key string) string { + name := ActiveProfile() + if name == "" { + return key + } + + if _, scoped := ProfileScopedKeys[key]; !scoped { + return key + } + + return profilesKey + "." + name + "." + key } // Note: Keys NOT in candidates are preserved as-is from the existing node. diff --git a/internal/config/write_test.go b/internal/config/write_test.go new file mode 100644 index 000000000..9fc03cc1f --- /dev/null +++ b/internal/config/write_test.go @@ -0,0 +1,185 @@ +// Copyright 2026 DataRobot, Inc. and its affiliates. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/datarobot/cli/internal/testutil" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// setupWriteTest sets up an isolated home dir + viper instance and returns +// the path UpdateConfigFile will write to. +func setupWriteTest(t *testing.T) string { + t.Helper() + + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, CreateConfigFileDirIfNotExists()) + + return filepath.Join(tempDir, ".config", "datarobot", "drconfig.yaml") +} + +func readRawYAML(t *testing.T, path string) map[string]any { + t.Helper() + + data, err := os.ReadFile(path) + require.NoError(t, err) + + var m map[string]any + + require.NoError(t, yaml.Unmarshal(data, &m)) + + return m +} + +func TestUpdateConfigFile_ExplicitWriteLandsUnderActiveProfile(t *testing.T) { + configFile := setupWriteTest(t) + + viper.Set(ProfileKey, "eu-mtsaas") + viper.Set(DataRobotURL, "https://app.eu.datarobot.com/api/v2") + viper.Set(DataRobotAPIKey, "eu-token") + + require.NoError(t, UpdateConfigFile(DataRobotURL, DataRobotAPIKey)) + + raw := readRawYAML(t, configFile) + + profiles, ok := raw["profiles"].(map[string]any) + require.True(t, ok, "expected a profiles: block") + + eu, ok := profiles["eu-mtsaas"].(map[string]any) + require.True(t, ok, "expected profiles.eu-mtsaas to be created") + + assert.Equal(t, "https://app.eu.datarobot.com/api/v2", eu["endpoint"]) + assert.Equal(t, "eu-token", eu["token"]) + + // The default (top-level) endpoint/token must not exist yet: this is a + // brand-new profile, nothing was ever written at the top level. + assert.NotContains(t, raw, "endpoint") + assert.NotContains(t, raw, "token") +} + +func TestUpdateConfigFile_TopLevelUnchangedAfterProfileWrite(t *testing.T) { + configFile := setupWriteTest(t) + + initial := "endpoint: https://app.datarobot.com/api/v2\ntoken: default-token\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + require.NoError(t, ReadConfigFile("")) + + viper.Set(ProfileKey, "eu-mtsaas") + viper.Set(DataRobotURL, "https://app.eu.datarobot.com/api/v2") + viper.Set(DataRobotAPIKey, "eu-token") + + require.NoError(t, UpdateConfigFile(DataRobotURL, DataRobotAPIKey)) + + raw := readRawYAML(t, configFile) + + assert.Equal(t, "https://app.datarobot.com/api/v2", raw["endpoint"], "default profile's endpoint must be untouched") + assert.Equal(t, "default-token", raw["token"], "default profile's token must be untouched") +} + +func TestUpdateConfigFile_PreservesComments(t *testing.T) { + configFile := setupWriteTest(t) + + initial := "# my datarobot config\nendpoint: https://app.datarobot.com/api/v2\ntoken: default-token\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + require.NoError(t, ReadConfigFile("")) + + viper.Set(DataRobotAPIKey, "new-token") + require.NoError(t, UpdateConfigFile(DataRobotAPIKey)) + + data, err := os.ReadFile(configFile) + require.NoError(t, err) + + var doc yaml.Node + + require.NoError(t, yaml.Unmarshal(data, &doc)) + require.Len(t, doc.Content, 1) + + root := doc.Content[0] + require.NotEmpty(t, root.Content) + assert.Equal(t, "my datarobot config", root.Content[0].HeadComment[2:], "head comment should be preserved verbatim") +} + +func TestUpdateConfigFile_BareSweepDoesNotForkInheritedKeyIntoProfile(t *testing.T) { + configFile := setupWriteTest(t) + + // eu-mtsaas exists but defines nothing of its own yet: endpoint, token, + // and ca-cert all come from the default profile via ReadConfigFile's + // merge. + initial := "endpoint: https://app.datarobot.com/api/v2\ntoken: default-token\nca-cert: /etc/ssl/corp.pem\nprofiles:\n eu-mtsaas: {}\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + + viper.Set(ProfileKey, "eu-mtsaas") + require.NoError(t, ReadConfigFile("")) + + // A bare sweep (no explicit keys) must not copy any of those inherited + // values down into profiles.eu-mtsaas. + require.NoError(t, UpdateConfigFile()) + + raw := readRawYAML(t, configFile) + + profiles, ok := raw["profiles"].(map[string]any) + require.True(t, ok) + + eu, ok := profiles["eu-mtsaas"].(map[string]any) + require.True(t, ok) + assert.Empty(t, eu, "inherited default-profile values must not be forked into the profile by a bare sweep") +} + +func TestUpdateConfigFile_GlobalKeyWritesTopLevelUnderActiveProfile(t *testing.T) { + configFile := setupWriteTest(t) + + initial := "endpoint: https://app.datarobot.com/api/v2\ntoken: default-token\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + require.NoError(t, ReadConfigFile("")) + + viper.Set(ProfileKey, "eu-mtsaas") + viper.Set(DefaultLLMID, "some-deployment-id") + + require.NoError(t, UpdateConfigFile(DefaultLLMID)) + + raw := readRawYAML(t, configFile) + + assert.Equal(t, "some-deployment-id", raw[DefaultLLMID], "default-llm-id is global, so it writes at the top level even with a profile active") + assert.NotContains(t, raw, "profiles") +} + +func TestUpdateConfigFile_ProfileNameWrittenLowercase(t *testing.T) { + configFile := setupWriteTest(t) + + viper.Set(ProfileKey, "EU-MTSaaS") + viper.Set(DataRobotURL, "https://app.eu.datarobot.com/api/v2") + viper.Set(DataRobotAPIKey, "eu-token") + + require.NoError(t, UpdateConfigFile(DataRobotURL, DataRobotAPIKey)) + + raw := readRawYAML(t, configFile) + + profiles, ok := raw["profiles"].(map[string]any) + require.True(t, ok) + + _, hasLowercase := profiles["eu-mtsaas"] + assert.True(t, hasLowercase, "the profile section must be written lowercase so ReadConfigFile's case-insensitive lookup finds it again") +} diff --git a/internal/plugin/exec_test.go b/internal/plugin/exec_test.go index 6d03a15c6..72d65b8a2 100644 --- a/internal/plugin/exec_test.go +++ b/internal/plugin/exec_test.go @@ -381,10 +381,12 @@ func setupUniversalTestFlags(t *testing.T) *pflag.FlagSet { fs.Bool("disable-telemetry", false, "") fs.Bool("skip-certificate-check", false, "") fs.String("ca-cert", "", "") + fs.String(config.ProfileKey, "", "") fs.Lookup("debug").Annotations = map[string][]string{config.UniversalAnnotationKey: {"DEBUG"}} fs.Lookup("disable-telemetry").Annotations = map[string][]string{config.UniversalAnnotationKey: {"DISABLE_TELEMETRY"}} fs.Lookup("skip-certificate-check").Annotations = map[string][]string{config.UniversalAnnotationKey: {"SKIP_CERTIFICATE_CHECK"}} fs.Lookup("ca-cert").Annotations = map[string][]string{config.UniversalAnnotationKey: {"CA_CERT"}} + fs.Lookup(config.ProfileKey).Annotations = map[string][]string{config.UniversalAnnotationKey: {"PROFILE"}} return fs } @@ -489,6 +491,36 @@ func TestUniversalFlagEnv_StringEmptyOmitted(t *testing.T) { "empty string flags must not be emitted") } +func TestUniversalFlagEnv_ProfileSet(t *testing.T) { + viperx.Reset() + + fs := setupUniversalTestFlags(t) + + viperx.Set(config.ProfileKey, "eu-mtsaas") + + result := universalFlagEnv(fs) + + assert.Contains(t, result, config.EnvPrefix+"PROFILE=eu-mtsaas", + "the active profile must be forwarded to plugin subprocesses") +} + +func TestBuildPluginEnv_ForwardsActiveProfileCredentials(t *testing.T) { + viperx.Reset() + t.Cleanup(viperx.Reset) + + fs := setupUniversalTestFlags(t) + + viperx.Set(config.ProfileKey, "eu-mtsaas") + viperx.Set(config.DataRobotURL, "https://app.eu.datarobot.com/api/v2") + viperx.Set(config.DataRobotAPIKey, "eu-token") + + env := buildPluginEnv("", true, fs) + + assert.Contains(t, env, config.EnvPrefix+"PROFILE=eu-mtsaas") + assert.Contains(t, env, "DATAROBOT_ENDPOINT=https://app.eu.datarobot.com/api/v2") + assert.Contains(t, env, "DATAROBOT_API_TOKEN=eu-token") +} + // --- TraverseChildren / core-blind invariant tests --- // buildTestTree returns an isolated cobra command tree that mirrors the real CLI From 785c16f75855e6c1787cc4e2436ee5335edcf037 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Tue, 1 Sep 2026 18:29:10 -0600 Subject: [PATCH 2/3] Make profiles case sensitive --- cmd/auth/profile/show/cmd.go | 1 + internal/config/write.go | 56 +++++++++++++++++++++++++++++++++-- internal/config/write_test.go | 26 ++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/cmd/auth/profile/show/cmd.go b/cmd/auth/profile/show/cmd.go index 626eab470..199bb7d28 100644 --- a/cmd/auth/profile/show/cmd.go +++ b/cmd/auth/profile/show/cmd.go @@ -145,6 +145,7 @@ func resolveDetail(name string, own, def config.ProfileInfo, active bool) profil detail.HasToken = own.HasToken if !detail.HasTokenOwn { + detail.Endpoint = def.Endpoint detail.HasToken = def.HasToken } diff --git a/internal/config/write.go b/internal/config/write.go index 85d0d2619..90e5a88fb 100644 --- a/internal/config/write.go +++ b/internal/config/write.go @@ -161,7 +161,7 @@ func applyAllowedKeysToNode(node *yaml.Node, keys []string) { continue } - setNestedKeyInNode(node, profileDestPath(key), viper.Get(key)) + setNestedKeyInNode(node, profileDestPath(node, key), viper.Get(key)) } } @@ -210,7 +210,7 @@ func candidateKeys(keys []string) []string { // to: the key itself for the default profile, or profiles.. when // a named profile is active and the key is profile-scoped. PersistableKeys // is always matched against the logical key, never against this path. -func profileDestPath(key string) string { +func profileDestPath(node *yaml.Node, key string) string { name := ActiveProfile() if name == "" { return key @@ -220,7 +220,57 @@ func profileDestPath(key string) string { return key } - return profilesKey + "." + name + "." + key + return profilesKey + "." + profileSectionName(node, name) + "." + key +} + +func profileSectionName(node *yaml.Node, name string) string { + profilesNode := profilesMappingNode(node) + if profilesNode == nil { + return name + } + + return existingProfileName(profilesNode, name) +} + +func profilesMappingNode(node *yaml.Node) *yaml.Node { + if node == nil || node.Kind != yaml.MappingNode { + return nil + } + + for i := 0; i < len(node.Content)-1; i += 2 { + keyNode := node.Content[i] + if keyNode.Value != profilesKey { + continue + } + + profilesNode := node.Content[i+1] + if profilesNode.Kind == yaml.MappingNode { + return profilesNode + } + + return nil + } + + return nil +} + +func existingProfileName(profilesNode *yaml.Node, name string) string { + for i := 0; i < len(profilesNode.Content)-1; i += 2 { + sectionKey := profilesNode.Content[i] + if sectionKey.Value == name { + return name + } + + if sectionKey.Value == "" { + continue + } + + if NormalizeProfileName(sectionKey.Value) == name { + return sectionKey.Value + } + } + + return name } // Note: Keys NOT in candidates are preserved as-is from the existing node. diff --git a/internal/config/write_test.go b/internal/config/write_test.go index 9fc03cc1f..36381a929 100644 --- a/internal/config/write_test.go +++ b/internal/config/write_test.go @@ -183,3 +183,29 @@ func TestUpdateConfigFile_ProfileNameWrittenLowercase(t *testing.T) { _, hasLowercase := profiles["eu-mtsaas"] assert.True(t, hasLowercase, "the profile section must be written lowercase so ReadConfigFile's case-insensitive lookup finds it again") } + +func TestUpdateConfigFile_ReusesExistingMixedCaseProfileSection(t *testing.T) { + configFile := setupWriteTest(t) + + initial := "profiles:\n EU-MTSaaS:\n endpoint: https://app.eu.datarobot.com/api/v2\n token: old-token\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + require.NoError(t, ReadConfigFile("")) + + viper.Set(ProfileKey, "EU-MTSaaS") + viper.Set(DataRobotURL, "https://app.eu.datarobot.com/api/v2") + viper.Set(DataRobotAPIKey, "new-token") + + require.NoError(t, UpdateConfigFile(DataRobotURL, DataRobotAPIKey)) + + raw := readRawYAML(t, configFile) + + profiles, ok := raw["profiles"].(map[string]any) + require.True(t, ok) + require.Len(t, profiles, 1) + + _, hasOriginalCase := profiles["EU-MTSaaS"] + assert.True(t, hasOriginalCase, "an existing mixed-case profile section must be reused instead of creating a lowercase duplicate") + + _, hasLowercase := profiles["eu-mtsaas"] + assert.False(t, hasLowercase, "a second lowercase profile section must not be created when the existing section uses a different case") +} From ae059fa425c7040d2132e69afeaeda5265ff08a5 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Fri, 4 Sep 2026 15:49:25 -0600 Subject: [PATCH 3/3] Resolved review comments --- cmd/auth/profile/cmd.go | 2 +- cmd/auth/profile/show/cmd.go | 55 +++++++----------------- cmd/auth/profile/show/cmd_test.go | 18 ++++++++ docs/commands/auth.md | 1 - docs/development/configuration.md | 9 ++-- docs/user-guide/configuration.md | 10 ++--- internal/auth/auth.go | 6 +-- internal/config/profile.go | 61 +++++++++++++------------- internal/config/profile_test.go | 58 +++++++++++++++++++++++++ internal/config/write.go | 71 ++++++++++++++++++++----------- internal/config/write_test.go | 64 ++++++++++++++++++++++++++++ 11 files changed, 247 insertions(+), 108 deletions(-) diff --git a/cmd/auth/profile/cmd.go b/cmd/auth/profile/cmd.go index 284e2157e..bccd889ac 100644 --- a/cmd/auth/profile/cmd.go +++ b/cmd/auth/profile/cmd.go @@ -32,7 +32,7 @@ func Cmd() *cobra.Command { Long: `Inspect the named profiles stored in drconfig.yaml. A profile is a named set of credentials (endpoint + token, optionally -ca-cert and ssl_verify) alongside the default one, so you can work against +ca-cert) alongside the default one, so you can work against several DataRobot installations without re-authenticating each time. Select one with --profile or DATAROBOT_CLI_PROFILE= on any command. diff --git a/cmd/auth/profile/show/cmd.go b/cmd/auth/profile/show/cmd.go index 199bb7d28..fff92975b 100644 --- a/cmd/auth/profile/show/cmd.go +++ b/cmd/auth/profile/show/cmd.go @@ -31,23 +31,21 @@ const defaultProfileLabel = config.DefaultProfileLabel // profileDetail is the JSON representation of a single profile's resolved // settings for --output-format json. type profileDetail struct { - Name string `json:"name"` - Active bool `json:"active"` - Endpoint string `json:"endpoint"` - EndpointOwn bool `json:"endpoint_own"` - HasToken bool `json:"has_token"` - HasTokenOwn bool `json:"has_token_own"` - CACert string `json:"ca_cert,omitempty"` - CACertOwn bool `json:"ca_cert_own"` - SSLVerify *bool `json:"ssl_verify,omitempty"` - SSLVerifyOwn bool `json:"ssl_verify_own"` + Name string `json:"name"` + Active bool `json:"active"` + Endpoint string `json:"endpoint"` + EndpointOwn bool `json:"endpoint_own"` + HasToken bool `json:"has_token"` + HasTokenOwn bool `json:"has_token_own"` + CACert string `json:"ca_cert,omitempty"` + CACertOwn bool `json:"ca_cert_own"` } func Cmd() *cobra.Command { cmd := &cobra.Command{ Use: "show [name]", Short: "🔍 Show a profile's resolved settings", - Long: `Show a profile's resolved settings: its own endpoint/token/ca-cert/ssl_verify, + Long: `Show a profile's resolved settings: its own endpoint/token/ca-cert, falling back to the default profile's values for anything it doesn't define. Defaults to the active profile (selected via --profile or @@ -134,17 +132,16 @@ func resolveDetail(name string, own, def config.ProfileInfo, active bool) profil detail := profileDetail{Name: name, Active: active} isDefault := name == defaultProfileLabel - detail.EndpointOwn = isDefault || own.Endpoint != "" - detail.Endpoint = own.Endpoint - - if !detail.EndpointOwn { - detail.Endpoint = def.Endpoint - } + // config.applyProfile merges endpoint and token atomically, so a profile + // that defines either one owns both and never inherits the other. + credOwn := isDefault || own.Endpoint != "" || own.HasToken - detail.HasTokenOwn = isDefault || own.HasToken + detail.EndpointOwn = credOwn + detail.HasTokenOwn = credOwn + detail.Endpoint = own.Endpoint detail.HasToken = own.HasToken - if !detail.HasTokenOwn { + if !credOwn { detail.Endpoint = def.Endpoint detail.HasToken = def.HasToken } @@ -156,13 +153,6 @@ func resolveDetail(name string, own, def config.ProfileInfo, active bool) profil detail.CACert = def.CACert } - detail.SSLVerifyOwn = isDefault || own.SSLVerify != nil - detail.SSLVerify = own.SSLVerify - - if !detail.SSLVerifyOwn { - detail.SSLVerify = def.SSLVerify - } - return detail } @@ -177,7 +167,6 @@ func printDetail(d profileDetail) { printField("endpoint", valueOrDash(d.Endpoint), d.EndpointOwn) printField("token", tokenStatus(d.HasToken), d.HasTokenOwn) printField("ca-cert", valueOrDash(d.CACert), d.CACertOwn) - printField("ssl_verify", sslVerifyStatus(d.SSLVerify), d.SSLVerifyOwn) } func printField(label, value string, own bool) { @@ -204,15 +193,3 @@ func tokenStatus(hasToken bool) string { return "not set" } - -func sslVerifyStatus(v *bool) string { - if v == nil { - return "-" - } - - if *v { - return "true" - } - - return "false" -} diff --git a/cmd/auth/profile/show/cmd_test.go b/cmd/auth/profile/show/cmd_test.go index 3186a62b6..b06327284 100644 --- a/cmd/auth/profile/show/cmd_test.go +++ b/cmd/auth/profile/show/cmd_test.go @@ -52,6 +52,8 @@ profiles: endpoint: https://onprem.example.com/api/v2 token: onprem-token ca-cert: /etc/ssl/onprem.pem + staging: + endpoint: https://staging.example.com/api/v2 ` require.NoError(t, os.WriteFile(filepath.Join(configDir, "drconfig.yaml"), []byte(raw), 0o600)) require.NoError(t, config.ReadConfigFile("")) @@ -129,6 +131,22 @@ func TestShow_NamedProfile_InheritsCACertFromDefault(t *testing.T) { assert.False(t, detail.CACertOwn) } +// TestShow_EndpointOnlyProfileDoesNotInheritCredentials mirrors +// config.applyProfile, which merges endpoint and token atomically: a profile +// created by `auth set-url` alone owns its endpoint and an empty token, and +// must never be shown pairing its own endpoint with the default's token (or +// the default's endpoint under its own name). +func TestShow_EndpointOnlyProfileDoesNotInheritCredentials(t *testing.T) { + writeTestConfig(t) + + detail, _ := runShowJSON(t, "staging") + + assert.Equal(t, "https://staging.example.com/api/v2", detail.Endpoint) + assert.True(t, detail.EndpointOwn) + assert.False(t, detail.HasToken, "a profile that owns an endpoint but no token must not borrow the default's") + assert.True(t, detail.HasTokenOwn) +} + func TestShow_NoArgsDefaultsToActiveProfile(t *testing.T) { writeTestConfig(t) diff --git a/docs/commands/auth.md b/docs/commands/auth.md index e42f58765..e1de49bb2 100644 --- a/docs/commands/auth.md +++ b/docs/commands/auth.md @@ -402,7 +402,6 @@ eu-mtsaas endpoint: https://app.eu.datarobot.com/api/v2 token: set ca-cert: - (inherited from default) - ssl_verify: - (inherited from default) ``` Both support `--output-format json`. To work against a specific profile with any command, diff --git a/docs/development/configuration.md b/docs/development/configuration.md index abbaa923c..0c180f113 100644 --- a/docs/development/configuration.md +++ b/docs/development/configuration.md @@ -147,9 +147,12 @@ mechanism lives in `internal/config/profile.go`: Deliberately **not** `viperx.Set`: `Set` writes the override layer, which outranks flags and env, so a profile's `endpoint` would beat an explicit `DATAROBOT_CLI_ENDPOINT` instead of losing to it. -- Only `config.ProfileScopedKeys` (`endpoint`, `token`, `ca-cert`, - `ssl_verify`) may live under a profile; every other persistable key stays - global at the top level, shared by all profiles. +- Only `config.ProfileScopedKeys` (`endpoint`, `token`, `ca-cert`) may live + under a profile; every other persistable key stays global at the top level, + shared by all profiles. `ssl_verify` is deliberately excluded: nothing in + the CLI reads it (TLS is driven by `--ca-cert` and + `--skip-certificate-check`), so it stays a global passthrough value that + `UpdateConfigFile` preserves. - `endpoint`/`token` merge atomically: if a profile defines either one, both are merged (substituting `""` for the one it omits), so a profile can never end up pairing its own endpoint with the default profile's token. diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index 55fad2425..9de7bd765 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -108,10 +108,10 @@ export DATAROBOT_CLI_PROFILE=eu-mtsaas dr templates list ``` -Only `endpoint`, `token`, `ca-cert`, and `ssl_verify` are profile-scoped. A -profile that doesn't define `ca-cert` or `ssl_verify` inherits the default -profile's value. Every other setting (e.g. `default-llm-id`) is global and -shared by all profiles. +Only `endpoint`, `token`, and `ca-cert` are profile-scoped. A profile that +doesn't define `ca-cert` inherits the default profile's value. Every other +setting (e.g. `default-llm-id`, `ssl_verify`) is global and shared by all +profiles. There's no `create` or `delete` subcommand: a profile is created the first time you run `dr --profile auth login` (or `auth set-url`) for a name @@ -251,7 +251,7 @@ When the CLI needs configuration settings, it looks for them in this order (high 1. **Command-line flags** (e.g., `--config `, `--profile `)—overrides everything. 2. **Environment variables** (e.g., `DATAROBOT_CLI_CONFIG`, `DATAROBOT_CLI_PROFILE`)—overrides config files. -3. **The active named profile's own settings**—shadows the default profile's `endpoint`/`token`/`ca-cert`/`ssl_verify`. +3. **The active named profile's own settings**—shadows the default profile's `endpoint`/`token`/`ca-cert`. 4. **Config files** (e.g., `~/.config/datarobot/drconfig.yaml`)—default (top-level) profile. 5. **Built-in defaults**—fallback values. diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 96248d505..16ab1d1e8 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -437,10 +437,8 @@ func WriteConfigFileSilent() error { // Also sweep the rest of config.PersistableKeys (e.g. ca-cert), same as // before named profiles existed, so a flag like --ca-cert still persists - // across invocations. The bare sweep still restricts any OTHER - // profile-scoped key (ca-cert, ssl_verify) to ones the active profile's - // section already owns, so this can't fork an inherited value into a - // brand-new profile the way passing them explicitly here would. + // across invocations. The bare sweep still refuses to fork a merely + // inherited profile-scoped value down into the active profile's section. if err := config.UpdateConfigFile(); err != nil { log.Error(err) return err diff --git a/internal/config/profile.go b/internal/config/profile.go index bc4e80ae6..245431880 100644 --- a/internal/config/profile.go +++ b/internal/config/profile.go @@ -17,10 +17,12 @@ package config import ( "errors" "fmt" + "os" "regexp" "sort" "strings" + "github.com/datarobot/cli/internal/log" "github.com/spf13/viper" ) @@ -38,11 +40,15 @@ const DefaultProfileLabel = "default" // ProfileScopedKeys are the persistable keys a named profile may override. // Every other persistable key (e.g. default-llm-id) is global: it lives only // at the top level and is shared by every profile. +// +// ssl_verify is deliberately absent: nothing in the CLI reads it (TLS is +// driven by --ca-cert and --skip-certificate-check), so it stays a global +// passthrough value the config writer preserves rather than a profile knob +// that would imply it gates this CLI's TLS behaviour. var ProfileScopedKeys = map[string]struct{}{ DataRobotURL: {}, DataRobotAPIKey: {}, "ca-cert": {}, - "ssl_verify": {}, } // profileNamePattern is what viper can address as a dotted-path key segment: @@ -109,10 +115,24 @@ func profileSection(name string) (map[string]any, bool) { // ProfileNames returns the sorted profile names present in the config file. func ProfileNames() []string { - profiles := viper.GetStringMap(profilesKey) + return namedProfileKeys(viper.GetStringMap(profilesKey)) +} + +// namedProfileKeys returns the sorted, addressable section names in a raw +// profiles map. A hand-edited "profiles.default" section is dropped: +// ValidateProfileName reserves that name for the top-level profile, so such a +// section can never be selected and must not be listed as if it could. +func namedProfileKeys(profiles map[string]any) []string { names := make([]string, 0, len(profiles)) for name := range profiles { + if NormalizeProfileName(name) == DefaultProfileLabel { + log.Debugf("ignoring %s.%s section in drconfig.yaml: %q is reserved for the top-level default profile", + profilesKey, name, DefaultProfileLabel) + + continue + } + names = append(names, name) } @@ -182,11 +202,10 @@ func stringOrEmpty(v any) string { // profile. A field is the zero value when the profile does not define it // (it inherits from the default profile at merge time; see applyProfile). type ProfileInfo struct { - Name string - Endpoint string - HasToken bool - CACert string - SSLVerify *bool + Name string + Endpoint string + HasToken bool + CACert string } // LoadProfiles re-reads the config file drconfig.yaml is currently pointed @@ -209,8 +228,11 @@ func LoadProfiles() (ProfileInfo, []ProfileInfo, error) { v.SetConfigFile(configFile) v.SetConfigType("yaml") + // SetConfigFile bypasses viper's search, so a missing file surfaces as a + // plain os.ErrNotExist rather than viper.ConfigFileNotFoundError. Treat it + // as "no profiles configured" so a config deleted mid-session still lists. err := v.ReadInConfig() - if err != nil && errors.As(err, &viper.ConfigFileNotFoundError{}) { + if err != nil && errors.Is(err, os.ErrNotExist) { return ProfileInfo{}, nil, nil } @@ -227,12 +249,7 @@ func LoadProfiles() (ProfileInfo, []ProfileInfo, error) { // via applyProfile (which uses the same GetStringMap access pattern). rawProfiles := v.GetStringMap(profilesKey) - names := make([]string, 0, len(rawProfiles)) - for name := range rawProfiles { - names = append(names, name) - } - - sort.Strings(names) + names := namedProfileKeys(rawProfiles) profiles := make([]ProfileInfo, 0, len(names)) @@ -247,24 +264,10 @@ func LoadProfiles() (ProfileInfo, []ProfileInfo, error) { // profileInfoFromSection builds a ProfileInfo from a profile's own raw // section (or, for the default profile, the config file's top-level map). func profileInfoFromSection(name string, section map[string]any) ProfileInfo { - info := ProfileInfo{ + return ProfileInfo{ Name: name, Endpoint: stringOrEmpty(section[DataRobotURL]), HasToken: stringOrEmpty(section[DataRobotAPIKey]) != "", CACert: stringOrEmpty(section["ca-cert"]), } - - raw, ok := section["ssl_verify"] - if !ok { - return info - } - - b, ok := raw.(bool) - if !ok { - return info - } - - info.SSLVerify = &b - - return info } diff --git a/internal/config/profile_test.go b/internal/config/profile_test.go index a85c01af1..611923d8a 100644 --- a/internal/config/profile_test.go +++ b/internal/config/profile_test.go @@ -164,6 +164,64 @@ profiles: assert.Equal(t, []string{"empty-profile", "eu-mtsaas"}, names) } +// TestLoadProfiles_ReservedDefaultSectionIgnored covers a hand-edited +// "profiles.default" section: ValidateProfileName reserves that name for the +// top-level profile, so listing it would render an unreachable second +// "default" row in `dr auth profile list`. +func TestLoadProfiles_ReservedDefaultSectionIgnored(t *testing.T) { + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, CreateConfigFileDirIfNotExists()) + + configFile := filepath.Join(tempDir, ".config", "datarobot", "drconfig.yaml") + rawYAML := `endpoint: https://default.example.com/api/v2 +token: default-token +profiles: + default: + endpoint: https://impostor.example.com/api/v2 + eu-mtsaas: + endpoint: https://eu.example.com/api/v2 +` + require.NoError(t, os.WriteFile(configFile, []byte(rawYAML), 0o600)) + require.NoError(t, ReadConfigFile("")) + + _, profiles, err := LoadProfiles() + require.NoError(t, err) + + names := make([]string, len(profiles)) + for i, p := range profiles { + names[i] = p.Name + } + + assert.Equal(t, []string{"eu-mtsaas"}, names) + assert.Equal(t, []string{"eu-mtsaas"}, ProfileNames()) +} + +// TestLoadProfiles_MissingFileIsNotAnError covers a config file deleted after +// viper recorded it: SetConfigFile bypasses viper's search, so the failure is +// a plain os.ErrNotExist rather than viper.ConfigFileNotFoundError. +func TestLoadProfiles_MissingFileIsNotAnError(t *testing.T) { + tempDir := t.TempDir() + testutil.SetTestHomeDir(t, tempDir) + viper.Reset() + t.Cleanup(viper.Reset) + + require.NoError(t, CreateConfigFileDirIfNotExists()) + + configFile := filepath.Join(tempDir, ".config", "datarobot", "drconfig.yaml") + require.NoError(t, os.WriteFile(configFile, []byte("endpoint: https://default.example.com/api/v2\n"), 0o600)) + require.NoError(t, ReadConfigFile("")) + require.NoError(t, os.Remove(configFile)) + + def, profiles, err := LoadProfiles() + require.NoError(t, err) + assert.Empty(t, def.Endpoint) + assert.Empty(t, profiles) +} + func TestApplyProfile_EndpointTokenAtomicity(t *testing.T) { tempDir := t.TempDir() testutil.SetTestHomeDir(t, tempDir) diff --git a/internal/config/write.go b/internal/config/write.go index 90e5a88fb..c9f8823cb 100644 --- a/internal/config/write.go +++ b/internal/config/write.go @@ -152,7 +152,7 @@ func readYAMLNode(path string) (*yaml.Node, error) { // and non-allowlisted keys. It navigates to nested keys using dotted notation // (e.g. "foo.bar.baz"). func applyAllowedKeysToNode(node *yaml.Node, keys []string) { - for _, key := range candidateKeys(keys) { + for _, key := range candidateKeys(node, keys) { if _, ok := PersistableKeys[key]; !ok { continue } @@ -171,13 +171,13 @@ func applyAllowedKeysToNode(node *yaml.Node, keys []string) { // For a bare sweep (keys is empty) with no profile active, every allowlisted // key is a candidate, as before. // -// For a bare sweep with a profile active, profile-scoped candidates are -// restricted to keys the profile's own section already defines. Without -// this, the sweep would copy every inherited top-level value (including -// endpoint and token belonging to a *different* instance) down into the -// profile and permanently fork it. Global (non-profile-scoped) keys sweep -// as usual. -func candidateKeys(keys []string) []string { +// For a bare sweep with a profile active, a profile-scoped key is a candidate +// only when it belongs to the profile rather than being inherited (see +// profileOwnsKey). Without this, the sweep would copy every inherited +// top-level value (including endpoint and token belonging to a *different* +// instance) down into the profile and permanently fork it. Global +// (non-profile-scoped) keys sweep as usual. +func candidateKeys(node *yaml.Node, keys []string) []string { if len(keys) > 0 { return keys } @@ -186,17 +186,10 @@ func candidateKeys(keys []string) []string { activeProfile := ActiveProfile() - var owned map[string]any - - if activeProfile != "" { - owned, _ = profileSection(activeProfile) - } - for key := range PersistableKeys { _, scoped := ProfileScopedKeys[key] - _, owns := owned[key] - if scoped && activeProfile != "" && !owns { + if scoped && activeProfile != "" && !profileOwnsKey(node, activeProfile, key) { continue } @@ -206,6 +199,30 @@ func candidateKeys(keys []string) []string { return candidates } +// profileOwnsKey reports whether a bare sweep should write key into the active +// profile's section. Ownership is resolved against node -- the config file as +// it currently exists on disk -- rather than against the process-global viper +// instance, whose profiles map is a snapshot from startup and so never sees a +// section an earlier UpdateConfigFile call in the same process just created. +// +// A key is owned when the profile's section already defines it, or when the +// live viper value differs from the default profile's on-disk value, which +// means a flag or environment variable overrode what would otherwise have been +// inherited (e.g. `dr --profile onprem --ca-cert ... auth login`). +func profileOwnsKey(node *yaml.Node, profile, key string) bool { + section := mappingValueNode(profilesMappingNode(node), profileSectionName(node, profile)) + if mappingValueNode(section, key) != nil { + return true + } + + inherited := "" + if valNode := mappingValueNode(node, key); valNode != nil { + inherited = valNode.Value + } + + return fmt.Sprintf("%v", viper.Get(key)) != inherited +} + // profileDestPath returns the YAML path a persistable key should be written // to: the key itself for the default profile, or profiles.. when // a named profile is active and the key is profile-scoped. PersistableKeys @@ -233,22 +250,24 @@ func profileSectionName(node *yaml.Node, name string) string { } func profilesMappingNode(node *yaml.Node) *yaml.Node { + profilesNode := mappingValueNode(node, profilesKey) + if profilesNode == nil || profilesNode.Kind != yaml.MappingNode { + return nil + } + + return profilesNode +} + +// mappingValueNode returns the value node for key in a mapping node, or nil. +func mappingValueNode(node *yaml.Node, key string) *yaml.Node { if node == nil || node.Kind != yaml.MappingNode { return nil } for i := 0; i < len(node.Content)-1; i += 2 { - keyNode := node.Content[i] - if keyNode.Value != profilesKey { - continue - } - - profilesNode := node.Content[i+1] - if profilesNode.Kind == yaml.MappingNode { - return profilesNode + if node.Content[i].Value == key { + return node.Content[i+1] } - - return nil } return nil diff --git a/internal/config/write_test.go b/internal/config/write_test.go index 36381a929..daa225f1e 100644 --- a/internal/config/write_test.go +++ b/internal/config/write_test.go @@ -148,6 +148,70 @@ func TestUpdateConfigFile_BareSweepDoesNotForkInheritedKeyIntoProfile(t *testing assert.Empty(t, eu, "inherited default-profile values must not be forked into the profile by a bare sweep") } +// TestUpdateConfigFile_BareSweepWritesFlagOverriddenKeyIntoProfile covers +// `dr --profile eu-mtsaas --ca-cert /etc/ssl/eu.pem auth login`: ca-cert is +// not yet in the profile's section, but the flag value differs from the +// default profile's, so it is the profile's own value and must persist. +func TestUpdateConfigFile_BareSweepWritesFlagOverriddenKeyIntoProfile(t *testing.T) { + configFile := setupWriteTest(t) + + initial := "endpoint: https://app.datarobot.com/api/v2\ntoken: default-token\nca-cert: /etc/ssl/corp.pem\nprofiles:\n eu-mtsaas: {}\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + + viper.Set(ProfileKey, "eu-mtsaas") + require.NoError(t, ReadConfigFile("")) + + viper.Set("ca-cert", "/etc/ssl/eu.pem") + + require.NoError(t, UpdateConfigFile()) + + raw := readRawYAML(t, configFile) + + assert.Equal(t, "/etc/ssl/corp.pem", raw["ca-cert"], "the default profile's ca-cert must be left alone") + + profiles, ok := raw["profiles"].(map[string]any) + require.True(t, ok) + + eu, ok := profiles["eu-mtsaas"].(map[string]any) + require.True(t, ok) + assert.Equal(t, "/etc/ssl/eu.pem", eu["ca-cert"]) +} + +// TestUpdateConfigFile_BareSweepSeesSectionWrittenEarlierInProcess covers the +// two-call sequence in auth.WriteConfigFileSilent: the explicit endpoint/token +// write creates the profile section on disk, but the process-global viper +// profiles map is still the startup snapshot, so ownership must be resolved +// from the file rather than from viper. +func TestUpdateConfigFile_BareSweepSeesSectionWrittenEarlierInProcess(t *testing.T) { + configFile := setupWriteTest(t) + + initial := "endpoint: https://app.datarobot.com/api/v2\ntoken: default-token\n" + require.NoError(t, os.WriteFile(configFile, []byte(initial), 0o600)) + require.NoError(t, ReadConfigFile("")) + + // Mirror defaultConfigInitializer's brand-new-profile path: the section + // does not exist yet, so viper never learns about it and the inherited + // credentials are cleared before auth login supplies its own. + viper.Set(ProfileKey, "onprem") + viper.Set(DataRobotURL, "https://onprem.example.com/api/v2") + viper.Set(DataRobotAPIKey, "onprem-token") + require.NoError(t, UpdateConfigFile(DataRobotURL, DataRobotAPIKey)) + + viper.Set(DataRobotAPIKey, "rotated-onprem-token") + require.NoError(t, UpdateConfigFile()) + + raw := readRawYAML(t, configFile) + + assert.Equal(t, "default-token", raw[DataRobotAPIKey], "the default profile's token must be left alone") + + profiles, ok := raw["profiles"].(map[string]any) + require.True(t, ok) + + onprem, ok := profiles["onprem"].(map[string]any) + require.True(t, ok) + assert.Equal(t, "rotated-onprem-token", onprem[DataRobotAPIKey]) +} + func TestUpdateConfigFile_GlobalKeyWritesTopLevelUnderActiveProfile(t *testing.T) { configFile := setupWriteTest(t)