Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support path based routing in core packages #164

Merged
merged 4 commits into from
Mar 4, 2024

Conversation

nabuskey
Copy link
Collaborator

@nabuskey nabuskey commented Mar 1, 2024

This introduces new flags that can be used when creating a new cluster.

  • --protocol. Either http or https. We have been using https but http is useful when we want to front it with other proxy services like codespaces.
  • --host. Specifies the host name that ingress nginx should use to route requests. When fronted by another proxy, you can set this to match the host name of the proxy, if required.
  • --use-path-routing. Allows you to have core packages under a single domain name. e.g. instead of going to https://argocd.cnoe.localtest.me, go to https://cnoe.localtest.me/argocd. This flag applies to core packages only.

nabuskey added 2 commits March 1, 2024 22:31
Signed-off-by: Manabu McCloskey <[email protected]>
@greghaynes
Copy link
Contributor

@nabuskey interesting. I dont understand what on codespaces breaks this. Is there a doc or a hint you can leave about how it is different?

@nabuskey
Copy link
Collaborator Author

nabuskey commented Mar 2, 2024

@greghaynes See my comment here: #148 (comment)

It pretty much comes down to many web based IDE environments not supporting raw TCP connection forwarding. It makes sense that browsers don't support it, but it makes running and interacting with idpbuilder in a browser impossible. I think having full support for Codespaces is quite useful for letting new users play with idpbuidler and realize the benefit of it quickly.

@greghaynes
Copy link
Contributor

Ah that makes sense! I wonder if we should just switch everything to be path based, then? I'm fine with this regardless, thanks!

@nabuskey
Copy link
Collaborator Author

nabuskey commented Mar 2, 2024

Yeah that's what I was discussing with Nima earlier. I wonder if it makes sense to switch over to path based pattern instead.

Obviously, path based approach is less similar to what a normal environments would look like. E.g. Git servers are accessible at git.mycompnay.com or auth services are available at auth.mycompany.com. Although it's entirely possible for them to be available under a single domain, it's less common from what I've seen.

@nabuskey
Copy link
Collaborator Author

nabuskey commented Mar 2, 2024

Noticing some strange behaviours in Codespaces. Will troubleshoot next week before merging.

Notes:

rules:
  - host: localhost

^ works
https://github.com/nabuskey/idpbuilder/blob/4a6bfda8415d1cdbb4e15da75727ed4a5b135530/pkg/controllers/gitrepository/controller.go#L176

./idpbuilder create --protocol http --host ${CODESPACE_NAME}-8080.${GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN} --port 8080 --use-path-routing --ingress-host-name localhost

@jessesanford
Copy link
Contributor

I like the idea of everything squishing into a path based routing sandbox and playing nice together, but we would be creating more pain for ourselves down the road as we try to adopt projects that might be incompatible. At the same time, it may be more of a burden to support both methods for each and every capability we publish. Maybe in the future we shift our default to path based and allow for individual capabilities to use their own hostnames on a case by case basis? It would be up to the capability owner to support the special case then.

@nabuskey nabuskey force-pushed the feature/path-routing branch from 3ac024b to 615bbb7 Compare March 4, 2024 20:23
@nabuskey
Copy link
Collaborator Author

nabuskey commented Mar 4, 2024

Ok it works with codespaces now. The command to run is something like:

./idpbuilder create --protocol http  \
  --host ${CODESPACE_NAME}-8080.${GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN} \
  --port 8080 --use-path-routing --ingress-host-name localhost

We need to supply the host name expected by ingress otherwise, ingress-nginx cannot route requests because the proxy created by codespaces uses localhost to talk to our service

./replace.sh ${CODESPACE_NAME}-8080.${GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN} 443

@nabuskey nabuskey merged commit 988756c into cnoe-io:main Mar 4, 2024
2 checks passed
@nimakaviani
Copy link
Contributor

super awesome to see this land. thanks for all the hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants