Skip to content

fix: order ingress rules by path specificity before hostname catch-all - #46

Open
etomm wants to merge 1 commit into
StringKe:mainfrom
etomm:fix/ingress-path-specificity-order
Open

etomm wants to merge 1 commit into
StringKe:mainfrom
etomm:fix/ingress-path-specificity-order

Conversation

@etomm

@etomm etomm commented Nov 5, 2026 •

Copy link
Copy Markdown

Description

Cloudflare evaluates tunnel ingress rules in order, first match wins.

The ingress aggregation currently sorts rules lexicographically by path:

return rules[i].Path < rules[j].Path

Since "" (the hostname-only catch-all) sorts before any non-empty path,
a catch-all such as:

cloud.example.com -> http://internal:9443

is emitted before specific paths like:

cloud.example.com/svc1 -> http://svc1:80
cloud.example.com/svc2 -> http://svc2:80

Cloudflare matches the catch-all first, so the specific path rules are never
reached. The same issue exists in TunnelConfig.AggregateRules(), which only
sorts by source priority and hostname and does not consider path specificity.

This change orders rules by path specificity before deterministic tie-breakers:

  1. path-specific rules before hostname-only catch-all rules
  2. longer paths before shorter paths (more specific first)
  3. hostname/path as deterministic tie-breakers

Applied to:

  • internal/controller/ingress/converters.go — buildIngressRules
  • internal/controller/tunnelconfig/types.go — AggregateRules

Both use sort.SliceStable so equal keys preserve deterministic order.

Container image namespace

The release workflow and Makefile previously hardcoded
ghcr.io/stringke/cloudflare-operator, which makes it awkward for forks to
build and push their own images without editing files.

This change makes the image path configurable via IMAGE_NAME:

  • .github/workflows/release.yml
    IMAGE_NAME: ${{ vars.IMAGE_NAME || 'stringke/cloudflare-operator' }}
  • Makefile
    IMAGE_TAG_BASE ?= ghcr.io/$(if $(IMAGE_NAME),$(IMAGE_NAME),stringke/cloudflare-operator)

IMAGE_NAME can be set as a repository/org/environment variable
(Settings → Secrets and variables → Actions → Variables), e.g.
IMAGE_NAME=myuser/cloudflare-operator. When it is not set, the upstream
stringke/cloudflare-operator image name is used.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

Related Issues

None.

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the project's coding standards
  • I have added/updated tests for my changes
  • I have updated documentation as needed (not required for this fix)
  • All new and existing tests pass (make test) (relevant package tests pass; full make test not run locally)
  • Linting passes (make lint) (not run locally)
  • CRD manifests are up-to-date (make manifests generate) (no API/CRD changes)

Testing

Added TestAggregateRulesOrdersByPathSpecificity covering:

  • /api/v2 before /api
  • path rules before the hostname catch-all
  • longer paths before shorter paths
  • deterministic hostname ordering

Commands run:

go test ./internal/controller/tunnelconfig/...
go test ./internal/controller/ingress/...
go vet ./internal/controller/tunnelconfig/... ./internal/controller/ingress/...

All passed. Also validated .github/workflows/release.yml parses as YAML.

The Docker push path was not run locally; it still requires a valid GHCR login
for the target namespace.

Additional Notes

No API or CRD changes for the ingress ordering fix. The behavior for disjoint
paths is unchanged; the fix only affects ordering where a hostname catch-all
would otherwise shadow more specific path rules.

The IMAGE_NAME change is configuration-only. Forks should set the repository
variable to a lowercase GHCR path, e.g. myuser/cloudflare-operator.

Alternative

Another option would be to simply invert the comparison:

return rules[i].Path > rules[j].Path

This would invert the sort, putting path prefixes before their descendants.
It works for the common prefix-extension case and moves the empty catch-all
last, but it does not guarantee longest-path-first ordering for unrelated
paths. The explicit specificity sort is used instead because it is deterministic
and easier to reason about.

@etomm
etomm force-pushed the fix/ingress-path-specificity-order branch from ee5b716 to c459435 Compare November 5, 2026 07:58
@etomm
etomm force-pushed the fix/ingress-path-specificity-order branch from c459435 to 4eae320 Compare November 5, 2026 08:04
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