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

[FIX] Issue 904 - Vue app doesn't take into account uri_prefix #967

Merged
merged 2 commits into from
Feb 24, 2024

Conversation

Nactik
Copy link
Contributor

@Nactik Nactik commented Feb 6, 2024

Fix #904 - uri_prefix not taken into account for the UI

Hey there, here's a quick PR to fix the way tegola UI is served when sitting behind a reverse proxy. I still need to implement and fix test but the idea is here (I will implement them asap)

Changes:

  • Added a new setting proxy_protocol (I can change it if you think of a better name) in the config file (config.toml) to set a custom protocol that will be used to generate the URLs included in the capabilities endpoint responses. (Useful when working under a reverse proxy and when the X-Forwarded-Proto is not a solution - Fix Specify client protocol in config #134 )
  • When serving embedded files under a reverse proxy, strip the uri_prefix from the request (server/viewer_embed.go)
  • Adjust vite settings to include static file using a relative path

@Nactik Nactik changed the title [FIX] Issue 904 - Vue app not taken into account uri_prefix [FIX] Issue 904 - Vue app doesn't into account uri_prefix Feb 6, 2024
@Nactik Nactik changed the title [FIX] Issue 904 - Vue app doesn't into account uri_prefix [FIX] Issue 904 - Vue app doesn't take into account uri_prefix Feb 6, 2024
@coveralls
Copy link

coveralls commented Feb 6, 2024

Pull Request Test Coverage Report for Build c9799a7f2-PR-967

Details

  • -3 of 10 (70.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.006%) to 46.861%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/tegola/cmd/server.go 0 3 0.0%
Totals Coverage Status
Change from base Build b7ad33baf: 0.006%
Covered Lines: 6575
Relevant Lines: 14031

💛 - Coveralls

@ARolek ARolek requested a review from mapl February 6, 2024 21:23
@ARolek
Copy link
Member

ARolek commented Feb 6, 2024

@Nactik nice! LMK when this is out of draft and I will give it a review.

@Nactik Nactik marked this pull request as ready for review February 8, 2024 17:24
@Nactik
Copy link
Contributor Author

Nactik commented Feb 8, 2024

@ARolek
It should be good now :)

Copy link
Member

@iwpnd iwpnd left a comment

Choose a reason for hiding this comment

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

Nice, lgtm. 👍

was not aware http.StripPrefix was a thing.

Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

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

This looks excellent! I left a couple of nits line, but no blockers. Can you please squash your commits into a single commit and then I will get this merged in?

🙏

@@ -477,6 +479,163 @@ func TestParse(t *testing.T) {
expected: config.Config{},
expectedErr: env.ErrEnvVar("I_AM_MISSING"),
},
"4 test empty proxy_protocol": {
config: `
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should move these inline configs to external files. it sure would make this easier to look it ;-)

config/config_test.go Show resolved Hide resolved
@Nactik Nactik force-pushed the issue-904-fix_uri_prefix_for_ui branch from c24f900 to 9d0020a Compare February 18, 2024 14:16
Remove prefix from static file to serve behind proxy

Config vite to serve static file as embedded (Relative path)

Update lock file

Add Scheme tests

Add test for BuildCapabilitiesUrl

Add comment on ProxyProtol settings

Convert interface{} to any

Reset ProxyProtocol when not set
@Nactik Nactik force-pushed the issue-904-fix_uri_prefix_for_ui branch from 7217051 to 387be73 Compare February 19, 2024 21:27
@gdey gdey removed their request for review February 22, 2024 07:50
@ARolek ARolek merged commit 387be73 into go-spatial:master Feb 24, 2024
10 of 11 checks passed
@ARolek
Copy link
Member

ARolek commented Feb 24, 2024

Just merged this into master. Great for contribution! Thank you.

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