Skip to content

Fix for 'oc login should avoid clobbering kubeconfig contexts #283' #2185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pietergobin wants to merge 1 commit into from
Closed

Fix for 'oc login should avoid clobbering kubeconfig contexts #283' #2185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/cli/login/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func NewCmdLogin(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.
cmds.Flags().BoolVarP(&o.WebLogin, "web", "w", o.WebLogin, "Login with web browser. Starts a local HTTP callback server to perform the OAuth2 Authorization Code Grant flow. Use with caution on multi-user systems, as the server's port will be open to all users.")
cmds.Flags().Int32VarP(&o.CallbackPort, "callback-port", "c", o.CallbackPort, "Port for the callback server when using --web. Defaults to a random open port")

cmds.Flags().BoolVar(&o.KeepCtxName, "no-clobber", o.KeepCtxName, "Experimental: Keep the current context name in the kubeconfig file.")
Copy link
Member

@ardaguclu ardaguclu Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for opening a PR to fix this. But we would not prefer a new flag for this functionality.

cmds.Flags().StringVar(&o.OIDCExecPluginType, "exec-plugin", o.OIDCExecPluginType, "Experimental: Specify credentials exec plugin type to be used to authenticate external OIDC issuer. Currently only 'oc-oidc' is supported")
cmds.Flags().StringVar(&o.OIDCClientID, "client-id", o.OIDCClientID, "Experimental: Client ID for external OIDC issuer. Only supports Auth Code + PKCE. Required.")
cmds.Flags().StringVar(&o.OIDCClientSecret, "client-secret", o.OIDCClientSecret, "Experimental: Client secret for external OIDC issuer. Optional.")
Expand Down
7 changes: 7 additions & 0 deletions pkg/cli/login/loginoptions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/client-go/pkg/apis/clientauthentication"
restclient "k8s.io/client-go/rest"
kclientcmd "k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/clientcmd/api"
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find the file
find . -name "loginoptions.go" -type f

Repository: openshift/oc

Length of output: 87

🏁 Script executed:

# Read the imports section (first ~50 lines) to see the imports
head -50 ./pkg/cli/login/loginoptions.go | cat -n

Repository: openshift/oc

Length of output: 1864

🏁 Script executed:

# Check the usage around line 562-565 to see if api.Context is being used
sed -n '555,570p' ./pkg/cli/login/loginoptions.go | cat -n

Repository: openshift/oc

Length of output: 623

🏁 Script executed:

# Let's verify if this file can compile by checking if there are any compilation errors
# Try to get the full file to see the context better
wc -l ./pkg/cli/login/loginoptions.go

Repository: openshift/oc

Length of output: 91

🏁 Script executed:

# Let's verify the complete imports and usage context
# Check if there's a go.mod file and try to understand the module structure
head -n 100 ./pkg/cli/login/loginoptions.go | tail -n 50

Repository: openshift/oc

Length of output: 1338

🏁 Script executed:

# Search for usage of "api." in the file to see what's using the non-aliased import
grep -n "api\." ./pkg/cli/login/loginoptions.go

Repository: openshift/oc

Length of output: 506

🏁 Script executed:

# Let's verify this is indeed a Go compilation error by checking if there's a way to test this
# Search for any other uses of the non-aliased "api" that might be imported
grep -n " api\." ./pkg/cli/login/loginoptions.go | head -20

Repository: openshift/oc

Length of output: 38

🏁 Script executed:

# Let's search for the exact error by looking at the grep output more carefully
# Check if line 564 specifically uses "api.Context"
sed -n '564p' ./pkg/cli/login/loginoptions.go | cat -n

Repository: openshift/oc

Length of output: 158

🏁 Script executed:

# Verify: Is there any use of just "api." (not kclientcmdapi.) in the file?
grep -E '\bapi\.[A-Z]' ./pkg/cli/login/loginoptions.go

Repository: openshift/oc

Length of output: 151

Duplicate import will cause compilation failure.

Lines 33-34 both import k8s.io/client-go/tools/clientcmd/api—line 33 without an alias and line 34 aliased as kclientcmdapi. Go forbids the same package path twice. Remove the unaliased import on line 33 and update line 564 to use the existing alias.

🛠️ Suggested fix 
-	"k8s.io/client-go/tools/clientcmd/api"
@@
-		newConfig.Contexts = map[string]*api.Context{o.StartingKubeConfig.CurrentContext: newContext}
+		newConfig.Contexts = map[string]*kclientcmdapi.Context{o.StartingKubeConfig.CurrentContext: newContext}
🤖 Prompt for AI Agents 
In `@pkg/cli/login/loginoptions.go` at line 33, Remove the duplicate unaliased
import of "k8s.io/client-go/tools/clientcmd/api" and keep the existing aliased
import kclientcmdapi; then update any code that references the package with the
unaliased name (e.g., occurrences of api.SomeType or api.SomeFunc around the
login options usage) to use the alias kclientcmdapi.SomeType /
kclientcmdapi.SomeFunc so the single aliased import is used consistently.

kclientcmdapi "k8s.io/client-go/tools/clientcmd/api"
"k8s.io/klog/v2"

Expand Down Expand Up @@ -70,6 +71,7 @@ type LoginOptions struct {
Project string
WebLogin bool
CallbackPort int32
KeepCtxName bool

// infra
StartingKubeConfig *kclientcmdapi.Config
Expand Down Expand Up @@ -557,6 +559,11 @@ func (o *LoginOptions) SaveConfig() (bool, error) {
if err != nil {
return false, err
}
if o.KeepCtxName {
newContext := newConfig.Contexts[newConfig.CurrentContext]
Copy link

@vkrizan vkrizan Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this take the context from the --context global option, if provided; and default to current context?

Thanks!

Copy link

@vkrizan vkrizan Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..likely resolved by #2187

newConfig.Contexts = map[string]*api.Context{o.StartingKubeConfig.CurrentContext: newContext}
newConfig.CurrentContext = o.StartingKubeConfig.CurrentContext
Comment on lines +562 to +565
Copy link

@coderabbitai coderabbitai bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard --no-clobber when there is no current context.

If StartingKubeConfig.CurrentContext is empty (e.g., first-time login), this block sets CurrentContext to empty and writes a kubeconfig with no active context. Gate the remap to only run when a current context exists.

🛠️ Suggested fix 
-	if o.KeepCtxName {
+	if o.KeepCtxName && o.StartingKubeConfig != nil && o.StartingKubeConfig.CurrentContext != "" {
 		newContext := newConfig.Contexts[newConfig.CurrentContext]
 		newConfig.Contexts = map[string]*api.Context{o.StartingKubeConfig.CurrentContext: newContext}
 		newConfig.CurrentContext = o.StartingKubeConfig.CurrentContext
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if o.KeepCtxName {
newContext := newConfig.Contexts[newConfig.CurrentContext]
newConfig.Contexts = map[string]*api.Context{o.StartingKubeConfig.CurrentContext: newContext}
newConfig.CurrentContext = o.StartingKubeConfig.CurrentContext
if o.KeepCtxName && o.StartingKubeConfig != nil && o.StartingKubeConfig.CurrentContext != "" {
newContext := newConfig.Contexts[newConfig.CurrentContext]
newConfig.Contexts = map[string]*api.Context{o.StartingKubeConfig.CurrentContext: newContext}
newConfig.CurrentContext = o.StartingKubeConfig.CurrentContext
}
🤖 Prompt for AI Agents 
In `@pkg/cli/login/loginoptions.go` around lines 562 - 565, The current remap
under KeepCtxName runs even when o.StartingKubeConfig.CurrentContext is empty,
causing newConfig.CurrentContext to be set to an empty string; change the block
in the KeepCtxName branch of loginoptions (the code manipulating
newConfig.Contexts and newConfig.CurrentContext) to first check that
o.StartingKubeConfig.CurrentContext is non-empty before performing the remap —
only assign newContext into newConfig.Contexts and set newConfig.CurrentContext
when o.StartingKubeConfig.CurrentContext != "".

}

cwd, err := os.Getwd()
if err != nil {
Expand Down