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: add KDP for the migration of rule-specific fields #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
272 changes: 272 additions & 0 deletions proposals/rule_specific_fields_migration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,272 @@
# Rule-specific Fields Migration

- **Authors**: [Mariam Fahmy](https://github.com/MariamFahmy98)
- **Created**: July 22th, 2024
- **Abstract**: Migration of fields that are specific to a rule type under their corresponding rules.

## Overview

This document proposes the migration of the following fields under their corresponding rules:
1. `validationFailureAction`
2. `validationFailureOverrides`
3. `mutateExistingOnPolicyUpdate`
4. `generateExisting`

## Motivation

We're getting feedback from the community that some fields defined under the `spec` have to be moved under their corresponding rules. By updating the schema, we want to make it easier for users to describe their policies in a way that makes more sense, stays consistent, and can be extended when needed.

## Proposal

### Validation Failure Action

- Both `validationFailureAction` and `validationFailureOverrides` fields will be moved under the `validate` rule. If they aren't defined under the `validate` rule, they will use the default value under the `spec` field.

- The `VALIDATE ACTION` column will be removed since the field is moved under the `validate` rule.

- The policy cache should be updated to filter policies based on the value of the fields under the `validate` rule. It will return policies with either `enforce` or `audit` rules.

For example:

```yaml
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: check-ns-labels
spec:
rules:
- name: require-ns-purpose-label
match:
any:
- resources:
kinds:
- Namespace
validate:
validationFailureAction: Enforce
message: "You must have label `purpose` with value `production` set on all new namespaces."
pattern:
metadata:
labels:
purpose: production
- name: require-ns-env-label
match:
any:
- resources:
kinds:
- Namespace
validate:
validationFailureAction: Audit
message: "You must have label `environment` with value `production` set on all new namespaces."
pattern:
metadata:
labels:
environment: production
```

During applying policies in the `enforce` mode, the policy cache will return the policy with the rules that have the `enforce` mode only. The same applies to the `audit` mode. This is because we don't want to block the API request if the policy has an `audit` rule.

- The `validationFailureAction` field will be introduced under the `verifyImages` rules due to the fact that image verification mutation failure is later blocked by the validating webhook.

For example:

```yaml
verifyImages:
# validationFailureAction: Audit
- attestors:
- count: 1
entries:
- keys:
publicKeys: |-
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFN8gGjQua2g8N+aLx3Eff+/j5HxL
bV+H2z50/0A4d8XyMUvizPQBtcgei43pqLj1850m3wSwI08z2+6zT1QaEg==
-----END PUBLIC KEY-----
signatureAlgorithm: sha256
imageReferences:
- '*'
mutateDigest: true
required: true
useCache: true
verifyDigest: true
validationFailureAction: Audit
```

#### Autogen Rules

The autogenerated rules for both `validate` and `verifyImage` will have the `validationFailureAction` and the `validationFailureActionOverrides` field set if they are defined under the rule level.

For example:

```yaml
autogen:
rules:
- match:
any:
- resources:
kinds:
- DaemonSet
- Deployment
- Job
- ReplicaSet
- ReplicationController
- StatefulSet
name: autogen-require-image-tag
validate:
validationFailureAction: Audit
message: An image tag is required.
pattern:
spec:
template:
spec:
containers:
- image: '*:*'
- match:
any:
- resources:
kinds:
- CronJob
name: autogen-cronjob-require-image-tag
validate:
validationFailureAction: Audit
message: An image tag is required.
pattern:
spec:
jobTemplate:
spec:
template:
spec:
containers:
- image: '*:*'
- match:
any:
- resources:
kinds:
- DaemonSet
- Deployment
- Job
- ReplicaSet
- ReplicationController
- StatefulSet
name: autogen-validate-image-tag
validate:
validationFailureAction: Audit
message: Using a mutable image tag e.g. 'latest' is not allowed.
pattern:
spec:
template:
spec:
containers:
- image: '!*:latest'
- match:
any:
- resources:
kinds:
- CronJob
name: autogen-cronjob-validate-image-tag
validate:
validationFailureAction: Audit
message: Using a mutable image tag e.g. 'latest' is not allowed.
pattern:
spec:
jobTemplate:
spec:
template:
spec:
containers:
- image: '!*:latest'
```

#### CLI

There are no major changes in the CLI except for the `warn` status. Currently, the rule can have the `warn` status in case the flag `--auditWarn` is used and the `validationFailureAction` is set to `Audit`.

By introducing the field under the validation rule, the `warn` status will be set in case of the following:

1. The `--auditWarn` flag is used.
2. The rule is of type `Validation`.
3. The `validationFailureAction` under the rule is set to `Audit`.

#### Events

Since `HandleValidationEnforce()` and `HandleValidationAudit()` are executed concurrently and each generate its own event, there are some cases where two events are generated for the same resource and the same policy. This mostly happens when the policy has multiple rules with different validation action modes.

For example:

```yaml
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: check-ns-labels
spec:
rules:
- name: require-ns-purpose-label
match:
any:
- resources:
kinds:
- Namespace
validate:
validationFailureAction: Enforce
message: "You must have label `purpose` with value `production` set on all new namespaces."
pattern:
metadata:
labels:
purpose: production
- name: require-ns-env-label
match:
any:
- resources:
kinds:
- Namespace
validate:
validationFailureAction: Audit
message: "You must have label `environment` with value `production` set on all new namespaces."
pattern:
metadata:
labels:
environment: production
```

When applying the following namespace that have the `environment: production` label, two events will be generated for the same resource. The first event will be generated by the `require-ns-purpose-label` rule with the `enforce` mode, and the second event will be generated by the `require-ns-env-label` rule with the `audit` mode.

```yaml
apiVersion: v1
kind: Namespace
metadata:
name: bad-ns-1
labels:
environment: production
```

The generated events:
```yaml
LAST SEEN TYPE REASON OBJECT MESSAGE
8s Normal PolicyApplied clusterpolicy/check-ns-labels Namespace bad-ns-1: pass
8s Warning PolicyViolation clusterpolicy/check-ns-labels Namespace bad-ns-1: [require-ns-purpose-label] fail (blocked); validation error: You must have label `purpose` with value `production` set on all new namespaces. rule require-ns-purpose-label failed at path /metadata/labels/purpose/
```

One possible solution is to aggregate the engine responses returned from the `HandleValidationEnforce()` and `HandleValidationAudit()` functions and generate a single event for the same resource and policy.

#### Reports

Although each of `HandleValidationEnforce()` and `HandleValidationAudit()` functions generates its own report, the reports are aggregated and stored in the final CRD by the reports controller. Therefore, there is no effect on the reporting system.

### Generate Existing

### Mutate Existing

### Deprecation Plan

- The following fields:
1. `spec.validationFailureAction`
2. `spec.validationFailureOverrides`
3. `spec.mutateExistingOnPolicyUpdate`
4. `spec.generateExisting`

will be deprecated in Kyverno 1.13. It is expected that they will be completely removed when introduing a new API version `v2` for the CRDs in Kyverno 1.14.

- The Kyverno CLI `fix` command will be used to update the existing policies and move those fields under their corresponding rules.

- In case the deprecated fields are used in the policy, a warning message will be printed indicating that the field is deprecated and needs to be moved under its corresponding rule.

- Upgrading to Kyverno 1.13 will not affect the existing policies. The policies will continue to work as expected.