From 21ff81b758d3638c11f9e9709c19c65ece6762a6 Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Wed, 5 Sep 2018 13:01:15 -0700 Subject: [PATCH 1/5] Add multibases test with namereference nonconflict --- .../base/kustomization.yaml | 4 +++ .../base/rolebinding.yaml | 11 +++++++ .../base/serviceaccount.yaml | 4 +++ .../combined/kustomization.yaml | 3 ++ .../expected.yaml | 33 +++++++++++++++++++ .../overlays/a/kustomization.yaml | 4 +++ .../overlays/b/kustomization.yaml | 4 +++ .../testcase-multibases-nonconflict/test.yaml | 4 +++ 8 files changed, 67 insertions(+) create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/base/rolebinding.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/base/serviceaccount.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/combined/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/expected.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/overlays/a/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/overlays/b/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-nonconflict/test.yaml diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml new file mode 100644 index 000000000..9084ca255 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml @@ -0,0 +1,4 @@ +resources: +- serviceaccount.yaml +- rolebinding.yaml +namePrefix: base- \ No newline at end of file diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/base/rolebinding.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/base/rolebinding.yaml new file mode 100644 index 000000000..c74e189ce --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/base/rolebinding.yaml @@ -0,0 +1,11 @@ +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: serviceaccount diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/base/serviceaccount.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/base/serviceaccount.yaml new file mode 100644 index 000000000..f1fff56b2 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/base/serviceaccount.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: serviceaccount diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/combined/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/combined/kustomization.yaml new file mode 100644 index 000000000..5d1ecdfc7 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/combined/kustomization.yaml @@ -0,0 +1,3 @@ +bases: +- ../overlays/a +- ../overlays/b diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/expected.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/expected.yaml new file mode 100644 index 000000000..efa826d0e --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/expected.yaml @@ -0,0 +1,33 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: a-base-serviceaccount +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: b-base-serviceaccount +--- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: a-base-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: a-base-serviceaccount +--- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: b-base-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: b-base-serviceaccount diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/overlays/a/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/overlays/a/kustomization.yaml new file mode 100644 index 000000000..2b15edc85 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/overlays/a/kustomization.yaml @@ -0,0 +1,4 @@ +bases: +- ../../base/ + +namePrefix: a- diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/overlays/b/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/overlays/b/kustomization.yaml new file mode 100644 index 000000000..1e19a7607 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/overlays/b/kustomization.yaml @@ -0,0 +1,4 @@ +bases: +- ../../base/ + +namePrefix: b- diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/test.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/test.yaml new file mode 100644 index 000000000..cbbcc93ba --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/test.yaml @@ -0,0 +1,4 @@ +description: multibases with name reference +args: [] +filename: testdata/testcase-multibases-nonconflict/combined +expectedStdout: testdata/testcase-multibases-nonconflict/expected.yaml From 7811d9f2bc3780ae4197433c118442602dfb1165 Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Wed, 5 Sep 2018 13:01:56 -0700 Subject: [PATCH 2/5] Add multibases test with namereference conflict --- .../base/kustomization.yaml | 4 ++++ .../base/rolebinding.yaml | 11 +++++++++++ .../base/serviceaccount.yaml | 4 ++++ .../combined/kustomization.yaml | 3 +++ .../overlays/a/kustomization.yaml | 7 +++++++ .../overlays/a/serviceaccount.yaml | 4 ++++ .../overlays/b/kustomization.yaml | 4 ++++ .../testdata/testcase-multibases-conflict/test.yaml | 4 ++++ 8 files changed, 41 insertions(+) create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/base/rolebinding.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/base/serviceaccount.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/combined/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/overlays/a/serviceaccount.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/overlays/b/kustomization.yaml create mode 100644 pkg/commands/testdata/testcase-multibases-conflict/test.yaml diff --git a/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml new file mode 100644 index 000000000..9084ca255 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml @@ -0,0 +1,4 @@ +resources: +- serviceaccount.yaml +- rolebinding.yaml +namePrefix: base- \ No newline at end of file diff --git a/pkg/commands/testdata/testcase-multibases-conflict/base/rolebinding.yaml b/pkg/commands/testdata/testcase-multibases-conflict/base/rolebinding.yaml new file mode 100644 index 000000000..c74e189ce --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/base/rolebinding.yaml @@ -0,0 +1,11 @@ +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: serviceaccount diff --git a/pkg/commands/testdata/testcase-multibases-conflict/base/serviceaccount.yaml b/pkg/commands/testdata/testcase-multibases-conflict/base/serviceaccount.yaml new file mode 100644 index 000000000..f1fff56b2 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/base/serviceaccount.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: serviceaccount diff --git a/pkg/commands/testdata/testcase-multibases-conflict/combined/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-conflict/combined/kustomization.yaml new file mode 100644 index 000000000..5d1ecdfc7 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/combined/kustomization.yaml @@ -0,0 +1,3 @@ +bases: +- ../overlays/a +- ../overlays/b diff --git a/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml new file mode 100644 index 000000000..0ff4668b5 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml @@ -0,0 +1,7 @@ +bases: +- ../../base/ + +namePrefix: a- + +resources: +- serviceaccount.yaml \ No newline at end of file diff --git a/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/serviceaccount.yaml b/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/serviceaccount.yaml new file mode 100644 index 000000000..f1fff56b2 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/serviceaccount.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: serviceaccount diff --git a/pkg/commands/testdata/testcase-multibases-conflict/overlays/b/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-conflict/overlays/b/kustomization.yaml new file mode 100644 index 000000000..1e19a7607 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/overlays/b/kustomization.yaml @@ -0,0 +1,4 @@ +bases: +- ../../base/ + +namePrefix: b- diff --git a/pkg/commands/testdata/testcase-multibases-conflict/test.yaml b/pkg/commands/testdata/testcase-multibases-conflict/test.yaml new file mode 100644 index 000000000..496ce95b8 --- /dev/null +++ b/pkg/commands/testdata/testcase-multibases-conflict/test.yaml @@ -0,0 +1,4 @@ +description: multibases with name reference +args: [] +filename: testdata/testcase-multibases-conflict/combined +expectedError: detected conflicts when resolving name references serviceaccount From 4a297fa1387ef6ed2d8343bf4a7499d99bdad383 Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Wed, 5 Sep 2018 13:02:51 -0700 Subject: [PATCH 3/5] improve idslice --- .../testdata/testcase-variable-ref/expected.yaml | 16 ++++++++-------- pkg/resmap/idslice.go | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/commands/testdata/testcase-variable-ref/expected.yaml b/pkg/commands/testdata/testcase-variable-ref/expected.yaml index 43845e41d..cb60233e6 100644 --- a/pkg/commands/testdata/testcase-variable-ref/expected.yaml +++ b/pkg/commands/testdata/testcase-variable-ref/expected.yaml @@ -78,16 +78,10 @@ metadata: apiVersion: v1 kind: Service metadata: - annotations: - prometheus.io/path: _status/vars - prometheus.io/port: "8080" - prometheus.io/scrape: "true" - service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" labels: app: cockroachdb - name: dev-base-cockroachdb + name: dev-base-cockroachdb-public spec: - clusterIP: None ports: - name: grpc port: 26257 @@ -101,10 +95,16 @@ spec: apiVersion: v1 kind: Service metadata: + annotations: + prometheus.io/path: _status/vars + prometheus.io/port: "8080" + prometheus.io/scrape: "true" + service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" labels: app: cockroachdb - name: dev-base-cockroachdb-public + name: dev-base-cockroachdb spec: + clusterIP: None ports: - name: grpc port: 26257 diff --git a/pkg/resmap/idslice.go b/pkg/resmap/idslice.go index 3b4652608..38ed4d518 100644 --- a/pkg/resmap/idslice.go +++ b/pkg/resmap/idslice.go @@ -34,7 +34,7 @@ func (a IdSlice) Less(i, j int) bool { if a[i].Gvk().String() != a[j].Gvk().String() { return gvkLess(a[i].Gvk(), a[j].Gvk()) } - return a[i].Name() < a[j].Name() + return a[i].String() < a[j].String() } var order = []string{"Namespace", "CustomResourceDefinition", "ServiceAccount", From 7b301446fab1202b14eb4974d91786aa46f5dc7a Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Wed, 5 Sep 2018 13:10:33 -0700 Subject: [PATCH 4/5] filter by namespace and nameprefix in namereference transformer --- pkg/resmap/resmap.go | 12 ++++++++++++ pkg/resource/resid.go | 27 ++++++++++++++++++++++++++- pkg/transformers/namereference.go | 15 ++++++++++++++- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/pkg/resmap/resmap.go b/pkg/resmap/resmap.go index d0a2586fd..60ec6f131 100644 --- a/pkg/resmap/resmap.go +++ b/pkg/resmap/resmap.go @@ -140,6 +140,18 @@ func (m ResMap) insert(newName string, obj *unstructured.Unstructured) error { return nil } +// FilterBy returns a ResMap containing ResIds with the same namespace and nameprefix +// with the inputId +func (m ResMap) FilterBy(inputId resource.ResId) ResMap { + result := ResMap{} + for id, res := range m { + if id.Namespace() == inputId.Namespace() && id.HasSamePrefix(inputId) { + result[id] = res + } + } + return result +} + // NewResourceSliceFromPatches returns a slice of resources given a patch path slice from a kustomization file. func NewResourceSliceFromPatches( loader loader.Loader, paths []patch.PatchStrategicMerge) ([]*resource.Resource, error) { diff --git a/pkg/resource/resid.go b/pkg/resource/resid.go index 5104a81cd..563e8701e 100644 --- a/pkg/resource/resid.go +++ b/pkg/resource/resid.go @@ -97,10 +97,35 @@ func (n ResId) Namespace() string { // CopyWithNewPrefix make a new copy from current ResId and append a new prefix func (n ResId) CopyWithNewPrefix(p string) ResId { - return ResId{gvk: n.gvk, name: n.name, prefix: p + n.prefix, namespace: n.namespace} + return ResId{gvk: n.gvk, name: n.name, prefix: n.concatePrefix(p), namespace: n.namespace} } // CopyWithNewNamespace make a new copy from current ResId and set a new namespace func (n ResId) CopyWithNewNamespace(ns string) ResId { return ResId{gvk: n.gvk, name: n.name, prefix: n.prefix, namespace: ns} } + +// HasSamePrefix check if two ResIds have the same leading prefix +func (n ResId) HasSamePrefix(id ResId) bool { + prefixes1 := n.prefixList() + prefixes2 := id.prefixList() + return len(prefixes1) == 0 || len(prefixes2) == 0 || prefixes1[0] == prefixes2[0] +} + +func (n ResId) concatePrefix(p string) string { + if p == "" { + return n.prefix + } + if n.prefix == "" { + return p + } + return p + ":" + n.prefix +} + +func (n ResId) prefixList() []string { + var plist []string + if n.prefix == "" { + return plist + } + return strings.Split(n.prefix, ":") +} diff --git a/pkg/transformers/namereference.go b/pkg/transformers/namereference.go index 008f76fc4..89e887b01 100644 --- a/pkg/transformers/namereference.go +++ b/pkg/transformers/namereference.go @@ -21,6 +21,7 @@ import ( "fmt" "github.com/kubernetes-sigs/kustomize/pkg/resmap" + "github.com/kubernetes-sigs/kustomize/pkg/resource" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -58,7 +59,7 @@ func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { continue } err := mutateField(objMap, path.Path, path.CreateIfNotPresent, - o.updateNameReference(referencePathConfig.referencedGVK, m)) + o.updateNameReference(referencePathConfig.referencedGVK, m.FilterBy(id))) if err != nil { return err } @@ -81,9 +82,21 @@ func (o *nameReferenceTransformer) updateNameReference( continue } if id.Name() == s { + err := o.detectConflict(id, m, s) + if err != nil { + return nil, err + } return res.GetName(), nil } } return in, nil } } + +func (o *nameReferenceTransformer) detectConflict(id resource.ResId, m resmap.ResMap, name string) error { + matchedIds := m.FindByGVKN(id) + if len(matchedIds) > 1 { + return fmt.Errorf("detected conflicts when resolving name references %s:\n%v", name, matchedIds) + } + return nil +} From 829cb2baa386e473b86a90f588e0e8007b8d06ae Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Wed, 5 Sep 2018 16:00:41 -0700 Subject: [PATCH 5/5] address comments --- .../base/kustomization.yaml | 2 +- .../overlays/a/kustomization.yaml | 2 +- .../base/kustomization.yaml | 2 +- pkg/resmap/resmap.go | 2 +- pkg/resource/resid.go | 15 ++++++--------- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml index 9084ca255..f913dbaf5 100644 --- a/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml +++ b/pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml @@ -1,4 +1,4 @@ resources: - serviceaccount.yaml - rolebinding.yaml -namePrefix: base- \ No newline at end of file +namePrefix: base- diff --git a/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml index 0ff4668b5..166dba8ac 100644 --- a/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml +++ b/pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml @@ -4,4 +4,4 @@ bases: namePrefix: a- resources: -- serviceaccount.yaml \ No newline at end of file +- serviceaccount.yaml diff --git a/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml b/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml index 9084ca255..f913dbaf5 100644 --- a/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml +++ b/pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml @@ -1,4 +1,4 @@ resources: - serviceaccount.yaml - rolebinding.yaml -namePrefix: base- \ No newline at end of file +namePrefix: base- diff --git a/pkg/resmap/resmap.go b/pkg/resmap/resmap.go index 60ec6f131..5471541e8 100644 --- a/pkg/resmap/resmap.go +++ b/pkg/resmap/resmap.go @@ -145,7 +145,7 @@ func (m ResMap) insert(newName string, obj *unstructured.Unstructured) error { func (m ResMap) FilterBy(inputId resource.ResId) ResMap { result := ResMap{} for id, res := range m { - if id.Namespace() == inputId.Namespace() && id.HasSamePrefix(inputId) { + if id.Namespace() == inputId.Namespace() && id.HasSameLeftmostPrefix(inputId) { result[id] = res } } diff --git a/pkg/resource/resid.go b/pkg/resource/resid.go index 563e8701e..c970eb9b1 100644 --- a/pkg/resource/resid.go +++ b/pkg/resource/resid.go @@ -97,7 +97,7 @@ func (n ResId) Namespace() string { // CopyWithNewPrefix make a new copy from current ResId and append a new prefix func (n ResId) CopyWithNewPrefix(p string) ResId { - return ResId{gvk: n.gvk, name: n.name, prefix: n.concatePrefix(p), namespace: n.namespace} + return ResId{gvk: n.gvk, name: n.name, prefix: n.concatPrefix(p), namespace: n.namespace} } // CopyWithNewNamespace make a new copy from current ResId and set a new namespace @@ -105,14 +105,15 @@ func (n ResId) CopyWithNewNamespace(ns string) ResId { return ResId{gvk: n.gvk, name: n.name, prefix: n.prefix, namespace: ns} } -// HasSamePrefix check if two ResIds have the same leading prefix -func (n ResId) HasSamePrefix(id ResId) bool { +// HasSameLeftmostPrefix check if two ResIds have the same +// left most prefix. +func (n ResId) HasSameLeftmostPrefix(id ResId) bool { prefixes1 := n.prefixList() prefixes2 := id.prefixList() - return len(prefixes1) == 0 || len(prefixes2) == 0 || prefixes1[0] == prefixes2[0] + return prefixes1[0] == prefixes2[0] } -func (n ResId) concatePrefix(p string) string { +func (n ResId) concatPrefix(p string) string { if p == "" { return n.prefix } @@ -123,9 +124,5 @@ func (n ResId) concatePrefix(p string) string { } func (n ResId) prefixList() []string { - var plist []string - if n.prefix == "" { - return plist - } return strings.Split(n.prefix, ":") }