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

adding --audit-warn flag #5321

Merged
merged 5 commits into from
Nov 21, 2022
Merged

adding --audit-warn flag #5321

merged 5 commits into from
Nov 21, 2022

Conversation

stelah1423
Copy link
Contributor

@stelah1423 stelah1423 commented Nov 11, 2022

Explanation

This PR implements a feature requested in #4375.

Related issue

Closes #4375

Milestone of this PR

What type of PR is this

feature

Proposed Changes

This PR will add a new flag of --audit-warn to the kyverno apply CLI subcommand. This is intended to treat any policies that are set to audit to NOT exit with a non-zero code, and be counted as a warning instead. In other words, if the policy is set to audit, and --warn-audit is set, it would exit 0 if audit policies matched. If enforcing policies are matched, it would exit 1.

This is very similar to the policies.kyverno.io/scored annotation that can treat an audited policy as a warning instead of a failure, but with some extra benefits from the CLI tool. This flag does take into account the validationFailureActionOverrides spec for a resource, whereas the annotation does not (as far as I know).

In this implementation, I also propose having kyverno apply still report the audit policy to stdout when --policy-report is not set, similar to a failure, but with a slightly different message stating this was an audit warning. I am proposing this since this tool is commonly used in ci/cd pipelines, and still having the verbosity around the audit findings would prove useful. In contrary, when using the scored annotation with kyverno apply, it does NOT report this finding to stdout, but would still exit with 0. I find that not necessarily useful. Perhaps this should be an additional flag to report these warnings, but I would have to defer that decision to you fine folks.

To recap:

When --audit-warn is set:

  • Policies set to audit will report as warnings instead
  • These warnings will exit 0; unless there is an enforcing failure
  • When --policy-report is not set (default return value), we also print a message to stdout that the audit warning was logged as well

Example output

This output utilizes validationFailureActionOverrides for the namespace default1 to audit instead of enforce. You can see the output is slightly different for audit failures. I'm wide open to changing the verbiage around these messages.

kyverno apply disallow-latest-tag.yaml --resource=echo-test.yaml --audit-warn

Applying 2 policy rules to 5 resources...

policy disallow-latest-tag -> resource default1/Deployment/echo-deployment2 failed as audit warning:
1. autogen-require-image-tag: validation error: An image tag is required. rule autogen-require-image-tag failed at path /spec/template/spec/containers/0/image/

policy disallow-latest-tag -> resource default/Deployment/echo-deployment failed:
1. autogen-require-image-tag: validation error: An image tag is required. rule autogen-require-image-tag failed at path /spec/template/spec/containers/0/image/

pass: 2, fail: 1, warn: 1, error: 0, skip: 26

Proof Manifests

Create echo-test.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: echo-deployment2
  namespace: default1
spec:
  replicas: 1
  selector:
    matchLabels:
      app: echo-server2
  template:
    metadata:
      labels:
        app: echo-server2
    spec:
      containers:
        - name: echo-server
          image: hashicorp/http-echo
          args:
          - -text="hello there!"
          ports:
            - name: http-port
              containerPort: 5678
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: echo-deployment
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      app: echo-server
  template:
    metadata:
      labels:
        app: echo-server
    spec:
      containers:
        - name: echo-server
          image: hashicorp/http-echo
          args:
          - -text="hello there!"
          ports:
            - name: http-port
              containerPort: 5678

Create disallow-latest-tag.yaml

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: disallow-latest-tag
  annotations:
    policies.kyverno.io/title: Disallow Latest Tag
    policies.kyverno.io/category: Best Practices
    policies.kyverno.io/severity: medium
    policies.kyverno.io/subject: Pod
    policies.kyverno.io/description: >-
      The ':latest' tag is mutable and can lead to unexpected errors if the
      image changes. A best practice is to use an immutable tag that maps to
      a specific version of an application Pod. This policy validates that the image
      specifies a tag and that it is not called `latest`.      
spec:
  validationFailureAction: audit
  validationFailureActionOverrides:
    - action: enforce     # Action to apply
      namespaces:       # List of affected namespaces
        - default
  background: true
  rules:
  - name: require-image-tag
    match:
      resources:
        kinds:
        - Pod
    validate:
      message: "An image tag is required."
      pattern:
        spec:
          containers:
          - image: "*:*"
  - name: validate-image-tag
    match:
      resources:
        kinds:
        - Pod
    validate:
      message: "Using a mutable image tag e.g. 'latest' is not allowed."
      pattern:
        spec:
          containers:
          - image: "!*:latest"

Run kyverno apply

kyverno apply disallow-latest-tag.yaml --resource=echo-test.yaml --audit-warn

I have tested this with/without a combination of validationFailureActionOverrides rules in the policy, as well as with/without --policy-report set. I may be overlooking some other functionality to test; please let me know.

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

Further Comments

@welcome
Copy link

welcome bot commented Nov 11, 2022

Thanks for opening your first Pull Request here! Please check out our Contributing guidelines and confirm that you Signed off.

@chipzoller
Copy link
Member

Steven, thank you for this contribution and extra THANK YOU for the docs! We appreciate your efforts here.

@codecov
Copy link

codecov bot commented Nov 14, 2022

Codecov Report

Merging #5321 (dd85840) into main (815a0e4) will decrease coverage by 0.02%.
The diff coverage is 26.08%.

@@            Coverage Diff             @@
##             main    #5321      +/-   ##
==========================================
- Coverage   36.29%   36.27%   -0.03%     
==========================================
  Files         171      171              
  Lines       19076    19091      +15     
==========================================
+ Hits         6924     6925       +1     
- Misses      11360    11370      +10     
- Partials      792      796       +4     
Impacted Files Coverage Δ
cmd/cli/kubectl-kyverno/utils/common/common.go 14.81% <23.80%> (-0.25%) ⬇️
cmd/cli/kubectl-kyverno/apply/apply_command.go 22.41% <50.00%> (+0.19%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Steven Lahouchuc <stelah1423@gmail.com>
@vyankyGH vyankyGH enabled auto-merge (squash) November 21, 2022 13:51
@vyankyGH vyankyGH merged commit 1960ced into kyverno:main Nov 21, 2022
@stelah1423 stelah1423 mentioned this pull request Dec 5, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Provide kyverno apply --audit-warn option
4 participants