fixed ports merging issue and refactored some kyaml.walk code

This commit is contained in:
Natasha Sarkar
2021-02-08 17:44:05 -08:00
parent a3a11bf3f4
commit 71f4cecb4c
7 changed files with 147 additions and 125 deletions

View File

@@ -723,12 +723,12 @@ spec:
- name: consul
image: "hashicorp/consul:1.9.1"
ports:
- containerPort: 8301
protocol: "TCP"
- containerPort: 8301
protocol: "UDP"
- containerPort: 8500
name: http
- containerPort: 8301
protocol: "UDP"
- containerPort: 8301
protocol: "UDP"
`,
},
}

View File

@@ -24,3 +24,5 @@ require (
sigs.k8s.io/kustomize/kyaml v0.10.7
sigs.k8s.io/yaml v1.2.0
)
replace sigs.k8s.io/kustomize/kyaml => ../kyaml

View File

@@ -1124,3 +1124,92 @@ metadata:
}
}
}
// test for #3513
func TestSmpWithDifferentKeysOnDifferentPorts(t *testing.T) {
th := kusttest_test.MakeHarness(t)
th.WriteK(".", `
resources:
- resource.yaml
patches:
- path: patch.yaml
target:
kind: StatefulSet
name: myapp
`)
th.WriteF("resource.yaml", `
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: myapp
spec:
template:
spec:
containers:
- name: consul
image: "hashicorp/consul:1.9.1"
ports:
- containerPort: 8500
name: http
- containerPort: 8501
name: https
- containerPort: 8301
protocol: "TCP"
name: serflan-tcp
- containerPort: 8301
protocol: "UDP"
name: serflan-udp
- containerPort: 8302
name: serfwan
- containerPort: 8300
name: server
- containerPort: 8600
name: dns-tcp
protocol: "TCP"
- containerPort: 8600
name: dns-udp
protocol: "UDP"`)
th.WriteF("patch.yaml", `
kind: StatefulSet
metadata:
name: myapp
labels:
test: label
`)
m := th.Run(".", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: apps/v1
kind: StatefulSet
metadata:
labels:
test: label
name: myapp
spec:
template:
spec:
containers:
- image: hashicorp/consul:1.9.1
name: consul
ports:
- containerPort: 8301
name: serflan-tcp
protocol: TCP
- containerPort: 8301
name: serflan-udp
protocol: UDP
- containerPort: 8600
name: dns-tcp
protocol: TCP
- containerPort: 8600
name: dns-udp
protocol: UDP
- containerPort: 8500
name: http
- containerPort: 8501
name: https
- containerPort: 8302
name: serfwan
- containerPort: 8300
name: server
`)
}

View File

@@ -106,9 +106,7 @@ func (e ElementSetter) Filter(rn *RNode) (*RNode, error) {
var err error
found := true
for j := range e.Keys {
if j >= len(e.Values) {
val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: ""})
} else {
if j < len(e.Values) {
val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]})
}
if err != nil {

View File

@@ -127,15 +127,16 @@ func TestElementSetter(t *testing.T) {
`, assertNoErrorString(t)(node.String()))
node = orig.Copy()
// Return error because ElementSetter doesn't support a single key
// when there is a scalar value in the list
// When the element is not found, return an empty ist
_, err = node.Pipe(ElementSetter{Keys: []string{"a"}})
assert.EqualError(t, err, "wrong Node Kind for expected: MappingNode was ScalarNode: value: {scalarValue}")
assert.NoError(t, err)
assert.Equal(t, `[]
`, assertNoErrorString(t)(node.String()))
// Return error because ElementSetter will assume all elements are scalar when
// there is only value provided.
_, err = node.Pipe(ElementSetter{Values: []string{"b"}})
assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}")
assert.NoError(t, err)
assert.Equal(t, `[]
`, assertNoErrorString(t)(node.String()))
node = MustParse(`
- a: b

View File

@@ -16,7 +16,7 @@ import (
// src and dst should be both sequence node. key is used to call ElementSetter.
// ElementSetter will use key-value pair to find and set the element in sequence
// node.
func appendListNode(dst, src *yaml.RNode, keys []string, merge3 bool) (*yaml.RNode, error) {
func appendListNode(dst, src *yaml.RNode, keys []string) (*yaml.RNode, error) {
var err error
for _, elem := range src.Content() {
// If key is empty, we know this is a scalar value and we can directly set the
@@ -33,10 +33,6 @@ func appendListNode(dst, src *yaml.RNode, keys []string, merge3 bool) (*yaml.RNo
continue
}
if len(keys) > 1 && !merge3 {
continue
}
// we need to get the value for key so that we can find the element to set
// in sequence.
v := []string{}
@@ -59,7 +55,6 @@ func appendListNode(dst, src *yaml.RNode, keys []string, merge3 bool) (*yaml.RNo
// We use the key and value from elem to find the corresponding element in dst.
// Then we will use ElementSetter to replace the element with elem. If we cannot
// find the item, the element will be appended.
_, err = dst.Pipe(yaml.ElementSetter{
Element: elem,
Keys: keys,
@@ -73,96 +68,37 @@ func appendListNode(dst, src *yaml.RNode, keys []string, merge3 bool) (*yaml.RNo
return dst, nil
}
// setPrimitiveSequenceElements sets elements in a primitive list
func (l *Walker) setPrimitiveSequenceElements(values []string, key string, dest *yaml.RNode) (*yaml.RNode, error) {
// itemsToBeAdded contains the items that will be added to dest
itemsToBeAdded := yaml.NewListRNode()
var schema *openapi.ResourceSchema
if l.Schema != nil {
schema = l.Schema.Elements()
}
for _, value := range values {
val, err := Walker{
VisitKeysAsScalars: l.VisitKeysAsScalars,
InferAssociativeLists: l.InferAssociativeLists,
Visitor: l,
Schema: schema,
Sources: l.elementValue(key, value),
MergeOptions: l.MergeOptions,
}.Walk()
if err != nil {
return nil, err
}
// delete the node from **dest** if it's null or empty
if yaml.IsMissingOrNull(val) || yaml.IsEmptyMap(val) {
_, err = dest.Pipe(yaml.ElementSetter{Keys: []string{key}, Values: []string{value}})
if err != nil {
return nil, err
}
continue
}
if val.Field(key) == nil {
// make sure the key is set on the field
_, err = val.Pipe(yaml.SetField(key, yaml.NewScalarRNode(value)))
if err != nil {
return nil, err
}
}
// Add the val to the sequence. val will replace the item in the sequence if
// there is an item that matches the key-value pair. Otherwise val will be appended
// the the sequence.
_, err = itemsToBeAdded.Pipe(yaml.ElementSetter{
Element: val.YNode(),
Keys: []string{key},
Values: []string{value},
})
if err != nil {
return nil, err
}
}
var err error
if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend {
// items from patches are needed to be prepended. so we append the
// dest to itemsToBeAdded
dest, err = appendListNode(itemsToBeAdded, dest, []string{""}, len(l.Sources) > 2)
} else {
// append the items
dest, err = appendListNode(dest, itemsToBeAdded, []string{""}, len(l.Sources) > 2)
}
if err != nil {
return nil, err
}
// sequence is empty
if yaml.IsMissingOrNull(dest) {
return nil, nil
}
return dest, nil
}
// validateKeys returns a list of valid key-value pairs
// if secondary merge key values are missing, use only the available merge keys
func validateKeys(value []string, keys []string) ([]string, []string) {
func validateKeys(valuesList [][]string, values []string, keys []string) ([]string, []string) {
validKeys := make([]string, 0)
validValues := make([]string, 0)
for i, v := range value {
if v != "" {
validKeys = append(validKeys, keys[i])
validValues = append(validValues, v)
validKeySet := sets.String{}
for _, values := range valuesList {
for i, v := range values {
if v != "" {
validKeySet.Insert(keys[i])
}
}
}
if len(validKeys) == 0 { // if values missing, fall back to primary keys
validKeys = keys
validValues = value
if validKeySet.Len() == 0 { // if values missing, fall back to primary keys
return keys, values
}
for _, k := range keys {
if validKeySet.Has(k) {
validKeys = append(validKeys, k)
}
}
for i, v := range values {
if v != "" || validKeySet.Has(keys[i]) {
validValues = append(validValues, v)
}
}
return validKeys, validValues
}
// setAssociativeSequenceElements recursively set the elements in the list
func (l *Walker) setAssociativeSequenceElements(values [][]string, keys []string, dest *yaml.RNode) (*yaml.RNode, error) {
func (l *Walker) setAssociativeSequenceElements(valuesList [][]string, keys []string, dest *yaml.RNode) (*yaml.RNode, error) {
// itemsToBeAdded contains the items that will be added to dest
itemsToBeAdded := yaml.NewListRNode()
var schema *openapi.ResourceSchema
@@ -170,12 +106,20 @@ func (l *Walker) setAssociativeSequenceElements(values [][]string, keys []string
schema = l.Schema.Elements()
}
for _, value := range values {
if len(value) == 0 {
// each element in valuesList is a list of values corresponding to the keys
// for example, for the following yaml:
// - containerPort: 8080
// protocol: UDP
// - containerPort: 8080
// protocol: TCP
// `keys` would be [containerPort, protocol]
// and `valuesList` would be [ [8080, UDP], [8080, TCP] ]
for _, values := range valuesList {
if len(values) == 0 {
continue
}
validKeys, validValues := validateKeys(value, keys)
validKeys, validValues := validateKeys(valuesList, values, keys)
val, err := Walker{
VisitKeysAsScalars: l.VisitKeysAsScalars,
InferAssociativeLists: l.InferAssociativeLists,
@@ -218,7 +162,7 @@ func (l *Walker) setAssociativeSequenceElements(values [][]string, keys []string
_, err = itemsToBeAdded.Pipe(yaml.ElementSetter{
Element: val.YNode(),
Keys: validKeys,
Values: validValues,
Values: values,
})
if err != nil {
return nil, err
@@ -226,17 +170,18 @@ func (l *Walker) setAssociativeSequenceElements(values [][]string, keys []string
}
var err error
for _, v := range values {
validKeys, _ := validateKeys(v, keys)
if len(valuesList) > 0 {
validKeys, _ := validateKeys(valuesList, valuesList[0], keys)
if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend {
// items from patches are needed to be prepended. so we append the
// dest to itemsToBeAdded
dest, err = appendListNode(itemsToBeAdded, dest, validKeys, len(l.Sources) > 2)
dest, err = appendListNode(itemsToBeAdded, dest, validKeys)
} else {
// append the items
dest, err = appendListNode(dest, itemsToBeAdded, validKeys, len(l.Sources) > 2)
dest, err = appendListNode(dest, itemsToBeAdded, validKeys)
}
}
if err != nil {
return nil, err
}
@@ -278,7 +223,7 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
}
// primitive associative list -- merge the values
return l.setPrimitiveSequenceElements(l.elementPrimitiveValues(), "", dest)
return l.setAssociativeSequenceElements(l.elementPrimitiveValues(), []string{""}, dest)
}
// elementKey returns the merge key to use for the associative list
@@ -338,9 +283,9 @@ func (l Walker) elementValues(keys []string) [][]string {
}
// elementPrimitiveValues returns the primitive values in an associative list -- eg. finalizers
func (l Walker) elementPrimitiveValues() []string {
func (l Walker) elementPrimitiveValues() [][]string {
// use slice to to keep elements in the original order
var returnValues []string
var returnValues [][]string
seen := sets.String{}
// if we are doing append, dest node should be the first.
// otherwise dest node should be the last.
@@ -360,26 +305,13 @@ func (l Walker) elementPrimitiveValues() []string {
if seen.Has(item.Value) {
continue
}
returnValues = append(returnValues, item.Value)
returnValues = append(returnValues, []string{item.Value})
seen.Insert(item.Value)
}
}
return returnValues
}
// fieldValue returns a slice containing each source's value for fieldName
func (l Walker) elementValue(key, value string) []*yaml.RNode {
var fields []*yaml.RNode
for i := range l.Sources {
if l.Sources[i] == nil {
fields = append(fields, nil)
continue
}
fields = append(fields, l.Sources[i].Element(key, value))
}
return fields
}
// fieldValue returns a slice containing each source's value for fieldName
func (l Walker) elementValueList(keys []string, values []string) []*yaml.RNode {
var fields []*yaml.RNode

View File

@@ -803,10 +803,10 @@ spec:
name: test-deployment
ports:
- containerPort: 8080
name: take-over-the-world
name: disappearing-act
protocol: TCP
- containerPort: 8080
name: disappearing-act
name: take-over-the-world
protocol: TCP
`, `
apiVersion: apps/v1