fix bugs around ingress, tls, and dev-password#495
Conversation
96e6bc1 to
4aa7d91
Compare
|
Marking this as a draft, i think there's more bugs here and i wanna make sure I'm solving them right |
d259e7b to
1ff0ba3
Compare
Signed-off-by: Caleb Boylan <calebboylan@gmail.com>
When using the --ingress-host-name flag we end up needing two separate ingresses per service. This is because the cluster services will continue to use the "host" domain to access other resources within the cluster. In order to properly setup these extra ingresses we need to add a SAN for the separate ingress host name. Signed-off-by: Caleb Boylan <calebboylan@gmail.com>
1ff0ba3 to
45daaff
Compare
| @@ -157,7 +157,7 @@ func (r *LocalbuildReconciler) setGiteaToken(ctx context.Context, secret corev1. | |||
| // gitea URL reachable within the cluster with proper coredns config. Mainly for argocd | |||
| func giteaInternalBaseUrl(config v1alpha1.BuildCustomizationSpec) string { | |||
There was a problem hiding this comment.
This could be simplified a bit to just use the pkg/util/gitea.go GiteaBaseUrl function, not sure if we want that change in this PR though, it's already getting a bit bloated
There was a problem hiding this comment.
I think this function is specific for returning in-cluster URL and I can see it is only used once. And the way this PR changes it's logic, it is identical to pkg/util/gitea.go/GiteaBaseUrl function. So we should use that and get rid of this function.
punkwalker
left a comment
There was a problem hiding this comment.
The changes LGTM. I am not sure how/if these changes will impact the current assumptions that we have made in configuration of various components. Let's see how e2e test perform on this PR.
| @@ -157,7 +157,7 @@ func (r *LocalbuildReconciler) setGiteaToken(ctx context.Context, secret corev1. | |||
| // gitea URL reachable within the cluster with proper coredns config. Mainly for argocd | |||
| func giteaInternalBaseUrl(config v1alpha1.BuildCustomizationSpec) string { | |||
There was a problem hiding this comment.
I think this function is specific for returning in-cluster URL and I can see it is only used once. And the way this PR changes it's logic, it is identical to pkg/util/gitea.go/GiteaBaseUrl function. So we should use that and get rid of this function.
|
/e2e |
So e2e completed successfully. So we can merge this if you are done with changes |
Let's merge this, I'll remove the duplicate function in a future PR, this PR is already getting larger than it should be. |
Signed-off-by: Caleb Boylan <calebboylan@gmail.com>
This PR fixes a few bugs around using the
--ingress-host-nameflag,--dev-passwordflag, and--use-path-routingflag. I initially wanted to separate these but decided they make sense to keep together since they're all a little bit interdependent.Before this change using the
--ingress-host-nameflag would cause argo to fail to fetch repos from gitea due to missing SANs in the TLS cert. It also fixes a bug where we're hardcoding the domain for argocd and gitea tocnoe.localtest.mein the--dev-passwordcodepath.