diff --git a/pkg/accumulator/resaccumulator.go b/pkg/accumulator/resaccumulator.go index e7e460c59..ef2f4b9bc 100644 --- a/pkg/accumulator/resaccumulator.go +++ b/pkg/accumulator/resaccumulator.go @@ -64,20 +64,14 @@ func (ra *ResAccumulator) GetTransformerConfig() *config.TransformerConfig { func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { for _, v := range incoming { - // Warning: Do not change GvknEquals to Equals until the - // namespace is part of the variable declaration. - // The namespace will eventually be added to the variable - // declaration to solve the following issue: - // https://github.com/kubernetes-sigs/kustomize/issues/1298 - // If two resources of the same kind with the same name but - // in different namespaces, any variable aiming to put at one - // of those resources will fail since it is not possible to - // specify the namespace and use Id.Equals instead of Id.GvknEquals - // The change will also need to be backward compatible, i.e. - // no namespace specified means wildcard namespace not "default" - // namespace. - matched := ra.resMap.GetMatchingResourcesByOriginalId( - resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name).GvknEquals) + targetId := resid.NewResIdWithNamespace(v.ObjRef.GVK(), v.ObjRef.Name, v.ObjRef.Namespace) + idMatcher := targetId.GvknEquals + if targetId.Namespace != "" || !targetId.IsNamespaceableKind() { + // Preserve backward compatibility. An empty namespace means + // wildcard search on the namespace hence we still use GvknEquals + idMatcher = targetId.Equals + } + matched := ra.resMap.GetMatchingResourcesByOriginalId(idMatcher) if len(matched) > 1 { return fmt.Errorf( "found %d resId matches for var %s "+ diff --git a/pkg/target/namespaces_test.go b/pkg/target/namespaces_test.go index 481cdf2cc..582879ad5 100644 --- a/pkg/target/namespaces_test.go +++ b/pkg/target/namespaces_test.go @@ -104,8 +104,8 @@ rules: // https://github.com/kubernetes-sigs/kustomize/issues/1298 const namespaceNeedInVarMyApp string = ` resources: -- elasticsearch-test-service.yaml - elasticsearch-dev-service.yaml +- elasticsearch-test-service.yaml vars: - name: elasticsearch-test-service-name objref: @@ -256,11 +256,8 @@ spec: ` // TestVariablesAmbiguous demonstrates how two variables pointing at two different resources -// using the same name in different namespaces are treated as ambiguous -// The fundamental reason is that objRef field in the variable does not contain a namespace -// qualifier. -// Once the namespace qualifer is added, as for the other resources lookup in the coder, -// the ResId.GvknEquals method usage will have to be phased out and replaced by ResId.Equals. +// using the same name in different namespaces are treated as ambiguous if the namespace is +// not specified func TestVariablesAmbiguous(t *testing.T) { th := kusttest_test.NewKustTestHarness(t, "/namespaceNeedInVar/myapp") th.WriteK("/namespaceNeedInVar/myapp", namespaceNeedInVarMyApp) @@ -335,3 +332,56 @@ resources: } th.AssertActualEqualsExpected(m, namespaceNeedInVarExpectedOutput) } + +const namespaceNeedInVarMyAppWithNamespace string = ` +resources: +- elasticsearch-dev-service.yaml +- elasticsearch-test-service.yaml +vars: +- name: elasticsearch-test-service-name + objref: + kind: Service + name: elasticsearch + namespace: test + apiVersion: v1 + fieldref: + fieldpath: metadata.name +- name: elasticsearch-test-protocol + objref: + kind: Service + name: elasticsearch + namespace: test + apiVersion: v1 + fieldref: + fieldpath: spec.ports[0].protocol +- name: elasticsearch-dev-service-name + objref: + kind: Service + name: elasticsearch + namespace: dev + apiVersion: v1 + fieldref: + fieldpath: metadata.name +- name: elasticsearch-dev-protocol + objref: + kind: Service + name: elasticsearch + namespace: dev + apiVersion: v1 + fieldref: + fieldpath: spec.ports[0].protocol +` + +// TestVariablesDisambiguatedWithNamespace demonstrates that adding the namespace +// to the variable declarations allows to disambiguate the variables. +func TestVariablesDisambiguatedWithNamespace(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/namespaceNeedInVar/myapp") + th.WriteK("/namespaceNeedInVar/myapp", namespaceNeedInVarMyAppWithNamespace) + th.WriteF("/namespaceNeedInVar/myapp/elasticsearch-dev-service.yaml", namespaceNeedInVarDevResources) + th.WriteF("/namespaceNeedInVar/myapp/elasticsearch-test-service.yaml", namespaceNeedInVarTestResources) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, namespaceNeedInVarExpectedOutput) +} diff --git a/pkg/types/var.go b/pkg/types/var.go index 4b70caf7f..44bd0f2c8 100644 --- a/pkg/types/var.go +++ b/pkg/types/var.go @@ -54,6 +54,7 @@ type Target struct { APIVersion string `json:"apiVersion,omitempty" yaml:"apiVersion,omitempty"` gvk.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"` Name string `json:"name" yaml:"name"` + Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` } // FieldSelector contains the fieldPath to an object field.