Skip to content

Conversation

@mkilchhofer
Copy link
Contributor

This PR here:

introduced a regression for everyone using this Kyverno policy: https://kyverno.io/policies/other/pdb-maxunavailable/pdb-maxunavailable/

validation error: The value of maxUnavailable must be greater than zero. rule deny-pdb-maxunavailable failed at path /spec/maxUnavailable/

Since both fields are mutual exclusive, I suggest that we only render the related field according to values:

$ kubectl explain pdb.spec
(..)
FIELDS:
  maxUnavailable	<IntOrString>
    An eviction is allowed if at most "maxUnavailable" pods selected by
    "selector" are unavailable after the eviction, i.e. even in absence of the
    evicted pod. For example, one can prevent all voluntary evictions by
    specifying 0. This is a mutually exclusive setting with "minAvailable".

  minAvailable	<IntOrString>
    An eviction is allowed if at least "minAvailable" pods selected by
    "selector" will still be available after the eviction, i.e. even in the
    absence of the evicted pod.  So for example you can prevent all voluntary
    evictions by specifying "100%".
(..)

@mkilchhofer mkilchhofer changed the title fix(PDB): Don't set null inside PDB fix(PDB): Don't render null keys inside PDB Jul 21, 2025
@mkilchhofer mkilchhofer force-pushed the bugfix/dont_set_null_fields branch from 217be9d to 7bdfe73 Compare July 21, 2025 15:38
@tuunit
Copy link
Member

tuunit commented Jul 21, 2025

@pinkavaj not what I was expecting but my concern was valid.

Copy link
Member

@tuunit tuunit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@the-technat the-technat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkilchhofer pinged me internally to have a look and I would confirm that fix is necessary since maxUnavailable and minAvailable are mutually exclusive.

@tuunit
Copy link
Member

tuunit commented Jul 21, 2025

@mkilchhofer pinged me internally to have a look and I would confirm that fix is necessary since maxUnavailable and minAvailable are mutually exclusive.

Yes but null would be valid. It should work without kyverno and doesn't void the mutual exclusivity of those parameters as far as I know

@tuunit tuunit merged commit cb54873 into oauth2-proxy:main Jul 21, 2025
1 check passed
@tuunit
Copy link
Member

tuunit commented Jul 21, 2025

Never the less this implementation is cleaner

@mkilchhofer mkilchhofer deleted the bugfix/dont_set_null_fields branch July 22, 2025 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants