return error for duplicate keys rather than panicking

This commit is contained in:
Natasha Sarkar
2021-03-03 12:03:36 -08:00
parent 93dd571df9
commit 722b0131f0
15 changed files with 86 additions and 31 deletions

View File

@@ -30,12 +30,16 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
return nil
}
for _, r := range m.Resources() {
if r.IsEmpty() {
empty, err := r.IsEmpty()
if err != nil {
return err
}
if empty {
// Don't mutate empty objects?
continue
}
r.StorePreviousId()
err := r.ApplyFilter(namespace.Filter{
err = r.ApplyFilter(namespace.Filter{
Namespace: p.Namespace,
FsSlice: p.FieldSpecs,
})

View File

@@ -76,7 +76,7 @@ type Kunstructured interface {
GetString(string) (string, error)
// Several uses.
Map() map[string]interface{}
Map() (map[string]interface{}, error)
// Used by Resource.AsYAML and Resource.String
MarshalJSON() ([]byte, error)

View File

@@ -399,7 +399,8 @@ func find(name string, resMap resmap.ResMap) *resource.Resource {
func getCommand(r *resource.Resource) string {
var m map[string]interface{}
var c []interface{}
m, _ = r.Map()["spec"].(map[string]interface{})
resourceMap, _ := r.Map()
m, _ = resourceMap["spec"].(map[string]interface{})
m, _ = m["template"].(map[string]interface{})
m, _ = m["spec"].(map[string]interface{})
c, _ = m["containers"].([]interface{})

View File

@@ -43,7 +43,11 @@ func (o *multiTransformer) transform(m resmap.ResMap) error {
}
}
for _, r := range m.Resources() {
if r.IsEmpty() {
empty, err := r.IsEmpty()
if err != nil {
return err
}
if empty {
err := m.Remove(r.CurId())
if err != nil {
return err

View File

@@ -306,13 +306,16 @@ items:
assert.False(t, tc.exp.isErr)
assert.Equal(t, len(tc.exp.out), len(rs))
for i := range rs {
rsMap, err := rs[i].Map()
assert.NoError(t, err)
assert.Equal(
t, fmt.Sprintf("%v", tc.exp.out[i]), fmt.Sprintf("%v", rs[i].Map()))
t, fmt.Sprintf("%v", tc.exp.out[i]), fmt.Sprintf("%v", rsMap))
if n != "listWithAnchors" {
// https://github.com/kubernetes-sigs/kustomize/issues/3271
if !reflect.DeepEqual(tc.exp.out[i], rs[i].Map()) {
m, _ := rs[i].Map()
if !reflect.DeepEqual(tc.exp.out[i], m) {
t.Fatalf("%s:\nexpected: %v\n actual: %v",
n, tc.exp.out[i], rs[i].Map())
n, tc.exp.out[i], m)
}
}
}

View File

@@ -220,7 +220,7 @@ func (wn *WNode) GetString(path string) (string, error) {
}
// Map implements ifc.Kunstructured.
func (wn *WNode) Map() map[string]interface{} {
func (wn *WNode) Map() (map[string]interface{}, error) {
return wn.node.Map()
}

View File

@@ -559,7 +559,9 @@ func TestGetSlice(t *testing.T) {
}
func TestMapEmpty(t *testing.T) {
assert.Equal(t, 0, len(NewWNode().Map()))
newNodeMap, err := NewWNode().Map()
assert.NoError(t, err)
assert.Equal(t, 0, len(newNodeMap))
}
func TestMap(t *testing.T) {
@@ -577,7 +579,8 @@ func TestMap(t *testing.T) {
},
}
actual := wn.Map()
actual, err := wn.Map()
assert.NoError(t, err)
if diff := cmp.Diff(expected, actual); diff != "" {
t.Fatalf("actual map does not deep equal expected map:\n%v", diff)
}

View File

@@ -90,9 +90,14 @@ func (m *merginator) makeError(cd resource.ConflictDetector, index int) error {
if conflict == nil {
return fmt.Errorf("expected conflict for %s", m.incoming[index].OrgId())
}
conflictMap, _ := conflict.Map()
incomingIndexMap, _ := m.incoming[index].Map()
return fmt.Errorf(
"conflict between %#v at index %d and %#v",
m.incoming[index].Map(), index, conflict.Map())
incomingIndexMap,
index,
conflictMap,
)
}
// findConflict looks for a conflict in a resource slice.

View File

@@ -119,7 +119,11 @@ func (m *resWrangler) Debug(title string) {
fmt.Println("---")
}
fmt.Printf("# %d %s\n", i, r.OrgId())
blob, err := yaml.Marshal(r.Map())
m, err := r.Map()
if err != nil {
panic(err)
}
blob, err := yaml.Marshal(m)
if err != nil {
panic(err)
}
@@ -272,7 +276,8 @@ func (m *resWrangler) AsYaml() ([]byte, error) {
for _, res := range m.Resources() {
out, err := res.AsYAML()
if err != nil {
return nil, errors.Wrapf(err, "%#v", res.Map())
m, _ := res.Map()
return nil, errors.Wrapf(err, "%#v", m)
}
if firstObj {
firstObj = false
@@ -602,14 +607,23 @@ func (m *resWrangler) ApplySmPatch(
// Some unknown error, let it through.
return err
}
if !res.IsEmpty() {
empty, err := res.IsEmpty()
if err != nil {
return err
}
if !empty {
m, _ := res.Map()
return errors.Wrapf(
err, "with unexpectedly non-empty object map of size %d",
len(res.Map()))
len(m))
}
// Fall through to handle deleted object.
}
if !res.IsEmpty() {
empty, err := res.IsEmpty()
if err != nil {
return err
}
if !empty {
// IsEmpty means all fields have been removed from the object.
// This can happen if a patch required deletion of the
// entire resource (not just a part of it). This means

View File

@@ -114,7 +114,11 @@ func (rf *Factory) SliceFromBytes(in []byte) ([]*Resource, error) {
u := kunStructs[0]
kunStructs = kunStructs[1:]
if strings.HasSuffix(u.GetKind(), "List") {
items := u.Map()["items"]
m, err := u.Map()
if err != nil {
return nil, err
}
items := m["items"]
itemsSlice, ok := items.([]interface{})
if !ok {
if items == nil {

View File

@@ -98,11 +98,12 @@ func (r *Resource) GetString(p string) (string, error) {
return r.kunStr.GetString(p)
}
func (r *Resource) IsEmpty() bool {
return len(r.kunStr.Map()) == 0
func (r *Resource) IsEmpty() (bool, error) {
m, err := r.kunStr.Map()
return len(m) == 0, err
}
func (r *Resource) Map() map[string]interface{} {
func (r *Resource) Map() (map[string]interface{}, error) {
return r.kunStr.Map()
}
@@ -488,7 +489,11 @@ func (r *Resource) ApplySmPatch(patch *Resource) error {
if err != nil {
return err
}
if !r.IsEmpty() {
empty, err := r.IsEmpty()
if err != nil {
return err
}
if !empty {
r.SetName(n)
r.SetNamespace(ns)
}

View File

@@ -46,7 +46,11 @@ func (w Writer) WriteIndividualFiles(dirPath string, m resmap.ResMap) error {
}
func (w Writer) write(path, fName string, res *resource.Resource) error {
yml, err := yaml.Marshal(res.Map())
m, err := res.Map()
if err != nil {
return err
}
yml, err := yaml.Marshal(m)
if err != nil {
return err
}

View File

@@ -816,17 +816,18 @@ func FromMap(m map[string]interface{}) (*RNode, error) {
return Parse(string(c))
}
func (rn *RNode) Map() map[string]interface{} {
func (rn *RNode) Map() (map[string]interface{}, error) {
if rn == nil || rn.value == nil {
return make(map[string]interface{})
return make(map[string]interface{}), nil
}
var result map[string]interface{}
if err := rn.value.Decode(&result); err != nil {
// Should not be able to create an RNode that cannot be decoded;
// this is an unrecoverable error.
log.Fatalf("failed to decode ynode: %v", err)
str, _ := rn.String()
return nil, fmt.Errorf("received error %w for the following resource:\n%s", err, str)
}
return result
return result, nil
}
// ConvertJSONToYamlNode parses input json string and returns equivalent yaml node

View File

@@ -386,7 +386,9 @@ func TestRNodeGetValidatedMetadata(t *testing.T) {
}
func TestRNodeMapEmpty(t *testing.T) {
assert.Equal(t, 0, len(NewRNode(nil).Map()))
newRNodeMap, err := NewRNode(nil).Map()
assert.NoError(t, err)
assert.Equal(t, 0, len(newRNodeMap))
}
func TestRNodeMap(t *testing.T) {
@@ -411,7 +413,8 @@ func TestRNodeMap(t *testing.T) {
},
}
actual := wn.Map()
actual, err := wn.Map()
assert.NoError(t, err)
if diff := cmp.Diff(expected, actual); diff != "" {
t.Fatalf("actual map does not deep equal expected map:\n%v", diff)
}

View File

@@ -34,12 +34,16 @@ func (p *plugin) Transform(m resmap.ResMap) error {
return nil
}
for _, r := range m.Resources() {
if r.IsEmpty() {
empty, err := r.IsEmpty()
if err != nil {
return err
}
if empty {
// Don't mutate empty objects?
continue
}
r.StorePreviousId()
err := r.ApplyFilter(namespace.Filter{
err = r.ApplyFilter(namespace.Filter{
Namespace: p.Namespace,
FsSlice: p.FieldSpecs,
})