mirrors/kyverno
1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2024-12-14 11:57:48 +00:00
Code Activity

fix: early return in policy validation (#5200)

Browse source 
* fix: early return in policy validation

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* fix test

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
This commit is contained in:
Charles-Edouard Brétéché 2022-11-03 09:05:23 +00:00 committed by GitHub
parent 3fc157717a
commit f52da91b72
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 55 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

81
pkg/policy/validate.go
View file

@ -25,7 +25,6 @@ import (
"github.com/kyverno/kyverno/pkg/utils"
kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
"github.com/pkg/errors"
admissionv1 "k8s.io/api/admission/v1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
@ -110,7 +109,8 @@ func validateJSONPatch(patch string, ruleIdx int) error {
}
// Validate checks the policy and rules declarations for required configurations
func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, openApiManager openapi.Manager) (*admissionv1.AdmissionResponse, error) {
func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, openApiManager openapi.Manager) ([]string, error) {
var warnings []string
namespaced := policy.IsNamespaced()
spec := policy.GetSpec()
background := spec.BackgroundProcessingEnabled()
@ -124,13 +124,13 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
err := ValidateVariables(policy, background)
if err != nil {
return nil, err
return warnings, err
}
if onPolicyUpdate {
err := ValidateOnPolicyUpdate(policy, onPolicyUpdate)
if err != nil {
return nil, err
return warnings, err
}
}
@ -140,7 +140,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
// Get all the cluster type kind supported by cluster
res, err := discovery.ServerPreferredResources(client.Discovery().DiscoveryInterface())
if err != nil {
return nil, err
return warnings, err
}
for _, resList := range res {
for _, r := range resList.APIResources {
@ -152,13 +152,13 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
if errs := policy.Validate(clusterResources); len(errs) != 0 {
return nil, errs.ToAggregate()
return warnings, errs.ToAggregate()
}
if !namespaced {
err := validateNamespaces(spec, specPath.Child("validationFailureActionOverrides"))
if err != nil {
return nil, err
return warnings, err
}
}
@ -168,34 +168,31 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
rulePath := rulesPath.Index(i)
// check for forward slash
if err := validateJSONPatchPathForForwardSlash(rule.Mutation.PatchesJSON6902); err != nil {
return nil, fmt.Errorf("path must begin with a forward slash: spec.rules[%d]: %s", i, err)
return warnings, fmt.Errorf("path must begin with a forward slash: spec.rules[%d]: %s", i, err)
}
if err := validateJSONPatch(rule.Mutation.PatchesJSON6902, i); err != nil {
return nil, fmt.Errorf("%s", err)
return warnings, fmt.Errorf("%s", err)
}
if jsonPatchOnPod(rule) {
msg := "Pods managed by workload controllers should not be directly mutated using policies. " +
"Use the autogen feature or write policies that match Pod controllers."
logging.V(1).Info(msg)
return &admissionv1.AdmissionResponse{
Allowed: true,
Warnings: []string{msg},
}, nil
warnings = append(warnings, msg)
}
// validate resource description
if path, err := validateResources(rulePath, rule); err != nil {
return nil, fmt.Errorf("path: spec.rules[%d].%s: %v", i, path, err)
return warnings, fmt.Errorf("path: spec.rules[%d].%s: %v", i, path, err)
}
err := validateElementInForEach(rule)
if err != nil {
return nil, err
return warnings, err
}
if err := validateRuleContext(rule); err != nil {
return nil, fmt.Errorf("path: spec.rules[%d]: %v", i, err)
return warnings, fmt.Errorf("path: spec.rules[%d]: %v", i, err)
}
// If a rule's match block does not match any kind,
@ -203,25 +200,25 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if len(rule.MatchResources.Any) > 0 {
for _, rmr := range rule.MatchResources.Any {
if len(rmr.Kinds) == 0 {
return nil, validateMatchKindHelper(rule)
return warnings, validateMatchKindHelper(rule)
}
}
} else if len(rule.MatchResources.All) > 0 {
for _, rmr := range rule.MatchResources.All {
if len(rmr.Kinds) == 0 {
return nil, validateMatchKindHelper(rule)
return warnings, validateMatchKindHelper(rule)
}
}
} else {
if len(rule.MatchResources.Kinds) == 0 {
return nil, validateMatchKindHelper(rule)
return warnings, validateMatchKindHelper(rule)
}
}
// validate Cluster Resources in namespaced policy
// For namespaced policy, ClusterResource type field and values are not allowed in match and exclude
if namespaced {
return nil, checkClusterResourceInMatchAndExclude(rule, clusterResources, mock, res)
return warnings, checkClusterResourceInMatchAndExclude(rule, clusterResources, mock, res)
}
// validate rule actions
@ -229,26 +226,26 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
// - Validate
// - Generate
if err := validateActions(i, &rules[i], client, mock); err != nil {
return nil, err
return warnings, err
}
if utils.ContainsString(rule.MatchResources.Kinds, "*") && spec.BackgroundProcessingEnabled() {
return nil, fmt.Errorf("wildcard policy not allowed in background mode. Set spec.background=false to disable background mode for this policy rule ")
return warnings, fmt.Errorf("wildcard policy not allowed in background mode. Set spec.background=false to disable background mode for this policy rule ")
}
if (utils.ContainsString(rule.MatchResources.Kinds, "*") && len(rule.MatchResources.Kinds) > 1) || (utils.ContainsString(rule.ExcludeResources.Kinds, "*") && len(rule.ExcludeResources.Kinds) > 1) {
return nil, fmt.Errorf("wildard policy can not deal more than one kind")
return warnings, fmt.Errorf("wildard policy can not deal more than one kind")
}
if utils.ContainsString(rule.MatchResources.Kinds, "*") || utils.ContainsString(rule.ExcludeResources.Kinds, "*") {
if rule.HasGenerate() || rule.HasVerifyImages() || rule.Validation.ForEachValidation != nil {
return nil, fmt.Errorf("wildcard policy does not support rule type")
return warnings, fmt.Errorf("wildcard policy does not support rule type")
}
if rule.HasValidate() {
if rule.Validation.GetPattern() != nil || rule.Validation.GetAnyPattern() != nil {
if !ruleOnlyDealsWithResourceMetaData(rule) {
return nil, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
@ -260,7 +257,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
for _, condition := range typedConditions {
key := condition.GetKey()
if !strings.Contains(key.(string), "request.object.metadata.") && (!wildCardAllowedVariables.MatchString(key.(string)) || strings.Contains(key.(string), "request.object.spec")) {
return nil, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
@ -270,7 +267,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if rule.HasMutate() {
if !ruleOnlyDealsWithResourceMetaData(rule) {
return nil, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
@ -283,7 +280,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
if len(errs) != 0 {
return nil, errs.ToAggregate()
return warnings, errs.ToAggregate()
}
}
@ -293,7 +290,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
errs = append(errs, i.Validate(verifyImagePath.Index(index))...)
}
if len(errs) != 0 {
return nil, errs.ToAggregate()
return warnings, errs.ToAggregate()
}
}
@ -303,11 +300,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
msg := "Policies that match Pods apply to all Pods including those created and managed by controllers " +
"excluded from autogen. Use preconditions to exclude the Pods managed by controllers which are " +
"excluded from autogen. Refer to https://kyverno.io/docs/writing-policies/autogen/ for details."
return &admissionv1.AdmissionResponse{
Allowed: true,
Warnings: []string{msg},
}, nil
warnings = append(warnings, msg)
}
// Validate Kind with match resource kinds
@ -317,7 +310,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
return nil, errors.Wrapf(err, "the kind defined in the any match resource is invalid")
return warnings, errors.Wrapf(err, "the kind defined in the any match resource is invalid")
}
}
}
@ -325,7 +318,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
return nil, errors.Wrapf(err, "the kind defined in the all match resource is invalid")
return warnings, errors.Wrapf(err, "the kind defined in the all match resource is invalid")
}
}
}
@ -333,7 +326,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
return nil, errors.Wrapf(err, "the kind defined in the any exclude resource is invalid")
return warnings, errors.Wrapf(err, "the kind defined in the any exclude resource is invalid")
}
}
}
@ -341,24 +334,24 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
return nil, errors.Wrapf(err, "the kind defined in the all exclude resource is invalid")
return warnings, errors.Wrapf(err, "the kind defined in the all exclude resource is invalid")
}
}
}
if !utils.ContainsString(rule.MatchResources.Kinds, "*") {
err := validateKinds(rule.MatchResources.Kinds, mock, client, policy)
if err != nil {
return nil, errors.Wrapf(err, "match resource kind is invalid")
return warnings, errors.Wrapf(err, "match resource kind is invalid")
}
err = validateKinds(rule.ExcludeResources.Kinds, mock, client, policy)
if err != nil {
return nil, errors.Wrapf(err, "exclude resource kind is invalid")
return warnings, errors.Wrapf(err, "exclude resource kind is invalid")
}
}
// Validate string values in labels
if !isLabelAndAnnotationsString(rule) {
return nil, fmt.Errorf("labels and annotations supports only string values, \"use double quotes around the non string values\"")
return warnings, fmt.Errorf("labels and annotations supports only string values, \"use double quotes around the non string values\"")
}
// add label to source mentioned in policy
@ -402,13 +395,13 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
}
if spec.SchemaValidation == nil || *spec.SchemaValidation {
if !mock && (spec.SchemaValidation == nil || *spec.SchemaValidation) {
if err := openApiManager.ValidatePolicyMutation(policy); err != nil {
return nil, err
return warnings, err
}
}
return nil, nil
return warnings, nil
}
func ValidateVariables(p kyvernov1.PolicyInterface, backgroundMode bool) error {

12
pkg/policy/validate_test.go
View file

@ -1533,10 +1533,8 @@ func Test_PodControllerAutoGenExclusion_Not_All_Controllers_Policy(t *testing.T)
assert.NilError(t, err)
openApiManager, _ := openapi.NewManager()
res, err := Validate(policy, nil, true, openApiManager)
if res != nil {
assert.Assert(t, res.Warnings != nil)
}
warnings, err := Validate(policy, nil, true, openApiManager)
assert.Assert(t, warnings != nil)
assert.NilError(t, err)
}
@ -1592,10 +1590,8 @@ func Test_PodControllerAutoGenExclusion_None_Policy(t *testing.T) {
assert.NilError(t, err)
openApiManager, _ := openapi.NewManager()
res, err := Validate(policy, nil, true, openApiManager)
if res != nil {
assert.Assert(t, res.Warnings != nil)
}
warnings, err := Validate(policy, nil, true, openApiManager)
assert.Assert(t, warnings == nil)
assert.NilError(t, err)
}

12
pkg/utils/admission/utils.go
View file

@ -111,3 +111,15 @@ func GetResourceName(request *admissionv1.AdmissionRequest) string {
}
return resourceName
}
func ValidationResponse(err error, warnings ...string) *admissionv1.AdmissionResponse {
response := Response(err == nil)
if err != nil {
response.Result = &metav1.Status{
Status: metav1.StatusFailure,
Message: err.Error(),
}
}
response.Warnings = warnings
return response
}

7
pkg/webhooks/policy/handlers.go
View file

@ -35,15 +35,12 @@ func (h *handlers) Validate(logger logr.Logger, request *admissionv1.AdmissionRe
logger.Error(err, "failed to unmarshal policies from admission request")
return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to validate policy, check kyverno controller logs for details: %v", err))
}
response, err := policyvalidate.Validate(policy, h.client, false, h.openApiManager)
warnings, err := policyvalidate.Validate(policy, h.client, false, h.openApiManager)
if err != nil {
logger.Error(err, "policy validation errors")
return admissionutils.ResponseWithMessage(false, err.Error())
}
if response != nil && len(response.Warnings) != 0 {
return response
}
return admissionutils.Response(true)
return admissionutils.ValidationResponse(err, warnings...)
}
func (h *handlers) Mutate(logger logr.Logger, request *admissionv1.AdmissionRequest, _ time.Time) *admissionv1.AdmissionResponse {