Codecov Report

❌ Patch coverage is 89.81481% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.25%. Comparing base (7e69452) to head (1079c75).
⚠️ Report is 55 commits behind head on main.

@@            Coverage Diff             @@
##             main     #985      +/-   ##
==========================================
- Coverage   71.27%   68.25%   -3.02%     
==========================================
  Files          22       22              
  Lines        3046     3141      +95     
==========================================
- Hits         2171     2144      -27     
- Misses        706      831     +125     
+ Partials      169      166       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hey,

First of all, thank you very much for your contribution!

Per-port specific configuration for load balancers is currently part of a broader discussion. This issue summarizes the situation quite well. As mentioned there, I’ll review the options for per-port configuration and draft a proposal to ensure a consistent experience. If the team agrees with the direction, we can move forward with the implementation, and I’ll revisit your PR at that point.

The failing e2e tests on forks are expected.

Best regards,
Lukas

I'm

Hey,

First of all, thank you very much for your contribution!

Per-port specific configuration for load balancers is currently part of a broader discussion. This issue summarizes the situation quite well. As mentioned there, I’ll review the options for per-port configuration and draft a proposal to ensure a consistent experience. If the team agrees with the direction, we can move forward with the implementation, and I’ll revisit your PR at that point.

The failing e2e tests on forks are expected.

Best regards, Lukas

Hey Lukas

Thanks, and I'm happy to help with implementation. For the time being I've switched to proxy protocol which fits my use-case well (forwarding IP headers would be nice, but my Ingress supports this kind of termination).

I've thought about this some more and if I were to move forward on this I'd probably rewrite this to maintain the same annotation names as upstream, but with an added port at the end of the name, eg. load-balancer.hetzner.cloud/protocol/443: https - you'd have multiple annotations for a mapping, but this would be easier to understand and more extendable.

but with an added port at the end of the name, eg. load-balancer.hetzner.cloud/protocol/443: https - you'd have multiple annotations for a mapping, but this would be easier to understand and more extendable.

This will not work unfortunately, as there is only one slash allowed in the annotation to separate key/value. Ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set

but with an added port at the end of the name, eg. load-balancer.hetzner.cloud/protocol/443: https - you'd have multiple annotations for a mapping, but this would be easier to understand and more extendable.

This will not work unfortunately, as there is only one slash allowed in the annotation to separate key/value. Ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set

I think my main issue with my current approach is that it's fairly convoluted. We could use a different separation character for the port, since ultimately the actual separation character does not really matter. I think that's still a cleaner solution than encoding a map in annotations, even if the Kubernetes annotation spec specifically allows for structured data in annotations. I still think it should be human-readable.

This PR has been marked as stale because it has not had recent activity. The bot will close the PR if no further action occurs.