Skip to content

Commit

Permalink
Throwing error instead of silently ignoring invalid input
Browse files Browse the repository at this point in the history
  • Loading branch information
m-Bilal committed Jan 1, 2022
1 parent b28f1e5 commit ff7b2f2
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 11 deletions.
5 changes: 4 additions & 1 deletion api/internal/accumulator/namereferencetransformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error {
fMap := t.determineFilters(m.Resources())
debug(fMap)
for r, fList := range fMap {
c := m.SubsetThatCouldBeReferencedByResource(r)
c, err := m.SubsetThatCouldBeReferencedByResource(r)
if err != nil {
return err
}
for _, f := range fList {
f.Referrer = r
f.ReferralCandidates = c
Expand Down
2 changes: 1 addition & 1 deletion api/resmap/resmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ type ResMap interface {
// This is a filter; it excludes things that cannot be
// referenced by the resource, e.g. objects in other
// namespaces. Cluster wide objects are never excluded.
SubsetThatCouldBeReferencedByResource(*resource.Resource) ResMap
SubsetThatCouldBeReferencedByResource(*resource.Resource) (ResMap, error)

// DeAnchor replaces YAML aliases with structured data copied from anchors.
// This cannot be undone; if desired, call DeepCopy first.
Expand Down
21 changes: 13 additions & 8 deletions api/resmap/reswrangler.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,14 +389,17 @@ func (m *resWrangler) makeCopy(copier resCopier) ResMap {

// SubsetThatCouldBeReferencedByResource implements ResMap.
func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
referrer *resource.Resource) ResMap {
referrer *resource.Resource) (ResMap, error) {
referrerId := referrer.CurId()
if referrerId.IsClusterScoped() {
// A cluster scoped resource can refer to anything.
return m
return m, nil
}
result := newOne()
roleBindingNamespaces := getNamespacesForRoleBinding(referrer)
roleBindingNamespaces, err := getNamespacesForRoleBinding(referrer)
if err != nil {
return nil, err
}
for _, possibleTarget := range m.rList {
id := possibleTarget.CurId()
if id.IsClusterScoped() {
Expand All @@ -416,20 +419,20 @@ func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
result.append(possibleTarget)
}
}
return result
return result, nil
}

// getNamespacesForRoleBinding returns referenced ServiceAccount namespaces
// if the resource is a RoleBinding
func getNamespacesForRoleBinding(r *resource.Resource) map[string]bool {
func getNamespacesForRoleBinding(r *resource.Resource) (map[string]bool, error) {
result := make(map[string]bool)
if r.GetKind() != "RoleBinding" {
return result
return result, nil
}
//nolint staticcheck
subjects, err := r.GetSlice("subjects")
if err != nil || subjects == nil {
return result
return result, nil
}
for _, s := range subjects {
subject := s.(map[string]interface{})
Expand All @@ -438,12 +441,14 @@ func getNamespacesForRoleBinding(r *resource.Resource) map[string]bool {
if kind.(string) == "ServiceAccount" {
if n, ok3 := ns.(string); ok3 {
result[n] = true
} else {
return nil, errors.New("Invalid Input: namespace is blank")
}
}
}
}
}
return result
return result, nil
}

// AppendAll implements ResMap.
Expand Down
5 changes: 4 additions & 1 deletion api/resmap/reswrangler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,10 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) {
for name, test := range tests {
test := test
t.Run(name, func(t *testing.T) {
got := m.SubsetThatCouldBeReferencedByResource(test.filter)
got, err1 := m.SubsetThatCouldBeReferencedByResource(test.filter)
if err1 != nil {
t.Fatalf("Expected error %v: ", err1)
}
err := test.expected.ErrorIfNotEqualLists(got)
if err != nil {
test.expected.Debug("expected")
Expand Down

0 comments on commit ff7b2f2

Please sign in to comment.