kubernetes-sigs/kustomize
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/kustomize.git synced 2026-05-17 18:25:26 +00:00
Code Issues Packages Projects Releases 1 Wiki Activity

Update setter comments during 3-way merge

Browse Source
This commit is contained in:
Phani Teja Marupaka
2020-08-02 18:24:56 -07:00
parent 17f935452f
commit 4e74947731
6 changed files with 222 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
kyaml/yaml/merge3/element_test.go
View File

@@ -8,7 +8,8 @@ var elementTestCases = []testCase{
//
// Test Case
//
{description: `Add an element to an existing list`,
{
description: `Add an element to an existing list`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -57,7 +58,8 @@ spec:
//
// Test Case
//
{description: `Add an element to a non-existing list`,
{
description: `Add an element to a non-existing list`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
@@ -86,7 +88,8 @@ spec:
name: foo
`},
{description: `Add an element to a non-existing list, existing in dest`,
{
description: `Add an element to a non-existing list, existing in dest`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
@@ -127,7 +130,8 @@ spec:
// Test Case
// TODO(pwittrock): Figure out if there is something better we can do here
// This element is missing from the destination -- only the new fields are added
{description: `Add a field to the element, element missing from dest`,
{
description: `Add a field to the element, element missing from dest`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -168,7 +172,8 @@ spec:
//
// Test Case
//
{description: `Update a field on the elem, element missing from the dest`,
{
description: `Update a field on the elem, element missing from the dest`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -212,7 +217,8 @@ spec:
//
// Test Case
//
{description: `Update a field on the elem, element present in the dest`,
{
description: `Update a field on the elem, element present in the dest`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -308,7 +314,8 @@ spec:
//
// Test Case
//
{description: `Add a field on the elem, element and field present in the dest`,
{
description: `Add a field on the elem, element and field present in the dest`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -356,7 +363,8 @@ spec:
//
// Test Case
//
{description: `Ignore an element`,
{
description: `Ignore an element`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -397,7 +405,8 @@ spec:
//
// Test Case
//
{description: `Leave deleted`,
{
description: `Leave deleted`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -424,7 +433,8 @@ kind: Deployment
//
// Test Case
//
{description: `Remove an element -- matching`,
{
description: `Remove an element -- matching`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -463,7 +473,8 @@ spec:
//
// Test Case
//
{description: `Remove an element -- field missing from update`,
{
description: `Remove an element -- field missing from update`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -503,7 +514,8 @@ spec:
//
// Test Case
//
{description: `Remove an element -- element missing`,
{
description: `Remove an element -- element missing`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -554,7 +566,8 @@ spec:
//
// Test Case
//
{description: `Remove an element -- empty containers`,
{
description: `Remove an element -- empty containers`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -595,7 +608,8 @@ spec:
//
// Test Case
//
{description: `Remove an element -- missing list field`,
{
description: `Remove an element -- missing list field`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -635,7 +649,8 @@ spec:
//
// Test Case
//
{description: `infer merge keys merge'`,
{
description: `infer merge keys merge'`,
origin: `
kind: Deployment
spec:
@@ -678,7 +693,8 @@ spec:
//
// Test Case
//
{description: `no infer merge keys merge using schema`,
{
description: `no infer merge keys merge using schema`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -725,7 +741,8 @@ spec:
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema as line comment'`,
{
description: `no infer merge keys merge using explicit schema as line comment'`,
origin: `
apiVersion: custom
kind: Deployment
@@ -772,7 +789,8 @@ spec:
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema as head comment'`,
{
description: `no infer merge keys merge using explicit schema as head comment'`,
origin: `
apiVersion: custom
kind: Deployment
@@ -821,7 +839,8 @@ spec:
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema to parent field'`,
{
description: `no infer merge keys merge using explicit schema to parent field'`,
origin: `
apiVersion: custom
kind: Deployment
@@ -860,7 +879,8 @@ spec: # {"$ref":"#/definitions/io.k8s.api.core.v1.PodSpec"}
//
// Test Case
//
{description: `no infer merge keys merge using explicit schema to parent field header'`,
{
description: `no infer merge keys merge using explicit schema to parent field header'`,
origin: `
apiVersion: custom
kind: Deployment

6
kyaml/yaml/merge3/kustomization_test.go
View File

@@ -6,7 +6,8 @@ package merge3_test
var kustomizationTestCases = []testCase{
// Kustomization Test Cases
{description: `ConfigMapGenerator merge`,
{
description: `ConfigMapGenerator merge`,
origin: `
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
@@ -48,7 +49,8 @@ configMapGenerator:
- configkey=configs/another_configfile2
name: a-configmap2`},
{description: `SecretGenerator merge`,
{
description: `SecretGenerator merge`,
origin: `
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

27
kyaml/yaml/merge3/list_test.go
View File

@@ -9,7 +9,8 @@ var listTestCases = []testCase{
//
// Test Case
//
{description: `Replace list`,
{
description: `Replace list`,
origin: `
list:
- 1
@@ -34,7 +35,8 @@ list:
//
// Test Case
//
{description: `Add an updated list`,
{
description: `Add an updated list`,
origin: `
apiVersion: apps/v1
list: # old value
@@ -62,7 +64,8 @@ list:
//
// Test Case
//
{description: `Add keep an omitted field`,
{
description: `Add keep an omitted field`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
@@ -89,7 +92,8 @@ kind: StatefulSet
// Test Case
//
// TODO(#36): consider making this an error
{description: `Change an updated field`,
{
description: `Change an updated field`,
origin: `
apiVersion: apps/v1
list: # old value
@@ -119,7 +123,8 @@ list: # conflicting value
//
// Test Case
//
{description: `Ignore a field -- set`,
{
description: `Ignore a field -- set`,
origin: `
apiVersion: apps/v1
list: # ignore value
@@ -151,7 +156,8 @@ list:
//
// Test Case
//
{description: `Ignore a field -- empty`,
{
description: `Ignore a field -- empty`,
origin: `
apiVersion: apps/v1
list: # ignore value
@@ -174,7 +180,8 @@ apiVersion: apps/v1
//
// Test Case
//
{description: `Explicitly clear a field`,
{
description: `Explicitly clear a field`,
origin: `
apiVersion: apps/v1`,
update: `
@@ -192,7 +199,8 @@ apiVersion: apps/v1`},
//
// Test Case
//
{description: `Implicitly clear a field`,
{
description: `Implicitly clear a field`,
origin: `
apiVersion: apps/v1
list: # clear value
@@ -214,7 +222,8 @@ apiVersion: apps/v1`},
// Test Case
//
// TODO(#36): consider making this an error
{description: `Implicitly clear a changed field`,
{
description: `Implicitly clear a changed field`,
origin: `
apiVersion: apps/v1
list: # old value

37
kyaml/yaml/merge3/map_test.go
View File

@@ -7,7 +7,8 @@ var mapTestCases = []testCase{
//
// Test Case
//
{description: `Add the annotations map field`,
{
description: `Add the annotations map field`,
origin: `
kind: Deployment`,
update: `
@@ -27,7 +28,8 @@ metadata:
//
// Test Case
//
{description: `Add an annotation to the field`,
{
description: `Add an annotation to the field`,
origin: `
kind: Deployment
metadata:
@@ -54,7 +56,8 @@ metadata:
//
// Test Case
//
{description: `Add an annotation to the field, field missing from dest`,
{
description: `Add an annotation to the field, field missing from dest`,
origin: `
kind: Deployment
metadata:
@@ -64,7 +67,7 @@ metadata:
kind: Deployment
metadata:
annotations:
a: b # ignore because unchanged
a: b # ignored because unchanged
d: e`,
local: `
kind: Deployment`,
@@ -77,7 +80,8 @@ metadata:
//
// Test Case
//
{description: `Update an annotation on the field, field messing rom the dest`,
{
description: `Update an annotation on the field, field messing rom the dest`,
origin: `
kind: Deployment
metadata:
@@ -105,7 +109,8 @@ metadata:
//
// Test Case
//
{description: `Add an annotation to the field, field missing from dest`,
{
description: `Add an annotation to the field, field missing from dest`,
origin: `
kind: Deployment
metadata:
@@ -115,7 +120,7 @@ metadata:
kind: Deployment
metadata:
annotations:
a: b # ignore because unchanged
a: b # ignored because unchanged
d: e`,
local: `
kind: Deployment`,
@@ -128,7 +133,8 @@ metadata:
//
// Test Case
//
{description: `Remove an annotation`,
{
description: `Remove an annotation`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -158,7 +164,8 @@ metadata:
// Test Case
//
// TODO(#36) support ~annotations~: {} deletion
{description: `Specify a field as empty that isn't present in the source`,
{
description: `Specify a field as empty that isn't present in the source`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -186,7 +193,8 @@ metadata:
//
// Test Case
//
{description: `Remove an annotation`,
{
description: `Remove an annotation`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -213,7 +221,8 @@ metadata:
//
// Test Case
//
{description: `Remove annotations field`,
{
description: `Remove annotations field`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -238,7 +247,8 @@ metadata:
//
// Test Case
//
{description: `Remove annotations field, but keep in dest`,
{
description: `Remove annotations field, but keep in dest`,
origin: `
apiVersion: apps/v1
kind: Deployment
@@ -266,7 +276,8 @@ metadata:
//
// Test Case
//
{description: `Remove annotations, but they are already empty`,
{
description: `Remove annotations, but they are already empty`,
origin: `
apiVersion: apps/v1
kind: Deployment

133
kyaml/yaml/merge3/scalar_test.go
View File

@@ -8,13 +8,15 @@ var scalarTestCases = []testCase{
//
// Test Case
//
{description: `Set and updated a field`,
origin: `kind: Deployment`,
update: `kind: StatefulSet`,
local: `kind: Deployment`,
expected: `kind: StatefulSet`},
{
description: `Set and updated a field`,
origin: `kind: Deployment`,
update: `kind: StatefulSet`,
local: `kind: Deployment`,
expected: `kind: StatefulSet`},
{description: `Add an updated field`,
{
description: `Add an updated field`,
origin: `
apiVersion: apps/v1
kind: Deployment # old value`,
@@ -27,7 +29,109 @@ apiVersion: apps/v1`,
apiVersion: apps/v1
kind: StatefulSet # new value`},
{description: `Add keep an omitted field`,
//
// Test Case
//
{
description: `Ensure comments are updated`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'dest'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 3`,
update: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'updated'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 3 # {"$openapi":"replicas"}`,
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'original'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 3
`,
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'updated'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 3 # {"$openapi":"replicas"}
`},
{
description: `Ensure deleted comments are not updated`,
origin: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'dest'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 3 # {"$openapi":"replicas"}`,
update: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'updated'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 3 # {"$openapi":"replicas"}`,
local: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'original'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 4
`,
expected: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/merge-source: 'updated'
config.kubernetes.io/path: 'temp.yaml'
spec:
replicas: 4
`},
{
description: `Add keep an omitted field`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
@@ -48,7 +152,8 @@ kind: StatefulSet
// Test Case
//
// TODO(#36): consider making this an error
{description: `Change an updated field`,
{
description: `Change an updated field`,
origin: `
apiVersion: apps/v1
kind: Deployment # old value`,
@@ -62,7 +167,8 @@ kind: Service # conflicting value`,
apiVersion: apps/v1
kind: StatefulSet # new value`},
{description: `Ignore a field`,
{
description: `Ignore a field`,
origin: `
apiVersion: apps/v1
kind: Deployment # ignore this field`,
@@ -74,7 +180,8 @@ apiVersion: apps/v1`,
expected: `
apiVersion: apps/v1`},
{description: `Explicitly clear a field`,
{
description: `Explicitly clear a field`,
origin: `
apiVersion: apps/v1`,
update: `
@@ -102,7 +209,8 @@ apiVersion: apps/v1`},
// Test Case
//
// TODO(#36): consider making this an error
{description: `Implicitly clear a changed field`,
{
description: `Implicitly clear a changed field`,
origin: `
apiVersion: apps/v1
kind: Deployment`,
@@ -117,7 +225,8 @@ apiVersion: apps/v1`},
//
// Test Case
//
{description: `Merge an empty scalar value`,
{
description: `Merge an empty scalar value`,
origin: `
apiVersion: apps/v1
`,

15
kyaml/yaml/merge3/visitor.go
View File

@@ -4,6 +4,7 @@
package merge3
import (
"sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/openapi"
"sigs.k8s.io/kustomize/kyaml/yaml"
"sigs.k8s.io/kustomize/kyaml/yaml/walk"
@@ -66,6 +67,20 @@ func (m Visitor) VisitScalar(nodes walk.Sources, s *openapi.ResourceSchema) (*ya
return nodes.Dest(), nil
}
updatedStr, err := nodes.Updated().String()
if err != nil {
return nil, errors.Wrap(err)
}
originStr, err := nodes.Origin().String()
if err != nil {
return nil, errors.Wrap(err)
}
if updatedStr != originStr {
// change in update node
return nodes.Updated(), nil
}
if nodes.Updated().YNode().Value != nodes.Origin().YNode().Value {
// value changed in update
return nodes.Updated(), nil