Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Include oldObject and admission request metadata in expansion #3341

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/audit/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ func (am *Manager) reviewObjects(ctx context.Context, kind string, folderCount i
}

// Expand object and review any resultant resources
base := &mutationtypes.Mutable{
base := &expansion.Expandable{
Object: objFile,
Namespace: ns,
Username: "",
Expand Down
57 changes: 57 additions & 0 deletions pkg/expansion/fixtures/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,4 +622,61 @@ spec:
ports:
- containerPort: '80'
`

GeneratorOldCronJob = `
apiVersion: batch/v1
kind: CronJob
metadata:
name: my-cronjob
spec:
schedule: "* * * * *"
jobTemplate:
spec:
template:
spec:
containers:
- args:
- "/bin/sh"
image: nginx:1.14.0
imagePullPolicy: Always
name: nginx
ports:
- containerPort: '8080'
`

ResultantOldJob = `
apiVersion: batch/v1
kind: Job
metadata:
name: my-cronjob-job
namespace: default
spec:
template:
spec:
containers:
- args:
- "/bin/sh"
image: nginx:1.14.0
imagePullPolicy: Always
name: nginx
ports:
- containerPort: '8080'
`

ResultantRecursiveOldPod = `
apiVersion: v1
kind: Pod
metadata:
name: my-cronjob-job-pod
namespace: default
spec:
containers:
- args:
- "/bin/sh"
image: nginx:1.14.0
imagePullPolicy: Always
name: nginx
ports:
- containerPort: '8080'
`
)
31 changes: 18 additions & 13 deletions pkg/expansion/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ type System struct {
db templateDB
}

type Resultant struct {
Obj *unstructured.Unstructured
TemplateName string
EnforcementAction string
}

type TemplateID string

type IDSet map[TemplateID]bool
Expand Down Expand Up @@ -132,7 +126,7 @@ func genGVKToSchemaGVK(gvk expansionunversioned.GeneratedGVK) schema.GroupVersio
// Expand expands `base` into resultant resources, and applies any applicable
// mutators. If no ExpansionTemplates match `base`, an empty slice
// will be returned. If `s.mutationSystem` is nil, no mutations will be applied.
func (s *System) Expand(base *mutationtypes.Mutable) ([]*Resultant, error) {
func (s *System) Expand(base *Expandable) ([]*Resultant, error) {
s.lock.RLock()
defer s.lock.RUnlock()

Expand All @@ -143,7 +137,7 @@ func (s *System) Expand(base *mutationtypes.Mutable) ([]*Resultant, error) {
return res, nil
}

func (s *System) expandRecursive(base *mutationtypes.Mutable, resultants *[]*Resultant, depth int) error {
func (s *System) expandRecursive(base *Expandable, resultants *[]*Resultant, depth int) error {
if depth >= maxRecursionDepth {
return fmt.Errorf("maximum recursion depth of %d reached", maxRecursionDepth)
}
Expand All @@ -154,8 +148,9 @@ func (s *System) expandRecursive(base *mutationtypes.Mutable, resultants *[]*Res
}

for _, r := range res {
mut := &mutationtypes.Mutable{
mut := &Expandable{
Object: r.Obj,
OldObject: r.OldObj,
Namespace: base.Namespace,
Username: base.Username,
Source: base.Source,
Expand All @@ -169,7 +164,7 @@ func (s *System) expandRecursive(base *mutationtypes.Mutable, resultants *[]*Res
return nil
}

func (s *System) expand(base *mutationtypes.Mutable) ([]*Resultant, error) {
func (s *System) expand(base *Expandable) ([]*Resultant, error) {
gvk := base.Object.GroupVersionKind()
if gvk == (schema.GroupVersionKind{}) {
return nil, fmt.Errorf("cannot expand resource %s with empty GVK", base.Object.GetName())
Expand All @@ -180,14 +175,24 @@ func (s *System) expand(base *mutationtypes.Mutable) ([]*Resultant, error) {

for _, te := range templates {
res, err := expandResource(base.Object, base.Namespace, te)
if err != nil {
return nil, err
}

var oldRes *unstructured.Unstructured
if base.OldObject != nil {
oldRes, err = expandResource(base.OldObject, base.Namespace, te)
if err != nil {
return nil, err
}
}

resultants = append(resultants, &Resultant{
Obj: res,
OldObj: oldRes,
TemplateName: te.ObjectMeta.Name,
EnforcementAction: te.Spec.EnforcementAction,
})
if err != nil {
return nil, err
}
}

if s.mutationSystem == nil {
Expand Down
63 changes: 55 additions & 8 deletions pkg/expansion/system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import (

func TestExpand(t *testing.T) {
tests := []struct {
name string
generator *unstructured.Unstructured
ns *corev1.Namespace
templates []*expansionunversioned.ExpansionTemplate
mutators []types.Mutator
want []*Resultant
expectErr bool
name string
generator *unstructured.Unstructured
generatorOld *unstructured.Unstructured
ns *corev1.Namespace
templates []*expansionunversioned.ExpansionTemplate
mutators []types.Mutator
want []*Resultant
expectErr bool
}{
{
name: "generator with no templates or mutators",
Expand Down Expand Up @@ -248,6 +249,51 @@ func TestExpand(t *testing.T) {
{Obj: fixtures.LoadFixture(fixtures.ResultantRecursivePod, t), EnforcementAction: "", TemplateName: "expand-jobs"},
},
},
{
name: "recursive expansion with old object",
generator: fixtures.LoadFixture(fixtures.GeneratorCronJob, t),
generatorOld: fixtures.LoadFixture(fixtures.GeneratorOldCronJob, t),
ns: &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
mutators: []types.Mutator{},
templates: []*expansionunversioned.ExpansionTemplate{
fixtures.LoadTemplate(fixtures.TempExpCronJob, t),
},
want: []*Resultant{
{
Obj: fixtures.LoadFixture(fixtures.ResultantJob, t),
OldObj: fixtures.LoadFixture(fixtures.ResultantOldJob, t),
EnforcementAction: "",
TemplateName: "expand-cronjobs",
},
},
},
{
name: "recursive expansion with old object and mutators",
generator: fixtures.LoadFixture(fixtures.GeneratorCronJob, t),
generatorOld: fixtures.LoadFixture(fixtures.GeneratorOldCronJob, t),
ns: &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}},
mutators: []types.Mutator{
fixtures.LoadAssignMeta(fixtures.AssignMetaAnnotatePod, t),
},
templates: []*expansionunversioned.ExpansionTemplate{
fixtures.LoadTemplate(fixtures.TempExpCronJob, t),
fixtures.LoadTemplate(fixtures.TempExpJob, t),
},
want: []*Resultant{
{
Obj: fixtures.LoadFixture(fixtures.ResultantJob, t),
OldObj: fixtures.LoadFixture(fixtures.ResultantOldJob, t),
EnforcementAction: "",
TemplateName: "expand-cronjobs",
},
{
Obj: fixtures.LoadFixture(fixtures.ResultantRecursivePod, t),
OldObj: fixtures.LoadFixture(fixtures.ResultantRecursiveOldPod, t),
EnforcementAction: "",
TemplateName: "expand-jobs",
},
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -265,8 +311,9 @@ func TestExpand(t *testing.T) {
}
}

base := &types.Mutable{
base := &Expandable{
Object: tc.generator,
OldObject: tc.generatorOld,
Namespace: tc.ns,
Username: "unit-test",
Source: types.SourceTypeGenerated,
Expand Down
33 changes: 33 additions & 0 deletions pkg/expansion/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package expansion

import (
mutationtypes "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// Expandable represents an expandable object and its metadata.
type Expandable struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because object, oldObject, etc. only exist for webhook admission (not audit, gator, or any other enforcement point), we probably should not make these part of the interface. We can probably achieve the same result by modifying how the admission webhook invokes expansion.

// Object is the object to be expanded.
Object *unstructured.Unstructured

// OldObject is the old version of the object (if applicable) to be expanded.
OldObject *unstructured.Unstructured

// Namespace is the namespace of the expandable object.
Namespace *corev1.Namespace

// Username is the name of the user who initiates
// admission request of the expandable object.
Username string

// Source specifies which types of resources the mutator should be applied to
Source mutationtypes.SourceType
}

type Resultant struct {
Obj *unstructured.Unstructured
OldObj *unstructured.Unstructured
TemplateName string
EnforcementAction string
}
14 changes: 10 additions & 4 deletions pkg/gator/expand/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,24 @@ func NewExpander(resources []*unstructured.Unstructured) (*Expander, error) {
func (er *Expander) Expand(resource *unstructured.Unstructured) ([]*expansion.Resultant, error) {
ns, _ := er.NamespaceForResource(resource)

// Mutate the base resource before expanding it
base := &types.Mutable{
// Mutate the mutable resource before expanding it
mutable := &types.Mutable{
Object: resource,
Namespace: ns,
Username: "",
Source: types.SourceTypeOriginal,
}
if _, err := er.mutSystem.Mutate(base); err != nil {
if _, err := er.mutSystem.Mutate(mutable); err != nil {
return nil, fmt.Errorf("error mutating base resource %s: %w", resource.GetName(), err)
}

resultants, err := er.expSystem.Expand(base)
expandable := &expansion.Expandable{
Object: mutable.Object,
Namespace: mutable.Namespace,
Username: mutable.Username,
Source: mutable.Source,
}
resultants, err := er.expSystem.Expand(expandable)
if err != nil {
return nil, fmt.Errorf("error expanding resource %s: %w", resource.GetName(), err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/readiness/ready_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func Test_ExpansionTemplate(t *testing.T) {
panic(fmt.Errorf("error converting deployment to unstructured: %w", err))
}
u := unstructured.Unstructured{Object: o}
m := mutationtypes.Mutable{
m := expansion.Expandable{
Object: &u,
Namespace: testNS,
Username: "",
Expand Down
Loading