IAM-951 fix broken href for non-empty context-path #426
Conversation
There was a problem hiding this comment.
I don't really love this, but if it works, it works.
I have one question though:
If we are going to start templating/injecting the returned html, why don't we set the html base tag instead of editing paths? This seems much safer and extendable. Something like this:
diff --git a/pkg/ui/handlers.go b/pkg/ui/handlers.go
index b9b0a3e..842c9d8 100644
--- a/pkg/ui/handlers.go
+++ b/pkg/ui/handlers.go
@@ -102,10 +102,7 @@ func (a *API) uiFiles(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")
w.WriteHeader(http.StatusOK)
- normContextPath := a.contextPath
- if !strings.HasSuffix(normContextPath, "/") {
- normContextPath += "/"
- }
+ normContextPath := r.URL.JoinPath(a.contextPath, UIPrefix, "/")
err = t.Execute(w, normContextPath)
if err != nil {
diff --git a/ui/index.html b/ui/index.html
index 4dd324f..ea803e4 100644
--- a/ui/index.html
+++ b/ui/index.html
@@ -4,6 +4,7 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
+ <base href="{{ . }}" target="_blank">
<title>Identity platform</title>
<link rel="shortcut icon" href="https://assets.ubuntu.com/v1/49a1a858-favicon-32x32.png" type="image/x-icon" />
BarcoMasile
force-pushed
the
IAM-951-fix-fe-routes-hrefs
branch
from
d23879d
to
3c51af4
Compare
I think it's a good idea honestly, but from my notes from the meeting with @huwshimi we explicitly chose not to use the base path (I may have misinterpreted in this case though). If it works, I am open to it and actually think it's better (as you said). |
I think the previous issue with base was that it didn't help us resolve the issue with a |
|
The main problem was that url's with more path parts were not allowed. E.g. |
BarcoMasile
force-pushed
the
IAM-951-fix-fe-routes-hrefs
branch
from
e0e031c
to
81f8a9c
Compare
|
Just rebased on main. |
It looks like this removed my changes. Would you like me to push them again? |
@huwshimi |
Done. |
|
Just to be clear, @huwshimi I am curious why you think that the current approach is cleaner than the one proposed at #426 (review) |
(as discussed elsewhere) this is using |
Description
Switch to html/template for rendering context path dynamically for
index.htmlThis way when serving Admin UI under a context path we don't break the UI.
UI still expects to be under
/context-path(if present)/ui.N.B.
We still need web team intervention to make the produced
index.htmlto have the following prefix for its assets:{{ . }}uiExample
This PR was manually tested simulating a fake context-path under which the UI could be served. See the example with
with-context-pathused as a fake path. The index html file GETs the right path for the assets.Fixes
Fixes #350