Windows ingress support#1514
Conversation
c26a6ac to
612b2c8
Compare
76579bf to
37b823f
Compare
Signed-off-by: makhov <[email protected]>
apedriza
left a comment
apedriza
left a comment
There was a problem hiding this comment.
has the TestWindowsIngressProvisioning been executed succesfully? We could manually trigger before merge it as currently will not run until it is on main
| Name: kmc.GetIngressManifestsConfigMapName(), | ||
| }, | ||
| Secret: &corev1.SecretVolumeSource{ | ||
| SecretName: kmc.GetIngressManifestsConfigMapName(), |
There was a problem hiding this comment.
| SecretName: kmc.GetIngressManifestsConfigMapName(), | |
| SecretName: kmc.GetIngressManifestsSecret(), |
Now that it is stored as a secret
Anyway, this name is used in several places not related to configmaps/secrets. I would use something mroe general like GetIngressManifestsResourceName or something like that
| _ = kcontrollerutil.SetExternalOwnerReference(kmc, &configMap, scope.client.Scheme(), scope.externalOwner) | ||
| err = scope.reconcileResource(ctx, kmc, &configMap) | ||
|
|
||
| configMap, err := scope.generateIngressManifestsConfigMap(kmc, proxyCert, proxyKey, caCert) |
There was a problem hiding this comment.
Why not create the secret directly instead of creating it from a non-stored ConfigMap?
| err := kmcScope.ensureHAProxyCerts(ctx, kmc) | ||
| if err != nil { | ||
| return ctrl.Result{Requeue: true, RequeueAfter: time.Minute}, fmt.Errorf("error generating ingress certificates: %w", err) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we move this within reconcileIngress? I would encapsulate all the old logic support/considerations deeper, otherwise can be a bit confusing from a high-level POV and rename cert from ingress-haproxy to ingress-proxy, just in case proxy implementation is changed in the future
2 participants
Switching to traefik which supports windows