From e518a965e911b407f6c4d449006ab10e535f3531 Mon Sep 17 00:00:00 2001 From: Mariam Fahmy Date: Tue, 23 Jul 2024 16:21:55 +0300 Subject: [PATCH] feat: add KDP for the migration of rule-specific fields Signed-off-by: Mariam Fahmy --- proposals/rule_specific_fields_migration.md | 272 ++++++++++++++++++++ 1 file changed, 272 insertions(+) create mode 100644 proposals/rule_specific_fields_migration.md diff --git a/proposals/rule_specific_fields_migration.md b/proposals/rule_specific_fields_migration.md new file mode 100644 index 0000000..1a7a45c --- /dev/null +++ b/proposals/rule_specific_fields_migration.md @@ -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.