hcloud-cloud-controller-manager icon indicating copy to clipboard operation
hcloud-cloud-controller-manager copied to clipboard

Inconsistency in the naming for load balancer annotation.

Open DeprecatedLuke opened this issue 1 year ago • 4 comments

https://github.com/hetznercloud/hcloud-cloud-controller-manager/blob/e2b0ed63f6cd5a7499d356bacd784ded41df5f2a/internal/annotation/load_balancer.go#L161

Isn't this supposed to be http-redirect-https? Most guides also say that it's http-redirect-https, but in the code itself it's http-redirect-http.

DeprecatedLuke avatar Jun 01 '24 21:06 DeprecatedLuke

~~That should indeed be http-redirect-https. Though changing this now would be a breaking change. Perhaps best to add the correct option besides and use that if its set and fall back to the old annotation otherwise.~~

apricote avatar Jun 18 '24 13:06 apricote

I would be more concerned about people using http-redirect-https (as it is shown in the docs) in a misconfigured way causing it to break after an update. Perhaps just have a new annotation called ssl-redirect (same as ingress-nginx).

DeprecatedLuke avatar Jun 18 '24 13:06 DeprecatedLuke

After looking at it again, I understand why http-redirect-http was chosen in the first place. The API schema for this is { "http": {"redirect-http": true }}, so the annotation was chosen to be equivalent.

To help with some confusion, but keep it mostly consistent with the API we would prefer the following solution:

  • Add new annotation "load-balancer.hetzner.cloud/http-redirect-https"
  • If specified use the value from (new) http-redirect-https
  • If the new annotation is not specified, use the value of (old) http-redirect-http

apricote avatar Sep 17 '24 06:09 apricote

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

github-actions[bot] avatar Dec 25 '24 12:12 github-actions[bot]