Skip to content

[kubectl-plugin][Refactor] Standardize namespace resolution to respect kubeconfig context. #4367

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

Open
kash2104 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[kubectl-plugin][Refactor] Standardize namespace resolution to respect kubeconfig context. #4367

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
11 changes: 5 additions & 6 deletions kubectl-plugin/pkg/cmd/create/create_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func NewCreateClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptio
}

func (options *CreateClusterOptions) Complete(cmd *cobra.Command, args []string) error {
namespace, err := cmd.Flags().GetString("namespace")
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
if err != nil {
return fmt.Errorf("failed to get namespace: %w", err)
}
Expand Down Expand Up @@ -241,7 +241,10 @@ func (options *CreateClusterOptions) resolveClusterName() (string, error) {

// resolveNamespace resolves the namespace from the CLI flag and the config file
func (options *CreateClusterOptions) resolveNamespace() (string, error) {
namespace := "default"
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
if err != nil {
return "", fmt.Errorf("failed to get current namespace: %w", err)
}
Copy link

@cursor cursor bot Jan 12, 2026

Choose a reason for hiding this comment

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

Config file namespace ignored due to always-set options.namespace

High Severity

The resolveNamespace() function can no longer use the namespace from a config file. Previously, cmd.Flags().GetString("namespace") returned an empty string when the user didn't pass --namespace, allowing the config file namespace to be used. Now, ToRawKubeConfigLoader().Namespace() always returns a value (at minimum "default"), so options.namespace is never empty. This causes the condition if options.namespace != "" to always be true, making the else if branch that uses rayClusterConfig.Namespace unreachable. Additionally, the error check at line 249 will now incorrectly fail when a config file specifies a namespace different from the kubeconfig default.

Additional Locations (2)

Fix in Cursor Fix in Web

Copy link
Contributor

@400Ping 400Ping Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Agree, and I think you should add test to check this

Copy link
Collaborator

@fscnick fscnick Feb 7, 2026

Choose a reason for hiding this comment

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

There are some different behavior if the namespace is specified in config file, --file ray-cluster.yaml.

before applying this pr:

Image

after applying this pr:

Image


if options.rayClusterConfig.Namespace != nil && *options.rayClusterConfig.Namespace != "" && options.namespace != "" && options.namespace != *options.rayClusterConfig.Namespace {
return "", fmt.Errorf("the namespace in the config file %q does not match the namespace %q. You must pass --namespace=%s to perform this operation", *options.rayClusterConfig.Namespace, options.namespace, *options.rayClusterConfig.Namespace)
Expand Down Expand Up @@ -274,10 +277,6 @@ func (options *CreateClusterOptions) Run(ctx context.Context, k8sClient client.C
}
options.rayClusterConfig.Namespace = &namespace
} else {
if options.namespace == "" {
options.namespace = "default"
}

options.rayClusterConfig = &generation.RayClusterConfig{
Namespace: &options.namespace,
Name: &options.clusterName,
Expand Down
91 changes: 70 additions & 21 deletions kubectl-plugin/pkg/cmd/create/create_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/cli-runtime/pkg/genericclioptions"
kubefake "k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/tools/clientcmd/api"
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"k8s.io/utils/ptr"

Expand All @@ -23,48 +25,94 @@ import (
rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"
)

func createTempKubeConfigFile(t *testing.T, currentNamespace string) (string, error) {
tmpDir := t.TempDir()

// Set up fake config for kubeconfig
config := &api.Config{
Clusters: map[string]*api.Cluster{
"test-cluster": {
Server: "https://fake-kubernetes-cluster.example.com",
InsecureSkipTLSVerify: true, // For testing purposes
},
},
Contexts: map[string]*api.Context{
"test-context": {
Cluster: "test-cluster",
AuthInfo: "my-fake-user",
Namespace: currentNamespace,
},
},
CurrentContext: "test-context",
AuthInfos: map[string]*api.AuthInfo{
"my-fake-user": {
Token: "", // Empty for testing without authentication
},
},
}

fakeFile := filepath.Join(tmpDir, ".kubeconfig")

return fakeFile, clientcmd.WriteToFile(*config, fakeFile)
}

func TestRayCreateClusterComplete(t *testing.T) {
kubeConfigWithCurrentContext, err := createTempKubeConfigFile(t, "test-namespace")
require.NoError(t, err)
testStreams, _, _, _ := genericclioptions.NewTestIOStreams()

tests := map[string]struct {
image string
rayVersion string
expectedError string
expectedImage string
args []string
image string
namespace string
rayVersion string
expectedError string
expectedImage string
expectedNamespace string
args []string
}{
"should error when there are no args": {
args: []string{},
expectedError: "See 'cluster -h' for help and examples",
args: []string{},
expectedError: "See 'cluster -h' for help and examples",
expectedNamespace: "test-namespace",
},
"should error when too many args": {
args: []string{"testRayClusterName", "extra-arg"},
expectedError: "See 'cluster -h' for help and examples",
args: []string{"testRayClusterName", "extra-arg"},
expectedError: "See 'cluster -h' for help and examples",
expectedNamespace: "test-namespace",
},
"should succeed with default image when no image is specified": {
args: []string{"testRayClusterName"},
rayVersion: util.RayVersion,
expectedImage: defaultImageWithTag,
args: []string{"testRayClusterName"},
rayVersion: util.RayVersion,
expectedImage: defaultImageWithTag,
expectedNamespace: "test-namespace",
},
"should succeed with provided image when provided": {
args: []string{"testRayClusterName"},
image: "DEADBEEF",
expectedImage: "DEADBEEF",
args: []string{"testRayClusterName"},
image: "DEADBEEF",
expectedImage: "DEADBEEF",
expectedNamespace: "test-namespace",
},
"should set the image to the same version as the ray version when the image is the default and the ray version is not the default": {
args: []string{"testRayClusterName"},
image: defaultImageWithTag,
rayVersion: "2.52.0",
expectedImage: fmt.Sprintf("%s:2.52.0", defaultImage),
args: []string{"testRayClusterName"},
image: defaultImageWithTag,
namespace: "foo",
rayVersion: "2.52.0",
expectedImage: fmt.Sprintf("%s:2.52.0", defaultImage),
expectedNamespace: "foo",
},
}

for name, tc := range tests {
Copy link
Collaborator

@fscnick fscnick Jan 29, 2026

Choose a reason for hiding this comment

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

Would it be possible to add a test case that respects the namespace in kubeconfig?

t.Run(name, func(t *testing.T) {
cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true))
configFlags := &genericclioptions.ConfigFlags{KubeConfig: &kubeConfigWithCurrentContext}
if tc.namespace != "" {
configFlags.Namespace = &tc.namespace
}

cmdFactory := cmdutil.NewFactory(configFlags)
fakeCreateClusterOptions := NewCreateClusterOptions(cmdFactory, testStreams)
cmd := &cobra.Command{Use: "cluster"}
cmd.Flags().StringVarP(&fakeCreateClusterOptions.namespace, "namespace", "n", "", "")
configFlags.AddFlags(cmd.Flags())
fakeCreateClusterOptions.rayVersion = tc.rayVersion

if tc.image != "" {
Expand All @@ -78,6 +126,7 @@ func TestRayCreateClusterComplete(t *testing.T) {
} else {
require.NoError(t, err)
require.Equal(t, tc.expectedImage, fakeCreateClusterOptions.image)
require.Equal(t, tc.expectedNamespace, fakeCreateClusterOptions.namespace)
}
})
}
Expand Down
6 changes: 1 addition & 5 deletions kubectl-plugin/pkg/cmd/create/create_workergroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,12 @@ func NewCreateWorkerGroupCommand(cmdFactory cmdutil.Factory, streams genericclio
}

func (options *CreateWorkerGroupOptions) Complete(cmd *cobra.Command, args []string) error {
namespace, err := cmd.Flags().GetString("namespace")
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
if err != nil {
return fmt.Errorf("failed to get namespace: %w", err)
}
options.namespace = namespace

if options.namespace == "" {
options.namespace = "default"
}

if options.rayStartParams == nil {
options.rayStartParams = map[string]string{}
}
Expand Down
31 changes: 26 additions & 5 deletions kubectl-plugin/pkg/cmd/create/create_workergroup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/cli-runtime/pkg/genericclioptions"
cmdutil "k8s.io/kubectl/pkg/cmd/util"
"k8s.io/utils/ptr"

"github.com/ray-project/kuberay/kubectl-plugin/pkg/util"
Expand Down Expand Up @@ -104,10 +106,17 @@ func TestCreateWorkerGroupCommandComplete(t *testing.T) {
},
},
{
name: "Valid input without namespace flag",
args: []string{"example-group"},
flags: map[string]string{},
expectedError: "failed to get namespace: flag accessed but not defined: namespace",
name: "Valid input without namespace flag",
args: []string{"example-group"},
flags: map[string]string{
"namespace": "",
},
Comment on lines +111 to +113
Copy link
Collaborator

@fscnick fscnick Feb 7, 2026

Choose a reason for hiding this comment

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

Why should it add an empty namespace flag? It seems a bit of different from the description above.

expected: &CreateWorkerGroupOptions{
namespace: "default",
groupName: "example-group",
image: "rayproject/ray:latest",
rayVersion: "latest",
},
},
{
name: "mMissing group name",
Expand Down Expand Up @@ -181,12 +190,24 @@ func TestCreateWorkerGroupCommandComplete(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
configFlags := genericclioptions.NewConfigFlags(true)
cmd := &cobra.Command{}
configFlags.AddFlags(cmd.Flags())
cmd.Flags().String(
"worker-ray-start-params",
"",
"ray start parameters for worker",
)
for key, value := range tt.flags {
cmd.Flags().String(key, value, "")
err := cmd.Flags().Set(key, value)
if err != nil {
require.NoError(t, err)
}
}

factory := cmdutil.NewFactory(configFlags)
options := &CreateWorkerGroupOptions{
cmdFactory: factory,
rayVersion: "latest",
}

Expand Down
5 changes: 1 addition & 4 deletions kubectl-plugin/pkg/cmd/delete/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,11 @@ func NewDeleteCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IOSt
}

func (options *DeleteOptions) Complete(cmd *cobra.Command, args []string) error {
namespace, err := cmd.Flags().GetString("namespace")
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
if err != nil {
return fmt.Errorf("failed to get namespace: %w", err)
}
options.namespace = namespace
if options.namespace == "" {
options.namespace = "default"
}

if options.resources == nil {
options.resources = map[util.ResourceType][]string{}
Expand Down
11 changes: 8 additions & 3 deletions kubectl-plugin/pkg/cmd/delete/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (

func TestComplete(t *testing.T) {
testStreams, _, _, _ := genericclioptions.NewTestIOStreams()
cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true))

tests := []struct {
name string
Expand Down Expand Up @@ -102,10 +101,16 @@ func TestComplete(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
fakeDeleteOptions := NewDeleteOptions(cmdFactory, testStreams)
configFlags := genericclioptions.NewConfigFlags(true)
if tc.namespace != "" {
configFlags.Namespace = &tc.namespace
}
cmdFactory := cmdutil.NewFactory(configFlags)

fakeDeleteOptions := NewDeleteOptions(cmdFactory, testStreams)
cmd := &cobra.Command{}
cmd.Flags().StringVarP(&fakeDeleteOptions.namespace, "namespace", "n", tc.namespace, "")
flags := cmd.Flags()
configFlags.AddFlags(flags)

err := fakeDeleteOptions.Complete(cmd, tc.args)

Expand Down
9 changes: 3 additions & 6 deletions kubectl-plugin/pkg/cmd/get/get_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewGetClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptions.
ValidArgsFunction: completion.RayClusterCompletionFunc(cmdFactory),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
if err := options.Complete(args, cmd); err != nil {
if err := options.Complete(args); err != nil {
return err
}
// running cmd.Execute or cmd.ExecuteE sets the context, which will be done by root
Expand All @@ -61,15 +61,12 @@ func NewGetClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptions.
return cmd
}

func (options *GetClusterOptions) Complete(args []string, cmd *cobra.Command) error {
namespace, err := cmd.Flags().GetString("namespace")
func (options *GetClusterOptions) Complete(args []string) error {
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
if err != nil {
return fmt.Errorf("failed to get namespace: %w", err)
}
options.namespace = namespace
if options.namespace == "" {
options.namespace = "default"
}

if len(args) >= 1 {
options.cluster = args[0]
Expand Down
Loading
Loading