Skip to content

Commit

Permalink
disable auto-gen when a rule has mixed of kinds: pod & pod controllers
Browse files Browse the repository at this point in the history
Signed-off-by: Shuting Zhao <shutting06@gmail.com>
  • Loading branch information
realshuting committed Apr 29, 2021
1 parent df6c896 commit 2704e40
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 16 deletions.
54 changes: 38 additions & 16 deletions pkg/policymutation/policymutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p

// scenario A
if !ok {
controllers = engine.PodControllers
annPatch, err := defaultPodControllerAnnotation(ann)
controllers = getControllers(&policy, log)
annPatch, err := defaultPodControllerAnnotation(ann, controllers)
if err != nil {
errs = append(errs, fmt.Errorf("failed to generate pod controller annotation for policy '%s': %v", policy.Name, err))
} else {
Expand All @@ -235,7 +235,7 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p

// scenario B
if controllers == "none" {
return nil, nil
return patches, nil
}

log.V(3).Info("auto generating rule for pod controllers", "controllers", controllers)
Expand All @@ -246,6 +246,37 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p
return
}

// getControllers gets applicable pod controllers, it returns
// - "none" if:
// - name or selector is defined
// - mixed kinds (Pod + pod controller) is defined
// - mutate.Patches/mutate.PatchesJSON6902/validate.deny/generate rule is defined
// - otherwise it returns all pod controllers
func getControllers(policy *kyverno.ClusterPolicy, log logr.Logger) string {
for _, rule := range policy.Spec.Rules {
match := rule.MatchResources
exclude := rule.ExcludeResources

if match.ResourceDescription.Name != "" || match.ResourceDescription.Selector != nil ||
exclude.ResourceDescription.Name != "" || exclude.ResourceDescription.Selector != nil {
log.Info("skip generating rule on pod controllers: Name / Selector in resource description may not be applicable.", "rule", rule.Name)
return "none"
}

if (len(match.Kinds) > 1 && utils.ContainsString(match.Kinds, "Pod")) ||
(len(exclude.Kinds) > 1 && utils.ContainsString(exclude.Kinds, "Pod")) {
return "none"
}

if rule.Mutation.Patches != nil || rule.Mutation.PatchesJSON6902 != "" ||
rule.Validation.Deny != nil || rule.HasGenerate() {
return "none"
}
}

return engine.PodControllers
}

func createRuleMap(rules []kyverno.Rule) map[string]kyvernoRule {
var ruleMap = make(map[string]kyvernoRule)
for _, rule := range rules {
Expand Down Expand Up @@ -398,10 +429,6 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr.
return kyvernoRule{}
}

if rule.Mutation.Overlay == nil && !rule.HasValidate() && rule.Mutation.PatchStrategicMerge == nil {
return kyvernoRule{}
}

// Support backwards compatibility
skipAutoGeneration := false
var controllersValidated []string
Expand All @@ -420,11 +447,6 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr.
}

if skipAutoGeneration {
if match.ResourceDescription.Name != "" || match.ResourceDescription.Selector != nil ||
exclude.ResourceDescription.Name != "" || exclude.ResourceDescription.Selector != nil {
logger.Info("skip generating rule on pod controllers: Name / Selector in resource description may not be applicable.", "rule", rule.Name)
return kyvernoRule{}
}
if controllers == "all" {
controllers = "DaemonSet,Deployment,Job,StatefulSet"
} else {
Expand Down Expand Up @@ -534,11 +556,11 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr.
}

// defaultPodControllerAnnotation inserts an annotation
// "pod-policies.kyverno.io/autogen-controllers=DaemonSet,Deployment,Job,StatefulSet" to policy
func defaultPodControllerAnnotation(ann map[string]string) ([]byte, error) {
// "pod-policies.kyverno.io/autogen-controllers=<controllers>" to policy
func defaultPodControllerAnnotation(ann map[string]string, controllers string) ([]byte, error) {
if ann == nil {
ann = make(map[string]string)
ann[engine.PodControllersAnnotation] = engine.PodControllers
ann[engine.PodControllersAnnotation] = controllers
jsonPatch := struct {
Path string `json:"path"`
Op string `json:"op"`
Expand All @@ -563,7 +585,7 @@ func defaultPodControllerAnnotation(ann map[string]string) ([]byte, error) {
}{
"/metadata/annotations/pod-policies.kyverno.io~1autogen-controllers",
"add",
engine.PodControllers,
controllers,
}

patchByte, err := json.Marshal(jsonPatch)
Expand Down
77 changes: 77 additions & 0 deletions pkg/policymutation/policymutation_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package policymutation

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"

kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
"github.com/kyverno/kyverno/pkg/engine"
"github.com/kyverno/kyverno/pkg/utils"
"gotest.tools/assert"
Expand Down Expand Up @@ -152,3 +155,77 @@ func Test_CronJobAndDeployment(t *testing.T) {

assert.DeepEqual(t, rulePatches, expectedPatches)
}

func Test_getControllers(t *testing.T) {
testCases := []struct {
name string
policy []byte
expectedControllers string
}{
{
name: "rule-with-match-name",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"test","match":{"resources":{"kinds":["Namespace"],"name":"*"}}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-match-selector",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test-getcontrollers"},"spec":{"background":false,"rules":[{"name":"test-getcontrollers","match":{"resources":{"kinds":["Pod"],"selector":{"matchLabels":{"foo":"bar"}}}}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-exclude-name",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test-getcontrollers"},"spec":{"background":false,"rules":[{"name":"test-getcontrollers","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"name":"test"}}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-exclude-selector",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test-getcontrollers"},"spec":{"background":false,"rules":[{"name":"test-getcontrollers","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"selector":{"matchLabels":{"foo":"bar"}}}}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-deny",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"require-network-policy","match":{"resources":{"kinds":["Pod"]}},"validate":{"message":"testpolicy","deny":{"conditions":[{"key":"{{request.object.metadata.labels.foo}}","operator":"Equals","value":"bar"}]}}}]}}`),
expectedControllers: "none",
},

{
name: "rule-with-match-mixed-kinds-pod-podcontrollers",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-service-labels-env"},"spec":{"background":false,"rules":[{"name":"set-service-label","match":{"resources":{"kinds":["Pod","Deployment"]}},"preconditions":{"any":[{"key":"{{request.operation}}","operator":"Equals","value":"CREATE"}]},"mutate":{"patchStrategicMerge":{"metadata":{"labels":{"+(service)":"{{request.object.spec.template.metadata.labels.app}}"}}}}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-exclude-mixed-kinds-pod-podcontrollers",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-service-labels-env"},"spec":{"background":false,"rules":[{"name":"set-service-label","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"kinds":["Pod","Deployment"]}},"mutate":{"patchStrategicMerge":{"metadata":{"labels":{"+(service)":"{{request.object.spec.template.metadata.labels.app}}"}}}}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-match-kinds-pod-only",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"require-network-policy","match":{"resources":{"kinds":["Pod"]}},"validate":{"message":"testpolicy","pattern":{"metadata":{"labels":{"foo":"bar"}}}}}]}}`),
expectedControllers: engine.PodControllers,
},
{
name: "rule-with-exclude-kinds-pod-only",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"require-network-policy","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"kinds":["Pod"],"namespaces":["test"]}},"validate":{"message":"testpolicy","pattern":{"metadata":{"labels":{"foo":"bar"}}}}}]}}`),
expectedControllers: engine.PodControllers,
},
{
name: "rule-with-mutate-patches",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"test","match":{"resources":{"kinds":["Pod"]}},"mutate":{"patchesJson6902":"-op:add\npath:/spec/containers/0/env/-1\nvalue:{\"name\":\"SERVICE\",\"value\":{{request.object.spec.template.metadata.labels.app}}}"}}]}}`),
expectedControllers: "none",
},
{
name: "rule-with-generate",
policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"add-networkpolicy"},"spec":{"rules":[{"name":"default-deny-ingress","match":{"resources":{"kinds":["Namespace"],"name":"*"}},"exclude":{"resources":{"namespaces":["kube-system","default","kube-public","kyverno"]}},"generate":{"kind":"NetworkPolicy","name":"default-deny-ingress","namespace":"{{request.object.metadata.name}}","synchronize":true,"data":{"spec":{"podSelector":{},"policyTypes":["Ingress"]}}}}]}}`),
expectedControllers: "none",
},
}

for _, test := range testCases {
var policy kyverno.ClusterPolicy
err := json.Unmarshal(test.policy, &policy)
assert.NilError(t, err)

controllers := getControllers(&policy, log.Log)
assert.Equal(t, test.expectedControllers, controllers, fmt.Sprintf("test %s failed", test.name))
}
}

0 comments on commit 2704e40

Please sign in to comment.