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

UI works on subpaths #3992

Merged
merged 20 commits into from
Sep 13, 2023
Merged

UI works on subpaths #3992

merged 20 commits into from
Sep 13, 2023

Conversation

foot
Copy link
Contributor

@foot foot commented Sep 7, 2023

Fixes #2529

What changed?

  • Enables support for running under weave-gitops under a subpath e.g. example.com/wego/ in the UI router

Why was this change made?

  • Common to expose weave-gitops dashboard under some subpath

How was this change implemented?

UI routing now checks for and respects a <base href="/foo/" /> tag if present.

If its not present everything works as it did before.

Understand

  • The UI uses react-router with the <BrowserRouter />.
  • The UI handles navigation etc itself.
  • The golang server file router serves up /index.html on just about any path except those with extensions (for css/js/images) and the api endpoints
  • This means that when you land at example.com/wego/applications/detail, index.html will be served.
    • The UI code is responsible for choosing what to show at this point.
    • From this route, we need to know where we can fetch index.js and index.css from. The correct url respectively would be /wego/index.js and /wego/index.css
  • Setting <base href="/wego/" /> tells the UI where to fetch relative resources from and where the api request can be made from (/wego/v1/api etc)

On the backend

How to set the <base /> tag in the served index.html

  • Add a cli flag to set it and add it
  • <base /> tag injected from cli
  • Mount the server handler under a subpath and add a redirect at / to point to it. e.g. / 302 -> /weave-gitops

How did you validate the change?

Test

Test to check ingress works too..

Spin up a kind cluster w/ ingress support:

  • Run ./tools/kind-with-ingress.sh
  • Uncomment ingress and additionalArgs sections in tools/helm-dev-values.yaml
  • tilt up
  • open http://localhost:30080 note the 404 from ingress controller
  • open http://localhost:30080/wego/ everything should be working okay

Release notes

Documentation Changes

@foot foot marked this pull request as ready for review September 8, 2023 12:19
@foot
Copy link
Contributor Author

foot commented Sep 8, 2023

@LappleApple I'll write up some docs for this. "How to run weave-gitops on a sub path of your ingress controller", thoughts about where it should go?

Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document how to do this?

pkg/server/route_prefix.go Outdated Show resolved Hide resolved
pkg/server/route_prefix.go Show resolved Hide resolved
pkg/server/route_prefix.go Show resolved Hide resolved
@bigkevmcd
Copy link
Contributor

Looks good, but we probably need some Helm and documentation!

Co-authored-by: Kevin McDermott <[email protected]>
Copy link

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Love the UI tests and extra helper script.

tools/kind-with-ingress.sh Outdated Show resolved Hide resolved
@LappleApple
Copy link
Contributor

@foot Let's chat next week about docs-y things :)

Copy link
Contributor

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it locally, and it works, let's get the exported functions either not-exported, if they don't need to be, or documented.

Otherwise, looks good.

website/docs/open-source/getting-started/configuration.mdx Outdated Show resolved Hide resolved
pkg/server/route_prefix.go Show resolved Hide resolved
pkg/server/route_prefix.go Show resolved Hide resolved
foot and others added 5 commits September 12, 2023 10:34
- Helps the user find what they are after
The UI is built w/ relative asset links. If you land on a subpage
e.g. /git-repo/details, then the base tag must still be present to
indicate the relative js/css should be loaded from "/" and not from
"/git-repo/details"
- Add Makefile helper too
Copy link

@heubeck heubeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

@foot foot merged commit e03898d into main Sep 13, 2023
26 checks passed
@foot foot deleted the ui-works-on-subpaths branch September 13, 2023 07:10
@gre9ory
Copy link

gre9ory commented Sep 13, 2023

@foot
I can confirm it works in my setup too after building an image of the repo with the route-prefix support and setting up the Helm chart to use that dev image.

Still misses a new proper image and an updated Helm chart to reference that image to be usable by end users if not mistaken?

Anyhow, thank you for this - in my view - important feature!

@bigkevmcd
Copy link
Contributor

@gre9ory it should be in the next release

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.

How to configure the app and Ingress with different base path
6 participants