EPMDEDP-16652: test: Add comprehensive unit tests for login cluster config

Refactored deeply nested cluster config population logic into separate
functions (populateClusterConfig, fetchClusterMetadata) for improved
testability and maintainability. Fixes regression where namespace warning
was swallowed by early returns on errors.

Signed-off-by: Sergiy Kulanov <sergiy_kulanov@epam.com>

Author: SergK

README.md

@@ -192,50 +192,34 @@ port 3001 directly.
 make build    # → dist/krci
 ```
 
-### Step 1 — Login
+### Login
 
-The portal runs over HTTP, so the CLI cannot auto-discover the OIDC issuer.
-Pass it explicitly via `KRCI_ISSUER_URL` (copy the value from the portal's
-`OIDC_ISSUER_URL` in `.env` or from its startup output):
+The portal runs over HTTP, so the CLI cannot auto-discover the OIDC issuer or
+cluster metadata. Supply all values upfront via environment variables (copy
+`OIDC_ISSUER_URL` from the portal's `.env` or its startup output):
 
 ```bash
 KRCI_ISSUER_URL=https://idp.example.com/realms/my-realm \
+KRCI_CLUSTER_NAME=my-cluster \
+KRCI_NAMESPACE=my-namespace \
   ./dist/krci auth login --portal-url http://localhost:3001
 ```
 
-A browser window opens for authentication. On success you'll see:
+A browser window opens for authentication. On success:
 
 ```
 Logged in as user@example.com (User Name)
-Warning: could not fetch cluster config: portal URL must use HTTPS ...
-Warning: namespace not configured; set KRCI_NAMESPACE or re-run login
 ```
 
-These warnings are expected — the CLI cannot fetch cluster metadata over HTTP.
-The portal URL and issuer are saved to `~/.config/krci/config.yaml`, so you
-won't need `--portal-url` on subsequent commands.
-
-### Step 2 — Run commands
-
-Supply the cluster name and namespace that would normally be auto-discovered.
-These are required on every command:
-
-```bash
-export KRCI_CLUSTER_NAME=my-cluster
-export KRCI_NAMESPACE=my-namespace
-```
-
-Then use the CLI normally:
+All values are saved to `~/.config/krci/config.yaml`, so subsequent commands
+need no flags or environment variables:
 
 ```bash
 ./dist/krci project list
 ./dist/krci deployment list
 ./dist/krci pipelinerun list --project my-app --reason
 ```
 
-> **Tip:** add the exports to a local `.envrc` (direnv) or a shell alias to
-> avoid repeating them.
-
 ## Prerequisites
 
 - Go 1.26+ (for building from source)

internal/config/config.go

@@ -89,8 +89,9 @@ func Resolve() (*Config, error) {
 	return &cfg, nil
 }
 
-// Save persists user-provided values to the config file.
-// Only writes non-empty values so defaults and env vars are not baked in.
+// Save persists resolved configuration values to the config file.
+// Non-empty values (including those resolved from env vars) are written
+// so they survive across sessions without requiring env vars again.
 func Save(cfg *Config) error {
 	configDir := cfg.ConfigDir
 	if err := os.MkdirAll(configDir, 0700); err != nil {

pkg/cmd/auth/login/login.go

@@ -2,8 +2,10 @@
 package login
 
 import (
+	"context"
 	"errors"
 	"fmt"
+	"io"
 	"regexp"
 
 	"github.com/spf13/cobra"
@@ -18,6 +20,9 @@ import (
 // dns1123LabelRegexp validates a Kubernetes namespace name.
 var dns1123LabelRegexp = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`)
 
+// clusterFetchFunc fetches cluster configuration from a portal URL using a bearer token.
+type clusterFetchFunc func(portalURL, token string) (*portal.ClusterConfig, error)
+
 // LoginOptions holds all inputs for the login command.
 type LoginOptions struct {
 	IO            *iostreams.IOStreams
@@ -107,39 +112,60 @@ func loginRun(cmd *cobra.Command, opts *LoginOptions) error {
 	// Clone cfg so Save() writes only the fields we intend to persist.
 	cfgCopy := *cfg
 
-	// Fetch cluster config (authenticated) to auto-populate cluster name and namespace.
-	tok, err := tp.GetToken(cmd.Context())
-	if err != nil {
-		_, _ = fmt.Fprintf(opts.IO.ErrOut, "Warning: could not get token for cluster config: %v\n", err)
-	} else {
-		clusterCfg, err := portal.FetchClusterConfig(cfg.PortalURL, tok)
-		if err != nil {
-			_, _ = fmt.Fprintf(opts.IO.ErrOut, "Warning: could not fetch cluster config: %v\n", err)
-		} else {
-			if cfgCopy.ClusterName == "" && clusterCfg.ClusterName != "" {
-				cfgCopy.ClusterName = clusterCfg.ClusterName
-			}
+	populateClusterConfig(cmd.Context(), tp, &cfgCopy, opts.IO.ErrOut, portal.FetchClusterConfig)
 
-			if cfgCopy.Namespace == "" && clusterCfg.DefaultNamespace != "" {
-				if dns1123LabelRegexp.MatchString(clusterCfg.DefaultNamespace) {
-					cfgCopy.Namespace = clusterCfg.DefaultNamespace
-					_, _ = fmt.Fprintf(opts.IO.ErrOut, "Namespace: %s (from portal)\n", cfgCopy.Namespace)
-				} else {
-					_, _ = fmt.Fprintf(opts.IO.ErrOut,
-						"Warning: portal returned invalid namespace %q, ignoring\n", clusterCfg.DefaultNamespace)
-				}
-			}
-		}
+	if err := config.Save(&cfgCopy); err != nil {
+		_, _ = fmt.Fprintf(opts.IO.ErrOut, "Warning: could not save config: %v\n", err)
+	}
+
+	return nil
+}
+
+// populateClusterConfig fetches cluster name and namespace from the portal
+// when they are not already provided via env vars or config file.
+func populateClusterConfig(
+	ctx context.Context, tp auth.TokenProvider, cfg *config.Config,
+	errOut io.Writer, fetch clusterFetchFunc,
+) {
+	if cfg.ClusterName != "" && cfg.Namespace != "" {
+		return
 	}
 
-	if cfgCopy.Namespace == "" {
-		_, _ = fmt.Fprintf(opts.IO.ErrOut,
+	fetchClusterMetadata(ctx, tp, cfg, errOut, fetch)
+
+	if cfg.Namespace == "" {
+		_, _ = fmt.Fprintf(errOut,
 			"Warning: namespace not configured; set KRCI_NAMESPACE or re-run login\n")
 	}
+}
 
-	if err := config.Save(&cfgCopy); err != nil {
-		_, _ = fmt.Fprintf(opts.IO.ErrOut, "Warning: could not save config: %v\n", err)
+func fetchClusterMetadata(
+	ctx context.Context, tp auth.TokenProvider, cfg *config.Config,
+	errOut io.Writer, fetch clusterFetchFunc,
+) {
+	tok, err := tp.GetToken(ctx)
+	if err != nil {
+		_, _ = fmt.Fprintf(errOut, "Warning: could not get token for cluster config: %v\n", err)
+		return
 	}
 
-	return nil
+	clusterCfg, err := fetch(cfg.PortalURL, tok)
+	if err != nil {
+		_, _ = fmt.Fprintf(errOut, "Warning: could not fetch cluster config: %v\n", err)
+		return
+	}
+
+	if cfg.ClusterName == "" && clusterCfg.ClusterName != "" {
+		cfg.ClusterName = clusterCfg.ClusterName
+	}
+
+	if cfg.Namespace == "" && clusterCfg.DefaultNamespace != "" {
+		if dns1123LabelRegexp.MatchString(clusterCfg.DefaultNamespace) {
+			cfg.Namespace = clusterCfg.DefaultNamespace
+			_, _ = fmt.Fprintf(errOut, "Namespace: %s (from portal)\n", cfg.Namespace)
+		} else {
+			_, _ = fmt.Fprintf(errOut,
+				"Warning: portal returned invalid namespace %q, ignoring\n", clusterCfg.DefaultNamespace)
+		}
+	}
 }

pkg/cmd/auth/login/login_test.go

@@ -0,0 +1,148 @@
+package login
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/KubeRocketCI/cli/internal/auth"
+	"github.com/KubeRocketCI/cli/internal/config"
+	"github.com/KubeRocketCI/cli/internal/portal"
+)
+
+// mockTokenProvider implements auth.TokenProvider for testing.
+type mockTokenProvider struct {
+	token    string
+	tokenErr error
+}
+
+func (m *mockTokenProvider) GetToken(_ context.Context) (string, error) {
+	return m.token, m.tokenErr
+}
+
+func (m *mockTokenProvider) Login(_ context.Context) error     { return nil }
+func (m *mockTokenProvider) Logout() error                     { return nil }
+func (m *mockTokenProvider) UserInfo() (*auth.UserInfo, error) { return nil, nil }
+
+func TestPopulateClusterConfig(t *testing.T) {
+	t.Parallel()
+
+	okFetcher := func(cluster, ns string) clusterFetchFunc {
+		return func(_, _ string) (*portal.ClusterConfig, error) {
+			return &portal.ClusterConfig{ClusterName: cluster, DefaultNamespace: ns}, nil
+		}
+	}
+
+	errFetcher := func(_, _ string) (*portal.ClusterConfig, error) {
+		return nil, errors.New("connection refused")
+	}
+
+	tests := []struct {
+		name         string
+		cfg          config.Config
+		tp           auth.TokenProvider
+		fetcher      clusterFetchFunc
+		wantCluster  string
+		wantNS       string
+		wantOutput   []string
+		wantNoOutput []string
+	}{
+		{
+			name: "both already set — skips fetch",
+			cfg: config.Config{
+				ClusterName: "existing",
+				Namespace:   "existing-ns",
+			},
+			tp:           &mockTokenProvider{token: "tok"},
+			fetcher:      errFetcher,
+			wantCluster:  "existing",
+			wantNS:       "existing-ns",
+			wantNoOutput: []string{"Warning"},
+		},
+		{
+			name:        "fetch succeeds — populates both",
+			cfg:         config.Config{PortalURL: "https://portal.test"},
+			tp:          &mockTokenProvider{token: "tok"},
+			fetcher:     okFetcher("discovered", "discovered-ns"),
+			wantCluster: "discovered",
+			wantNS:      "discovered-ns",
+			wantOutput:  []string{"Namespace: discovered-ns (from portal)"},
+		},
+		{
+			name:        "fetch succeeds — does not overwrite existing cluster name",
+			cfg:         config.Config{PortalURL: "https://portal.test", ClusterName: "existing"},
+			tp:          &mockTokenProvider{token: "tok"},
+			fetcher:     okFetcher("discovered", "discovered-ns"),
+			wantCluster: "existing",
+			wantNS:      "discovered-ns",
+		},
+		{
+			name:        "fetch succeeds — does not overwrite existing namespace",
+			cfg:         config.Config{PortalURL: "https://portal.test", Namespace: "existing-ns"},
+			tp:          &mockTokenProvider{token: "tok"},
+			fetcher:     okFetcher("discovered", "other-ns"),
+			wantCluster: "discovered",
+			wantNS:      "existing-ns",
+		},
+		{
+			name:        "token error — warns and emits namespace warning",
+			cfg:         config.Config{PortalURL: "https://portal.test"},
+			tp:          &mockTokenProvider{tokenErr: errors.New("token expired")},
+			fetcher:     errFetcher,
+			wantCluster: "",
+			wantNS:      "",
+			wantOutput: []string{
+				"Warning: could not get token",
+				"Warning: namespace not configured",
+			},
+		},
+		{
+			name:        "fetch error — warns and emits namespace warning",
+			cfg:         config.Config{PortalURL: "https://portal.test"},
+			tp:          &mockTokenProvider{token: "tok"},
+			fetcher:     errFetcher,
+			wantCluster: "",
+			wantNS:      "",
+			wantOutput: []string{
+				"Warning: could not fetch cluster config",
+				"Warning: namespace not configured",
+			},
+		},
+		{
+			name:        "invalid namespace from portal — warns",
+			cfg:         config.Config{PortalURL: "https://portal.test"},
+			tp:          &mockTokenProvider{token: "tok"},
+			fetcher:     okFetcher("c", "INVALID NS!"),
+			wantCluster: "c",
+			wantNS:      "",
+			wantOutput: []string{
+				"Warning: portal returned invalid namespace",
+				"Warning: namespace not configured",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			cfg := tt.cfg
+			var buf bytes.Buffer
+			populateClusterConfig(context.Background(), tt.tp, &cfg, &buf, tt.fetcher)
+
+			assert.Equal(t, tt.wantCluster, cfg.ClusterName, "ClusterName")
+			assert.Equal(t, tt.wantNS, cfg.Namespace, "Namespace")
+
+			output := buf.String()
+			for _, want := range tt.wantOutput {
+				assert.Contains(t, output, want)
+			}
+			for _, notWant := range tt.wantNoOutput {
+				assert.NotContains(t, output, notWant)
+			}
+		})
+	}
+}