Refactor Policy violation owner logic

[shivdudhani]

• Remove cluster and namespaced policy violation controller

• Policy violation owner is the resource

• Handler policy deletion manually.

• Refactor policy violation generation logic, as there was a lot of duplicate code for cluster and namespaced PV. The initial proposal, code can be improved further.

• Update the vendor files.

• Support mandatory fields in webhook configuration required since 1.16.( test with minikube 1.15.2 & 1.16.2).

  • Sideeffect: NoneOnDryRun
  • AdmissionReviewVersions: ['v1beta1']

fixes #526
fixes #524
fixes #528
fixes #442
fixes #429

Testing Done:

• cleanup namespaced PV when the resource is deleted.
• clean namespaced PV when the policy is deleted.

[realshuting]

Also need to set resource version if it's an update, https://github.com/nirmata/kyverno/blob/a5aa8669ff772134aa2b1dac92b9bb15c7b1e68c/pkg/policyviolation/namespacedpv.go#L102

[shivdudhani]

Corrected

[realshuting]

When info.Resource.GetName() == "", assume it's a pod created by podcontroller, and for the first creation request, it doesn't come with the name, so here we are generating a violation on resource owner even if it's not blocked?

[shivdudhani]

this is related to https://github.com/nirmata/kyverno/issues/535, as nothing has been decided. so adding as a placeholder.

[realshuting]

Can we remove info.Resource.GetName() == "" condition here? As this would create incorrect violations on resourceOwner. Maybe switch back to what we used to have, which is to skip creating violations if name is not assigned yet, until we decide how to solve #535?

[shivdudhani]

ok will remove, but by when will be have the solution for #535 ?
As the current implementation works but is incorrect.

[shivdudhani]

removed the fix and jst left the placeholder.

[realshuting]

ok will remove, but by when will be have the solution for #535 ?
As the current implementation works but is incorrect.

Let's merge this PR first and move the discussion to 535.

[realshuting]

Had a separate question apart from this PR:
https://github.com/nirmata/kyverno/blob/4f174779dcbf69cdf1ef382bf729231e81777010/pkg/webhooks/validation.go#L100-L101

For these two lines, seems like we are checking engine response and validate action separately. A problem with this: say if I have 2 policies, A(audit) and B(enfoce), while policy A fails and B passes, so in this case we will block the resource creation since A fails and B is set to enforce. Is this correct? We only want to block the request if B fails, while in this case A fails which is in audit mode, shouldn't we pass this check?

[shivdudhani]

Had a separate question apart from this PR:
https://github.com/nirmata/kyverno/blob/4f174779dcbf69cdf1ef382bf729231e81777010/pkg/webhooks/validation.go#L100-L101

For these two lines, seems like we are checking engine response and validate action separately. A problem with this: say if I have 2 policies, A(audit) and B(enfoce), while policy A fails and B passes, so in this case we will block the resource creation since A fails and B is set to enforce. Is this correct? We only want to block the request if B fails, while in this case A fails which is in audit mode, shouldn't we pass this check?

If there is a policy blocking a resource, then the rest policies will also not be applied. A policy in audit mode might be mutating the resource. And this might affect the policy B validation. In short, the order in which policies are applied is not defined. So we reject the request for any block.