Skip to content

fix: validate empty route parentRefs and omit empty hostnames - #2386

Open
somaz94 wants to merge 1 commit into
goharbor:mainfrom
somaz94:fix/route-empty-parentrefs-hosts
Open

fix: validate empty route parentRefs and omit empty hostnames#2386
somaz94 wants to merge 1 commit into
goharbor:mainfrom
somaz94:fix/route-empty-parentrefs-hosts

Conversation

@somaz94

@somaz94 somaz94 commented Jul 3, 2026

Copy link
Copy Markdown

Problem

When expose.type: route is used with the chart's own default values (expose.route.parentRefs: [] and expose.route.hosts: []), helm template fails with:

Error: YAML parse error on harbor/templates/gateway-apis/route.yaml: error converting YAML to JSON: yaml: line 9: could not find expected ':'

toYaml on an empty list returns the flow literal [], and nindent 2 put it at the same indentation as the parent parentRefs: / hostnames: key, so it parsed as a sibling node. expose.type: route was broken out of the box, with a cryptic error and no user override required.

Fixes #2385

Fix

parentRefs and hostnames are treated differently, matching their roles in the Gateway API HTTPRoute spec:

  • parentRefs — an HTTPRoute with no parentRefs attaches to no Gateway and cannot serve traffic, so it is effectively required in route mode. When it is empty the template now fails with a clear message instead of emitting invalid YAML.
  • hostnames — genuinely optional per the spec (omitting it matches all hostnames), so it is wrapped in a {{- with }} guard and omitted when the list is empty, rendered at nindent 4 when present.

Validation

  • helm template --set expose.type=route,expose.tls.enabled=false . now fails with a clear expose.route.parentRefs must be set … message (previously a cryptic YAML parse error).
  • With parentRefs set and hosts empty, parentRefs renders and hostnames is omitted.
  • helm unittest -f 'test/unittest/gateway-apis/*.yaml' . passes (updated the empty-defaults case to assert the clear failure; added the parentRefs-set/hosts-empty case).
  • helm lint . passes.

somaz94 added a commit to somaz94/somaz94 that referenced this pull request Jul 3, 2026
@MinerYang

Copy link
Copy Markdown
Collaborator

Hi @somaz94 ,

the parentRef define which Gateways this Route wants to be attached to and normally we regard it as not omitted.
Could you elaborate more your use case and ping me for a review once it's ready.

Thanks,
Miner

@somaz94
somaz94 force-pushed the fix/route-empty-parentrefs-hosts branch from e3f0b41 to 44591f4 Compare July 6, 2026 06:13
@somaz94 somaz94 changed the title fix: render valid HTTPRoute when route parentRefs and hosts are empty fix: validate empty route parentRefs and omit empty hostnames Jul 6, 2026
@somaz94

somaz94 commented Jul 6, 2026

Copy link
Copy Markdown
Author

Hi @MinerYang, thanks for the review!

You're right that parentRefs shouldn't be omitted — an HTTPRoute with no parentRefs attaches to no Gateway and can't serve traffic. My original patch was too lenient there.

To clarify the use case: this started as a pure rendering bug (#2385). The chart ships expose.route.parentRefs: [] and hosts: [] as its own defaults, so helm template --set expose.type=route fails out of the box with a cryptic could not find expected ':' YAML parse error — toYaml on an empty list emits [] at the wrong indent. There's no user "use case" for empty parentRefs; the problem is just that the default value produces an unreadable error instead of a clear one.

I've reworked the PR to match your view:

  • parentRefs: no longer omitted. If it's empty under expose.type: route, the template now fails with a clear message instead of emitting invalid YAML.
  • hostnames: kept optional via {{- with }}, since it's genuinely optional in the Gateway API HTTPRoute spec (omitting it matches all hostnames).

helm unittest / helm lint pass. Flipping to ready for review now.

Thanks!

@somaz94
somaz94 marked this pull request as ready for review July 6, 2026 06:27
Fixes goharbor#2385

Signed-off-by: somaz <genius5711@gmail.com>
@somaz94
somaz94 force-pushed the fix/route-empty-parentrefs-hosts branch from 44591f4 to 67a2843 Compare July 6, 2026 06:28
@somaz94

somaz94 commented Jul 15, 2026

Copy link
Copy Markdown
Author

Hi @MinerYang — thanks, and I think we're actually in full agreement: this PR does not make parentRefs optional/omittable. It keeps it required and just replaces a broken out-of-the-box experience with a clear error.

The use case is the chart's own defaults (issue #2385): with expose.type: route and the shipped defaults expose.route.parentRefs: [] and expose.route.hosts: [], helm template fails with a cryptic YAML parse error:

Error: YAML parse error on harbor/templates/gateway-apis/route.yaml: error converting YAML to JSON: yaml: line 9: could not find expected ':'

toYaml [] renders the flow literal [], and at nindent 2 it lands at the same indentation as the parent parentRefs: key, so it parses as a sibling and the document is invalid — route mode is unusable until the user happens to set a value, with no hint why.

The fix keeps the Gateway API semantics you describe:

  • parentRefs — since an HTTPRoute with no parentRefs attaches to no Gateway and can't serve traffic, it stays required. When empty the template now fails with expose.route.parentRefs must be set when expose.type is "route" … instead of the cryptic parse error. (Verified it renders that message on the defaults.)
  • hostnames — genuinely optional per the spec (omitting matches all hostnames), so it's wrapped in {{ with }} and omitted when empty, which also removes the same invalid-YAML issue for that field.

helm unittest (6/6) and helm lint pass. So the behavior you'd expect — parentRefs required — is preserved; this only turns a silent broken default into an actionable message. Ready for review whenever you have a moment 🙏

@somaz94

somaz94 commented Jul 28, 2026

Copy link
Copy Markdown
Author

@MinerYang I see the needs/follow-up label — could you clarify what you'd like from me here? I want to make sure I address the right thing rather than guess.

To restate the scope: this isn't about making parentRefs optional. With the chart's own default values the rendered HTTPRoute is invalid (empty parentRefs, and an empty hostnames entry), which is the #2385 report. The change validates parentRefs and omits hostnames when empty, so the default render is valid.

If the follow-up is about something else — more unit-test coverage, narrowing the scope to just the hostnames omission, or a different approach entirely — happy to do it, just let me know which. Also note the Lint/Unit/Integration workflows are still pending approval to run on this fork PR, so CI hasn't actually exercised the added unittest yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

expose.type=route: HTTPRoute template fails to render with chart's own default (empty) parentRefs/hosts

2 participants