ClusterAdmissionPolicy: provide feedback when `Rules` are invalid

[flavio]

ClusterAdmissionPolicy v1alpha2 has a Rules attribute, which contains RuleWithOperations objects.

The controller must keep track of the ClusterAdmissionPolicy resources that cannot be properly created because of invalid rules.

PS: we know the the right approach would be to provide a validating admission webhook for ClusterAdmissionPolicy resources. We want to investigate how reliable it would be to provide such a validation policy via a Kubewarden policy, instead of using a built-in webhook provided by kubebuilder. This is research is going to

[ereslibre]

PS: we know the the right approach would be to provide a validating admission webhook for ClusterAdmissionPolicy resources. We want to investigate how reliable it would be to provide such a validation policy via a Kubewarden policy,

My opinion here is that replicating logic already baked in the apiserver is not the ideal way to go, and that instead, we can update the status subresource of the ClusterAdmissionPolicy resource, so it has some fields that represent different stages of the reconciliation, e.g.: policyServerReconciliated, serviceReconciliated, webhookConfigurationReconciliated for example. A condition in the status field would also allow us to easily either report success, or failure with a reason, where we can basically forward the error from the apiserver.

This would ensure that we don't have to repeat these rules, and that we forward this information to the end user, so they can take action in order to amend the situation.

[flavio]

I like that approach, can you plan (and write here):

• How the ClusterAdmissionPolicy/status will look like. The new attributes, their possible values and meaning
• How the user would interact with that. How would he discover a policy is not yet enforced, how he could see why this policy is not enforced

Thanks :bow:

[ereslibre]

As a first approach I suggest to do have a status object like so:

status:
  conditions:
  - lastTransitionTime: "<timestamp>"
    lastUpdateTime: "<timestamp>"
    message: "<message>"
    status: "True"|"False"
    type: PolicyServerSecretReconciled
  - lastTransitionTime: "<timestamp>"
    lastUpdateTime: "<timestamp>"
    message: "<message>"
    status: "True"|"False"
    type: PolicyServerConfigMapReconciled
  - lastTransitionTime: "<timestamp>"
    lastUpdateTime: "<timestamp>"
    message: "<message>"
    status: "True"|"False"
    type: PolicyServerDeploymentReconciled
  - lastTransitionTime: "<timestamp>"
    lastUpdateTime: "<timestamp>"
    message: "<message>"
    status: "True"|"False"
    type: PolicyServerServiceReconciled
  - lastTransitionTime: "<timestamp>"
    lastUpdateTime: "<timestamp>"
    message: "<message>"
    status: "True"|"False"
    type: WebhookRegistered

Each condition represents one of the major steps in the reconciliation process. It's also true that many of them will be a no-op (e.g. reconciling the policy server service). However, I think it's good to have this observability, at least at the beginning.

The kubewarden-controller will be the component that will update the status.conditions on every ClusterAdmissionPolicy object as it observes/reconciles each policy. It requires to be able to patch ClusterAdmissionPolicy/status subresource.

As a regular way of condition annotation, lastTransitionTime is only updated whenever the status changes. lastUpdateTime is updated always, even if the status does not flip.

The status field will only contain a raw True or False depending on whether the reconciliation of that bit in that resource did succeed or not. If it failed, we will write on the message field the err provided by the underlying code. This ensures that the user will have visibility on what is wrong on that policy, and why it is not active.

[ereslibre]

Besides the conditions, we can have a top level attribute on the status subresource:

status:
  policyActive: true|false

This field is updated depending whether all the reconciliation phase did succeed completely.

In the future we can also include interesting information here as other attributes, such as the SHA-256 sum of the policy (as reported by the policy-server itself), or some kind of GPG signature of the policy itself, as reported by the policy-server too.

[flavio]

As a first approach I suggest to do have a status object like so:

I'm fine with that. I would leave the policy-server object as the last one. IMHO the most important one (hence the first to implement) would be the WebhookRegistered entry.

[ereslibre]

I think that if an external actor needs to check the status of the policy, ideally they should use status.policyActive. For debugging purposes, conditions are useful, but it's a bit cumbersome to check conditions given they are an array.