fix: handle empty spec for modifyset (#2585)

Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> Co-authored-by: Max Smythe <smythe@google.com>
open-policy-agent · Feb 22, 2023 · 4ed4663 · 4ed4663
1 parent 0ba3c15
commit 4ed4663
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 4 deletions.
diff --git a/pkg/mutation/mutators/assign/assign_mutator_test.go b/pkg/mutation/mutators/assign/assign_mutator_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/open-policy-agent/gatekeeper/pkg/mutation/match"
 	path "github.com/open-policy-agent/gatekeeper/pkg/mutation/path/tester"
 	"github.com/open-policy-agent/gatekeeper/pkg/mutation/types"
+	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -1070,3 +1071,12 @@ func nestedMapSlice(u map[string]interface{}, fields ...string) ([]map[string]in
 	}
 	return out, nil
 }
+
+// Tests the Assign mutator MutatorForAssign call with an empty spec for graceful handling.
+func Test_Assign_emptySpec(t *testing.T) {
+	assign := &mutationsunversioned.Assign{}
+	mutator, err := MutatorForAssign(assign)
+
+	require.ErrorContains(t, err, "empty path")
+	require.Nil(t, mutator)
+}
diff --git a/pkg/mutation/mutators/assignmeta/assignmeta_mutator_test.go b/pkg/mutation/mutators/assignmeta/assignmeta_mutator_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned"
 	"github.com/open-policy-agent/gatekeeper/pkg/externaldata"
 	"github.com/open-policy-agent/gatekeeper/pkg/mutation/types"
+	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
@@ -107,3 +108,12 @@ func TestAssignMetadata(t *testing.T) {
 		})
 	}
 }
+
+// Tests the AssignMeta mutator MutatorForAssignMetadata call with an empty spec for graceful handling.
+func Test_AssignMeta_emptySpec(t *testing.T) {
+	assignMeta := &unversioned.AssignMetadata{}
+	mutator, err := MutatorForAssignMetadata(assignMeta)
+
+	require.ErrorContains(t, err, "invalid location for assignmetadat")
+	require.Nil(t, mutator)
+}
diff --git a/pkg/mutation/mutators/modifyset/modify_set_mutator.go b/pkg/mutation/mutators/modifyset/modify_set_mutator.go
@@ -138,7 +138,7 @@ func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, e
 		return nil, fmt.Errorf("modifyset %s can't change metadata", modifySet.GetName())
 	}
 
-	if path.Nodes[len(path.Nodes)-1].Type() == parser.ListNode {
+	if len(path.Nodes) > 0 && path.Nodes[len(path.Nodes)-1].Type() == parser.ListNode {
 		return nil, fmt.Errorf("final node in a modifyset location cannot be a keyed list")
 	}
 

diff --git a/pkg/mutation/mutators/modifyset/modify_set_mutator_benchmark_test.go b/pkg/mutation/mutators/modifyset/modify_set_mutator_benchmark_test.go
@@ -13,7 +13,7 @@ import (
 )
 
 func modifyset(value interface{}, location string) *unversioned.ModifySet {
-	result := &unversioned.ModifySet{
+	return &unversioned.ModifySet{
 		Spec: unversioned.ModifySetSpec{
 			ApplyTo: []match.ApplyTo{{
 				Groups:   []string{"*"},
@@ -29,8 +29,6 @@ func modifyset(value interface{}, location string) *unversioned.ModifySet {
 			},
 		},
 	}
-
-	return result
 }
 
 func benchmarkModifySetMutator(b *testing.B, n int) {

diff --git a/pkg/mutation/mutators/modifyset/modify_set_mutator_test.go b/pkg/mutation/mutators/modifyset/modify_set_mutator_test.go
@@ -0,0 +1,17 @@
+package modifyset
+
+import (
+	"testing"
+
+	mutationsunversioned "github.com/open-policy-agent/gatekeeper/apis/mutations/unversioned"
+	"github.com/stretchr/testify/require"
+)
+
+// Tests the ModifySet mutator MutatorForModifySet call with an empty spec for graceful handling.
+func Test_ModifySet_emptySpec(t *testing.T) {
+	modifySet := &mutationsunversioned.ModifySet{}
+	mutator, err := MutatorForModifySet(modifySet)
+
+	require.ErrorContains(t, err, "applyTo required for ModifySet mutator")
+	require.Nil(t, mutator)
+}