Add labels for traefik.frontend.entryPoints & PassTLSCert to Kubernetes
#2324
Conversation
ldez
added
status/2-needs-review
kind/enhancement
ldez
changed the title
traefik.frontend.entryPoints & PassTLSCert to Kubernetes
|
Ping @dtomcej or @timoreimann, WDYT ? |
|
Ah wait, this doesn't seem to be about a new annotation, so maybe no docs update strictly needed. Would it be worthwhile to extend the guide somehow though? @dtomcej, WDYT? |
provider/kubernetes/kubernetes.go
Outdated
| log.Warnf("Unknown value '%s' for %s, falling back to %s", passHostHeaderAnnotation, types.LabelFrontendPassHostHeader, PassHostHeader) | ||
| } | ||
| PassHostHeader := getRulePassHostHeader(i, p) | ||
| PassTLSCert := getRulePassTLSCert(i, p) |
There was a problem hiding this comment.
The two variables probably don't need to be exported. Can we decapitalize them?
provider/kubernetes/kubernetes.go
Outdated
| if realm := i.Annotations[annotationKubernetesAuthRealm]; realm != "" && realm != traefikDefaultRealm { | ||
| log.Errorf("Value for annotation %q on ingress %s/%s invalid: no realm customization supported", annotationKubernetesAuthRealm, i.ObjectMeta.Namespace, i.ObjectMeta.Name) | ||
| delete(templateObjects.Backends, r.Host+pa.Path) | ||
| continue | ||
| } | ||
|
|
||
| EntryPoints := getEntrypoints(i) |
There was a problem hiding this comment.
Unexport this one too?
provider/kubernetes/kubernetes.go
Outdated
| Routes: make(map[string]types.Route), | ||
| Priority: priority, | ||
| BasicAuth: basicAuthCreds, | ||
| WhitelistSourceRange: whitelistSourceRange, | ||
| EntryPoints: EntryPoints, |
There was a problem hiding this comment.
Are the entry points needed for the pass TLS functionality? If not, then I'd prefer to handle them in a separate PR.
There was a problem hiding this comment.
Did i really need to make two separate PR?
| if ok { | ||
| return strings.Split(entrypointsAnnotation, ",") | ||
| } | ||
| return nil |
There was a problem hiding this comment.
If the nil slice is handled equally to the empty slice case by Traefik, then we don't need the extra ok check it seems?
There was a problem hiding this comment.
strings.Split return a non empty slice when his first arg is empty, so if i didn't check extra ok , i will need to check if first of slice is empty. It seem more maintainable that way, no?
There was a problem hiding this comment.
Good point, I missed that property of strings.Split. Let's keep things like they are right now then.
provider/kubernetes/kubernetes.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func getRulePassHostHeader(i *v1beta1.Ingress, p *Provider) bool { |
There was a problem hiding this comment.
It's not really a rule it seems to me. Can we drop that part from the function name?
provider/kubernetes/kubernetes.go
Outdated
| return passHostHeader | ||
| } | ||
|
|
||
| func getRulePassTLSCert(i *v1beta1.Ingress, p *Provider) bool { |
There was a problem hiding this comment.
Drop Rule from the name here too?
provider/kubernetes/kubernetes.go
Outdated
| default: | ||
| log.Warnf("Unknown value '%s' for %s, falling back to %s", passTLSCertAnnotation, types.LabelFrontendPassTLSCert, passTLSCert) | ||
| } | ||
| return passTLSCert |
There was a problem hiding this comment.
Is the parsing logic identical to what we have in the function just above? If so, can we extract a little helper and use it from both implementation?
|
@ryarnyah So that your PR is integrated in the 1.5, it is imperative that it is ready by Tuesday 21 November at the latest. 🚀 |
ryarnyah
force-pushed
the
feat/add-pass-tls-cert-providers
branch
2 times, most recently
from
2734f2c
to
daf02eb
Compare
|
I also added some docs on these. |
provider/kubernetes/kubernetes.go
Outdated
|
|
||
| } | ||
|
|
||
| func getBoolAnnoatation(meta v1.ObjectMeta, name string, defaultValue bool) bool { |
There was a problem hiding this comment.
Typo: Annoatation -> Annotation.
ryarnyah
force-pushed
the
feat/add-pass-tls-cert-providers
branch
from
daf02eb
to
66d91d4
Compare
Description
Add labels:
traefik.frontend.entryPointstraefik.frontend.passTLSCert