From 85ce6f2f9d5232b21fbfe66d0076dc38b5e6b4f1 Mon Sep 17 00:00:00 2001 From: alex <8968914+acpana@users.noreply.github.com> Date: Tue, 21 Feb 2023 22:20:29 -0800 Subject: [PATCH] fix: handle empty spec for modifyset (#2585) Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> Co-authored-by: Max Smythe --- .../mutators/assign/assign_mutator_test.go | 10 ++++++++++ .../assignmeta/assignmeta_mutator_test.go | 10 ++++++++++ .../mutators/modifyset/modify_set_mutator.go | 2 +- .../modify_set_mutator_benchmark_test.go | 4 +--- .../modifyset/modify_set_mutator_test.go | 17 +++++++++++++++++ 5 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 pkg/mutation/mutators/modifyset/modify_set_mutator_test.go diff --git a/pkg/mutation/mutators/assign/assign_mutator_test.go b/pkg/mutation/mutators/assign/assign_mutator_test.go index 3bd363e5d2b..473e9cac9d0 100644 --- 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 index 1f222c77666..3c47eb65ff3 100644 --- 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 index 6be2a422fb4..b878a5b8980 100644 --- 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 index bb15d688017..0f1a907cb4f 100644 --- 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 new file mode 100644 index 00000000000..740925b5afe --- /dev/null +++ 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) +}