Skip to content

Commit

Permalink
Merge branch 'master' into release-v3.15.0-beta.0
Browse files Browse the repository at this point in the history
  • Loading branch information
JaydipGabani authored Nov 16, 2023
2 parents 5ac80c8 + 8f5201f commit 545bd7c
Show file tree
Hide file tree
Showing 15 changed files with 281 additions and 70 deletions.
16 changes: 8 additions & 8 deletions docs/Release_Management.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ No plan to move to 4.0.0 unless there is a major design change like an incompati

**Minor Releases**

- X.Y.0-alpha.W, W >= 0 (Branch : master)
- X.Y.0-alpha.W, W >= 0 (Branch: master)
- Released as needed before we cut a beta X.Y release
- Alpha release, cut from master branch
- X.Y.0-beta.W, W >= 0 (Branch : master)
- X.Y.0-beta.W, W >= 0 (Branch: master)
- Released as needed before we cut a stable X.Y release
- More stable than the alpha release to signal users to test things out
- Beta release, cut from master branch
- X.Y.0-rc.W, W >= 0 (Branch : master)
- X.Y.0-rc.W, W >= 0 (Branch: release-X.Y)
- Released as needed before we cut a stable X.Y release
- soak for ~ 2 weeks before cutting a stable release
- Release candidate release, cut from master branch
- X.Y.0 (Branch: master)
- Released every ~ 3 months
- Stable release, cut from master when X.Y milestone is complete
- Soak for a total of ~2 weeks before cutting a stable release
- Release candidate release, cut from release-X.Y branch
- X.Y.0 (Branch: release-X.Y)
- Released every ~3 months
- Stable release, cut from release-X.Y branch when X.Y milestone is complete

**Patch Releases**

Expand Down
3 changes: 3 additions & 0 deletions pkg/mutation/mutators/assign/assign_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ func (m *Mutator) String() string {

// MutatorForAssign returns a mutator built from the given assign instance.
func MutatorForAssign(assign *mutationsunversioned.Assign) (*Mutator, error) {
if err := core.ValidateName(assign.Name); err != nil {
return nil, err
}
// This is not always set by the kubernetes API server
assign.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "Assign"})

Expand Down
35 changes: 29 additions & 6 deletions pkg/mutation/mutators/assign/assign_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
path "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/path/tester"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -1073,11 +1075,32 @@ 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)
func Test_Assign_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *mutationsunversioned.Assign
errMsg string
}{
{
name: "empty path",
mut: &mutationsunversioned.Assign{},
errMsg: "empty path",
},
{
name: "name > 63",
mut: &mutationsunversioned.Assign{
ObjectMeta: metav1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForAssign(tt.mut)

require.ErrorContains(t, err, "empty path")
require.Nil(t, mutator)
require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
4 changes: 4 additions & 0 deletions pkg/mutation/mutators/assignimage/assignimage_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ func (m *Mutator) String() string {
// MutatorForAssignImage returns a mutator built from
// the given assignImage instance.
func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Mutator, error) {
if err := core.ValidateName(assignImage.Name); err != nil {
return nil, err
}

// This is not always set by the kubernetes API server
assignImage.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignImage"})

Expand Down
33 changes: 33 additions & 0 deletions pkg/mutation/mutators/assignimage/assignimage_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import (

mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
"github.com/open-policy-agent/gatekeeper/v3/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"
Expand Down Expand Up @@ -392,3 +395,33 @@ func TestMutatorForAssignImage(t *testing.T) {
})
}
}

func Test_AssignImage_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *mutationsunversioned.AssignImage
errMsg string
}{
{
name: "empty path",
mut: &mutationsunversioned.AssignImage{},
errMsg: "empty path",
},
{
name: "name > 63",
mut: &mutationsunversioned.AssignImage{
ObjectMeta: metav1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForAssignImage(tt.mut)

require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
4 changes: 4 additions & 0 deletions pkg/mutation/mutators/assignmeta/assignmeta_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func (m *Mutator) String() string {

// MutatorForAssignMetadata builds a Mutator from the given AssignMetadata object.
func MutatorForAssignMetadata(assignMeta *mutationsunversioned.AssignMetadata) (*Mutator, error) {
if err := core.ValidateName(assignMeta.Name); err != nil {
return nil, err
}

// This is not always set by the kubernetes API server
assignMeta.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignMetadata"})

Expand Down
35 changes: 29 additions & 6 deletions pkg/mutation/mutators/assignmeta/assignmeta_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (

"github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -109,11 +111,32 @@ 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)
func Test_AssignMetadata_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *unversioned.AssignMetadata
errMsg string
}{
{
name: "empty spec",
mut: &unversioned.AssignMetadata{},
errMsg: "invalid location for assignmetadata",
},
{
name: "name > 63",
mut: &unversioned.AssignMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForAssignMetadata(tt.mut)

require.ErrorContains(t, err, "invalid location for assignmetadat")
require.Nil(t, mutator)
require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
5 changes: 4 additions & 1 deletion pkg/mutation/mutators/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ import "errors"

// ErrNonKeyedSetter occurs when a setter that doesn't understand keyed lists
// is called against a keyed list.
var ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists")
var (
ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists")
ErrNameLength = errors.New("maximum name length is 63 characters")
)
10 changes: 9 additions & 1 deletion pkg/mutation/mutators/core/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
)

// NewTester returns a patht.Tester for the given object name, kind path and
// NewTester returns a path.Tester for the given object name, kind path and
// pathtests.
func NewTester(name string, kind string, path parser.Path, ptests []unversioned.PathTest) (*patht.Tester, error) {
pathTests, err := gatherPathTests(name, kind, ptests)
Expand Down Expand Up @@ -141,3 +141,11 @@ func MatchWithApplyTo(mut *types.Mutable, applies []match.ApplyTo, mat *match.Ma

return matches, nil
}

func ValidateName(name string) error {
if len(name) > 63 {
return ErrNameLength
}

return nil
}
4 changes: 4 additions & 0 deletions pkg/mutation/mutators/modifyset/modify_set_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ func (m *Mutator) String() string {
// MutatorForModifySet returns an Mutator built from
// the given modifyset instance.
func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, error) {
if err := core.ValidateName(modifySet.Name); err != nil {
return nil, err
}

// This is not always set by the kubernetes API server
modifySet.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "ModifySet"})

Expand Down
36 changes: 30 additions & 6 deletions pkg/mutation/mutators/modifyset/modify_set_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,38 @@ import (
"testing"

mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
"github.com/stretchr/testify/require"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// 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)
func Test_ModifySet_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *mutationsunversioned.ModifySet
errMsg string
}{
{
name: "empty spec",
mut: &mutationsunversioned.ModifySet{},
errMsg: "applyTo required for ModifySet mutator",
},
{
name: "name > 63",
mut: &mutationsunversioned.ModifySet{
ObjectMeta: v1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForModifySet(tt.mut)

require.ErrorContains(t, err, "applyTo required for ModifySet mutator")
require.Nil(t, mutator)
require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
6 changes: 6 additions & 0 deletions pkg/mutation/mutators/testhelpers/dummy_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package testhelpers

import (
"reflect"
"strings"

"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
Expand Down Expand Up @@ -65,3 +66,8 @@ func NewDummyMutator(name, path string, value interface{}) *DummyMutator {
}
return &DummyMutator{name: name, path: p, value: value}
}

// BigName returns a 64-length string.
func BigName() string {
return strings.Repeat("abigname", 8) // 8 X 8 = 64
}
2 changes: 2 additions & 0 deletions pkg/webhook/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ func (h *webhookHandler) isGatekeeperResource(req *admission.Request) bool {
req.AdmissionRequest.Kind.Group == "constraints.gatekeeper.sh" ||
req.AdmissionRequest.Kind.Group == mutationsGroup ||
req.AdmissionRequest.Kind.Group == "config.gatekeeper.sh" ||
req.AdmissionRequest.Kind.Group == externalDataGroup ||
req.AdmissionRequest.Kind.Group == "expansion.gatekeeper.sh" ||
req.AdmissionRequest.Kind.Group == "status.gatekeeper.sh" {
return true
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/webhook/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,13 +336,20 @@ func (h *validationHandler) getValidationMessages(res []*rtypes.Result, req *adm
// validateGatekeeperResources returns whether an issue is user error (vs internal) and any errors
// validating internal resources.
func (h *validationHandler) validateGatekeeperResources(ctx context.Context, req *admission.Request) (bool, error) {
if !h.isGatekeeperResource(req) {
return false, nil
}

if req.Operation == admissionv1.Delete && req.Name == "" {
// Allow the general DELETE of resources like "/apis/config.gatekeeper.sh/v1alpha1/namespaces/<ns>/configs"
return true, nil
}

gvk := req.AdmissionRequest.Kind
if len(req.Name) > 63 {
return false, fmt.Errorf("resource cannot have metadata.name larger than 63 char; length: %d", len(req.Name))
}

gvk := req.AdmissionRequest.Kind
switch {
case gvk.Group == "templates.gatekeeper.sh" && gvk.Kind == "ConstraintTemplate":
return h.validateTemplate(ctx, req)
Expand Down
Loading

0 comments on commit 545bd7c

Please sign in to comment.