Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Change failurePolicy to "Fail" and add reinvocationPolicy "IfNeeded" #265

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Neumann-Nils
Copy link
Contributor

Description

This PR changes two things:

  1. It changes the failurePolicy from Ignore to Fail. I argue that it might be problematic if the Karydia webhook fails silently. It should rather fail and throw an error. Also see FailurePolicy considerations #2

https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#failure-policy
2. It sets the reinvocationPolicy to IfNeeded. Thus, the webhook may be called if the object being admitted is modified by other webhooks after the initial webhook call. For example, if the Karydia webhook is invoked for an object and modifies it, another webhook could modify it later on, then the Karydia webhook should be called again to enforce the security policies.
(For me it seems like this scenario is limited in the number of calls if changes appears on an already mutated object. See kubernetes/enhancements#1049).
https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy

Checklist

Before submitting this PR, please make sure:

  • your code builds clean with make
  • your code lets succeed unit tests with make test
  • your code lets succeed integration tests

@Neumann-Nils Neumann-Nils requested a review from a team as a code owner June 22, 2020 06:47
@Neumann-Nils Neumann-Nils self-assigned this Jun 22, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added the ci/wanted Required to trigger CI build and test label Jun 22, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the ci/wanted Required to trigger CI build and test label Jun 22, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the ci/wanted Required to trigger CI build and test label Jun 22, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the ci/wanted Required to trigger CI build and test label Jun 22, 2020
@Neumann-Nils Neumann-Nils added the ci/wanted Required to trigger CI build and test label Oct 13, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the ci/wanted Required to trigger CI build and test label Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants