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

[Bug]: Fix wildcard any/all issue (#5387)

* Fix wildcard for any/all match/excude kinds

* remove non required test

* add kuttl test

* Revert "add kuttl test"

This reverts commit d2245bc248.

* add kuttl test

* fix test
This commit is contained in:
Vyankatesh Kudtarkar 2022-11-17 19:37:03 +05:30 committed by GitHub
parent 987e6d1cf6
commit 83a84c9d47
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 171 additions and 48 deletions

View file

@ -250,54 +250,6 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
return warnings, err
}
if utils.ContainsString(rule.MatchResources.Kinds, "*") && spec.BackgroundProcessingEnabled() {
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 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 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 warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
if rule.Validation.Deny != nil {
kyvernoConditions, _ := utils.ApiextensionsJsonToKyvernoConditions(rule.Validation.Deny.GetAnyAllConditions())
switch typedConditions := kyvernoConditions.(type) {
case []kyvernov1.Condition: // backwards compatibility
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 warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
}
}
}
if rule.HasMutate() {
if !ruleOnlyDealsWithResourceMetaData(rule) {
return warnings, fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
if len(errs) != 0 {
return warnings, errs.ToAggregate()
}
}
if rule.HasVerifyImages() {
verifyImagePath := rulePath.Child("verifyImages")
for index, i := range rule.VerifyImages {
@ -321,6 +273,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
match := rule.MatchResources
exclude := rule.ExcludeResources
for _, value := range match.Any {
wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule)
if wildcardErr != nil {
return warnings, wildcardErr
}
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
@ -329,6 +285,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
}
for _, value := range match.All {
wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule)
if wildcardErr != nil {
return warnings, wildcardErr
}
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
@ -337,6 +297,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
}
for _, value := range exclude.Any {
wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule)
if wildcardErr != nil {
return warnings, wildcardErr
}
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
@ -345,6 +309,10 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
}
for _, value := range exclude.All {
wildcardErr := validateWildcard(value.ResourceDescription.Kinds, spec, rule)
if wildcardErr != nil {
return warnings, wildcardErr
}
if !utils.ContainsString(value.ResourceDescription.Kinds, "*") {
err := validateKinds(value.ResourceDescription.Kinds, mock, client, policy)
if err != nil {
@ -352,6 +320,7 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
}
}
}
if !utils.ContainsString(rule.MatchResources.Kinds, "*") {
err := validateKinds(rule.MatchResources.Kinds, mock, client, policy)
if err != nil {
@ -361,6 +330,15 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b
if err != nil {
return warnings, errors.Wrapf(err, "exclude resource kind is invalid")
}
} else {
wildcardErr := validateWildcard(rule.MatchResources.Kinds, spec, rule)
if wildcardErr != nil {
return warnings, wildcardErr
}
wildcardErr = validateWildcard(rule.ExcludeResources.Kinds, spec, rule)
if wildcardErr != nil {
return warnings, wildcardErr
}
}
// Validate string values in labels
@ -1163,6 +1141,52 @@ func podControllerAutoGenExclusion(policy kyvernov1.PolicyInterface) bool {
return false
}
// validateWildcard check for an Match/Exclude block contains "*"
func validateWildcard(kinds []string, spec *kyvernov1.Spec, rule kyvernov1.Rule) error {
if utils.ContainsString(kinds, "*") && spec.BackgroundProcessingEnabled() {
return fmt.Errorf("wildcard policy not allowed in background mode. Set spec.background=false to disable background mode for this policy rule ")
}
if utils.ContainsString(kinds, "*") && len(kinds) > 1 {
return fmt.Errorf("wildard policy can not deal more than one kind")
}
if utils.ContainsString(kinds, "*") {
if rule.HasGenerate() || rule.HasVerifyImages() || rule.Validation.ForEachValidation != nil {
return 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 fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
if rule.Validation.Deny != nil {
kyvernoConditions, _ := utils.ApiextensionsJsonToKyvernoConditions(rule.Validation.Deny.GetAnyAllConditions())
switch typedConditions := kyvernoConditions.(type) {
case []kyvernov1.Condition: // backwards compatibility
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 fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
}
}
}
if rule.HasMutate() {
if !ruleOnlyDealsWithResourceMetaData(rule) {
return fmt.Errorf("policy can only deal with the metadata field of the resource if" +
" the rule does not match any kind")
}
}
}
return nil
}
// validateKinds verifies if an API resource that matches 'kind' is valid kind
// and found in the cache, returns error if not found
func validateKinds(kinds []string, mock bool, client dclient.Interface, p kyvernov1.PolicyInterface) error {

View file

@ -2222,3 +2222,60 @@ func testResourceList() []*metav1.APIResourceList {
},
}
}
func Test_Any_wildcard_policy(t *testing.T) {
var err error
rawPolicy := []byte(`{
"apiVersion": "kyverno.io/v1",
"kind": "ClusterPolicy",
"metadata": {
"name": "verify-image"
},
"spec": {
"validationFailureAction": "enforce",
"background": false,
"rules": [
{
"name": "verify-image",
"match": {
"any": [
{
"resources": {
"kinds": [
"*"
]
}
}
]
},
"verifyImages": [
{
"imageReferences": [
"ghcr.io/kyverno/test-verify-image:*"
],
"mutateDigest": true,
"attestors": [
{
"entries": [
{
"keys": {
"publicKeys": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM\n5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==\n-----END PUBLIC KEY----- \n"
}
}
]
}
]
}
]
}
]
}
}`)
var policy *kyverno.ClusterPolicy
err = json.Unmarshal(rawPolicy, &policy)
assert.NilError(t, err)
openApiManager, _ := openapi.NewManager()
_, err = Validate(policy, nil, true, openApiManager)
assert.Assert(t, err != nil)
}

View file

@ -0,0 +1,14 @@
# Checks that the manifests.yaml file CANNOT be successfully created. If it can, fail the test as this is incorrect.
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- script: |
if kubectl apply -f policy.yaml
then
echo "Tested failed. policy was allowed."
exit 1
else
echo "Test succeeded. policy was blocked."
exit 0
fi

View file

@ -0,0 +1,2 @@
## Description
Basic validate test to check that a verify-image policy cannot be created when the policy has wildcard(*) included in match any/all resource block.

View file

@ -0,0 +1,26 @@
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: verify-image
spec:
validationFailureAction: enforce
background: false
rules:
- name: verify-image
match:
any:
- resources:
kinds:
- "*"
verifyImages:
- imageReferences:
- "ghcr.io/kyverno/test-verify-image:*"
mutateDigest: true
attestors:
- entries:
- keys:
publicKeys: |
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM
5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==
-----END PUBLIC KEY-----