Skip to content

Commit

Permalink
don't traverse recursively for setting namespace since it only appear…
Browse files Browse the repository at this point in the history
…s at the parent level
  • Loading branch information
tejal29 committed Jul 21, 2022
1 parent f43aabb commit fb5bdd2
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 38 deletions.
5 changes: 2 additions & 3 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ spec:
`,
expectedOut: fmt.Sprintf(`apiVersion: v1
kind: Pod
metadata:
namespace: %s
spec:
containers:
- image: gcr.io/k8s-skaffold/skaffold:test
Expand Down Expand Up @@ -179,8 +181,6 @@ spec:
---
apiVersion: v1
kind: Pod
metadata:
name: my-pod-456
spec:
containers:
- image: gcr.io/project/image2
Expand All @@ -199,7 +199,6 @@ spec:
apiVersion: v1
kind: Pod
metadata:
name: my-pod-456
namespace: %s
spec:
containers:
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/kubernetes/manifest/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ spec:
description: "unexpected metadata type",
namespace: "test",
manifests: ManifestList{[]byte(`metadata: []`)},
expected: ManifestList{[]byte(`metadata: []`)},
shouldErr: true,
},
{
description: "single Pod manifest in the list with same namespace as set",
Expand Down
70 changes: 36 additions & 34 deletions pkg/skaffold/kubernetes/manifest/namespaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/yaml"
)

const namespaceField = "namespace"

// CollectNamespaces returns all the namespaces in the manifests.
func (l *ManifestList) CollectNamespaces() ([]string, error) {
replacer := newNamespaceCollector()
Expand Down Expand Up @@ -66,7 +69,7 @@ func (r *namespaceCollector) Visit(gk schema.GroupKind, navpath string, o map[st
if !ok {
return true
}
if nsValue, present := metadata["namespace"]; present {
if nsValue, present := metadata[namespaceField]; present {
nsString, ok := nsValue.(string)
if !ok || nsString == "" {
return true
Expand All @@ -84,49 +87,48 @@ func (l *ManifestList) SetNamespace(namespace string, rs ResourceSelector) (Mani
if namespace == "" {
return *l, nil
}
replacer := newNamespaceSetter(namespace)
updated, err := l.Visit(replacer, rs)
if err != nil {
return nil, nsSettingErr(err)
}
if replacer.inValid {
return nil, nsAlreadySetErr()
var updated ManifestList
for _, item := range *l {
m := make(map[string]interface{})
if err := yaml.Unmarshal(item, &m); err != nil {
return nil, fmt.Errorf("reading Kubernetes YAML: %w", err)
}
if len(m) == 0 {
continue
}
if errU := addOrUpdateNamespace(m, namespace); errU != nil {
return nil, errU
}
updatedManifest, err := yaml.Marshal(m)
if err != nil {
return nil, nsSettingErr(err)
}

updated = append(updated, updatedManifest)
}

log.Entry(context.TODO()).Debugln("manifests set with namespace", updated.String())

return updated, nil
}

type namespaceSetter struct {
ns string
inValid bool
}

func newNamespaceSetter(ns string) *namespaceSetter {
return &namespaceSetter{
ns: ns,
inValid: false,
}
}

func (r *namespaceSetter) Visit(gk schema.GroupKind, navpath string, o map[string]interface{}, k string, v interface{}, rs ResourceSelector) bool {
if k != metadataField {
return true
func addOrUpdateNamespace(manifest map[string]interface{}, ns string) error {
originalMetadata, ok := manifest[metadataField]
if !ok {
metadataAdded := make(map[string]interface{})
metadataAdded[namespaceField] = ns
manifest[metadataField] = metadataAdded
return nil
}

metadata, ok := v.(map[string]interface{})
metadata, ok := originalMetadata.(map[string]interface{})
if !ok {
return true
return nsSettingErr(fmt.Errorf("error converting %s to map[string]interface{}", originalMetadata))
}

nsValue, present := metadata["namespace"]
if !present || isEmptyOrEqual(nsValue, r.ns) {
metadata["namespace"] = r.ns
return false
nsValue, present := metadata[namespaceField]
if !present || isEmptyOrEqual(nsValue, ns) {
metadata[namespaceField] = ns
return nil
}
r.inValid = true
return false
return nsAlreadySetErr()
}

func isEmptyOrEqual(v interface{}, s string) bool {
Expand Down

0 comments on commit fb5bdd2

Please sign in to comment.