Skip to content

Conversation

@JohnLahr
Copy link

@JohnLahr JohnLahr commented Oct 3, 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 loop in the topology spread constraints helper vso.topologySpreadConstraints in the Helm chart templates.

Summary

While defining one constraint worked just fine (with or without labels—those were correctly added when missing), defining more than one was causing invalid YAML to be rendered and the following error to be displayed:

YAML parse error ... mapping values are not allowed in this context

For reference, the offending values were:

# values.yaml
---
controller:
  topologySpreadConstraints:
    - maxSkew: 1
      topologyKey: topology.kubernetes.io/zone
      whenUnsatisfiable: ScheduleAnyway
    - maxSkew: 1
      topologyKey: kubernetes.io/hostname
      whenUnsatisfiable: DoNotSchedule

Root Cause

Digging into it, I noticed that because of the aggressive whitespace trimming ({{- ... -}}) combined with emitting each item separately, the rendered list items were concatenated without a newline between them. With two constraints, I was seeing something like this:

# deployment.yaml
---
# ...

topologySpreadConstraints:
  - whenUnsatisfiable: ScheduleAnyway
    topologyKey: topology.kubernetes.io/zone
    maxSkew: 1- labelSelector:         # <-- the next item starts immediately after “1”
      matchLabels:
        app.kubernetes.io/name: vault-secrets-operator
        app.kubernetes.io/instance: vault-secrets-operator
    whenUnsatisfiable: DoNotSchedule
    topologyKey: kubernetes.io/hostname
    maxSkew: 1

This happens because the vso.topologySpreadConstraints helper emits each constraint as a single‑element YAML list and uses whitespace‑trimming ({{- ... -}}). The consecutive toYaml outputs get concatenated without a newline, so the next - starts immediately after the previous line’s text (e.g., maxSkew: 1- labelSelector:), producing invalid YAML.

This refactor avoids the above problem and fixes the whitespace trap.

Testing

I have tested this in my own cluster (Kubernetes v1.32) and confirmed that all of the following values configs are successfully rendered:

  • No topology spread constraints defined
  • One topology spread constraint defined without labels (correctly added)
  • Two topology spread constraints defined with labels
  • Two topology spread constraint defined without labels (correctly added)
  • One topology spread constraint defined without labels (correctly added) and one topology spread constraint defined with labels

@JohnLahr JohnLahr requested a review from a team as a code owner October 3, 2025 23:55
@hashicorp-cla-app
Copy link

hashicorp-cla-app bot commented Oct 3, 2025

CLA assistant check
All committers have signed the CLA.

@hashicorp-cla-app
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@JohnLahr JohnLahr force-pushed the bugfix/topology-spread-constraints branch 2 times, most recently from 0f8b4e3 to 299f6c0 Compare October 4, 2025 06:42
- Fixed loop in topology spread constraint helper definition
@JohnLahr JohnLahr force-pushed the bugfix/topology-spread-constraints branch from 299f6c0 to 89f9f02 Compare October 4, 2025 06:43
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