Skip to content

remoteManifests: respect skaffold namespace #9936

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
hown3d wants to merge 2 commits into
base: main
Choose a base branch
from
+62 −13
Open

remoteManifests: respect skaffold namespace #9936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
592f4aa
remoteManifests: respect skaffold namespace
hown3d Dec 17, 2025
3c174c9
tests
hown3d Dec 17, 2025
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
6 changes: 4 additions & 2 deletions pkg/skaffold/render/generate/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ import (
)

// NewGenerator instantiates a Generator object.
func NewGenerator(workingDir string, config latest.Generate, hydrationDir string) Generator {
func NewGenerator(workingDir string, config latest.Generate, hydrationDir string, namespace string) Generator {
return Generator{
workingDir: workingDir,
config: config,
hydrationDir: hydrationDir,
namespace: namespace,
}
}

Expand All @@ -54,6 +55,7 @@ type Generator struct {
workingDir string
hydrationDir string
config latest.Generate
namespace string
}

func localManifests(paths []string, workdir string) ([]string, error) {
Expand Down Expand Up @@ -157,7 +159,7 @@ func (g Generator) Generate(ctx context.Context, out io.Writer) (manifest.Manife
// context in the specified namespace and for the specified type
func (g Generator) readRemoteManifest(ctx context.Context, rm latest.RemoteManifest) ([]byte, error) {
var args []string
ns := ""
ns := g.namespace
name := rm.Manifest
if parts := strings.Split(name, ":"); len(parts) > 1 {
ns = parts[0]
Expand Down
54 changes: 52 additions & 2 deletions pkg/skaffold/render/generate/generate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestGenerate(t *testing.T) {
Touch("empty.ignored").
Chdir()

g := NewGenerator(".", test.generateConfig, "")
g := NewGenerator(".", test.generateConfig, "", "")
var output bytes.Buffer
actual, err := g.Generate(context.Background(), &output)
t.CheckNoError(err)
Expand All @@ -121,7 +121,7 @@ func TestGenerateFromURLManifest(t *testing.T) {
defer os.RemoveAll(manifest.ManifestTmpDir)
g := NewGenerator(".", latest.Generate{
RawK8s: []string{ts.URL},
}, "")
}, "", "")
var output bytes.Buffer
actual, err := g.Generate(context.Background(), &output)
testutil.Run(t, "", func(t *testutil.T) {
Expand All @@ -131,6 +131,56 @@ func TestGenerateFromURLManifest(t *testing.T) {
})
}

func TestReadRemoteManifests(t *testing.T) {
tests := []struct {
description string
manifest latest.RemoteManifest
expectedCommand string
namespace string
}{
{
description: "explicit namespace",
manifest: latest.RemoteManifest{
Manifest: "mynamespace:deployment/foo",
},
expectedCommand: "kubectl --namespace mynamespace get deployment/foo -o yaml",
},
{
description: "explicit namespace with skaffold namespace should use explicit one",
manifest: latest.RemoteManifest{
Manifest: "mynamespace:deployment/foo",
},
expectedCommand: "kubectl --namespace mynamespace get deployment/foo -o yaml",
namespace: "anyotherone",
},
{
description: "no namespace should not set namespace flag",
manifest: latest.RemoteManifest{
Manifest: "deployment/foo",
},
expectedCommand: "kubectl get deployment/foo -o yaml",
},
{
description: "no explicit namespace but skaffold namespace should set namespace flag",
manifest: latest.RemoteManifest{
Manifest: "deployment/foo",
},
expectedCommand: "kubectl --namespace skaffold get deployment/foo -o yaml",
namespace: "skaffold",
},
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 17, 2025

Choose a reason for hiding this comment

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

medium

These test cases are great for covering the new namespace logic. To improve robustness, consider adding a test case for manifest names that contain colons, which is valid for some Kubernetes resources. The current parsing logic in readRemoteManifest with strings.Split might not handle this correctly. Adding a test case like the one below would help expose this and encourage a more robust implementation (e.g., using strings.SplitN).

{
	description: "resource name with colon",
	manifest: latest.RemoteManifest{
		Manifest: "mynamespace:my-crd:instance-1",
	},
	expectedCommand: "kubectl --namespace mynamespace get my-crd:instance-1 -o yaml",
},

}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&util.DefaultExecCommand, testutil.CmdRun(
test.expectedCommand,
))
g := Generator{namespace: test.namespace}
_, err := g.readRemoteManifest(t.Context(), test.manifest)
t.CheckNoError(err)
})
}
}

func TestManifestDeps(t *testing.T) {
tests := []struct {
description string
Expand Down
11 changes: 5 additions & 6 deletions pkg/skaffold/render/renderer/helm/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ import (
sUtil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util"
)

var (
// RendererHelmVersionOverride allows replacing the Helm version for testing purposes (to avoid calling installed `helm` binary in tests)
RendererHelmVersionOverride *semver.Version = nil
)
// RendererHelmVersionOverride allows replacing the Helm version for testing purposes (to avoid calling installed `helm` binary in tests)
var RendererHelmVersionOverride *semver.Version = nil

type Helm struct {
configName string
Expand Down Expand Up @@ -92,7 +90,8 @@ func New(ctx context.Context, cfg render.Config, rCfg latest.RenderConfig, label
}
}

generator := generate.NewGenerator(cfg.GetWorkingDir(), rCfg.Generate, "")
namespace := cfg.GetKubeNamespace()
generator := generate.NewGenerator(cfg.GetWorkingDir(), rCfg.Generate, "", namespace)
transformAllowlist, transformDenylist, err := util.ConsolidateTransformConfiguration(cfg)
if err != nil {
return Helm{}, err
Expand All @@ -108,7 +107,7 @@ func New(ctx context.Context, cfg render.Config, rCfg latest.RenderConfig, label
kubeContext: cfg.GetKubeContext(),
kubeConfig: cfg.GetKubeConfig(),
labels: labels,
namespace: cfg.GetKubeNamespace(),
namespace: namespace,
manifestOverrides: manifestOverrides,
helmVersion: helmVersion,

Expand Down
4 changes: 1 addition & 3 deletions pkg/skaffold/render/renderer/kubectl/kubectl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type Kubectl struct {

func New(cfg render.Config, rCfg latest.RenderConfig, labels map[string]string, configName string, ns string, manifestOverrides map[string]string, injectNs bool) (Kubectl, error) {
transformAllowlist, transformDenylist, err := rUtil.ConsolidateTransformConfiguration(cfg)
generator := generate.NewGenerator(cfg.GetWorkingDir(), rCfg.Generate, "")
generator := generate.NewGenerator(cfg.GetWorkingDir(), rCfg.Generate, "", ns)
if err != nil {
return Kubectl{}, err
}
Expand Down Expand Up @@ -116,7 +116,6 @@ func (r Kubectl) Render(ctx context.Context, out io.Writer, builds []graph.Artif
}

manifests, err = r.transformer.Transform(ctx, manifests)

if err != nil {
return manifest.ManifestListByConfig{}, err
}
Expand All @@ -136,7 +135,6 @@ func (r Kubectl) Render(ctx context.Context, out io.Writer, builds []graph.Artif
InjectNamespace: r.injectNs,
}
manifests, err = rUtil.BaseTransform(ctx, manifests, builds, opts, r.labels, r.namespace)

if err != nil {
return manifest.ManifestListByConfig{}, err
}
Expand Down