From a64c83753f5199394e16ddc9fe0f8d4fe3f0443f Mon Sep 17 00:00:00 2001 From: sethiyash Date: Tue, 8 Nov 2022 10:16:28 +0530 Subject: [PATCH 01/15] started supporting regex in config path Signed-off-by: sethiyash --- pkg/kapp/resources/matcher_empty_field.go | 12 +++++----- pkg/kapp/resources/mod_field_copy.go | 25 ++++++++++----------- pkg/kapp/resources/mod_field_remove.go | 12 +++++----- pkg/kapp/resources/mod_object_ref_set.go | 12 +++++----- pkg/kapp/resources/mod_path.go | 23 ++++++++++--------- pkg/kapp/resources/mod_string_map_append.go | 12 +++++----- test/e2e/cluster_resource.go | 8 +++---- 7 files changed, 53 insertions(+), 51 deletions(-) diff --git a/pkg/kapp/resources/matcher_empty_field.go b/pkg/kapp/resources/matcher_empty_field.go index 0fa348d39..e279eba91 100644 --- a/pkg/kapp/resources/matcher_empty_field.go +++ b/pkg/kapp/resources/matcher_empty_field.go @@ -33,9 +33,9 @@ func (m EmptyFieldMatcher) check(obj interface{}, path Path) bool { return true } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: switch { - case part.ArrayIndex.All != nil: + case part.IndexAndRegex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return obj == nil @@ -50,21 +50,21 @@ func (m EmptyFieldMatcher) check(obj interface{}, path Path) bool { return true - case part.ArrayIndex.Index != nil: + case part.IndexAndRegex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return obj == nil } - if *part.ArrayIndex.Index < len(typedObj) { - obj = typedObj[*part.ArrayIndex.Index] + if *part.IndexAndRegex.Index < len(typedObj) { + obj = typedObj[*part.IndexAndRegex.Index] } else { // Index not found, it's empty return true } default: - panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } default: diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index ea02eb433..69515359b 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -75,13 +75,13 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ typedObj[*part.MapKey] = obj } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: if isLast { return false, fmt.Errorf("Expected last part of the path to be map key") } switch { - case part.ArrayIndex.All != nil: + case part.IndexAndRegex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return false, fmt.Errorf("Unexpected non-array found: %T", obj) @@ -93,7 +93,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ objI := objI newFullPath := append([]*PathPart{}, fullPath...) - newFullPath[len(newFullPath)-1] = &PathPart{ArrayIndex: &PathPartArrayIndex{Index: &objI}} + newFullPath[len(newFullPath)-1] = &PathPart{IndexAndRegex: &PathPartIndexAndRegex{Index: &objI}} updated, err := t.apply(obj, path[i+1:], newFullPath, srcs) if err != nil { @@ -106,21 +106,20 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return anyUpdated, nil // dealt with children, get out - case part.ArrayIndex.Index != nil: + case part.IndexAndRegex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return false, fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.ArrayIndex.Index < len(typedObj) { - obj = typedObj[*part.ArrayIndex.Index] + if *part.IndexAndRegex.Index < len(typedObj) { + obj = typedObj[*part.IndexAndRegex.Index] return t.apply(obj, path[i+1:], fullPath, srcs) } return false, nil // index not found, nothing to append to - default: - panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } default: @@ -174,26 +173,26 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return nil, false, nil // index is not found return } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: if isLast { return nil, false, fmt.Errorf("Expected last part of the path to be map key") } switch { - case part.ArrayIndex.Index != nil: + case part.IndexAndRegex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return nil, false, fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.ArrayIndex.Index < len(typedObj) { - obj = typedObj[*part.ArrayIndex.Index] + if *part.IndexAndRegex.Index < len(typedObj) { + obj = typedObj[*part.IndexAndRegex.Index] } else { return nil, false, nil // index not found, return } default: - panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } default: diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 3f7d0569e..04c41923b 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -60,13 +60,13 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return nil // map key is not found, nothing to remove } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: if isLast { return fmt.Errorf("Expected last part of the path to be map key") } switch { - case part.ArrayIndex.All != nil: + case part.IndexAndRegex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) @@ -81,20 +81,20 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return nil // dealt with children, get out - case part.ArrayIndex.Index != nil: + case part.IndexAndRegex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.ArrayIndex.Index < len(typedObj) { - obj = typedObj[*part.ArrayIndex.Index] + if *part.IndexAndRegex.Index < len(typedObj) { + obj = typedObj[*part.IndexAndRegex.Index] } else { return nil // index not found, nothing to remove } default: - panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } default: diff --git a/pkg/kapp/resources/mod_object_ref_set.go b/pkg/kapp/resources/mod_object_ref_set.go index 40d3d46f7..f290f624b 100644 --- a/pkg/kapp/resources/mod_object_ref_set.go +++ b/pkg/kapp/resources/mod_object_ref_set.go @@ -41,9 +41,9 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { return nil } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: switch { - case part.ArrayIndex.All != nil: + case part.IndexAndRegex.All != nil: if obj == nil { return nil } @@ -61,20 +61,20 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { return nil // dealt with children, get out - case part.ArrayIndex.Index != nil: + case part.IndexAndRegex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.ArrayIndex.Index < len(typedObj) { - return t.apply(typedObj[*part.ArrayIndex.Index], path[i+1:]) + if *part.IndexAndRegex.Index < len(typedObj) { + return t.apply(typedObj[*part.IndexAndRegex.Index], path[i+1:]) } return nil // index not found, nothing to append to default: - panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } default: diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index ca84392a1..d74df9fbd 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -21,15 +21,16 @@ type ResourceModWithMultiple interface { type Path []*PathPart type PathPart struct { - MapKey *string - ArrayIndex *PathPartArrayIndex + MapKey *string + IndexAndRegex *PathPartIndexAndRegex } var _ json.Unmarshaler = &PathPart{} -type PathPartArrayIndex struct { +type PathPartIndexAndRegex struct { Index *int All *bool `json:"allIndexes"` + Regex *string `json:"regex"` } func NewPathFromStrings(strs []string) Path { @@ -88,22 +89,24 @@ func NewPathPartFromString(str string) *PathPart { } func NewPathPartFromIndex(i int) *PathPart { - return &PathPart{ArrayIndex: &PathPartArrayIndex{Index: &i}} + return &PathPart{IndexAndRegex: &PathPartIndexAndRegex{Index: &i}} } func NewPathPartFromIndexAll() *PathPart { trueBool := true - return &PathPart{ArrayIndex: &PathPartArrayIndex{All: &trueBool}} + return &PathPart{IndexAndRegex: &PathPartIndexAndRegex{All: &trueBool}} } func (p *PathPart) AsString() string { switch { case p.MapKey != nil: return *p.MapKey - case p.ArrayIndex != nil && p.ArrayIndex.Index != nil: - return fmt.Sprintf("%d", *p.ArrayIndex.Index) - case p.ArrayIndex != nil && p.ArrayIndex.All != nil: + case p.IndexAndRegex != nil && p.IndexAndRegex.Index != nil: + return fmt.Sprintf("%d", *p.IndexAndRegex.Index) + case p.IndexAndRegex != nil && p.IndexAndRegex.All != nil: return "(all)" + case p.IndexAndRegex != nil && p.IndexAndRegex.Regex != nil: + return *p.IndexAndRegex.Regex default: panic("Unknown path part") } @@ -111,13 +114,13 @@ func (p *PathPart) AsString() string { func (p *PathPart) UnmarshalJSON(data []byte) error { var str string - var idx PathPartArrayIndex + var idx PathPartIndexAndRegex switch { case json.Unmarshal(data, &str) == nil: p.MapKey = &str case json.Unmarshal(data, &idx) == nil: - p.ArrayIndex = &idx + p.IndexAndRegex = &idx default: return fmt.Errorf("Unknown path part") } diff --git a/pkg/kapp/resources/mod_string_map_append.go b/pkg/kapp/resources/mod_string_map_append.go index b4c8cbe57..00aa104b9 100644 --- a/pkg/kapp/resources/mod_string_map_append.go +++ b/pkg/kapp/resources/mod_string_map_append.go @@ -51,9 +51,9 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { typedObj[*part.MapKey] = obj } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: switch { - case part.ArrayIndex.All != nil: + case part.IndexAndRegex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) @@ -68,20 +68,20 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { return nil // dealt with children, get out - case part.ArrayIndex.Index != nil: + case part.IndexAndRegex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.ArrayIndex.Index < len(typedObj) { - return t.apply(typedObj[*part.ArrayIndex.Index], path[i+1:]) + if *part.IndexAndRegex.Index < len(typedObj) { + return t.apply(typedObj[*part.IndexAndRegex.Index], path[i+1:]) } return nil // index not found, nothing to append to default: - panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } default: diff --git a/test/e2e/cluster_resource.go b/test/e2e/cluster_resource.go index 15cac63a0..9b32ca04f 100644 --- a/test/e2e/cluster_resource.go +++ b/test/e2e/cluster_resource.go @@ -104,15 +104,15 @@ func (r ClusterResource) RawPath(path ctlres.Path) interface{} { panic(fmt.Sprintf("Expected to find key %s", *part.MapKey)) } - case part.ArrayIndex != nil: + case part.IndexAndRegex != nil: typedResult, ok := result.([]interface{}) if !ok { panic("Expected to find array") } switch { - case part.ArrayIndex.Index != nil: - result = typedResult[*part.ArrayIndex.Index] - case part.ArrayIndex.All != nil: + case part.IndexAndRegex.Index != nil: + result = typedResult[*part.IndexAndRegex.Index] + case part.IndexAndRegex.All != nil: panic("Unsupported array index all") default: panic("Unknown array index") From 7c6aa1c25b0e99fd5d1312f87e9f2ff0ab795ab2 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Mon, 14 Nov 2022 23:09:41 +0530 Subject: [PATCH 02/15] copying with calling apply recursively Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 47 +++++++++++++++++++++++++++- pkg/kapp/resources/mod_path.go | 11 ++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 69515359b..3f0593654 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -5,6 +5,7 @@ package resources import ( "fmt" + "regexp" ) type FieldCopyModSource string @@ -76,7 +77,8 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } case part.IndexAndRegex != nil: - if isLast { + //if isLast { + if isLast && part.IndexAndRegex.Regex == nil { return false, fmt.Errorf("Expected last part of the path to be map key") } @@ -118,6 +120,49 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } return false, nil // index not found, nothing to append to + + case part.IndexAndRegex.Regex != nil: + + regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + if err != nil { + return false, err + } + + typedObj, ok := obj.(map[string]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-map found: %T", obj) + } + //var newObj []interface{} + //path = append(path, &PathPart{ + // IndexAndRegex: &PathPartIndexAndRegex{ + // All: func() *bool { + // all := true + // return &all + // }(), + // }}) + success := true + for key, _ := range typedObj { + + if regex.MatchString(key) { + keyPointer := key + newFullPath := fullPath[:len(fullPath)-1] + //pathPart := PathPart{MapKey: &keyPointer} + //newFullPath = append(newFullPath, &pathPart) + //newPath := path[:len(path)-1] + //newPath = append(newPath, &pathPart) + + status, err := t.apply(obj, []*PathPart{&PathPart{MapKey: &keyPointer}}, newFullPath, srcs) + if err != nil { + return false, nil + } + success = success && status + //path = append(path, &PathPart{MapKey: &keyAddr}) + //newObj = append(newObj, &PathPart{MapKey: &keyAddr}) + } + } + //return t.apply(newObj, path[i+1:], fullPath, srcs) + return success, nil + default: panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index d74df9fbd..348f743d0 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -29,7 +29,7 @@ var _ json.Unmarshaler = &PathPart{} type PathPartIndexAndRegex struct { Index *int - All *bool `json:"allIndexes"` + All *bool `json:"allIndexes"` Regex *string `json:"regex"` } @@ -107,6 +107,8 @@ func (p *PathPart) AsString() string { return "(all)" case p.IndexAndRegex != nil && p.IndexAndRegex.Regex != nil: return *p.IndexAndRegex.Regex + //case p.RegexPath != nil && p.RegexPath.Regex != nil: + // return *p.RegexPath.Regex default: panic("Unknown path part") } @@ -115,12 +117,19 @@ func (p *PathPart) AsString() string { func (p *PathPart) UnmarshalJSON(data []byte) error { var str string var idx PathPartIndexAndRegex + //var regx PathPartRegex switch { case json.Unmarshal(data, &str) == nil: + //if strings.Contains(str, "regex") { + // fmt.Println(str) + //} p.MapKey = &str case json.Unmarshal(data, &idx) == nil: p.IndexAndRegex = &idx + //case json.Unmarshal(data, ®x) == nil: + // fmt.Println(regx) + // p.RegexPath = ®x default: return fmt.Errorf("Unknown path part") } From 2ec34f62b0a637bef51aa1900ee0e230a63d098f Mon Sep 17 00:00:00 2001 From: sethiyash Date: Fri, 18 Nov 2022 16:41:59 +0530 Subject: [PATCH 03/15] refactoring Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 31 +++++++------------------- pkg/kapp/resources/mod_field_remove.go | 2 +- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 3f0593654..767afc84a 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -77,7 +77,6 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } case part.IndexAndRegex != nil: - //if isLast { if isLast && part.IndexAndRegex.Regex == nil { return false, fmt.Errorf("Expected last part of the path to be map key") } @@ -132,36 +131,22 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ if !ok { return false, fmt.Errorf("Unexpected non-map found: %T", obj) } - //var newObj []interface{} - //path = append(path, &PathPart{ - // IndexAndRegex: &PathPartIndexAndRegex{ - // All: func() *bool { - // all := true - // return &all - // }(), - // }}) - success := true + var anyUpdated bool for key, _ := range typedObj { - if regex.MatchString(key) { keyPointer := key - newFullPath := fullPath[:len(fullPath)-1] - //pathPart := PathPart{MapKey: &keyPointer} - //newFullPath = append(newFullPath, &pathPart) - //newPath := path[:len(path)-1] - //newPath = append(newPath, &pathPart) + path[len(path)-1] = &PathPart{MapKey: &keyPointer} - status, err := t.apply(obj, []*PathPart{&PathPart{MapKey: &keyPointer}}, newFullPath, srcs) + updated, err := t.apply(obj, path[i:], fullPath[:len(fullPath)-1], srcs) if err != nil { - return false, nil + return false, err + } + if updated { + anyUpdated = true } - success = success && status - //path = append(path, &PathPart{MapKey: &keyAddr}) - //newObj = append(newObj, &PathPart{MapKey: &keyAddr}) } } - //return t.apply(newObj, path[i+1:], fullPath, srcs) - return success, nil + return anyUpdated, nil default: panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 04c41923b..c0fa4bc24 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -103,4 +103,4 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { } panic("unreachable") -} +} \ No newline at end of file From 36585b945ff94ae08dcf1c16614ab8f29e15f782 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Thu, 24 Nov 2022 13:36:31 +0530 Subject: [PATCH 04/15] fixing linting issues Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 6 +++--- pkg/kapp/resources/mod_field_remove.go | 2 +- pkg/kapp/resources/mod_path.go | 9 --------- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 767afc84a..0b8c98827 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -132,12 +132,12 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, fmt.Errorf("Unexpected non-map found: %T", obj) } var anyUpdated bool - for key, _ := range typedObj { + for key, obj := range typedObj { if regex.MatchString(key) { keyPointer := key path[len(path)-1] = &PathPart{MapKey: &keyPointer} - - updated, err := t.apply(obj, path[i:], fullPath[:len(fullPath)-1], srcs) + newFullPath := fullPath[:len(fullPath)-1] + updated, err := t.apply(obj, path[i:], newFullPath, srcs) if err != nil { return false, err } diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index c0fa4bc24..04c41923b 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -103,4 +103,4 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { } panic("unreachable") -} \ No newline at end of file +} diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 348f743d0..95a782fdd 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -107,8 +107,6 @@ func (p *PathPart) AsString() string { return "(all)" case p.IndexAndRegex != nil && p.IndexAndRegex.Regex != nil: return *p.IndexAndRegex.Regex - //case p.RegexPath != nil && p.RegexPath.Regex != nil: - // return *p.RegexPath.Regex default: panic("Unknown path part") } @@ -117,19 +115,12 @@ func (p *PathPart) AsString() string { func (p *PathPart) UnmarshalJSON(data []byte) error { var str string var idx PathPartIndexAndRegex - //var regx PathPartRegex switch { case json.Unmarshal(data, &str) == nil: - //if strings.Contains(str, "regex") { - // fmt.Println(str) - //} p.MapKey = &str case json.Unmarshal(data, &idx) == nil: p.IndexAndRegex = &idx - //case json.Unmarshal(data, ®x) == nil: - // fmt.Println(regx) - // p.RegexPath = ®x default: return fmt.Errorf("Unknown path part") } From c75a93b352af8a776395f101731cb01f08f84322 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Tue, 6 Dec 2022 14:07:42 +0530 Subject: [PATCH 05/15] stop overwriting object in copy mod and made changes in remove mod Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 7 +++---- pkg/kapp/resources/mod_field_remove.go | 23 ++++++++++++++++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 0b8c98827..b2ff2793b 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -121,7 +121,6 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, nil // index not found, nothing to append to case part.IndexAndRegex.Regex != nil: - regex, err := regexp.Compile(*part.IndexAndRegex.Regex) if err != nil { return false, err @@ -131,11 +130,11 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ if !ok { return false, fmt.Errorf("Unexpected non-map found: %T", obj) } + var anyUpdated bool - for key, obj := range typedObj { + for key, _ := range typedObj { if regex.MatchString(key) { - keyPointer := key - path[len(path)-1] = &PathPart{MapKey: &keyPointer} + path[len(path)-1] = &PathPart{MapKey: &key} newFullPath := fullPath[:len(fullPath)-1] updated, err := t.apply(obj, path[i:], newFullPath, srcs) if err != nil { diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 04c41923b..573f01b6f 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -5,6 +5,7 @@ package resources import ( "fmt" + "regexp" ) type FieldRemoveMod struct { @@ -61,7 +62,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { } case part.IndexAndRegex != nil: - if isLast { + if isLast && part.IndexAndRegex.Regex == nil { return fmt.Errorf("Expected last part of the path to be map key") } @@ -93,6 +94,26 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return nil // index not found, nothing to remove } + case part.IndexAndRegex.Regex != nil: + regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + if err != nil { + return err + } + + typedObj, ok := obj.(map[string]interface{}) + if !ok { + return fmt.Errorf("Unexpected non-map found: %T", obj) + } + for key, _ := range typedObj { + if regex.MatchString(key) { + path[len(path)-1] = &PathPart{MapKey: &key} + err := t.apply(obj, path[i:]) + if err != nil { + return err + } + } + } + return nil default: panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } From 4aceebeef32c129171a986648e606fb2b7e1c424 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Wed, 4 Jan 2023 10:47:05 +0530 Subject: [PATCH 06/15] tweaked the logic to resolve issues Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 83 +++++++++++++++++++------- pkg/kapp/resources/mod_field_remove.go | 37 ++++++++---- pkg/kapp/resources/mod_path.go | 2 +- 3 files changed, 87 insertions(+), 35 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index b2ff2793b..6de76e39b 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -121,32 +121,23 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, nil // index not found, nothing to append to case part.IndexAndRegex.Regex != nil: - regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + matchedKeys, err := t.matchRegexWithSources(path, srcs) if err != nil { return false, err } - - typedObj, ok := obj.(map[string]interface{}) - if !ok { - return false, fmt.Errorf("Unexpected non-map found: %T", obj) - } - - var anyUpdated bool - for key, _ := range typedObj { - if regex.MatchString(key) { - path[len(path)-1] = &PathPart{MapKey: &key} - newFullPath := fullPath[:len(fullPath)-1] - updated, err := t.apply(obj, path[i:], newFullPath, srcs) - if err != nil { - return false, err - } - if updated { - anyUpdated = true - } + allUpdated := true + for _, key := range matchedKeys { + newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) + newFullPath := fullPath[:len(fullPath)-1] + updated, err := t.apply(obj, newPath, newFullPath, srcs) + if err != nil { + return false, err + } + if !updated { + allUpdated = false } } - return anyUpdated, nil - + return allUpdated, nil default: panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } @@ -231,3 +222,53 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return obj, true, nil } + +func (t FieldCopyMod) matchRegexWithSources(path Path, srcs map[FieldCopyModSource]Resource) ([]string, error) { + // always looking into existing resource? + srcRes, found := srcs[FieldCopyModSourceExisting] + if !found || srcRes == nil { + return []string{}, fmt.Errorf("Existing resource not found") + } + return t.obtainMatchingRegexKeys(srcRes.unstructured().Object, path) +} + +func (t FieldCopyMod) obtainMatchingRegexKeys(obj interface{}, path Path) ([]string, error) { + var matchedKeys []string + for _, part := range path { + switch { + case part.MapKey != nil: + typedObj, ok := obj.(map[string]interface{}) + if !ok && typedObj != nil { + return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) + } + + var found bool + obj, found = typedObj[*part.MapKey] + if !found { + return matchedKeys, nil + } + + case part.IndexAndRegex != nil: + switch { + case part.IndexAndRegex.Regex != nil: + regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + if err != nil { + return matchedKeys, err + } + typedObj, ok := obj.(map[string]interface{}) + if !ok && typedObj != nil { + return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) + } + for key := range typedObj { + if regex.MatchString(key) { + matchedKeys = append(matchedKeys, key) + } + } + } + + default: + panic(fmt.Sprintf("Unexpected path part: %#v", part)) + } + } + return matchedKeys, nil +} diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 573f01b6f..512be5786 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -95,22 +95,15 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { } case part.IndexAndRegex.Regex != nil: - regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + matchedKeys, err := t.obtainMatchingRegexKeys(obj, part) if err != nil { return err } - - typedObj, ok := obj.(map[string]interface{}) - if !ok { - return fmt.Errorf("Unexpected non-map found: %T", obj) - } - for key, _ := range typedObj { - if regex.MatchString(key) { - path[len(path)-1] = &PathPart{MapKey: &key} - err := t.apply(obj, path[i:]) - if err != nil { - return err - } + for _, key := range matchedKeys { + newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) + err := t.apply(obj, newPath) + if err != nil { + return err } } return nil @@ -125,3 +118,21 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { panic("unreachable") } + +func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, part *PathPart) ([]string, error) { + var matchedKeys []string + regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + if err != nil { + return matchedKeys, err + } + typedObj, ok := obj.(map[string]interface{}) + if !ok && typedObj != nil { + return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) + } + for key := range typedObj { + if regex.MatchString(key) { + matchedKeys = append(matchedKeys, key) + } + } + return matchedKeys, nil +} diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 95a782fdd..400ebf971 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -77,7 +77,7 @@ func (p Path) AsString() string { func (p Path) ContainsNonMapKeys() bool { for _, part := range p { - if part.MapKey == nil { + if part.MapKey == nil && part.IndexAndRegex.Regex == nil { return true } } From a22ddeb79786382557b7f7329f2baab03ce227aa Mon Sep 17 00:00:00 2001 From: sethiyash Date: Wed, 4 Jan 2023 21:22:24 +0530 Subject: [PATCH 07/15] added e2e testcase with regex in config Signed-off-by: sethiyash --- test/e2e/config_test.go | 147 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index e97fe412d..d71c13906 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -4,6 +4,7 @@ package e2e import ( + "fmt" "math/rand" "strings" "testing" @@ -855,6 +856,152 @@ rules: }) } +func TestConfigHavingRegex(t *testing.T) { + yaml1 := ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-demo + annotations: + foo1: bar1 + foo2: bar2 +data: + player_initial_lives: "3" +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-test + annotations: + foo2: bar2 +data: + player_initial_lives: "3" +` + + yaml2 := ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-demo +data: + player_initial_lives: "3" +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: game-test +data: + player_initial_lives: "3" +` + + configYaml := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +rebaseRules: + - path: [metadata, annotations, {regex: "^foo"}] + type: %s + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} +` + + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + fieldsExcludedInMatch := []string{"kapp.k14s.io/app", "creationTimestamp:", "resourceVersion:", "uid:", "selfLink:", "kapp.k14s.io/association"} + name := "test-config-path-regex" + cleanUp := func() { + kapp.Run([]string{"delete", "-a", name}) + } + + cleanUp() + defer cleanUp() + + logger.Section("creating an app with multiple resources", func() { + out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, + RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1)}) + expectedOutput := ` +--- +# create: configmap/game-demo (v1) namespace: kapp-test +apiVersion: v1 +data: + player_initial_lives: "3" +kind: ConfigMap +metadata: + annotations: + foo1: bar1 + foo2: bar2 + labels: + name: game-demo + namespace: kapp-test +--- +# create: configmap/game-test (v1) namespace: kapp-test +apiVersion: v1 +data: + player_initial_lives: "3" +kind: ConfigMap +metadata: + annotations: + foo2: bar2 + labels: + name: game-test + namespace: kapp-test +` + out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) + out = clearKeys(fieldsExcludedInMatch, out) + + expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) + require.Contains(t, out, expectedOutput, "output does not match") + }) + + logger.Section("Deploy with new yaml with removed annotations and copying from existing resource", func() { + out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) + expectedOutput := `` + + out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) + out = clearKeys(fieldsExcludedInMatch, out) + + expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) + require.Contains(t, out, expectedOutput, "output does not match") + }) + + logger.Section("Remove all the annotation with remove config", func() { + out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, + RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1 + fmt.Sprintf(configYaml, "remove"))}) + + expectedOutput := ` +--- +# update: configmap/game-demo (v1) namespace: kapp-test +apiVersion: v1 +data: + player_initial_lives: "3" +kind: ConfigMap +metadata: + annotations: {} + labels: + name: game-demo + namespace: kapp-test +--- +# update: configmap/game-test (v1) namespace: kapp-test +apiVersion: v1 +data: + player_initial_lives: "3" +kind: ConfigMap +metadata: + annotations: {} + labels: + name: game-test + namespace: kapp-test +` + out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) + out = clearKeys(fieldsExcludedInMatch, out) + + expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) + require.Contains(t, out, expectedOutput, "output does not match") + }) +} + func RandomString(n int) string { var letters = []rune("abcdefghijklmnopqrstuvwxyz0123456789") From e9a8451c9d25c4e44344ed5d22e2dcb9724a8c56 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Thu, 12 Jan 2023 13:45:18 +0530 Subject: [PATCH 08/15] made suggested changes on e2e testcases Signed-off-by: sethiyash --- test/e2e/config_test.go | 39 ++++----------------------------------- 1 file changed, 4 insertions(+), 35 deletions(-) diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index d71c13906..7adab604b 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -918,44 +918,12 @@ rebaseRules: cleanUp() defer cleanUp() - logger.Section("creating an app with multiple resources", func() { - out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, + logger.Section("deploy configmaps with annotations", func() { + _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1)}) - expectedOutput := ` ---- -# create: configmap/game-demo (v1) namespace: kapp-test -apiVersion: v1 -data: - player_initial_lives: "3" -kind: ConfigMap -metadata: - annotations: - foo1: bar1 - foo2: bar2 - labels: - name: game-demo - namespace: kapp-test ---- -# create: configmap/game-test (v1) namespace: kapp-test -apiVersion: v1 -data: - player_initial_lives: "3" -kind: ConfigMap -metadata: - annotations: - foo2: bar2 - labels: - name: game-test - namespace: kapp-test -` - out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) - out = clearKeys(fieldsExcludedInMatch, out) - - expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) - require.Contains(t, out, expectedOutput, "output does not match") }) - logger.Section("Deploy with new yaml with removed annotations and copying from existing resource", func() { + logger.Section("deploy configmaps without annotations", func() { out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) expectedOutput := `` @@ -963,6 +931,7 @@ metadata: out = clearKeys(fieldsExcludedInMatch, out) expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) + // rebase rule should copy the annotations matched with regex require.Contains(t, out, expectedOutput, "output does not match") }) From 9e45923c1492254b8d19af1f82c9e29813ecb138 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Mon, 16 Jan 2023 14:10:10 +0530 Subject: [PATCH 09/15] regex as seprate path part Signed-off-by: sethiyash --- pkg/kapp/resources/matcher_empty_field.go | 12 +++--- pkg/kapp/resources/mod_field_copy.go | 42 +++++++++++---------- pkg/kapp/resources/mod_field_remove.go | 39 ++++++++++--------- pkg/kapp/resources/mod_object_ref_set.go | 12 +++--- pkg/kapp/resources/mod_path.go | 35 ++++++++++------- pkg/kapp/resources/mod_string_map_append.go | 12 +++--- test/e2e/cluster_resource.go | 8 ++-- 7 files changed, 84 insertions(+), 76 deletions(-) diff --git a/pkg/kapp/resources/matcher_empty_field.go b/pkg/kapp/resources/matcher_empty_field.go index e279eba91..0fa348d39 100644 --- a/pkg/kapp/resources/matcher_empty_field.go +++ b/pkg/kapp/resources/matcher_empty_field.go @@ -33,9 +33,9 @@ func (m EmptyFieldMatcher) check(obj interface{}, path Path) bool { return true } - case part.IndexAndRegex != nil: + case part.ArrayIndex != nil: switch { - case part.IndexAndRegex.All != nil: + case part.ArrayIndex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return obj == nil @@ -50,21 +50,21 @@ func (m EmptyFieldMatcher) check(obj interface{}, path Path) bool { return true - case part.IndexAndRegex.Index != nil: + case part.ArrayIndex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return obj == nil } - if *part.IndexAndRegex.Index < len(typedObj) { - obj = typedObj[*part.IndexAndRegex.Index] + if *part.ArrayIndex.Index < len(typedObj) { + obj = typedObj[*part.ArrayIndex.Index] } else { // Index not found, it's empty return true } default: - panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } default: diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 6de76e39b..972c2b7a9 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -76,13 +76,13 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ typedObj[*part.MapKey] = obj } - case part.IndexAndRegex != nil: - if isLast && part.IndexAndRegex.Regex == nil { + case part.ArrayIndex != nil: + if isLast && part.RegexObj == nil { return false, fmt.Errorf("Expected last part of the path to be map key") } switch { - case part.IndexAndRegex.All != nil: + case part.ArrayIndex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return false, fmt.Errorf("Unexpected non-array found: %T", obj) @@ -94,7 +94,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ objI := objI newFullPath := append([]*PathPart{}, fullPath...) - newFullPath[len(newFullPath)-1] = &PathPart{IndexAndRegex: &PathPartIndexAndRegex{Index: &objI}} + newFullPath[len(newFullPath)-1] = &PathPart{ArrayIndex: &PathPartArrayIndex{Index: &objI}} updated, err := t.apply(obj, path[i+1:], newFullPath, srcs) if err != nil { @@ -107,20 +107,25 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return anyUpdated, nil // dealt with children, get out - case part.IndexAndRegex.Index != nil: + case part.ArrayIndex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return false, fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.IndexAndRegex.Index < len(typedObj) { - obj = typedObj[*part.IndexAndRegex.Index] + if *part.ArrayIndex.Index < len(typedObj) { + obj = typedObj[*part.ArrayIndex.Index] return t.apply(obj, path[i+1:], fullPath, srcs) } return false, nil // index not found, nothing to append to - case part.IndexAndRegex.Regex != nil: + default: + panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + } + + case part.RegexObj != nil: + if part.RegexObj.Regex != nil { matchedKeys, err := t.matchRegexWithSources(path, srcs) if err != nil { return false, err @@ -138,10 +143,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } } return allUpdated, nil - default: - panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } - default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } @@ -193,26 +195,26 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return nil, false, nil // index is not found return } - case part.IndexAndRegex != nil: + case part.ArrayIndex != nil: if isLast { return nil, false, fmt.Errorf("Expected last part of the path to be map key") } switch { - case part.IndexAndRegex.Index != nil: + case part.ArrayIndex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return nil, false, fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.IndexAndRegex.Index < len(typedObj) { - obj = typedObj[*part.IndexAndRegex.Index] + if *part.ArrayIndex.Index < len(typedObj) { + obj = typedObj[*part.ArrayIndex.Index] } else { return nil, false, nil // index not found, return } default: - panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } default: @@ -224,7 +226,7 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool } func (t FieldCopyMod) matchRegexWithSources(path Path, srcs map[FieldCopyModSource]Resource) ([]string, error) { - // always looking into existing resource? + // always looking into existing resource srcRes, found := srcs[FieldCopyModSourceExisting] if !found || srcRes == nil { return []string{}, fmt.Errorf("Existing resource not found") @@ -248,10 +250,10 @@ func (t FieldCopyMod) obtainMatchingRegexKeys(obj interface{}, path Path) ([]str return matchedKeys, nil } - case part.IndexAndRegex != nil: + case part.RegexObj.Regex != nil: switch { - case part.IndexAndRegex.Regex != nil: - regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + case part.RegexObj.Regex != nil: + regex, err := regexp.Compile(*part.RegexObj.Regex) if err != nil { return matchedKeys, err } diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 512be5786..410af2c40 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -61,13 +61,13 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return nil // map key is not found, nothing to remove } - case part.IndexAndRegex != nil: - if isLast && part.IndexAndRegex.Regex == nil { + case part.ArrayIndex != nil: + if isLast && part.RegexObj.Regex == nil { return fmt.Errorf("Expected last part of the path to be map key") } switch { - case part.IndexAndRegex.All != nil: + case part.ArrayIndex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) @@ -79,37 +79,36 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return err } } - return nil // dealt with children, get out - case part.IndexAndRegex.Index != nil: + case part.ArrayIndex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.IndexAndRegex.Index < len(typedObj) { - obj = typedObj[*part.IndexAndRegex.Index] + if *part.ArrayIndex.Index < len(typedObj) { + obj = typedObj[*part.ArrayIndex.Index] } else { return nil // index not found, nothing to remove } - case part.IndexAndRegex.Regex != nil: - matchedKeys, err := t.obtainMatchingRegexKeys(obj, part) + default: + panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) + } + case part.RegexObj.Regex != nil: + matchedKeys, err := t.obtainMatchingRegexKeys(obj, part) + if err != nil { + return err + } + for _, key := range matchedKeys { + newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) + err := t.apply(obj, newPath) if err != nil { return err } - for _, key := range matchedKeys { - newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) - err := t.apply(obj, newPath) - if err != nil { - return err - } - } - return nil - default: - panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) } + return nil default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) @@ -121,7 +120,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, part *PathPart) ([]string, error) { var matchedKeys []string - regex, err := regexp.Compile(*part.IndexAndRegex.Regex) + regex, err := regexp.Compile(*part.RegexObj.Regex) if err != nil { return matchedKeys, err } diff --git a/pkg/kapp/resources/mod_object_ref_set.go b/pkg/kapp/resources/mod_object_ref_set.go index f290f624b..40d3d46f7 100644 --- a/pkg/kapp/resources/mod_object_ref_set.go +++ b/pkg/kapp/resources/mod_object_ref_set.go @@ -41,9 +41,9 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { return nil } - case part.IndexAndRegex != nil: + case part.ArrayIndex != nil: switch { - case part.IndexAndRegex.All != nil: + case part.ArrayIndex.All != nil: if obj == nil { return nil } @@ -61,20 +61,20 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { return nil // dealt with children, get out - case part.IndexAndRegex.Index != nil: + case part.ArrayIndex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.IndexAndRegex.Index < len(typedObj) { - return t.apply(typedObj[*part.IndexAndRegex.Index], path[i+1:]) + if *part.ArrayIndex.Index < len(typedObj) { + return t.apply(typedObj[*part.ArrayIndex.Index], path[i+1:]) } return nil // index not found, nothing to append to default: - panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } default: diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 400ebf971..9d69c584a 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -21,15 +21,19 @@ type ResourceModWithMultiple interface { type Path []*PathPart type PathPart struct { - MapKey *string - IndexAndRegex *PathPartIndexAndRegex + MapKey *string + RegexObj *PathPartRegex + ArrayIndex *PathPartArrayIndex } var _ json.Unmarshaler = &PathPart{} -type PathPartIndexAndRegex struct { +type PathPartArrayIndex struct { Index *int - All *bool `json:"allIndexes"` + All *bool `json:"allIndexes"` +} + +type PathPartRegex struct { Regex *string `json:"regex"` } @@ -77,7 +81,7 @@ func (p Path) AsString() string { func (p Path) ContainsNonMapKeys() bool { for _, part := range p { - if part.MapKey == nil && part.IndexAndRegex.Regex == nil { + if part.MapKey == nil { return true } } @@ -89,24 +93,24 @@ func NewPathPartFromString(str string) *PathPart { } func NewPathPartFromIndex(i int) *PathPart { - return &PathPart{IndexAndRegex: &PathPartIndexAndRegex{Index: &i}} + return &PathPart{ArrayIndex: &PathPartArrayIndex{Index: &i}} } func NewPathPartFromIndexAll() *PathPart { trueBool := true - return &PathPart{IndexAndRegex: &PathPartIndexAndRegex{All: &trueBool}} + return &PathPart{ArrayIndex: &PathPartArrayIndex{All: &trueBool}} } func (p *PathPart) AsString() string { switch { case p.MapKey != nil: return *p.MapKey - case p.IndexAndRegex != nil && p.IndexAndRegex.Index != nil: - return fmt.Sprintf("%d", *p.IndexAndRegex.Index) - case p.IndexAndRegex != nil && p.IndexAndRegex.All != nil: + case p.ArrayIndex != nil && p.ArrayIndex.Index != nil: + return fmt.Sprintf("%d", *p.ArrayIndex.Index) + case p.ArrayIndex != nil && p.ArrayIndex.All != nil: return "(all)" - case p.IndexAndRegex != nil && p.IndexAndRegex.Regex != nil: - return *p.IndexAndRegex.Regex + case p.RegexObj != nil && p.RegexObj.Regex != nil: + return *p.RegexObj.Regex default: panic("Unknown path part") } @@ -114,13 +118,16 @@ func (p *PathPart) AsString() string { func (p *PathPart) UnmarshalJSON(data []byte) error { var str string - var idx PathPartIndexAndRegex + var idx PathPartArrayIndex + var regx PathPartRegex switch { case json.Unmarshal(data, &str) == nil: p.MapKey = &str + case json.Unmarshal(data, ®x) == nil && regx.Regex != nil: + p.RegexObj = ®x case json.Unmarshal(data, &idx) == nil: - p.IndexAndRegex = &idx + p.ArrayIndex = &idx default: return fmt.Errorf("Unknown path part") } diff --git a/pkg/kapp/resources/mod_string_map_append.go b/pkg/kapp/resources/mod_string_map_append.go index 00aa104b9..b4c8cbe57 100644 --- a/pkg/kapp/resources/mod_string_map_append.go +++ b/pkg/kapp/resources/mod_string_map_append.go @@ -51,9 +51,9 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { typedObj[*part.MapKey] = obj } - case part.IndexAndRegex != nil: + case part.ArrayIndex != nil: switch { - case part.IndexAndRegex.All != nil: + case part.ArrayIndex.All != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) @@ -68,20 +68,20 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { return nil // dealt with children, get out - case part.IndexAndRegex.Index != nil: + case part.ArrayIndex.Index != nil: typedObj, ok := obj.([]interface{}) if !ok { return fmt.Errorf("Unexpected non-array found: %T", obj) } - if *part.IndexAndRegex.Index < len(typedObj) { - return t.apply(typedObj[*part.IndexAndRegex.Index], path[i+1:]) + if *part.ArrayIndex.Index < len(typedObj) { + return t.apply(typedObj[*part.ArrayIndex.Index], path[i+1:]) } return nil // index not found, nothing to append to default: - panic(fmt.Sprintf("Unknown array index: %#v", part.IndexAndRegex)) + panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } default: diff --git a/test/e2e/cluster_resource.go b/test/e2e/cluster_resource.go index 9b32ca04f..15cac63a0 100644 --- a/test/e2e/cluster_resource.go +++ b/test/e2e/cluster_resource.go @@ -104,15 +104,15 @@ func (r ClusterResource) RawPath(path ctlres.Path) interface{} { panic(fmt.Sprintf("Expected to find key %s", *part.MapKey)) } - case part.IndexAndRegex != nil: + case part.ArrayIndex != nil: typedResult, ok := result.([]interface{}) if !ok { panic("Expected to find array") } switch { - case part.IndexAndRegex.Index != nil: - result = typedResult[*part.IndexAndRegex.Index] - case part.IndexAndRegex.All != nil: + case part.ArrayIndex.Index != nil: + result = typedResult[*part.ArrayIndex.Index] + case part.ArrayIndex.All != nil: panic("Unsupported array index all") default: panic("Unknown array index") From a7696217c9d79508febfb14c428fa99c184a5ae2 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Tue, 17 Jan 2023 15:56:49 +0530 Subject: [PATCH 10/15] renamed regexObj to regex Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 57 ++++++++++++-------------- pkg/kapp/resources/mod_field_remove.go | 6 +-- pkg/kapp/resources/mod_path.go | 10 ++--- test/e2e/config_test.go | 11 +++-- 4 files changed, 42 insertions(+), 42 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 972c2b7a9..221ac45c8 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -77,7 +77,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } case part.ArrayIndex != nil: - if isLast && part.RegexObj == nil { + if isLast { return false, fmt.Errorf("Expected last part of the path to be map key") } @@ -124,26 +124,24 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } - case part.RegexObj != nil: - if part.RegexObj.Regex != nil { - matchedKeys, err := t.matchRegexWithSources(path, srcs) + case part.Regex != nil && part.Regex.Regex != nil: + matchedKeys, err := t.matchRegexWithSources(path, srcs) + if err != nil { + return false, err + } + allUpdated := true + for _, key := range matchedKeys { + newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) + newFullPath := fullPath[:len(fullPath)-1] + updated, err := t.apply(obj, newPath, newFullPath, srcs) if err != nil { return false, err } - allUpdated := true - for _, key := range matchedKeys { - newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) - newFullPath := fullPath[:len(fullPath)-1] - updated, err := t.apply(obj, newPath, newFullPath, srcs) - if err != nil { - return false, err - } - if !updated { - allUpdated = false - } + if !updated { + allUpdated = false } - return allUpdated, nil } + return allUpdated, nil default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } @@ -250,21 +248,18 @@ func (t FieldCopyMod) obtainMatchingRegexKeys(obj interface{}, path Path) ([]str return matchedKeys, nil } - case part.RegexObj.Regex != nil: - switch { - case part.RegexObj.Regex != nil: - regex, err := regexp.Compile(*part.RegexObj.Regex) - if err != nil { - return matchedKeys, err - } - typedObj, ok := obj.(map[string]interface{}) - if !ok && typedObj != nil { - return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) - } - for key := range typedObj { - if regex.MatchString(key) { - matchedKeys = append(matchedKeys, key) - } + case part.Regex != nil && part.Regex.Regex != nil: + regex, err := regexp.Compile(*part.Regex.Regex) + if err != nil { + return matchedKeys, err + } + typedObj, ok := obj.(map[string]interface{}) + if !ok && typedObj != nil { + return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) + } + for key := range typedObj { + if regex.MatchString(key) { + matchedKeys = append(matchedKeys, key) } } diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 410af2c40..6a825bb3e 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -62,7 +62,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { } case part.ArrayIndex != nil: - if isLast && part.RegexObj.Regex == nil { + if isLast && part.Regex.Regex == nil { return fmt.Errorf("Expected last part of the path to be map key") } @@ -96,7 +96,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { default: panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } - case part.RegexObj.Regex != nil: + case part.Regex != nil && part.Regex.Regex != nil: matchedKeys, err := t.obtainMatchingRegexKeys(obj, part) if err != nil { return err @@ -120,7 +120,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, part *PathPart) ([]string, error) { var matchedKeys []string - regex, err := regexp.Compile(*part.RegexObj.Regex) + regex, err := regexp.Compile(*part.Regex.Regex) if err != nil { return matchedKeys, err } diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 9d69c584a..3d59228c5 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -22,7 +22,7 @@ type Path []*PathPart type PathPart struct { MapKey *string - RegexObj *PathPartRegex + Regex *PathPartRegex ArrayIndex *PathPartArrayIndex } @@ -81,7 +81,7 @@ func (p Path) AsString() string { func (p Path) ContainsNonMapKeys() bool { for _, part := range p { - if part.MapKey == nil { + if part.MapKey == nil && part.Regex.Regex == nil { return true } } @@ -109,8 +109,8 @@ func (p *PathPart) AsString() string { return fmt.Sprintf("%d", *p.ArrayIndex.Index) case p.ArrayIndex != nil && p.ArrayIndex.All != nil: return "(all)" - case p.RegexObj != nil && p.RegexObj.Regex != nil: - return *p.RegexObj.Regex + case p.Regex != nil && p.Regex.Regex != nil: + return *p.Regex.Regex default: panic("Unknown path part") } @@ -125,7 +125,7 @@ func (p *PathPart) UnmarshalJSON(data []byte) error { case json.Unmarshal(data, &str) == nil: p.MapKey = &str case json.Unmarshal(data, ®x) == nil && regx.Regex != nil: - p.RegexObj = ®x + p.Regex = ®x case json.Unmarshal(data, &idx) == nil: p.ArrayIndex = &idx default: diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index 7adab604b..17f80c098 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -924,9 +924,14 @@ rebaseRules: }) logger.Section("deploy configmaps without annotations", func() { - out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) - expectedOutput := `` - + out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml", "-c", "--diff-summary=true"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) + expectedOutput := ` +Changes +Namespace Name Kind Age Op Op st. Wait to Rs Ri $ +Op: 0 create, 0 delete, 0 update, 0 noop, 0 exists +Wait to: 0 reconcile, 0 delete, 0 noop +Succeeded +` out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) out = clearKeys(fieldsExcludedInMatch, out) From 621e79d243f1af28143bef3c72e3600c13215a93 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Tue, 17 Jan 2023 23:20:35 +0530 Subject: [PATCH 11/15] fixing testcase Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 15 ++++++---- pkg/kapp/resources/mod_field_remove.go | 2 +- pkg/kapp/resources/mod_path.go | 9 ++++++ test/e2e/config_test.go | 40 +++++++++++++++++--------- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 221ac45c8..18ad9ff9d 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -69,7 +69,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ if !found || obj == nil { // create empty maps if there are no downstream array indexes; // if there are, we cannot make them anyway, so just exit - if path.ContainsNonMapKeys() { + if path.ContainsNonMapKeysAndRegex() { return false, nil } obj = map[string]interface{}{} @@ -224,12 +224,15 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool } func (t FieldCopyMod) matchRegexWithSources(path Path, srcs map[FieldCopyModSource]Resource) ([]string, error) { - // always looking into existing resource - srcRes, found := srcs[FieldCopyModSourceExisting] - if !found || srcRes == nil { - return []string{}, fmt.Errorf("Existing resource not found") + for _, src := range t.Sources { + if srcRes, found := srcs[src]; found && srcRes != nil { + matchedKeys, err := t.obtainMatchingRegexKeys(srcRes.unstructured().Object, path) + if err == nil && len(matchedKeys) > 0 { + return matchedKeys, err + } + } } - return t.obtainMatchingRegexKeys(srcRes.unstructured().Object, path) + return []string{}, fmt.Errorf("Field value not present in mentioned sources %s", t.Sources) } func (t FieldCopyMod) obtainMatchingRegexKeys(obj interface{}, path Path) ([]string, error) { diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 6a825bb3e..01ba11707 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -62,7 +62,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { } case part.ArrayIndex != nil: - if isLast && part.Regex.Regex == nil { + if isLast { return fmt.Errorf("Expected last part of the path to be map key") } diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 3d59228c5..bd2cc16e4 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -80,6 +80,15 @@ func (p Path) AsString() string { } func (p Path) ContainsNonMapKeys() bool { + for _, part := range p { + if part.MapKey == nil { + return true + } + } + return false +} + +func (p Path) ContainsNonMapKeysAndRegex() bool { for _, part := range p { if part.MapKey == nil && part.Regex.Regex == nil { return true diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index 17f80c098..ec6e6580b 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -906,6 +906,18 @@ rebaseRules: - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} ` + faultyConfigYaml := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config +rebaseRules: + - path: [metadata, annotations, {regex: }] + type: %s + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} +` + env := BuildEnv(t) logger := Logger{} kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} @@ -919,25 +931,25 @@ rebaseRules: defer cleanUp() logger.Section("deploy configmaps with annotations", func() { - _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, + _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1)}) }) logger.Section("deploy configmaps without annotations", func() { - out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml", "-c", "--diff-summary=true"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) - expectedOutput := ` -Changes -Namespace Name Kind Age Op Op st. Wait to Rs Ri $ -Op: 0 create, 0 delete, 0 update, 0 noop, 0 exists -Wait to: 0 reconcile, 0 delete, 0 noop -Succeeded -` - out = strings.TrimSpace(replaceTarget(replaceSpaces(replaceTs(out)))) - out = clearKeys(fieldsExcludedInMatch, out) + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) - expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) - // rebase rule should copy the annotations matched with regex - require.Contains(t, out, expectedOutput, "output does not match") + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("passing faulty config", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(faultyConfigYaml, "copy"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "panic: Unknown path part", "Expected to panic") }) logger.Section("Remove all the annotation with remove config", func() { From d99b646dd67e1113118b985e1ba28e6d85fcb8aa Mon Sep 17 00:00:00 2001 From: sethiyash Date: Fri, 20 Jan 2023 11:34:27 +0530 Subject: [PATCH 12/15] added nil check Signed-off-by: Yash Sethiya Signed-off-by: sethiyash --- pkg/kapp/resources/mod_path.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index bd2cc16e4..61c17a9c7 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -90,7 +90,7 @@ func (p Path) ContainsNonMapKeys() bool { func (p Path) ContainsNonMapKeysAndRegex() bool { for _, part := range p { - if part.MapKey == nil && part.Regex.Regex == nil { + if part.MapKey == nil && (part.Regex == nil || part.Regex.Regex == nil) { return true } } From ab33947c21bf00747ddb6fe1c0e156262344bb6d Mon Sep 17 00:00:00 2001 From: sethiyash Date: Fri, 3 Mar 2023 14:18:08 +0530 Subject: [PATCH 13/15] refactored logic to support regex with indexes Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 124 ++++++++-------- pkg/kapp/resources/mod_field_remove.go | 6 +- pkg/kapp/resources/mod_object_ref_set.go | 3 + pkg/kapp/resources/mod_path.go | 4 +- pkg/kapp/resources/mod_string_map_append.go | 3 + test/e2e/config_test.go | 148 +++++++++++++++++++- 6 files changed, 214 insertions(+), 74 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index 18ad9ff9d..f43cc6d44 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -31,29 +31,36 @@ func (t FieldCopyMod) IsResourceMatching(res Resource) bool { } func (t FieldCopyMod) ApplyFromMultiple(res Resource, srcs map[FieldCopyModSource]Resource) error { - // Make a copy of resource, to avoid modifications - // that may be done even in case when there is nothing to copy - updatedRes := res.DeepCopy() - - updated, err := t.apply(updatedRes.unstructured().Object, t.Path, Path{}, srcs) - if err != nil { - return fmt.Errorf("FieldCopyMod for path '%s' on resource '%s': %s", t.Path.AsString(), res.Description(), err) - } - - if updated { - res.setUnstructured(updatedRes.unstructured()) + for _, src := range t.Sources { + // Make a copy of resource, to avoid modifications + // that may be done even in case when there is nothing to copy + updatedRes := res.DeepCopy() + source, found := srcs[src] + if !found { + continue + } + updated, err := t.apply(updatedRes.unstructured().Object, source.unstructured().Object, t.Path, Path{}, srcs) + if err != nil { + return fmt.Errorf("FieldCopyMod for path '%s' on resource '%s': %s", t.Path.AsString(), res.Description(), err) + } + if updated { + res.setUnstructured(updatedRes.unstructured()) + } } - return nil } -func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[FieldCopyModSource]Resource) (bool, error) { +func (t FieldCopyMod) apply(obj interface{}, srcObj interface{}, path Path, fullPath Path, srcs map[FieldCopyModSource]Resource) (bool, error) { for i, part := range path { isLast := len(path) == i+1 fullPath = append(fullPath, part) switch { case part.MapKey != nil: + srcTypedObj, ok := srcObj.(map[string]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-map found: %T", srcObj) + } typedObj, ok := obj.(map[string]interface{}) if !ok { return false, fmt.Errorf("Unexpected non-map found: %T", obj) @@ -63,13 +70,21 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return t.copyIntoMap(typedObj, fullPath, srcs) } - var found bool + var ( + found bool + srcObjFound bool + ) + srcObj, srcObjFound = srcTypedObj[*part.MapKey] + if !srcObjFound || srcObj == nil { + return false, nil + } + obj, found = typedObj[*part.MapKey] // TODO check strictness? if !found || obj == nil { // create empty maps if there are no downstream array indexes; // if there are, we cannot make them anyway, so just exit - if path.ContainsNonMapKeysAndRegex() { + if path.ContainsArrayIndex() { return false, nil } obj = map[string]interface{}{} @@ -88,6 +103,11 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, fmt.Errorf("Unexpected non-array found: %T", obj) } + srcTypedObj, ok := srcObj.([]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-array found: %T", srcObj) + } + var anyUpdated bool for objI, obj := range typedObj { @@ -96,7 +116,11 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ newFullPath := append([]*PathPart{}, fullPath...) newFullPath[len(newFullPath)-1] = &PathPart{ArrayIndex: &PathPartArrayIndex{Index: &objI}} - updated, err := t.apply(obj, path[i+1:], newFullPath, srcs) + var srcTypeObj map[string]interface{} + if objI < len(srcTypedObj) { + srcTypeObj = srcTypedObj[objI].(map[string]interface{}) + } + updated, err := t.apply(obj, srcTypeObj, path[i+1:], newFullPath, srcs) if err != nil { return false, err } @@ -113,9 +137,15 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ return false, fmt.Errorf("Unexpected non-array found: %T", obj) } + srcTypedObj, ok := srcObj.([]interface{}) + if !ok { + return false, fmt.Errorf("Unexpected non-array found: %T", srcObj) + } + if *part.ArrayIndex.Index < len(typedObj) { obj = typedObj[*part.ArrayIndex.Index] - return t.apply(obj, path[i+1:], fullPath, srcs) + srcObj = srcTypedObj[*part.ArrayIndex.Index] + return t.apply(obj, srcObj, path[i+1:], fullPath, srcs) } return false, nil // index not found, nothing to append to @@ -125,7 +155,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ } case part.Regex != nil && part.Regex.Regex != nil: - matchedKeys, err := t.matchRegexWithSources(path, srcs) + matchedKeys, err := t.matchRegexWithSrcObj(*part.Regex.Regex, srcObj) if err != nil { return false, err } @@ -133,7 +163,7 @@ func (t FieldCopyMod) apply(obj interface{}, path Path, fullPath Path, srcs map[ for _, key := range matchedKeys { newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) newFullPath := fullPath[:len(fullPath)-1] - updated, err := t.apply(obj, newPath, newFullPath, srcs) + updated, err := t.apply(obj, srcObj, newPath, newFullPath, srcs) if err != nil { return false, err } @@ -223,51 +253,19 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return obj, true, nil } -func (t FieldCopyMod) matchRegexWithSources(path Path, srcs map[FieldCopyModSource]Resource) ([]string, error) { - for _, src := range t.Sources { - if srcRes, found := srcs[src]; found && srcRes != nil { - matchedKeys, err := t.obtainMatchingRegexKeys(srcRes.unstructured().Object, path) - if err == nil && len(matchedKeys) > 0 { - return matchedKeys, err - } - } - } - return []string{}, fmt.Errorf("Field value not present in mentioned sources %s", t.Sources) -} - -func (t FieldCopyMod) obtainMatchingRegexKeys(obj interface{}, path Path) ([]string, error) { +func (t FieldCopyMod) matchRegexWithSrcObj(regexString string, srcObj interface{}) ([]string, error) { var matchedKeys []string - for _, part := range path { - switch { - case part.MapKey != nil: - typedObj, ok := obj.(map[string]interface{}) - if !ok && typedObj != nil { - return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) - } - - var found bool - obj, found = typedObj[*part.MapKey] - if !found { - return matchedKeys, nil - } - - case part.Regex != nil && part.Regex.Regex != nil: - regex, err := regexp.Compile(*part.Regex.Regex) - if err != nil { - return matchedKeys, err - } - typedObj, ok := obj.(map[string]interface{}) - if !ok && typedObj != nil { - return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) - } - for key := range typedObj { - if regex.MatchString(key) { - matchedKeys = append(matchedKeys, key) - } - } - - default: - panic(fmt.Sprintf("Unexpected path part: %#v", part)) + regex, err := regexp.Compile(regexString) + if err != nil { + return matchedKeys, err + } + srcTypedObj, ok := srcObj.(map[string]interface{}) + if !ok && srcTypedObj != nil { + return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", srcObj) + } + for key := range srcTypedObj { + if regex.MatchString(key) { + matchedKeys = append(matchedKeys, key) } } return matchedKeys, nil diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 01ba11707..94ad913bf 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -97,7 +97,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } case part.Regex != nil && part.Regex.Regex != nil: - matchedKeys, err := t.obtainMatchingRegexKeys(obj, part) + matchedKeys, err := t.obtainMatchingRegexKeys(obj, *part.Regex.Regex) if err != nil { return err } @@ -118,9 +118,9 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { panic("unreachable") } -func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, part *PathPart) ([]string, error) { +func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, regexString string) ([]string, error) { var matchedKeys []string - regex, err := regexp.Compile(*part.Regex.Regex) + regex, err := regexp.Compile(regexString) if err != nil { return matchedKeys, err } diff --git a/pkg/kapp/resources/mod_object_ref_set.go b/pkg/kapp/resources/mod_object_ref_set.go index 40d3d46f7..723500755 100644 --- a/pkg/kapp/resources/mod_object_ref_set.go +++ b/pkg/kapp/resources/mod_object_ref_set.go @@ -77,6 +77,9 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } + case part.Regex != nil && part.Regex.Regex != nil: + panic(fmt.Sprintf("Regex in path part: %#v is only supported for rebase rules.", part)) + default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } diff --git a/pkg/kapp/resources/mod_path.go b/pkg/kapp/resources/mod_path.go index 61c17a9c7..8d6ef7d23 100644 --- a/pkg/kapp/resources/mod_path.go +++ b/pkg/kapp/resources/mod_path.go @@ -88,9 +88,9 @@ func (p Path) ContainsNonMapKeys() bool { return false } -func (p Path) ContainsNonMapKeysAndRegex() bool { +func (p Path) ContainsArrayIndex() bool { for _, part := range p { - if part.MapKey == nil && (part.Regex == nil || part.Regex.Regex == nil) { + if part.ArrayIndex != nil { return true } } diff --git a/pkg/kapp/resources/mod_string_map_append.go b/pkg/kapp/resources/mod_string_map_append.go index b4c8cbe57..f3643f602 100644 --- a/pkg/kapp/resources/mod_string_map_append.go +++ b/pkg/kapp/resources/mod_string_map_append.go @@ -84,6 +84,9 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } + case part.Regex != nil && part.Regex.Regex != nil: + panic(fmt.Sprintf("Regex in path part: %#v is only supported for rebase rules.", part)) + default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index ec6e6580b..bcb7ef343 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -857,7 +857,7 @@ rules: } func TestConfigHavingRegex(t *testing.T) { - yaml1 := ` + configMapResYaml := ` apiVersion: v1 kind: ConfigMap metadata: @@ -878,7 +878,7 @@ data: player_initial_lives: "3" ` - yaml2 := ` + updatedConfigMapResYaml := ` apiVersion: v1 kind: ConfigMap metadata: @@ -918,6 +918,85 @@ rebaseRules: - apiVersionKindMatcher: {apiVersion: v1, kind: ConfigMap} ` + deploymentResYaml := ` +apiVersion: apps/v1 +kind: Deployment +metadata: + namespace: default + name: simple-app +spec: + selector: + matchLabels: + simple-app: "" + template: + metadata: + labels: + simple-app: "" + spec: + containers: + - name: simple-app + image: docker.io/dkalinin/k8s-simple-app@sha256:4c8b96d4fffdfae29258d94a22ae4ad1fe36139d47288b8960d9958d1e63a9d0 + env: + - name: HELLO + value: strange + - name: HELLO_MSG + value: stranger +` + + updatedDeploymentResYaml := ` +apiVersion: apps/v1 +kind: Deployment +metadata: + namespace: default + name: simple-app +spec: + selector: + matchLabels: + simple-app: "" + template: + metadata: + labels: + simple-app: "" + spec: + containers: + - name: simple-app + image: docker.io/dkalinin/k8s-simple-app@sha256:4c8b96d4fffdfae29258d94a22ae4ad1fe36139d47288b8960d9958d1e63a9d0 + env: + - name: HELLO + - name: HELLO_MSG +` + + deploymentConfig := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config + +rebaseRules: + - path: [spec, template, spec, containers, {allIndexes: true}, env, %s] + type: %s + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment} +` + + deploymentConfigIndex := ` +--- +apiVersion: kapp.k14s.io/v1alpha1 +kind: Config + +rebaseRules: + - path: [spec, template, spec, containers, {allIndexes: true}, env, {index: 0}, %s] + type: copy + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment} + - path: [spec, template, spec, containers, {allIndexes: true}, env, {index: 1}, %s] + type: copy + sources: [new, existing] + resourceMatchers: + - apiVersionKindMatcher: {apiVersion: apps/v1, kind: Deployment} +` + env := BuildEnv(t) logger := Logger{} kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} @@ -932,12 +1011,12 @@ rebaseRules: logger.Section("deploy configmaps with annotations", func() { _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, - RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1)}) + RunOpts{IntoNs: true, StdinReader: strings.NewReader(configMapResYaml)}) }) logger.Section("deploy configmaps without annotations", func() { _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, - RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(configYaml, "copy"))}) + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedConfigMapResYaml + fmt.Sprintf(configYaml, "copy"))}) require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") @@ -946,7 +1025,7 @@ rebaseRules: logger.Section("passing faulty config", func() { _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, - RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(yaml2 + fmt.Sprintf(faultyConfigYaml, "copy"))}) + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedConfigMapResYaml + fmt.Sprintf(faultyConfigYaml, "copy"))}) require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "panic: Unknown path part", "Expected to panic") @@ -954,7 +1033,7 @@ rebaseRules: logger.Section("Remove all the annotation with remove config", func() { out, _ := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-changes-yaml"}, - RunOpts{IntoNs: true, StdinReader: strings.NewReader(yaml1 + fmt.Sprintf(configYaml, "remove"))}) + RunOpts{IntoNs: true, StdinReader: strings.NewReader(configMapResYaml + fmt.Sprintf(configYaml, "remove"))}) expectedOutput := ` --- @@ -986,6 +1065,63 @@ metadata: expectedOutput = strings.TrimSpace(replaceSpaces(expectedOutput)) require.Contains(t, out, expectedOutput, "output does not match") }) + + logger.Section("Deployment resource", func() { + _, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name}, + RunOpts{IntoNs: true, StdinReader: strings.NewReader(deploymentResYaml)}) + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, value", "copy"))}) + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule using regex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, {regex: \"^val\"}", "copy"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule using index and field", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "value", "value"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and copying with rebase rule using index and regex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "{regex: \"^val\"}", "{regex: \"^val\"}"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") + require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") + }) + + logger.Section("Deployment resource with remove value field and unmatched regex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "{regex: \"^tal\"}", "{regex: \"^tal\"}"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with pending changes (exit status 3)", "Expected to find stderr output") + }) + + logger.Section("Deployment resource with remove value field and unmatched regex and allIndex", func() { + _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, + RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, {regex: \"^tal\"}", "copy"))}) + + require.Errorf(t, err, "Expected to receive error") + require.Containsf(t, err.Error(), "Exiting after diffing with pending changes (exit status 3)", "Expected to find stderr output") + }) + } func RandomString(n int) string { From 566b3f93ecb5847cd3708800a9dd9d15797a663d Mon Sep 17 00:00:00 2001 From: sethiyash Date: Mon, 5 Jun 2023 18:47:20 +0530 Subject: [PATCH 14/15] adopted some nits Signed-off-by: sethiyash --- pkg/kapp/resources/mod_field_copy.go | 26 +++++++++++-------- pkg/kapp/resources/mod_field_remove.go | 28 ++++++--------------- pkg/kapp/resources/mod_object_ref_set.go | 4 +-- pkg/kapp/resources/mod_string_map_append.go | 4 +-- 4 files changed, 27 insertions(+), 35 deletions(-) diff --git a/pkg/kapp/resources/mod_field_copy.go b/pkg/kapp/resources/mod_field_copy.go index f43cc6d44..b6ebe5876 100644 --- a/pkg/kapp/resources/mod_field_copy.go +++ b/pkg/kapp/resources/mod_field_copy.go @@ -32,13 +32,13 @@ func (t FieldCopyMod) IsResourceMatching(res Resource) bool { func (t FieldCopyMod) ApplyFromMultiple(res Resource, srcs map[FieldCopyModSource]Resource) error { for _, src := range t.Sources { - // Make a copy of resource, to avoid modifications - // that may be done even in case when there is nothing to copy - updatedRes := res.DeepCopy() source, found := srcs[src] if !found { continue } + // Make a copy of resource, to avoid modifications + // that may be done even in case when there is nothing to copy + updatedRes := res.DeepCopy() updated, err := t.apply(updatedRes.unstructured().Object, source.unstructured().Object, t.Path, Path{}, srcs) if err != nil { return fmt.Errorf("FieldCopyMod for path '%s' on resource '%s': %s", t.Path.AsString(), res.Description(), err) @@ -47,6 +47,7 @@ func (t FieldCopyMod) ApplyFromMultiple(res Resource, srcs map[FieldCopyModSourc res.setUnstructured(updatedRes.unstructured()) } } + return nil } @@ -154,12 +155,15 @@ func (t FieldCopyMod) apply(obj interface{}, srcObj interface{}, path Path, full panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } - case part.Regex != nil && part.Regex.Regex != nil: - matchedKeys, err := t.matchRegexWithSrcObj(*part.Regex.Regex, srcObj) + case part.Regex != nil: + if part.Regex.Regex == nil { + panic("Regex should be non nil") + } + matchedKeys, err := matchRegexWithSrcObj(*part.Regex.Regex, srcObj) if err != nil { return false, err } - allUpdated := true + var anyUpdated bool for _, key := range matchedKeys { newPath := append(Path{&PathPart{MapKey: &key}}, path[i+1:]...) newFullPath := fullPath[:len(fullPath)-1] @@ -167,11 +171,13 @@ func (t FieldCopyMod) apply(obj interface{}, srcObj interface{}, path Path, full if err != nil { return false, err } - if !updated { - allUpdated = false + if updated { + anyUpdated = true } } - return allUpdated, nil + + return anyUpdated, nil + default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) } @@ -253,7 +259,7 @@ func (t FieldCopyMod) obtainValue(obj interface{}, path Path) (interface{}, bool return obj, true, nil } -func (t FieldCopyMod) matchRegexWithSrcObj(regexString string, srcObj interface{}) ([]string, error) { +func matchRegexWithSrcObj(regexString string, srcObj interface{}) ([]string, error) { var matchedKeys []string regex, err := regexp.Compile(regexString) if err != nil { diff --git a/pkg/kapp/resources/mod_field_remove.go b/pkg/kapp/resources/mod_field_remove.go index 94ad913bf..513e8603b 100644 --- a/pkg/kapp/resources/mod_field_remove.go +++ b/pkg/kapp/resources/mod_field_remove.go @@ -5,7 +5,6 @@ package resources import ( "fmt" - "regexp" ) type FieldRemoveMod struct { @@ -79,6 +78,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return err } } + return nil // dealt with children, get out case part.ArrayIndex.Index != nil: @@ -96,8 +96,11 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { default: panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } - case part.Regex != nil && part.Regex.Regex != nil: - matchedKeys, err := t.obtainMatchingRegexKeys(obj, *part.Regex.Regex) + case part.Regex != nil: + if part.Regex.Regex == nil { + panic("Regex should be non nil") + } + matchedKeys, err := matchRegexWithSrcObj(*part.Regex.Regex, obj) if err != nil { return err } @@ -108,6 +111,7 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { return err } } + return nil default: @@ -117,21 +121,3 @@ func (t FieldRemoveMod) apply(obj interface{}, path Path) error { panic("unreachable") } - -func (t FieldRemoveMod) obtainMatchingRegexKeys(obj interface{}, regexString string) ([]string, error) { - var matchedKeys []string - regex, err := regexp.Compile(regexString) - if err != nil { - return matchedKeys, err - } - typedObj, ok := obj.(map[string]interface{}) - if !ok && typedObj != nil { - return matchedKeys, fmt.Errorf("Unexpected non-map found: %T", obj) - } - for key := range typedObj { - if regex.MatchString(key) { - matchedKeys = append(matchedKeys, key) - } - } - return matchedKeys, nil -} diff --git a/pkg/kapp/resources/mod_object_ref_set.go b/pkg/kapp/resources/mod_object_ref_set.go index 723500755..9509247aa 100644 --- a/pkg/kapp/resources/mod_object_ref_set.go +++ b/pkg/kapp/resources/mod_object_ref_set.go @@ -77,8 +77,8 @@ func (t ObjectRefSetMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } - case part.Regex != nil && part.Regex.Regex != nil: - panic(fmt.Sprintf("Regex in path part: %#v is only supported for rebase rules.", part)) + case part.Regex != nil: + panic("Regex in path part is only supported for rebaseRules.") default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) diff --git a/pkg/kapp/resources/mod_string_map_append.go b/pkg/kapp/resources/mod_string_map_append.go index f3643f602..638d41262 100644 --- a/pkg/kapp/resources/mod_string_map_append.go +++ b/pkg/kapp/resources/mod_string_map_append.go @@ -84,8 +84,8 @@ func (t StringMapAppendMod) apply(obj interface{}, path Path) error { panic(fmt.Sprintf("Unknown array index: %#v", part.ArrayIndex)) } - case part.Regex != nil && part.Regex.Regex != nil: - panic(fmt.Sprintf("Regex in path part: %#v is only supported for rebase rules.", part)) + case part.Regex != nil: + panic("Regex in path part is only supported for rebaseRules.") default: panic(fmt.Sprintf("Unexpected path part: %#v", part)) From 04fc3c8337884314a00e90c56d4b9a4f3cacb549 Mon Sep 17 00:00:00 2001 From: sethiyash Date: Tue, 5 Sep 2023 17:13:30 +0530 Subject: [PATCH 15/15] Added comment in e2e testcases Signed-off-by: sethiyash --- test/e2e/config_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/e2e/config_test.go b/test/e2e/config_test.go index bcb7ef343..fc856c660 100644 --- a/test/e2e/config_test.go +++ b/test/e2e/config_test.go @@ -1074,6 +1074,8 @@ metadata: logger.Section("Deployment resource with remove value field and copying with rebase rule", func() { _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, value", "copy"))}) + + // no change as value field is copied again for all indexes in the updatedDeployment using config resource require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") @@ -1083,6 +1085,7 @@ metadata: _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, {regex: \"^val\"}", "copy"))}) + // no change as value field is copied again using regex for all indexes in the updatedDeployment using config resource require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") @@ -1092,6 +1095,7 @@ metadata: _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "value", "value"))}) + // no change as value field is copied again for both index of env 0 and 1 in the updatedDeployment using config resource require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") @@ -1101,6 +1105,7 @@ metadata: _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "{regex: \"^val\"}", "{regex: \"^val\"}"))}) + // no change as value field is copied again using regex for both index of env 0 and 1 in the updatedDeployment using config resource require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with no pending changes (exit status 2)", "Expected to find stderr output") require.Containsf(t, err.Error(), "exit code: '2'", "Expected to find exit code") @@ -1110,6 +1115,7 @@ metadata: _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfigIndex, "{regex: \"^tal\"}", "{regex: \"^tal\"}"))}) + // change exists as no field is present as per given regex and hence it was unable to copy the field require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with pending changes (exit status 3)", "Expected to find stderr output") }) @@ -1118,6 +1124,7 @@ metadata: _, err := kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", name, "--diff-run", "--diff-exit-status"}, RunOpts{IntoNs: true, AllowError: true, StdinReader: strings.NewReader(updatedDeploymentResYaml + fmt.Sprintf(deploymentConfig, "{allIndexes: true}, {regex: \"^tal\"}", "copy"))}) + // change exists as no field is present as per given regex on all the indexes and hence it was unable to copy the field require.Errorf(t, err, "Expected to receive error") require.Containsf(t, err.Error(), "Exiting after diffing with pending changes (exit status 3)", "Expected to find stderr output") })