Navigation Menu

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

use failurePolicy to block or allow requests, on policy errors #4183

Merged
merged 28 commits into from Aug 2, 2022
Merged

use failurePolicy to block or allow requests, on policy errors #4183

merged 28 commits into from Aug 2, 2022

Conversation

JimBugwadia
Copy link
Member

@JimBugwadia JimBugwadia commented Jul 2, 2022

Signed-off-by: Jim Bugwadia jim@nirmata.com

Explanation

Before this PR, if failurePolicy was set to Ignore it only meant that Kyverno could be skipped if a call back couldn't be made. After this PR, Ignore will additionally allow failing calls to image registries to be skipped over and the rule to be evaluated. This allows for rules like verifyImages or others which use image data to not block if the registry is temporarily down, useful in situations where images already exist on the nodes.

Related issue

Fixes #4168

Milestone of this PR

1.8.0

What type of PR is this

/kind cleanup

Proposed Changes

Use failurePolicy to determine if a rule error should result in admission requests being blocked.

If the failurePolicy is Ignore and a policy rule results in an error, then the admission request is allowed. If the failurePolicy is Fail and a policy rule results in an error, the admission request is blocked.

The validationFailureAction behavior stays the same. It controls the behavior for a policy violation (the status for the policy rule is fail, which may be a bit confusing).

Proof Manifests

added unit tests

From the CLI, test using this policy with failureMode: ignore:

---
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: check-image
  annotations:
    pod-policies.kyverno.io/autogen-controllers: none
spec:
  validationFailureAction: enforce
  background: false
  webhookTimeoutSeconds: 30
  failurePolicy: Ignore  
  rules:
    - name: verify-static-key
      match:
        resources:
          kinds:
            - Pod
      verifyImages:
      - image: "ghcr.io/kyverno/test-verify-image*"
        mutateDigest: false
        attestors:
        - count: 1
          entries:
          - keys: 
              publicKeys: |-
                -----BEGIN PUBLIC KEY-----
                MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM
                5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==
                -----END PUBLIC KEY-----          

Run this command, after disconnecting from the network:

❯ kubectl run nginx --image nginx --dry-run=server
Warning: policy check-image.verify-static-key: failed to verify signature for docker.io/nginx:latest: .attestors[0].entries[0].keys: Get "https://index.docker.io/v2/": dial tcp: lookup index.docker.io on 10.96.0.10:53: server misbehaving
pod/nginx created (server dry run)

Run this command, after connecting back to the network:

❯ kubectl run nginx --image nginx --dry-run=server
Error from server: admission webhook "mutate.kyverno.svc-ignore" denied the request:

policy Pod/default/nginx for resource violation:

check-image:
  verify-static-key: |
    failed to verify signature for docker.io/nginx:latest: .attestors[0].entries[0].keys: no matching signatures:

When a network error is received, the failureMode will be checked to return a policy error and warnings messages, instead of blocking the admission request.

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@JimBugwadia JimBugwadia marked this pull request as draft July 2, 2022 01:16
@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2022

Codecov Report

Merging #4183 (d14ae55) into main (6fa8a97) will increase coverage by 1.80%.
The diff coverage is 57.66%.

@@            Coverage Diff             @@
##             main    #4183      +/-   ##
==========================================
+ Coverage   28.86%   30.66%   +1.80%     
==========================================
  Files         143      146       +3     
  Lines       19372    19486     +114     
==========================================
+ Hits         5591     5976     +385     
+ Misses      13100    12778     -322     
- Partials      681      732      +51     
Impacted Files Coverage Δ
api/kyverno/v1/spec_types.go 20.22% <ø> (ø)
cmd/cli/kubectl-kyverno/utils/common/common.go 12.86% <0.00%> (ø)
pkg/config/fake.go 0.00% <0.00%> (ø)
pkg/config/metricsconfig.go 0.00% <0.00%> (ø)
pkg/engine/response/response.go 0.00% <0.00%> (ø)
pkg/engine/utils/utils.go 36.20% <ø> (+6.62%) ⬆️
pkg/openapi/fake.go 0.00% <0.00%> (ø)
pkg/openapi/validation.go 66.85% <ø> (ø)
pkg/policy/updaterequest.go 0.00% <0.00%> (ø)
pkg/policyreport/fake.go 0.00% <0.00%> (ø)
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@chipzoller
Copy link
Member

This is a behavioral change and will need to be documented.

@JimBugwadia
Copy link
Member Author

This is a behavioral change and will need to be documented.

kyverno/website#569

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@JimBugwadia JimBugwadia marked this pull request as ready for review July 5, 2022 07:32
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@realshuting
Copy link
Member

@prateekpandey14 - can you help review?

pkg/metrics/fake.go Outdated Show resolved Hide resolved
@realshuting
Copy link
Member

All e2e tests failed, looks like it cannot build the test image:

# github.com/kyverno/kyverno/pkg/metrics
Error: pkg/metrics/fake.go:13:[55](https://github.com/kyverno/kyverno/runs/7534505652?check_suite_focus=true#step:8:56): undefined: PromConfig
Error: pkg/metrics/fake.go:15:9: undefined: NewPromConfig
make: *** [Makefile:122: docker-build-kyverno-local] Error 2
Error: Process completed with exit code 2.

@JimBugwadia - can you please resolve conflicts and re-run tests?

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@prateekpandey14
Copy link
Contributor

@JimBugwadia some more conflicts to resolve after latest merge to main.

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
pkg/cosign/cosign.go Outdated Show resolved Hide resolved
pkg/cosign/cosign.go Outdated Show resolved Hide resolved
pkg/cosign/cosign.go Outdated Show resolved Hide resolved
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
cmd/initContainer/main.go Outdated Show resolved Hide resolved
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@JimBugwadia JimBugwadia enabled auto-merge (squash) August 2, 2022 03:54
cmd/kyverno/main.go Outdated Show resolved Hide resolved
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Copy link
Contributor

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

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

lgtm

@JimBugwadia JimBugwadia merged commit 943c3a1 into kyverno:main Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Allow policy failurePolicy to skip unexpected errors
5 participants