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

Pod policies should automatically be applied to pod controllers #518

Closed
JimBugwadia opened this issue Nov 19, 2019 · 17 comments · Fixed by #573
Closed

Pod policies should automatically be applied to pod controllers #518

JimBugwadia opened this issue Nov 19, 2019 · 17 comments · Fixed by #573
Assignees
Labels
enhancement New feature or request

Comments

@JimBugwadia
Copy link
Member

Is your feature request related to a problem? Please describe.

Pod policies should automatically be applied to pod controllers.

Several policies are written to apply on pods. When the policy is set to "audit" Kyverno automatically creates the violation on the pod controller.

However, when the policy is set to "enforce" pod controllers are not blocked during creation or edits.

Describe the solution you'd like

Pod controllers (deployments, etc.) should be blocked when policy is set to enforce.

@JimBugwadia JimBugwadia added the enhancement New feature or request label Nov 19, 2019
@realshuting
Copy link
Member

realshuting commented Nov 19, 2019

A few issues to be addressed:

  1. When the policy is set to "audit" Kyverno automatically creates the violation on the pod controller. I could reproduce this when I have a set of policies (#=N), and only one of them is set to enforce mode. Then I created a new deployment, kyverno blocks pod creation while N violations are reported on deployment. In this case, the incoming pod request matches N policies and one is to block, so we report N violations on resourceowner no matter what the policy mode is, enforce / audit. We could report only one violation on that enforce policy, but discussed with @shivdudhani , this would leave the other violations not reported (as this incoming request also violates the other policies but no violation is there)
  2. when the policy is set to "enforce" pod controllers are not blocked during edits, previously we had an issue that the enforce policy would block the deletion of the resource, because during delete, k8s updates the resource several times (to set deletion timestamp and such), and we added the logic to not process policies on updates request if the engine responds the same validate error. So in my case I added a label to a pod and this was not blocked.
  3. Pod controllers (deployments, etc.) should be blocked when policy is set to enforce, currently we cannot handle this, as if the policy applies on the pod, when we get the request to create the pod, the pod controller (deployments) is already created. Unless we handle this specifically, say if the request is on deployment (or other pod controllers), we extract the podtemplate and validate.

@JimBugwadia
Copy link
Member Author

@realshuting - my thoughts:

  1. This behavior seems correct. If any policy / rule blocks the resource, report all known violations so the user can fix. Is there anything you think we should improve?
  2. We should add a check if the existing violation is on a "ownedResource". Can you please log a separate issue for this?
  3. Yes, this would need to be specific behavior. We can either generate policies on pod controllers (from a pod policy) or handle dynamically.

@realshuting
Copy link
Member

  1. Nope, previously I thought we only want to report violation on enforce policy, it's clear to me now.
  2. I don't get your point ... @shivdudhani ?
  3. Track progress here.

@realshuting realshuting added this to the Kyverno Release 1.0.1 milestone Nov 25, 2019
@realshuting
Copy link
Member

realshuting commented Nov 25, 2019

@shivdudhani @JimBugwadia
Automatically generating policies on pod controllers for the validate policies (rules):

  1. pod controller itself would be blocked when policy is set to enforce
  2. violations will be (audit mode):
    • created on the pod controller as well as the pod
    • removed when the policy is removed
    • removed when the resource (pod controller / pod) is removed
    • removed when the resource no longer violates the policy
    • updated when the resource got updated

For scenario 1, as the pod controller will be blocked immediately during deploy, we don't need to report the violation on resource owner anymore #387, related code can be removed, for the creation as well as the cleanup. Also, related fields in violation CRD should be marked as deprecated and removed eventually.

The user could enable/disable this feature by setting the annotations in the validate rule. By default, this feature is enabled and the additional rule will be generated on all pod controllers.

  1. When policies.kyverno.io/podcontroller.generate set to true, Kyverno would add an additional rule in the same policy for all pod controllers (deployment, statefuleset, daemonset, job, cronjob).
  2. When policies.kyverno.io/podcontroller.list is specified, Kyverno would add an additional rule in the same policy for the given pod controllers, policies.kyverno.io/podcontroller.generate will be ignored if it is set to false.
  3. These 2 annotations can be set in the clusterpolicy metadata.annotation to enable this feature across the rules(within the same policy), but the most inner one takes precedence.

Take the following example, a rule on deployment and job will be generated for the rule validate-namespace, while a rule on deployment, job and statefuleset will be generated for the rule require-namespace as the annotation is not set in this rule but in the policy.

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: disallow-default-namespace
  annotations:
    policies.kyverno.io/podcontroller.list: deployment,job,statefuleset
spec:
  rules:
  - name: validate-namespace
    match:
      resources:
        kinds:
        - Pod
    validate:
      message: "Using 'default' namespace is not allowed"
      pattern:
        metadata:
          annotations:
            policies.kyverno.io/podcontroller.generate: true
            policies.kyverno.io/podcontroller.list: deployment,job
          namespace: "!default"
  - name: require-namespace
    match:
      resources:
        kinds:
        - Pod
    validate:
      message: "A namespace is required"
      pattern:
        metadata:
          namespace: "?*"

@JimBugwadia
Copy link
Member Author

@realshuting - sounds good overall.

Can we combine the annotation into 1?

The user can disable using:

policies.kyverno.io/podcontroller.generate: "none"

or:

policies.kyverno.io/podcontroller.generate: ""

The user can specify which controllers to generate using:

policies.kyverno.io/podcontroller.generate: "jobs,statefulset"

The default will be:

policies.kyverno.io/podcontroller.generate: "all"

@shivdudhani
Copy link
Contributor

Some queries:
With annotation policies.kyverno.io/podcontroller.generate, will the rules be generated in the mutating webhook of the policy for the specified kinds or handled internally?
If yes, then how will we manage updates on policy rules?

Also if we are running on a setup where the mutating webhook is disabled, then using the mutating webhook will be a limitation? Do we plan to support pod controllers in that scenario?

So let's say, I create a policy with rule r1 on Pod. This will create 2 more rules r2 and r3on RS and Deployment respectively.
Will the user be allowed to modify r2 & r3?
If rule r1 is modified, then r2 and r3 will also need to be modified.

If the annotation is updated (removed/added), we’ll have to update the policy accordingly
In case of violation, will the violation use r2 and r3 as references? If yes, then if we have a scenario where we have annotation added and removed. How would be clean up the violation as its referring to a rule that has been deleted?

@JimBugwadia
Copy link
Member Author

Also, should we suppress the violations on the pods if the pod controller has a violation for the same rule?

@JimBugwadia
Copy link
Member Author

@shivdudhani - we can start with the assumption that generated rules should not be updated by the user. Perhaps we add a new type or rule? Or some meta-data / field in the rule to indicate its a generated rule.

@realshuting
Copy link
Member

@shivdudhani

will the rules be generated in the mutating webhook of the policy for the specified kinds or handled internally?

We can generate in mutating webhook as this will creates the entire policy to database, if we handle internally, we could lose tracking of generated policies, violations might need to be handled specifically in this case.

if we are running on a setup where the mutating webhook is disabled

If mutating webhook is disabled, then Kyverno probably not functional for the webhook part, as all the resources request is forwarded to mutating webhook. So I guess in this case we do not support pod controller.

For the updates of a policy / rule, say there's a policy with rule r1 on Pod, we will only generate one additional rule2 on given pod controllers(kinds take a list of kind), so at maximum, we will update one generated rule per rule(on pod).

@realshuting
Copy link
Member

@JimBugwadia

Also, should we suppress the violations on the pods if the pod controller has a violation for the same rule?

I think it'd be good to suppress one of the violations, otherwise we will end up with 2x violations.
One question for this is, if we create violation on the pod controllers, then how to indicate the resources this violation is created for, probably add one more field kinds? So the violation will be one policy - pod controllers relationship.

@shivdudhani
Copy link
Contributor

shivdudhani commented Nov 27, 2019

FYI
As the controller for cluster PV and namespace PV are going to be removed after feature #524. And PV will have resource/ resource owner as ownerReference.
The pod controller feature will need to manage the cleanup of owners,
e.g.

  • Create Pod -> policy blocks creation of Pod -> generate PV on Deployment
  • The user corrects the Pod spec in deployment, then the PV on Deployment will also need to be removed.

@realshuting
Copy link
Member

One approach to generate policy on pod controller after the discussion with @JimBugwadia, could be using the data in the policy rules to build a new rule on pod controllers. Once #529 is available, we can refer to the validate.pattern block to substitute the pattern in the new rule. For example, given a policy on pod, with annotation policies.kyverno.io/podcontroller.generate: "jobs,statefulset":

kind: ClusterPolicy
metadata:
  name: disallow-latest-tag
  annotations:
    policies.kyverno.io/podcontroller.generate: "jobs,statefulset"
spec:
  rules:
  # this is a rule for pod
  - name: custompodrule
    match:
      resources:
        kinds:
        - Pod
        namepaces: # some namespaces
        selector: # selector
    validate:
      message: "custom message"
      pattern:
        spec:
          containers:
          - image: "!*:latest"

Kyverno would generate a new rule, adds to the same policy, based on the template:

  rules:
  # this is an auto-generated rule on the given pod controllers
  # we can indicate the rule type (auto-gen) by adding a prefix to rule name 
  - name: autogen-custompodrule
    match:
      resources:
        # kinds are defined in the annotation - policies.kyverno.io/podcontroller.generate
        kinds:
        - jobs
        - statefulset
        # refer by path, what this path looks like denpends on how we can query the resource data
        namepaces: /spec/rules/0/match/resources/namespaces
        selector:  /spec/rules/0/match/resources/selector
    validate:
      # or we can indicate the rule type (auto-gen) by adding the msg
      message: /spec/rules/0/mesage + <auto generated rule msg>
      pattern:
        # "spec" and "template" are keys Kyverno adds 
        spec:
          template:
            # insert entire pattern block from pod rule
            # directly put path here without any tag / key?
            /spec/rules/0/validate/pattern

For this template, we could either generate it at runtime (in the policy mutation webhook), or we could have a built-in policy to auto-generate this template policy, need to understand more for the latter approach.

FYI @shivdudhani

@shivdudhani
Copy link
Contributor

With variable substitution, we can refer to attribute in the policy.
The context will hold the incoming resource & the policy.

Suggestion 1:
Would we always auto-generate rules on pod-controller or that can be controlled via some flag. i.e. if user wants to have a policy on pod only.

Suggestion 2:
Auto-generate rules:

  • How would the resource matching/exclude to be mapped ? as this can be a direct variable substitution. As the resource block is mandatory now ?

As mentioned before we don't want to support the update to rules after creation in initial phase, so will be user be blocked from updating the rules in the policy. As the CR can be edited, and we should not user update auto-generated rules.

Suggestion 3:
A policy rule on Pod will generate, 2 additional rules(RS and Deployment). And this policy will be big.
Or we could only provide the flag to enforce on podControllers and handle this internally instead of adding rules. So the policy will look cleaner. As the logic will be fixed for only some (Deploy->RS->Pod) & (SS->Pod), using it internally won't be difficult.

@JimBugwadia
Copy link
Member Author

@shivdudhani - the proposal is to generate the additional rules when the policy is created. Hence, in the case the policy is the resource.

@realshuting
Copy link
Member

@shivdudhani
Suggestion 1: It is generated base on the annotation policies.kyverno.io/podcontroller.generate, described above

Suggestion 2: The selector and namespaces of matching/exclude block are inherited from whatever they defined in a pod's rule. I believe kind in resources of match block is mandatory: https://github.com/nirmata/kyverno/blob/a5aa8669ff772134aa2b1dac92b9bb15c7b1e68c/definitions/install.yaml#L72-L73

Suggestion 3: Only ONE additional rule will be generated as kinds takes a list of kind. We could handle this internally instead of adding a rule to the policy, but need to think about how the violations are reported.

Also, should we suppress the violations on the pods if the pod controller has a violation for the same rule?

In the resource block of the violation stores the info of pod controller, while it could be confused to user when they check the policy there is no such rule on pod controller.

@realshuting
Copy link
Member

realshuting commented Dec 24, 2019

The decision whether to generate rule on pod controllers (in mutation webhook):
Based on the annotation pod-policies.kyverno.io/autogen-controllers of a cluster policy:

  • If it is empty, Kyverno sets the default value to all and generate an additional rule on all pod controllers: DaemonSet,Deployment,Job,StatefulSet
  • If the user set to all, check the fields name and labelSelector in the resource description,
    • if any of them exists, skip generating the rule on pod controllers as the filter may not be applicable
    • if both not exist, generate an additional rule on all pod controllers
  • If it's set by user, generate on the given pod controllers

Process the rules (in policy engine):
Take the Deployment -> ReplicaSet -> Pod as an example:

  • First comes in a CREATION request for Deployment, engine applies the rule on deployment:
    • On success: mutate the podTemplate with annotationkyverno.io/policyapplied=true
    • On failure: report the violation if audit, mutate the podTemplate with annotationkyverno.io/policyapplied=true (if enforce, the resource creation will be blocked, so there is no need to annotate podTemplate)
  • Then comes in the CREATION request for Replicaset, skips it
  • Last receives a Pod CREATION request, if the pod has the annotation kyverno.io/policyapplied=true, which indicates the policy already been processed on the pod controller, skips applying the rule on pod. If no such annotation, which means the pod is created in standalone mode, then applies the rule to it.

Several points to note:

  1. We explicitly annotate the pod with kyverno.io/policyapplied=true, to mark that the rule has been already applied to the owner of this pod. Whenever we see this annotation, we'll skip processing the rule so the violation on pod would be suppressed.
  2. ReplicaSet is excluded from the pod controller list unless there is a specific use case for it, as it introduces the extra complexity to the processing and reporting components and adds no value. Kubernetes also recommends using Deployment instead.

@realshuting realshuting changed the title Pod policies should automatically be applied to pod controllers (when set to enforce) Pod policies should automatically be applied to pod controllers Dec 24, 2019
@JimBugwadia
Copy link
Member Author

@realshuting - suggestions on the annotations:

pod-policies.kyverno.io/autogen-controllers
pod-policies.kyverno.io/autogen-applied

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants