fix(ske): refine cluster config precedence

Author: Galin-Karabadzhakov

docs/stackit_ske_kubeconfig_login.md

@@ -8,6 +8,15 @@ Login plugin for kubernetes clients, that creates short-lived credentials to aut
 First you need to obtain a kubeconfig for use with the login command (first or second example).
 Secondly you use the kubeconfig with your chosen Kubernetes client (third example), the client will automatically retrieve the credentials via the STACKIT CLI.
 
+Project ID and region are resolved with the following precedence:
+
+| Explicit flag | Global config | Exec cluster config | Result |
+| --- | --- | --- | --- |
+| Not set | Set | Not set | Global config |
+| Set | Set | Not set | Explicit flag |
+| Not set | Set | Set | Exec cluster config |
+| Set | Set | Set | Explicit flag |
+
 ```
 stackit ske kubeconfig login [flags]
 ```

internal/cmd/ske/kubeconfig/login/login.go

@@ -59,7 +59,14 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
 		Long: fmt.Sprintf("%s\n%s\n%s",
 			"Login plugin for kubernetes clients, that creates short-lived credentials to authenticate against a STACKIT Kubernetes Engine (SKE) cluster.",
 			"First you need to obtain a kubeconfig for use with the login command (first or second example).",
-			"Secondly you use the kubeconfig with your chosen Kubernetes client (third example), the client will automatically retrieve the credentials via the STACKIT CLI.",
+			"Secondly you use the kubeconfig with your chosen Kubernetes client (third example), the client will automatically retrieve the credentials via the STACKIT CLI.\n\n"+
+				"Project ID and region are resolved with the following precedence:\n\n"+
+				"| Explicit flag | Global config | Exec cluster config | Result |\n"+
+				"| --- | --- | --- | --- |\n"+
+				"| Not set | Set | Not set | Global config |\n"+
+				"| Set | Set | Not set | Explicit flag |\n"+
+				"| Not set | Set | Set | Exec cluster config |\n"+
+				"| Set | Set | Set | Explicit flag |",
 		),
 		Args: args.NoArgs,
 		Example: examples.Build(
@@ -195,10 +202,10 @@ func parseClusterConfig(p *print.Printer, cmd *cobra.Command, idpMode, workloadI
 		clusterConfig.OrganizationID = organizationID
 	}
 	globalFlags := globalflags.Parse(p, cmd)
-	if globalFlags.ProjectId != "" {
+	if flagWasExplicitlySet(cmd, globalflags.ProjectIdFlag) || clusterConfig.STACKITProjectID == "" {
 		clusterConfig.STACKITProjectID = globalFlags.ProjectId
 	}
-	if globalFlags.Region != "" {
+	if flagWasExplicitlySet(cmd, globalflags.RegionFlag) || clusterConfig.Region == "" {
 		clusterConfig.Region = globalFlags.Region
 	}
 
@@ -229,6 +236,11 @@ func parseClusterConfig(p *print.Printer, cmd *cobra.Command, idpMode, workloadI
 	return clusterConfig, nil
 }
 
+func flagWasExplicitlySet(cmd *cobra.Command, name string) bool {
+	flag := cmd.Flag(name)
+	return flag != nil && flag.Changed
+}
+
 func loadExecCredentialFromEnv() (runtime.Object, error) {
 	execInfo := os.Getenv("KUBERNETES_EXEC_INFO")
 	if execInfo == "" {

internal/cmd/ske/kubeconfig/login/login_test.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/stackitcloud/stackit-cli/internal/pkg/auth"
 	"github.com/stackitcloud/stackit-cli/internal/pkg/config"
+	"github.com/stackitcloud/stackit-cli/internal/pkg/globalflags"
 	"github.com/stackitcloud/stackit-cli/internal/pkg/testparams"
 	"github.com/stackitcloud/stackit-cli/internal/pkg/utils"
 )
@@ -112,15 +113,104 @@ func TestParseClusterConfigWithoutExecClusterInfo(t *testing.T) {
 	}
 }
 
-func TestParseClusterConfigPrefersExplicitConfiguration(t *testing.T) {
+func TestParseClusterConfigPrecedence(t *testing.T) {
+	globalProjectID := uuid.NewString()
+	flagProjectID := uuid.NewString()
+	globalRegion := "global-region"
+	flagRegion := "flag-region"
+
+	tests := []struct {
+		name           string
+		clusterConfig  bool
+		explicitFlags  bool
+		expectedID     string
+		expectedRegion string
+	}{
+		{
+			name:           "global config without exec cluster config",
+			expectedID:     globalProjectID,
+			expectedRegion: globalRegion,
+		},
+		{
+			name:           "explicit flags without exec cluster config",
+			explicitFlags:  true,
+			expectedID:     flagProjectID,
+			expectedRegion: flagRegion,
+		},
+		{
+			name:           "exec cluster config overrides global config",
+			clusterConfig:  true,
+			expectedID:     testProjectId,
+			expectedRegion: testRegion,
+		},
+		{
+			name:           "explicit flags override exec cluster config",
+			clusterConfig:  true,
+			explicitFlags:  true,
+			expectedID:     flagProjectID,
+			expectedRegion: flagRegion,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			viper.Reset()
+			t.Cleanup(viper.Reset)
+			t.Setenv(envServiceAccountEmail, "workload@sa.stackit.cloud")
+
+			globalConfig := fmt.Sprintf(`{"project_id":%q,"region":%q}`, globalProjectID, globalRegion)
+			viper.SetConfigType("json")
+			if err := viper.ReadConfig(strings.NewReader(globalConfig)); err != nil {
+				t.Fatalf("read global config: %v", err)
+			}
+
+			execConfig := fixtureClusterConfig()
+			if !tt.clusterConfig {
+				execConfig.STACKITProjectID = ""
+				execConfig.Region = ""
+			}
+			configJSON, err := json.Marshal(execConfig)
+			if err != nil {
+				t.Fatalf("marshal cluster config: %v", err)
+			}
+			setExecCredentialEnv(t, &clientauthenticationv1.Cluster{
+				Server: "https://api.example.stackit.cloud",
+				Config: runtime.RawExtension{Raw: configJSON},
+			})
+
+			params := testparams.NewTestParams()
+			cmd := &cobra.Command{}
+			if err := globalflags.Configure(cmd.Flags()); err != nil {
+				t.Fatalf("configure global flags: %v", err)
+			}
+			configureFlags(cmd)
+			if tt.explicitFlags {
+				if err := cmd.Flags().Set(globalflags.ProjectIdFlag, flagProjectID); err != nil {
+					t.Fatalf("set project ID flag: %v", err)
+				}
+				if err := cmd.Flags().Set(globalflags.RegionFlag, flagRegion); err != nil {
+					t.Fatalf("set region flag: %v", err)
+				}
+			}
+
+			actual, err := parseClusterConfig(params.Printer, cmd, true, true, true)
+			if err != nil {
+				t.Fatalf("parse cluster config: %v", err)
+			}
+			expected := fixtureClusterConfig(func(config *clusterConfig) {
+				config.STACKITProjectID = tt.expectedID
+				config.Region = tt.expectedRegion
+			})
+			if diff := cmp.Diff(actual, expected, cmpopts.IgnoreFields(clusterConfig{}, "cacheKey")); diff != "" {
+				t.Fatalf("Data does not match: %s", diff)
+			}
+		})
+	}
+}
+
+func TestParseClusterConfigPrefersExplicitClusterIdentity(t *testing.T) {
 	viper.Reset()
 	t.Cleanup(viper.Reset)
-	explicitProjectID := uuid.NewString()
-	explicitRegion := "explicit-region"
-	explicitClusterName := "explicit-cluster"
-	explicitOrganizationID := uuid.NewString()
-	viper.Set(config.ProjectIdKey, explicitProjectID)
-	viper.Set(config.RegionKey, explicitRegion)
 	t.Setenv(envServiceAccountEmail, "workload@sa.stackit.cloud")
 
 	configJSON, err := json.Marshal(fixtureClusterConfig())
@@ -132,6 +222,8 @@ func TestParseClusterConfigPrefersExplicitConfiguration(t *testing.T) {
 		Config: runtime.RawExtension{Raw: configJSON},
 	})
 
+	explicitClusterName := "explicit-cluster"
+	explicitOrganizationID := uuid.NewString()
 	params := testparams.NewTestParams()
 	cmd := &cobra.Command{}
 	configureFlags(cmd)
@@ -141,13 +233,12 @@ func TestParseClusterConfigPrefersExplicitConfiguration(t *testing.T) {
 	if err := cmd.Flags().Set(organizationFlag, explicitOrganizationID); err != nil {
 		t.Fatalf("set organization flag: %v", err)
 	}
+
 	actual, err := parseClusterConfig(params.Printer, cmd, true, true, true)
 	if err != nil {
 		t.Fatalf("parse cluster config: %v", err)
 	}
 	expected := fixtureClusterConfig(func(config *clusterConfig) {
-		config.STACKITProjectID = explicitProjectID
-		config.Region = explicitRegion
 		config.ClusterName = explicitClusterName
 		config.OrganizationID = explicitOrganizationID
 	})