Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pkg/kubernetes/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (c *Core) PodsRun(ctx context.Context, namespace, name, image string, port
}
toCreate = append(toCreate, u)
}
return c.resourcesCreateOrUpdate(ctx, toCreate)
return c.resourcesCreateOrUpdate(ctx, toCreate, "")
}

func (c *Core) PodsTop(ctx context.Context, options api.PodsTopOptions) (*metrics.PodMetricsList, error) {
Expand Down
11 changes: 8 additions & 3 deletions pkg/kubernetes/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (c *Core) ResourcesGet(ctx context.Context, gvk *schema.GroupVersionKind, n
return c.DynamicClient().Resource(*gvr).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
}

func (c *Core) ResourcesCreateOrUpdate(ctx context.Context, resource string) ([]*unstructured.Unstructured, error) {
func (c *Core) ResourcesCreateOrUpdate(ctx context.Context, resource string, namespaceOverride string) ([]*unstructured.Unstructured, error) {
separator := regexp.MustCompile(`\r?\n---\r?\n`)
resources := separator.Split(resource, -1)
var parsedResources []*unstructured.Unstructured
Expand All @@ -70,7 +70,7 @@ func (c *Core) ResourcesCreateOrUpdate(ctx context.Context, resource string) ([]

parsedResources = append(parsedResources, &obj)
}
return c.resourcesCreateOrUpdate(ctx, parsedResources)
return c.resourcesCreateOrUpdate(ctx, parsedResources, namespaceOverride)
}

func (c *Core) ResourcesDelete(ctx context.Context, gvk *schema.GroupVersionKind, namespace, name string, gracePeriodSeconds *int64) error {
Expand Down Expand Up @@ -177,7 +177,7 @@ func (c *Core) resourcesListAsTable(ctx context.Context, gvk *schema.GroupVersio
return &unstructured.Unstructured{Object: unstructuredObject}, err
}

func (c *Core) resourcesCreateOrUpdate(ctx context.Context, resources []*unstructured.Unstructured) ([]*unstructured.Unstructured, error) {
func (c *Core) resourcesCreateOrUpdate(ctx context.Context, resources []*unstructured.Unstructured, namespaceOverride string) ([]*unstructured.Unstructured, error) {
for i, obj := range resources {
gvk := obj.GroupVersionKind()
gvr, rErr := c.resourceFor(&gvk)
Expand All @@ -186,6 +186,11 @@ func (c *Core) resourcesCreateOrUpdate(ctx context.Context, resources []*unstruc
}

namespace := obj.GetNamespace()
// If a namespace override was provided, it takes precedence over the namespace in the resource metadata
if namespaceOverride != "" {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems to be an "override always wins silently" behavior 🤔

kubectl apply -n only sets the namespace for resources that don't already have one — it does not override an explicit metadata.namespace in the manifest.

When both -n <namespace> and metadata.namespace are set and they differ, kubectl returns an error, that the provided namespace does not match the passed in namespace

A more correct implementation would only apply the override when the resource has no namespace set (matching kubectl's behavior):

if namespaceOverride != "" && namespace != "" && namespaceOverride != namespace {
    return nil, fmt.Errorf("the namespace from the provided object %q does not match the namespace %q", namespace, namespaceOverride)
}
if namespaceOverride != "" && namespace == "" {
    namespace = namespaceOverride
    obj.SetNamespace(namespace)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps worth to bake this into NamespaceOrDefault logic, instead of nesting the code above?

namespace = namespaceOverride
obj.SetNamespace(namespace)
}
// If it's a namespaced resource and namespace wasn't provided, try to use the default configured one
if namespaced, nsErr := c.isNamespaced(&gvk); nsErr == nil && namespaced {
namespace = c.NamespaceOrDefault(namespace)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't this handle already the case that if the namespace is not provided, we fall back to default ?

Not sure on adding this parameter

Expand Down
61 changes: 61 additions & 0 deletions pkg/mcp/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,67 @@ func (s *ResourcesSuite) TestResourcesCreateOrUpdate() {
s.Falsef(hasStatus, "status should not be present on the persisted resource")
})
})

s.Run("resources_create_or_update with namespace override and no namespace in resource", func() {
// Resource YAML has no namespace in metadata — namespace override should be applied
configMapYaml := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: a-cm-ns-override\ndata:\n key: value\n"
result, err := s.CallTool("resources_create_or_update", map[string]interface{}{
"resource": configMapYaml,
"namespace": "default",
})
s.Run("returns success", func() {
s.Nilf(err, "call tool failed %v", err)
s.Falsef(result.IsError, "call tool should not fail, got: %v", result.Content)
})
s.Run("creates ConfigMap in the overridden namespace", func() {
cm, cmErr := client.CoreV1().ConfigMaps("default").Get(s.T().Context(), "a-cm-ns-override", metav1.GetOptions{})
s.Require().Nilf(cmErr, "ConfigMap not found in overridden namespace")
s.Equalf("default", cm.Namespace, "ConfigMap should be in the overridden namespace")
})
})

s.Run("resources_create_or_update with namespace override takes precedence over namespace in resource", func() {
// Resource YAML has namespace: default but override should win
configMapYaml := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: a-cm-ns-override-wins\n namespace: default\ndata:\n key: value\n"
result, err := s.CallTool("resources_create_or_update", map[string]interface{}{
"resource": configMapYaml,
"namespace": "kube-public",
})
s.Run("returns success", func() {
s.Nilf(err, "call tool failed %v", err)
s.Falsef(result.IsError, "call tool should not fail, got: %v", result.Content)
})
s.Run("creates ConfigMap in the overridden namespace not the one in the resource", func() {
cm, cmErr := client.CoreV1().ConfigMaps("kube-public").Get(s.T().Context(), "a-cm-ns-override-wins", metav1.GetOptions{})
s.Require().Nilf(cmErr, "ConfigMap not found in overridden namespace")
s.Equalf("kube-public", cm.Namespace, "ConfigMap should be in the overridden namespace, not the one in the resource metadata")
})
s.Run("does not create ConfigMap in the namespace from the resource metadata", func() {
_, cmErr := client.CoreV1().ConfigMaps("default").Get(s.T().Context(), "a-cm-ns-override-wins", metav1.GetOptions{})
s.Errorf(cmErr, "ConfigMap should not exist in the namespace from the resource metadata")
})
})

s.Run("resources_create_or_update with namespace override applies to all resources in multi-document YAML", func() {
multiDocYaml := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: a-cm-multi-1\ndata:\n key: value1\n\n---\n\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: a-cm-multi-2\ndata:\n key: value2\n"
result, err := s.CallTool("resources_create_or_update", map[string]interface{}{
"resource": multiDocYaml,
"namespace": "default",
})
s.Run("returns success", func() {
s.Nilf(err, "call tool failed %v", err)
s.Falsef(result.IsError, "call tool should not fail, got: %v", result.Content)
})
s.Run("creates all ConfigMaps in the overridden namespace", func() {
cm1, cm1Err := client.CoreV1().ConfigMaps("default").Get(s.T().Context(), "a-cm-multi-1", metav1.GetOptions{})
s.Require().Nilf(cm1Err, "first ConfigMap not found in overridden namespace")
s.Equalf("default", cm1.Namespace, "first ConfigMap should be in the overridden namespace")

cm2, cm2Err := client.CoreV1().ConfigMaps("default").Get(s.T().Context(), "a-cm-multi-2", metav1.GetOptions{})
s.Require().Nilf(cm2Err, "second ConfigMap not found in overridden namespace")
s.Equalf("default", cm2.Namespace, "second ConfigMap should be in the overridden namespace")
})
})
}

func (s *ResourcesSuite) TestResourcesCreateOrUpdateForcesSSA() {
Expand Down
14 changes: 13 additions & 1 deletion pkg/toolsets/core/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ func initResources(o api.Openshift) []api.ServerTool {
Type: "string",
Description: "A JSON or YAML containing a representation of the Kubernetes resource. Should include top-level fields such as apiVersion,kind,metadata, and spec",
},
"namespace": {
Type: "string",
Description: "Optional namespace to apply the resource(s) to. Overrides the namespace defined in the resource metadata. Useful when resources are rendered without a hardcoded namespace (similar to kubectl apply -n <namespace>)",
},
},
Required: []string{"resource"},
},
Expand Down Expand Up @@ -270,7 +274,15 @@ func resourcesCreateOrUpdate(params api.ToolHandlerParams) (*api.ToolCallResult,
return api.NewToolCallResult("", fmt.Errorf("resource is not a string")), nil
}

resources, err := kubernetes.NewCore(params).ResourcesCreateOrUpdate(params, r)
ns := ""
if namespace := params.GetArguments()["namespace"]; namespace != nil {
ns, ok = namespace.(string)
if !ok {
return api.NewToolCallResult("", fmt.Errorf("namespace is not a string")), nil
}
}

resources, err := kubernetes.NewCore(params).ResourcesCreateOrUpdate(params, r, ns)
if err != nil {
return api.NewToolCallResult("", fmt.Errorf("failed to create or update resources: %w", err)), nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/toolsets/kubevirt/vm/create/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func create(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
}

// Create the VM in the cluster
resources, err := kubernetes.NewCore(params).ResourcesCreateOrUpdate(params, vmYaml)
resources, err := kubernetes.NewCore(params).ResourcesCreateOrUpdate(params, vmYaml, "")
if err != nil {
return api.NewToolCallResult("", fmt.Errorf("failed to create VirtualMachine: %w", err)), nil
}
Expand Down