From dd5674fe6b8393e51e78dff989e82ca0d7e8ad02 Mon Sep 17 00:00:00 2001 From: Jerome Brette Date: Sun, 7 Jul 2019 11:19:20 -0500 Subject: [PATCH] ResId.Equals usable for VariableRef. - Namespace need objRef field in variable declaration - Add namespace conflict test for variables The replacement of ResId.GkvnEquals reference by ResId.Equals highligthed the fact it is no possible yet when looking for variable targets because the namespace field is not allowed yet. This commit adds two tests to the namespaces_test.go regarding that use case. --- pkg/accumulator/resaccumulator.go | 2 + pkg/target/namespaces_test.go | 235 ++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+) diff --git a/pkg/accumulator/resaccumulator.go b/pkg/accumulator/resaccumulator.go index cf5825bc9..f22b43c38 100644 --- a/pkg/accumulator/resaccumulator.go +++ b/pkg/accumulator/resaccumulator.go @@ -64,6 +64,8 @@ func (ra *ResAccumulator) GetTransformerConfig() *config.TransformerConfig { func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { for _, v := range incoming { + // TODO(jeb): Do not change GvknEquals to Equals until the + // namespace is part of the variable declaration. matched := ra.resMap.GetMatchingResourcesByOriginalId( resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name).GvknEquals) if len(matched) > 1 { diff --git a/pkg/target/namespaces_test.go b/pkg/target/namespaces_test.go index 0ce86eea1..5ec603ecd 100644 --- a/pkg/target/namespaces_test.go +++ b/pkg/target/namespaces_test.go @@ -5,6 +5,7 @@ package target_test import ( "sigs.k8s.io/kustomize/v3/pkg/kusttest" + "strings" "testing" ) @@ -93,3 +94,237 @@ rules: - get `) } + +const ambiguousvarMyApp string = ` +resources: +- elasticsearch-test-service.yaml +- elasticsearch-dev-service.yaml +vars: +- name: elasticsearch-test-service-name + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: metadata.name +- name: elasticsearch-test-protocol + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: spec.ports[0].protocol +- name: elasticsearch-dev-service-name + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: metadata.name +- name: elasticsearch-dev-protocol + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: spec.ports[0].protocol +` + +const ambiguousvarDevResources string = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: elasticsearch + namespace: dev +spec: + template: + spec: + containers: + - name: elasticsearch + env: + - name: DISCOVERY_SERVICE + value: "$(elasticsearch-dev-service-name).monitoring.svc.cluster.local" + - name: DISCOVERY_PROTOCOL + value: "$(elasticsearch-dev-protocol)" +--- +apiVersion: v1 +kind: Service +metadata: + name: elasticsearch + namespace: dev +spec: + ports: + - name: transport + port: 9300 + protocol: TCP + clusterIP: None +` + +const ambiguousvarTestResources string = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: elasticsearch + namespace: test +spec: + template: + spec: + containers: + - name: elasticsearch + env: + - name: DISCOVERY_SERVICE + value: "$(elasticsearch-test-service-name).monitoring.svc.cluster.local" + - name: DISCOVERY_PROTOCOL + value: "$(elasticsearch-test-protocol)" +--- +apiVersion: v1 +kind: Service +metadata: + name: elasticsearch + namespace: test +spec: + ports: + - name: transport + port: 9300 + protocol: UDP + clusterIP: None +` + +const ambiguousvarExpectedOutput string = ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: elasticsearch + namespace: dev +spec: + template: + spec: + containers: + - env: + - name: DISCOVERY_SERVICE + value: elasticsearch.monitoring.svc.cluster.local + - name: DISCOVERY_PROTOCOL + value: TCP + name: elasticsearch +--- +apiVersion: v1 +kind: Service +metadata: + name: elasticsearch + namespace: dev +spec: + clusterIP: None + ports: + - name: transport + port: 9300 + protocol: TCP +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: elasticsearch + namespace: test +spec: + template: + spec: + containers: + - env: + - name: DISCOVERY_SERVICE + value: elasticsearch.monitoring.svc.cluster.local + - name: DISCOVERY_PROTOCOL + value: UDP + name: elasticsearch +--- +apiVersion: v1 +kind: Service +metadata: + name: elasticsearch + namespace: test +spec: + clusterIP: None + ports: + - name: transport + port: 9300 + protocol: UDP +` + +// 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. +func TestVariablesAmbiguous(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/ambiguousvar/myapp") + th.WriteK("/ambiguousvar/myapp", ambiguousvarMyApp) + th.WriteF("/ambiguousvar/myapp/elasticsearch-dev-service.yaml", ambiguousvarDevResources) + th.WriteF("/ambiguousvar/myapp/elasticsearch-test-service.yaml", ambiguousvarTestResources) + _, err := th.MakeKustTarget().MakeCustomizedResMap() + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), "unable to disambiguate") { + t.Fatalf("unexpected error %v", err) + } +} + +const ambiguousvarDevFolder string = ` +resources: +- elasticsearch-dev-service.yaml +vars: +- name: elasticsearch-dev-service-name + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: metadata.name +- name: elasticsearch-dev-protocol + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: spec.ports[0].protocol +` + +const ambiguousvarTestFolder string = ` +resources: +- elasticsearch-test-service.yaml +vars: +- name: elasticsearch-test-service-name + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: metadata.name +- name: elasticsearch-test-protocol + objref: + kind: Service + name: elasticsearch + apiVersion: v1 + fieldref: + fieldpath: spec.ports[0].protocol +` + +// TestVariablesAmbiguousWorkaround demonstrates a possible workaround +// to TestVariablesAmbiguous problem. It requires to separate the variables +// and resources into multiple kustomization context/folders instead of one. +func TestVariablesAmbiguousWorkaround(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/ambiguousvar/workaround") + th.WriteK("/ambiguousvar/dev", ambiguousvarDevFolder) + th.WriteF("/ambiguousvar/dev/elasticsearch-dev-service.yaml", ambiguousvarDevResources) + th.WriteK("/ambiguousvar/test", ambiguousvarTestFolder) + th.WriteF("/ambiguousvar/test/elasticsearch-test-service.yaml", ambiguousvarTestResources) + th.WriteK("/ambiguousvar/workaround", ` +resources: +- ../dev +- ../test +`) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, ambiguousvarExpectedOutput) +}