Although the enforcementAction of the constraint is set to warn, when the Rego syntax is wrong, webhook will reject all requests

[weiwei02]

What steps did you take and what happened: The policy constraints are configured as follows:

apiVersion: constraints.gatekeeper.sh/v1beta1
kind: EKSPodQos
spec:
  enforcementAction: warn
  match:
    kinds:
    - apiGroups:
      - ""
      kinds:
      - Pod
    scope: '*'
  parameters: null

When a syntax error occurs in the Rego associated with the policy template, the admissionWebhook rejects the modification request to the pod. The error message is as follows:

Warning  FailedPatchPod  4m32s (x4466 over 16h)  kubelet, 10.222.94.164  error patching pod annotations: admission webhook "validation.gatekeeper.sh" denied the request: admission.k8s.gatekeeper.sh: __modset_templates["admission.k8s.gatekeeper.sh"]["EKSPodQos"]_idx_1:12: eval_conflict_error: functions must not produce multiple outputs for same inputs

What did you expect to happen:

enforcementAction can be set to deny / warn / dryrun. I think that when enforcementAction is set to warn / dryrun, even if a Rego syntax error occurs, only a warning message should be given in the log and the admissionWebhook request should not be rejected.

Environment:

• Gatekeeper version: v3.5.1
• Kubernetes version: 1.18.9

[maxsmythe]

This is because when Rego encounters a runtime error like this, it halts all execution.

This means that not only is this constraint not getting executed, but there may be other constraints that are not getting evaluated as well, so we fail closed for security's sake.

Switching to using Golang to coordinate evaluation of constraint templates per https://github.com/open-policy-agent/frameworks/issues/135 would allow us to treat each runtime error discreetly.

[willbeason]

This will be fixed with the ongoing compiler sharding work in frameworks. Since each ConstraintTemplate will get its own Rego compiler, one failing to compile means the others will still be able to execute. Similarly - a Rego runtime error for a Constraint set to warn will be able to simply "warn" rather than halting execution.

[marcinkubica]

Hello, I'm new to rego. @willbeason is this problem the same as when using syntax error in the example below, when both template and policy is applied, however not enforced (ie. no violations are observed). Cheers

apiVersion: templates.gatekeeper.sh/v1beta1
kind: ConstraintTemplate
metadata:
  name: k8salwaysdeny
spec:
  crd:
    spec:
      names:
        kind: K8sAlwaysDeny
      validation:
        openAPIV3Schema:
          properties:
            message:
              type: string
  targets:
  - target: admission.k8s.gatekeeper.sh
    rego: |
      package k8salwaysdeny
      violation[{"msg": msg}] {
       1 > 0
      msg := input.parametes.message #should be `input.parameteRs.message` here
      }
---
apiVersion: constraints.gatekeeper.sh/v1beta1
kind: K8sAlwaysDeny
metadata:
  name: pod-always-deny
spec:
  match:
    kinds:
      - apiGroups: [""]
        kinds: ["Pod"]
  parameters:
    message: "ACCESS DENIED!"

[maxsmythe]

@marcinkubica The Rego syntax in your example is correct, but attempts to access a non-existing field. When that happens, that line is considered "undefined", which is "falsey", and Rego considers the rule not to have been matched, which leads to no violation.

TL;DR that behavior is WAI from a Rego perspective (though I, personally, would prefer a more fail-closed approach).

[sozercan]

@willbeason are we okay to close this original issue with compiler sharding changes in v3.8?

[maxsmythe]

It looks like the error issue has been mitigated somewhat by this code:

https://github.com/open-policy-agent/frameworks/blob/9db6a6b27c9741bcf9c5f4d8c6e3ef230b49f41d/constraint/pkg/client/drivers/local/driver.go#L248-L263

Which makes it so a failure only affects all constraints of the same kind (as opposed to all policy generally).

If we wanted to make this constraint-specific for debug-ability and accuracy for fail-open, that'd require more work.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.