Skip to content

Conversation

@JohnLahr
Copy link

@JohnLahr JohnLahr commented Oct 4, 2025

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

This PR aims to fix a broken PodDisruptionBudget when enabled.

Summary

When controller.podDisruptionBudget.enabled is set to true, the following error is thrown:

Failed sync attempt to : one or more objects failed to apply, reason: PodDisruptionBudget.policy "vault-secrets-operator" is invalid: spec: Invalid value: policy.PodDisruptionBudgetSpec{MinAvailable:(*intstr.IntOrString), Selector:(*v1.LabelSelector), MaxUnavailable:(*intstr.IntOrString), UnhealthyPodEvictionPolicy:(*policy.UnhealthyPodEvictionPolicyType)(nil)}: minAvailable and maxUnavailable cannot be both set

After trying multiple combinations of values for controller.podDisruptionBudget.minAvailable and controller.podDisruptionBudget.maxUnavailable, I discovered a temporary workaround: setting one to an explicit value and the other to an empty string ("") resolves this, but this is clunky and surely not the intended design.

Root Cause

The chart sets default values of 0 for both maxUnavailable and minUnavailable. This causing the template to render both maxUnavailable and minUnavailable simultaneously (and with some strange spacing):

---
# Source: vault-secrets-operator/templates/poddisruptionbudget.yaml
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
  name: vault-secrets-operator
  labels:
    app.kubernetes.io/component: controller-manager
    control-plane: controller-manager
    helm.sh/chart: vault-secrets-operator-1.0.1
    app.kubernetes.io/name: vault-secrets-operator
    app.kubernetes.io/instance: vault-secrets-operator
    app.kubernetes.io/version: "1.0.1"
    app.kubernetes.io/managed-by: Helm
  namespace: vault
spec:
  

  
  maxUnavailable:
    0

  
  minAvailable:
    0

  selector:
    matchLabels:
      app.kubernetes.io/name: vault-secrets-operator
      app.kubernetes.io/instance: vault-secrets-operator

Setting both values explicitly, even if one or both is 0, is disallowed by Kubernetes, so this always fails to apply.

This PR aims to fix this bug by using the following logic, keeping the current values of 0 as defaults for backward-compatibility:

  • If neither maxUnavailable nor minUnavailable are set, PDB defaults to a safe value minAvailable: 1 (user explicitly created a PDB, but they didn't set explicit constraints)
  • If either maxUnavailable or minUnavailable are set and the other is null/unset, use the constraint with the explicitly set value (as expected)
  • If both maxUnavailable and minUnavailable are set, but one is zero (the default if left unset), render only the non-zero value (maxUnavailable or minUnavailable)
  • If both maxUnavailable and minUnavailable are set to non-zero values, trigger the built-in failure message

Testing

I have tested all scenarios involving combinations of null values, explicitly set values, zero values, etc. to ensure that this template follows the above logic.

I now see correctly rendered PDBs:

---
# Source: vault-secrets-operator/templates/poddisruptionbudget.yaml
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
  name: vault-secrets-operator
  namespace: vault
  labels:
    app.kubernetes.io/component: controller-manager
    control-plane: controller-manager
    helm.sh/chart: vault-secrets-operator-1.0.1
    app.kubernetes.io/name: vault-secrets-operator
    app.kubernetes.io/instance: vault-secrets-operator
    app.kubernetes.io/version: "1.0.1"
    app.kubernetes.io/managed-by: Helm
spec:
  minAvailable: "34%"
  selector:
    matchLabels:
      app.kubernetes.io/name: vault-secrets-operator
      app.kubernetes.io/instance: vault-secrets-operator

- Fixed PDB rendering both an explicit minAvailable and maxUnavailable at once
@JohnLahr JohnLahr requested a review from a team as a code owner October 4, 2025 08:09
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.

1 participant