Skip to content
Permalink
Browse files
fix(secrets): panic and incorrect behaviour during secrets edit
Problem reproduced when adding, removing or changin order of map keys. Or when removing and changing order of array elements.

Signed-off-by: Timofey Kirillov <timofey.kirillov@flant.com>
  • Loading branch information
distorhead committed Jul 1, 2022
1 parent 2e404a9 commit 289400d3aec3794dc310942fb81d0b3423843753
Show file tree
Hide file tree
Showing 3 changed files with 325 additions and 4 deletions.
@@ -28,7 +28,14 @@ var _ = Describe("YamlEncoder", func() {
Expect(strings.TrimSpace(originalData)).To(Equal(strings.TrimSpace(string(resultData))), fmt.Sprintf("\n[EXPECTED]\n%q\n[GOT]\n%q\n", originalData, resultData))
},

Entry("simple yaml", `a: one`),
Entry("simple yaml", `
a: one
bbb:
url: http://service:3441/bbb
postgresql:
password:
_default: "1234"
`),

Entry("yaml with anchors", `
common_values: &common-values
@@ -41,14 +41,22 @@ func MergeEncodedYaml(oldData, newData, oldEncodedData, newEncodedData []byte) (
}

func MergeEncodedYamlNode(oldConfig, newConfig, oldEncodedConfig, newEncodedConfig *yaml_v3.Node) (*yaml_v3.Node, error) {
if oldConfig == nil {
return newEncodedConfig, nil
}
if newConfig.Kind != oldConfig.Kind {
return newEncodedConfig, nil
}

switch newEncodedConfig.Kind {
case yaml_v3.DocumentNode:
for pos := 0; pos < len(newEncodedConfig.Content); pos += 1 {
newValueNode, err := MergeEncodedYamlNode(oldConfig.Content[pos], newConfig.Content[pos], oldEncodedConfig.Content[pos], newEncodedConfig.Content[pos])
newEncodedValue := newEncodedConfig.Content[pos]
newValue := newConfig.Content[pos]
oldEncodedValue := getSubNodeByIndex(oldEncodedConfig, pos)
oldValue := getSubNodeByIndex(oldConfig, pos)

newValueNode, err := MergeEncodedYamlNode(oldValue, newValue, oldEncodedValue, newEncodedValue)
if err != nil {
return nil, fmt.Errorf("unable to process document key %d: %w", pos, err)
}
@@ -57,7 +65,14 @@ func MergeEncodedYamlNode(oldConfig, newConfig, oldEncodedConfig, newEncodedConf

case yaml_v3.MappingNode:
for pos := 0; pos < len(newEncodedConfig.Content); pos += 2 {
newValueNode, err := MergeEncodedYamlNode(oldConfig.Content[pos+1], newConfig.Content[pos+1], oldEncodedConfig.Content[pos+1], newEncodedConfig.Content[pos+1])
newKey := newEncodedConfig.Content[pos]

newEncodedValue := newEncodedConfig.Content[pos+1]
newValue := newConfig.Content[pos+1]
oldEncodedValue := getSubNodeByKey(oldEncodedConfig, newKey.Value)
oldValue := getSubNodeByKey(oldConfig, newKey.Value)

newValueNode, err := MergeEncodedYamlNode(oldValue, newValue, oldEncodedValue, newEncodedValue)
if err != nil {
return nil, fmt.Errorf("unable to process map key %q: %w", newEncodedConfig.Content[pos].Value, err)
}
@@ -66,7 +81,12 @@ func MergeEncodedYamlNode(oldConfig, newConfig, oldEncodedConfig, newEncodedConf

case yaml_v3.SequenceNode:
for pos := 0; pos < len(newEncodedConfig.Content); pos += 1 {
newValueNode, err := MergeEncodedYamlNode(oldConfig.Content[pos], newConfig.Content[pos], oldEncodedConfig.Content[pos], newEncodedConfig.Content[pos])
newEncodedValue := newEncodedConfig.Content[pos]
newValue := newConfig.Content[pos]
oldEncodedValue := getSubNodeByIndex(oldEncodedConfig, pos)
oldValue := getSubNodeByIndex(oldConfig, pos)

newValueNode, err := MergeEncodedYamlNode(oldValue, newValue, oldEncodedValue, newEncodedValue)
if err != nil {
return nil, fmt.Errorf("unable to process array key %d: %w", pos, err)
}
@@ -89,3 +109,20 @@ func MergeEncodedYamlNode(oldConfig, newConfig, oldEncodedConfig, newEncodedConf

return newEncodedConfig, nil
}

func getSubNodeByIndex(node *yaml_v3.Node, ind int) *yaml_v3.Node {
if ind < len(node.Content) {
return node.Content[ind]
}
return nil
}

func getSubNodeByKey(node *yaml_v3.Node, rawKey string) *yaml_v3.Node {
for i := 0; i < len(node.Content); i += 2 {
k, v := node.Content[i], node.Content[i+1]
if k.Value == rawKey {
return v
}
}
return nil
}
@@ -199,6 +199,283 @@ values:
key4: value4-changed-encoded1
# include common list
key5: *common-list
`),
}),

Entry("added elements into map and array", MergeEncodedYamlTest{
OldData: []byte(`
database:
user: vasya
password: gfhjkm
hosts:
- name: local1
address: a1
- name: local2
address: a2
`),
OldEncodedData: []byte(`
database:
user: enc1
password: enc2
hosts:
- name: enc3
address: enc4
- name: enc5
address: enc6
`),
NewData: []byte(`
database:
user: vasya
password: gfhjkm
admin: petya
adminPassword:
_default: "1234"
production: "#@$213sldfzjxXFLKJSdf1233s"
hosts:
- name: local1
address: a1
options:
timeout: 5s
- name: local2
address: a2
`),
NewEncodedData: []byte(`
database:
user: enc1-1
password: enc2-1
admin: enc3-1
adminPassword:
_default: enc4-1
production: enc5-1
hosts:
- name: enc6-1
address: enc7-1
options:
timeout: enc8-1
- name: enc9-1
address: enc10-1
`),
ExpectedResult: []byte(`
database:
user: enc1
password: enc2
admin: enc3-1
adminPassword:
_default: enc4-1
production: enc5-1
hosts:
- name: enc3
address: enc4
options:
timeout: enc8-1
- name: enc5
address: enc6
`),
}),

Entry("removed elements from map and array", MergeEncodedYamlTest{
OldData: []byte(`
database:
user: vasya
password: gfhjkm
admin: petya
adminPassword:
_default: "1234"
production: "#@$213sldfzjxXFLKJSdf1233s"
hosts:
- name: local1
address: a1
options:
timeout: 5s
- name: local2
address: a2
`),
OldEncodedData: []byte(`
database:
user: enc1
password: enc2
admin: enc3
adminPassword:
_default: enc4
production: enc5
hosts:
- name: enc6
address: enc7
options:
timeout: enc8
- name: enc9
address: enc10
`),
NewData: []byte(`
database:
user: vasya
adminPassword:
_default: "1234"
production: "#@$213sldfzjxXFLKJSdf1233s"
hosts:
- name: local2
address: a2
`),
NewEncodedData: []byte(`
database:
user: enc1-1
adminPassword:
_default: enc2-1
production: enc3-1
hosts:
- name: enc4-1
address: enc5-1
`),
ExpectedResult: []byte(`
database:
user: enc1
adminPassword:
_default: enc4
production: enc5
hosts:
- name: enc4-1
address: enc5-1
`),
}),

Entry("no changes in array", MergeEncodedYamlTest{
OldData: []byte(`
arr:
- k1: v1
k2: v2
- k1: v3
k2: v4
- k1: v5
k2: v6
`),
OldEncodedData: []byte(`
arr:
- k1: enc1
k2: enc2
- k1: enc3
k2: enc4
- k1: enc5
k2: enc6
`),
NewData: []byte(`
arr:
- k1: v1
k2: v2
- k1: v3
k2: v4
- k1: v5
k2: v6
`),
NewEncodedData: []byte(`
arr:
- k1: enc1-1
k2: enc2-1
- k1: enc3-1
k2: enc4-1
- k1: enc5-1
k2: enc6-1
`),
ExpectedResult: []byte(`
arr:
- k1: enc1
k2: enc2
- k1: enc3
k2: enc4
- k1: enc5
k2: enc6
`),
}),

Entry("random crazy mindless changes: change order of array elements, swap map keys with different values, change order of map values", MergeEncodedYamlTest{
OldData: []byte(`
database:
user: vasya
password: gfhjkm
admin: petya
adminPassword:
_default: "1234"
production: "#@$213sldfzjxXFLKJSdf1233s"
hosts:
- name: local0
address: a0
- name: local1
address: a1
options:
timeout: 5s
- name: local2
address: a2
`),
OldEncodedData: []byte(`
database:
user: enc1
password: enc2
admin: enc3
adminPassword:
_default: enc4
production: enc5
hosts:
- name: enc6
address: enc7
- name: enc8
address: enc9
options:
timeout: enc10
- name: enc11
address: enc12
`),
NewData: []byte(`
database:
password: gfhjkm
user: vasya
adminPassword: petya
admin:
_default: "1234"
production: "#@$213sldfzjxXFLKJSdf1233s"
hosts:
- name: local0
address: new-addr
- name: local2
address: a2
- name: local1
address: a1
options:
timeout: 5s
`),
NewEncodedData: []byte(`
database:
password: enc1-1
user: enc2-1
adminPassword: enc3-1
admin:
_default: enc4-1
production: enc5-1
hosts:
- name: enc6-1
address: enc7-1
- name: enc8-1
address: enc9-1
- name: enc10-1
address: enc11-1
options:
timeout: enc12-1
`),
ExpectedResult: []byte(`
database:
password: enc2
user: enc1
adminPassword: enc3-1
admin:
_default: enc4-1
production: enc5-1
hosts:
- name: enc6
address: enc7-1
- name: enc8-1
address: enc9-1
- name: enc10-1
address: enc11-1
options:
timeout: enc12-1
`),
}),
)

0 comments on commit 289400d

Please sign in to comment.