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

feat: added validation check for ensuring the existence of only 'any'/'all' #1791

Conversation

yashvardhan-kukreja
Copy link
Contributor

Related issue

closes #1765

What type of PR is this

/kind feature

Proposed changes

After this PR
If someone declares preconditions or validate.deny.conditions containing any/all, they won't be able to specify any other sub-field apart from any/all.
For example, the following policy:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: cluster-cm-array-example
spec:
  validationFailureAction: enforce 
  background: false 
  rules:
  - name: sample-rule
    context:
      - name: ops-cm
        configMap:
          name: ops-cm
          namespace: default
    match:
      resources:
        kinds:
        - Deployment
    validate:
      message: "The role {{ request.object.metadata.annotations.role }} is not in the allowed list of roles: {{ \"roles-dictionary\".data.\"allowed-roles\" }}."
      deny:
        conditions:
          any:
          - key: "{{ request.operation }}"
            operator: Equals
            value:  "{{ \"ops-cm\".data.\"deny-ops\"}}"
          foo:
          - key: "{{ request.operation }}"
            operator: Equals
            value:  "{{ \"ops-cm\".data.\"deny-ops\"}}"

When the above policy will be applied, the following error will be generated:

Error from server: error when creating "cpol.yaml": admission webhook "validate-policy.kyverno.svc" denied the request: path: spec.rules[0].validate.deny.conditions: error occurred while parsing preconditions/validate.deny.conditions: unknown field 'foo' found under preconditions/validate.deny.conditions

Before this PR
If someone specified an ambiguous sub-field like foo under preconditions or validate.deny.conditions, kyverno wouldn't raise an error like above.
For example, the following policy:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: cluster-cm-array-example
spec:
  validationFailureAction: enforce 
  background: false 
  rules:
  - name: sample-rule
    context:
      - name: ops-cm
        configMap:
          name: ops-cm
          namespace: default
    match:
      resources:
        kinds:
        - Deployment
    validate:
      message: "The role {{ request.object.metadata.annotations.role }} is not in the allowed list of roles: {{ \"roles-dictionary\".data.\"allowed-roles\" }}."
      deny:
        conditions:
          any:
          - key: "{{ request.operation }}"
            operator: Equals
            value:  "{{ \"ops-cm\".data.\"deny-ops\"}}"
          foo:
          - key: "{{ request.operation }}"
            operator: Equals
            value:  "{{ \"ops-cm\".data.\"deny-ops\"}}"

On applying the above policy, kyverno wouldn't raise any error. And behind the scenes, whenever the above policy's rule will evaluate an admission request, it would simplpy ignore the conditions specified under ambiguous sub-fields like foo but that's not ideal because user might be under the wrong assumption that that condition specified under foo would still be evaluating the incoming admission requests and working fine, (this might occur if the user, while writing a policy, unknowingly wrote a type like anyz or allz)

Checklist

Further comments

…l' fields are present under conditions

Signed-off-by: Yashvardhan Kukreja <yash.kukreja.98@gmail.com>
@yashvardhan-kukreja yashvardhan-kukreja force-pushed the issue-1765/validation-for-any-all-conditions branch from ab98462 to 1f6b235 Compare April 13, 2021 00:53
@yashvardhan-kukreja
Copy link
Contributor Author

yashvardhan-kukreja commented Apr 13, 2021

@chipzoller hope this looks fine :)

@chipzoller
Copy link
Member

Looks good. I'm not sure if we want to enforce there be a maximum of one all statement within those fields. Multiple any statements would be valid, but only a single all. Probably not a big deal either way, but it would seem to be a bit tidier.

@realshuting
Copy link
Member

Thanks @yashvardhan-kukreja, look good!

@realshuting realshuting merged commit 69c3418 into kyverno:main Apr 17, 2021
@realshuting realshuting self-assigned this Apr 17, 2021
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.

[BUG] Cleanup of any/all in conditions and preconditions
3 participants