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

[Feature] CLI test command should validate the policy under test #4365

Closed
2 tasks done
zswanson opened this issue Aug 21, 2022 · 11 comments · Fixed by #8387
Closed
2 tasks done

[Feature] CLI test command should validate the policy under test #4365

zswanson opened this issue Aug 21, 2022 · 11 comments · Fixed by #8387
Assignees
Labels
cli:test The CLI test command related issue. end user This label is used to track the issue that is raised by the end user. enhancement New feature or request type:cli cli releated issue

Comments

@zswanson
Copy link
Contributor

Problem Statement

It is currently possible to write a ClusterPolicy (or Policy) that is malformed such that the Kyverno controller would reject the ClusterPolicy if applied to a cluster. Depending on the the extent of the bugs in the policy, the kyverno test CLI command may continue to pass/fail tests appropriately.

For example:

  rules:
    - name: validate-gateway-port-protocol
      match:
        all:
          - resources:
              kinds:
                - Gateway
            namespaceSelector:
              matchLabels:
                istio/rev: "default"

The rule above had incorrect indentation where the namespaceSelector is at the same level as resources (it should be a child element of resources). The cli kyverno test command ran all tests without error; it was only on trying to apply the policy to a cluster that schema validation occurred and let us know that the policy wasn't constructed correctly.

In other more subtle problems this may cause kyverno test score the tests as 'error' or 'notfound' with little or no explanation of the issue even at max log verbosity. ie, using operator: Equal rather than operator: Equals in a precondition. A dryrun apply against the server immediately gives good feedback from the controller however.

Here is a second example of bad indentation that allowed tests to pass, but failed when trying to apply the policy. The preconditions stanza is incorrectly indented under the match.

  rules:

  - name: require-image-tag
    match:
      resources:
        kinds:
        - Pod

      preconditions:
        all:
          - key: "{{ request.operation }}"
            operator: AnyIn
            value:
              - CREATE
              - UPDATE

Solution Description

The kyverno CLI should either offer a separate validate command that can test a policy against the schema and report errors, and/or the kyverno CLI should incorporate that as part of the existing test command before the tests execute. If the policy violates the schema, the tests should not execute.

Alternatives

To workaround this problem a policy author must (dryrun) apply a policy repeatedly to a cluster running the kyverno controller.

Additional Context

No response

Slack discussion

No response

Research

  • I have read and followed the documentation AND the troubleshooting guide.
  • I have searched other issues in this repository and mine is not recorded.
@zswanson zswanson added enhancement New feature or request triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels Aug 21, 2022
@codeWithUtkarsh
Copy link

If no one is working on this I would like to work on this issue. @zswanson

@realshuting realshuting added type:cli cli releated issue and removed triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels Aug 22, 2022
@realshuting
Copy link
Member

Thanks for the interest @codeWithUtkarsh ! Kyverno policy does have the schema defined and can be used for policy validation. If you could share the investigated solutions, I'd gladly discuss them and help.

@codeWithUtkarsh
Copy link

Problem Statement

The cli kyverno test is not validating the policy manifest and hence fails at the time of appling the policy to a cluster.

For example:

rules:
    - name: validate-gateway-port-protocol
      match:
        all:
          - resources:
              kinds:
                - Gateway
            namespaceSelector:
              matchLabels:
                istio/rev: "default"

The example above has syntax error where resources and namespaceSelector are at the same level.

 

Solution

Validate the policy manifest with the defined schema and detect for any error. If found any error, print the error with proper message.

 

Definition

apiVersion : kyverno.io/v1alpha1
kind : Policy
metadata :
  name : example-policy
spec :
  rules:
  # General description
  - name: "<rule name>"
      match: # resources that are to be included
        resources:
          kinds: # Required, list of kinds
          - <some_kind>
          - <other_kind>
          name: <resource_name> # Optional, supports wildcards
          namespaces: # Optional, list of namespaces
          - <namespace>  
          - <namespace>  
          selector: <resources_selector> # Optional
      exclude: # resources that need to be excludes
        resources:
          kinds: # Required, list of kinds
          - <some_kind>
          - <other_kind>
          name: <resource_name> # Optional, supports wildcards
          namespaces: # Optional, list of namespaces
          - <namespace>  
          - <namespace>  
          selector: <resources_selector> # Optional
      # The simplest condition which checks label "app"
      validate:
        # Message is optional. The controller should show well-readable message by default.
        message: "Message why the current resource can't pass the validation"
        pattern:
          metadata:
            labels:
              # validate supports wildcard characters ? and * in string values.
              # ? - any single character
              # * - any characters (with length from 0 to infinity)
              # The next expression means that the "app" label should be explicitly set to the resource before creation
              app: ?
           
    # Check some Deployment properties + check whether the containers with "latest" version have imagePullPolicy: "Always".
    - name: "image pull policy check"
      match: 
        resources:
          kinds:
          - Deployment  # Matched for all deployments
      # The main fields of objects inside lists, which will be checked for neighboring and child fields.
      # If these fields don't set, every object field considered main: if the current example doesn't contain    
      validate:
        pattern:
          spec:
            replicas: !0
            revisionHistoryLimit: >5
            template:
              spec:
                containers:
                # Any field value in parentheses is an anchor and is used as the "if" block for all expressions in child tree
                # The next expression means, that the policy expects images with "latest" version in the list of containers
                - (image): "*:latest"
                  # This expression means that the image with "latest" version should have "Always" value in imagePullPolicy field
                  imagePullPolicy: "Always"
                  ports:
                  # This expression means that the container from image with "latest" version should have ports 443 or 6443
                  - containerPort: 443|6443
status:
  log:
  - timestamp: "2018-02-15T04:05:45Z"
    event: 'FAILED "Deployment of *nirmata* images" validate'
    resource: 'default/deployments/test1'
    message: '"spec/template/spec/containers/image" should have the "*:latest" format. Actual: "image:v1"'
  ...
 
### Specific validations ###
     
    # 1. Allow container from a list of registries
spec:
  rules:
    - resource:
        kind: Deployment
      validate:
        pattern:
          template:
            spec:
              containers:
              # Checks if the image path of "privateApp" container starts with "https://private.registry.io" OR "https://another-private.registry.io"
              # If some property contains operator | as a normal part of its value, it should be escaped by backslash: "\|".
              - name: privateApp
                path: "https://private.registry.io*|https://another-private.registry.io*"
               
    # 2. Check whether probe intervals are greater than 10s
    - name: "check probe intervals"
      match:
        resources:
          kinds:
          - Pod
      validate:
        pattern:
          containers:
          # In this case every object in containers list will be checked for pattern
          - (name): *
            livenessProbe:
              periodSeconds: >10
     
    # 3. Disallow hostPath in volumes
    - name: "disallow hostPath in volumes"
      match:
        resources:
          kinds:
          - Pod
      validate:
        pattern:
          volumes:
          - (name): *
            hostPath: null
           
    # 4. Disallow nodePort is services
    - name: "disallow nodePort in services"
      match:
        resources:
          kinds:
          - Service
      validate:
        pattern:
          spec:
            ports:
            - port: *
              nodePort: null
             
    # 5. Disallow memory resource requests > 8Gi
    - name: "diallow memory resource requests"
      match:
        resources:
          kinds:
          - Pod
      validate:
        pattern:
          spec:
            containers:
            - (name): *
              resources:
                requests:
                  # If the value contains a logical operator, the integer after it will be checked. Not numeric characters will be a part of pattern.
                  # The OR operator can combine the patterns with logical expressions and text patterns.
                  memory: "<9Gi|<8193Mi"

Source: https://github.com/kyverno/kyverno/wiki/Validation

 

Questions:

  1. Should we expose a new CLI to validate or improve test cli to validate?
  2. The validations will be restricted to the syntactical validation.
  3. Is there a need to consider letter case.
  4. Where to read the defined schema? Is there any API that can be used?
  5. Can there be any new keys present in policy schema other than defined in schema?
  6. Any standard format for error to be followed?

@realshuting Let me know when can we discuss my investigation and few doubts.

@realshuting
Copy link
Member

@codeWithUtkarsh - thanks for looking into the issue.

It seems there's some misunderstanding of Kyverno validate policies and validate Kyverno policies. The former is used to validate resources via Kyverno policies while the latter is to perform validation checks for Kyverno policies.

Take this policy as an example:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: validate-gateway-port-protocol
spec:
  validationFailureAction: enforce
  background: false
  rules:
    - name: validate-gateway-port-protocol
      match:
        all:
          - resources:
              kinds:
                - Gateway
            namespaceSelector:
              matchLabels:
                istio/rev: "default"

If you create that to the cluster, kubectl returns the below error saying that the policy is invalid due to the schema mismatch.

kubectl apply -f test.yaml
error: error validating "test": error validating data: ValidationError(ClusterPolicy.spec.rules[0].match.all[0]): unknown field "namespaceSelector" in io.kyverno.v1.ClusterPolicy.spec.rules.match.all; if you choose to ignore these errors, turn validation off with --validate=false

We want the same behavior with Kyverno CLI, possibly a new command validate to perform these checks. This can be done via OpenAPI schema validation which is already implemented in kubectl. Would be good to investigate how this can be done grammatically and add to Kyverno CLI.

@codeWithUtkarsh
Copy link

codeWithUtkarsh commented Aug 24, 2022

I meant the same @realshuting Maybe the added source have created confusion. I understand that the intend of this issue is to do a grammatical/syntactical check using kyverno cli. I will look at OpenAPI schema validation implemented in kubectl.

@chipzoller
Copy link
Contributor

What we need to do here is embed the same validation checks that Kyverno does as part of its internal policy validation into the CLI. It should run these checks implicitly for every policy fed to either the test or apply commands. The CLI had a validate option but that was deprecated and removed. I see no value is resurrecting that option specifically for this when the actual desire is to test a policy against resources (either test or apply).

@chipzoller
Copy link
Contributor

cc @vyankyGH and @Prateeknandle

@chipzoller
Copy link
Contributor

@codeWithUtkarsh are you still working on this issue? If not, please unassign yourself so other contributors know it is available.

@realshuting realshuting added cli:test The CLI test command related issue. end user This label is used to track the issue that is raised by the end user. labels Jul 31, 2023
@cbandy
Copy link
Contributor

cbandy commented Aug 30, 2023

Related: #2302

@eddycharly
Copy link
Member

We simply need to use strict marshalling.

@eddycharly
Copy link
Member

Thanks for reporting, this will be fixed in 1.11.

@eddycharly eddycharly self-assigned this Sep 13, 2023
@eddycharly eddycharly added this to the Kyverno Release 1.11.0 milestone Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli:test The CLI test command related issue. end user This label is used to track the issue that is raised by the end user. enhancement New feature or request type:cli cli releated issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants