Skip to content

feat: add fixed basepath /helm-dashboard #606

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add fixed basepath /helm-dashboard #606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/release.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@ jobs:
build-args: VER=${{ needs.pre_release.outputs.release_tag }}
platforms: linux/amd64,linux/arm64

- name: Build and push with base path
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it absolutely necessary to have a separate image for that?
From the first look it seems that just setting env variable should trigger the behavior. We have the ways to communicate configured base path from backend to frontend (via StatusInfo struct).

Copy link
Author

Choose a reason for hiding this comment

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

I'm +1 on having only one image and have configurable base path. I could not do it that way becasue VITE_BASE_PATH is only accesible at build time AFAIK. Once the image is built, we can only customize the base path for the backend via env var or as a flag as you suggest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After reading this doc part: https://vite.dev/guide/build.html#relative-base
I have a feeling that we could just use

export default defineConfig({
  base: './' 
})

And it would do the trick universally. Did you try investigating that direction?

uses: docker/build-push-action@v4
if: github.event_name != 'pull_request'
with:
context: .
push: ${{ github.event_name != 'pull_request' }}
tags: komodorio/helm-dashboard:${{ needs.pre_release.outputs.release_tag }}-basepath,komodorio/helm-dashboard:latest-basepath
labels: ${{ steps.meta.outputs.labels }}
build-args: VER=${{ needs.pre_release.outputs.release_tag }},VITE_BASE_PATH=/helm-dashboard
platforms: linux/amd64,linux/arm64

publish_chart:
runs-on: ubuntu-latest
needs: [ image, pre_release ]
Expand Down
3 changes: 2 additions & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ FROM node:latest as frontend
WORKDIR /build

COPY frontend ./
ARG VITE_BASE_PATH

RUN npm i && npm run build

Expand Down Expand Up @@ -49,4 +50,4 @@ COPY --from=builder /build/src/bin/dashboard /bin/helm-dashboard

ENTRYPOINT ["/bin/helm-dashboard", "--no-browser", "--bind=0.0.0.0", "--port=8080"]

# docker build . -t komodorio/helm-dashboard:0.0.0 && kind load docker-image komodorio/helm-dashboard:0.0.0
# docker build . -t komodorio/helm-dashboard:0.0.0 && kind load docker-image komodorio/helm-dashboard:0.0.0
8 changes: 4 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ pull: ; $(info $(M) Pulling source...) @
@git pull

.PHONY: build_go
build_go: $(BIN) ; $(info $(M) Building GO...) @ ## Build program binary
build_go: $(BIN) ; $(info $(M) Building GO...) @ ## Build program binary
go build \
-ldflags '-X main.version=$(VERSION) -X main.buildDate=$(DATE)' \
-o bin/dashboard .
-o bin/dashboard . ;

.PHONY: build_ui
build_ui: $(BIN) ; $(info $(M) Building UI...) @ ## Build program binary
cd frontend && npm i && npm run build && cd ..
cd frontend && npm i && npm run build && cd .. ;

.PHONY: build
build: build_ui build_go ; $(info $(M) Building executable...) @ ## Build program binary

.PHONY: debug
debug: ; $(info $(M) Running dashboard in debug mode...) @
@DEBUG=1 ./bin/dashboard
@DEBUG=1 ./bin/dashboard
5 changes: 4 additions & 1 deletion charts/helm-dashboard/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ Return the proper image name
{{- define "helm-dashboard.image" -}}
{{- $image := .Values.image -}}
{{- $tag := default .Chart.AppVersion $image.tag -}}
{{- if .Values.image.basePath }}
{{- $tag = printf "%s%s" $tag "-basepath" -}}
{{- end -}}
{{- $_ := set $image "tag" $tag -}}
{{ include "common.images.image" (dict "imageRoot" $_ "global" .Values.global) }}
{{- end -}}
Expand All @@ -73,4 +76,4 @@ Return the proper image name
*/}}
{{- define "test.image" -}}
{{ include "common.images.image" (dict "imageRoot" .Values.testImage "global" .Values.global) }}
{{- end -}}
{{- end -}}
4 changes: 4 additions & 0 deletions charts/helm-dashboard/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ spec:
value: /opt/dashboard/helm/data
- name: DEBUG
value: {{- ternary " '1'" "" .Values.debug }}
{{- if .Values.image.basePath }}
- name: HD_BASE_PATH
value: /helm-dashboard
{{end}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assuming base path is string, setting VITE_BASE_PATH here would be better chart design, not requiring separate image

{{- if .Values.dashboard.namespace }}
- name: HELM_NAMESPACE
value: {{ .Values.dashboard.namespace }}
Expand Down
2 changes: 2 additions & 0 deletions charts/helm-dashboard/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ image:
# Specifies the exact image digest to pull.
digest: ""
imagePullSecrets: []
# Flag for using an image with fixed base path /helm-dashboard
basePath: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this parameter string instead of boolean? This way user would be able to set any base path he wants, not only helm-dashboard. More flexible.


nameOverride: ""
fullnameOverride: ""
Expand Down
30 changes: 24 additions & 6 deletions frontend/src/API/apiService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,18 @@ interface ClustersResponse {
}
class ApiService {
currentCluster = "";
constructor(protected readonly isMockMode: boolean = false) {}
private readonly basePath: string;
constructor(protected readonly isMockMode: boolean = false) {
const fromEnv = (import.meta as any).env?.VITE_BASE_PATH as string | undefined;
// In production, Vite injects import.meta.env.BASE_URL. Prefer explicit VITE_BASE_PATH if provided.
const viteBase = (import.meta as any).env?.BASE_URL as string | undefined;
const computed = (fromEnv ?? viteBase ?? "/").trim();
// Normalize to leading slash, no trailing slash (except root)
let normalized = computed;
if (!normalized.startsWith("/")) normalized = "/" + normalized;
if (normalized !== "/") normalized = normalized.replace(/\/+$/g, "");
this.basePath = normalized;
}

setCluster = (cluster: string) => {
this.currentCluster = cluster;
Expand All @@ -28,14 +39,20 @@ class ApiService {
): Promise<T> {
let response;

const isAbsolute = /^https?:\/\//.test(url);
const fullUrl = isAbsolute
? url
: this.basePath === "/"
? url
: `${this.basePath}${url.startsWith("/") ? url : `/${url}`}`;
if (this.currentCluster) {
const headers = new Headers(options?.headers);
if (!headers.has("X-Kubecontext")) {
headers.set("X-Kubecontext", this.currentCluster);
}
response = await fetch(url, { ...options, headers });
response = await fetch(fullUrl, { ...options, headers });
} else {
response = await fetch(url, options);
response = await fetch(fullUrl, options);
}

if (!response.ok) {
Expand All @@ -55,7 +72,7 @@ class ApiService {
}

getToolVersion = async () => {
const response = await fetch("/status");
const response = await this.fetchWithDefaults("/status");
const data = await response.json();
return data;
};
Expand All @@ -73,8 +90,9 @@ class ApiService {
};

getClusters = async () => {
const response = await fetch("/api/k8s/contexts");
const data = (await response.json()) as ClustersResponse[];
const data = await this.fetchWithDefaults<ClustersResponse[]>(
"/api/k8s/contexts"
);
return data;
};

Expand Down
26 changes: 20 additions & 6 deletions frontend/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export default defineConfig(({ mode }) => {
const env = loadEnv(mode, process.cwd(), "");
const port = env.VITE_SERVER_PORT || 8080;
return {
base: env.VITE_BASE_PATH || "/",
plugins: [
react(),
viteStaticCopy({
Expand All @@ -31,12 +32,25 @@ export default defineConfig(({ mode }) => {
emptyOutDir: true,
},
server: {
proxy: {
"^/api/.*": `http://127.0.0.1:${port}`,
"^/status*": `http://127.0.0.1:${port}`,
"^/diff*": `http://127.0.0.1:${port}`,
"^/static*": `http://127.0.0.1:${port}`,
},
proxy: (() => {
const base = (env.VITE_BASE_PATH || "/").replace(/\/$/, "");
const target = `http://127.0.0.1:${port}`;
// Support both with and without base for local dev convenience
return {
"^/api/.*": target,
"^/status*": target,
"^/diff*": target,
"^/static*": target,
...(base
? {
[`^${base}/api/.*`]: target,
[`^${base}/status*`]: target,
[`^${base}/diff*`]: target,
[`^${base}/static*`]: target,
}
: {}),
} as Record<string, string>;
})(),
},
};
});
30 changes: 24 additions & 6 deletions pkg/dashboard/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"
"html"
"net/http"
"os"
"path"
"strings"

"github.com/gin-gonic/gin"
"github.com/komodorio/helm-dashboard/v2/pkg/dashboard/handlers"
Expand Down Expand Up @@ -99,13 +101,27 @@ func NewRouter(abortWeb context.CancelFunc, data *objects.DataLayer, debug bool)
api.Use(allowCORS)
}

configureStatic(api)
configureRoutes(abortWeb, data, api)
// Determine base path prefix for mounting the app under a subpath
Copy link
Collaborator

Choose a reason for hiding this comment

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

see my comment in server.go - the code would be simpler if the base path value is just passed to NewRouter()

basePath := strings.TrimSpace(os.Getenv("HD_BASE_PATH"))
if basePath == "/" {
basePath = ""
}
if basePath != "" {
if !strings.HasPrefix(basePath, "/") {
basePath = "/" + basePath
}
basePath = strings.TrimRight(basePath, "/")
}

root := api.Group(basePath)

configureStatic(root)
configureRoutes(abortWeb, data, root)

return api
}

func configureRoutes(abortWeb context.CancelFunc, data *objects.DataLayer, api *gin.Engine) {
func configureRoutes(abortWeb context.CancelFunc, data *objects.DataLayer, api *gin.RouterGroup) {
// server shutdown handler
api.DELETE("/", func(c *gin.Context) {
abortWeb()
Expand Down Expand Up @@ -188,18 +204,20 @@ func configureKubectls(api *gin.RouterGroup, data *objects.DataLayer) {
api.GET("/:kind/list", h.GetNameSpaces)
}

func configureStatic(api *gin.Engine) {
func configureStatic(api *gin.RouterGroup) {
fs := http.FS(frontend.StaticFS)

api.GET("/", func(c *gin.Context) {
c.FileFromFS("/dist/", fs)
})

api.GET("/assets/*filepath", func(c *gin.Context) {
c.FileFromFS(path.Join("dist", c.Request.URL.Path), fs)
fp := strings.TrimPrefix(c.Param("filepath"), "/")
c.FileFromFS(path.Join("dist", "assets", fp), fs)
})

api.GET("/static/*filepath", func(c *gin.Context) {
c.FileFromFS(path.Join("dist", c.Request.URL.Path), fs)
fp := strings.TrimPrefix(c.Param("filepath"), "/")
c.FileFromFS(path.Join("dist", "static", fp), fs)
})
}
6 changes: 4 additions & 2 deletions pkg/dashboard/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ func TestConfigureStatic(t *testing.T) {

// Create an API Engine
api := gin.Default()
root := api.Group("/")

// Configure static routes
configureStatic(api)
configureStatic(root)

// Start the server
api.ServeHTTP(w, req)
Expand All @@ -109,6 +110,7 @@ func TestConfigureRoutes(t *testing.T) {

// Create a API Engine
api := gin.Default()
root := api.Group("/")

// Required arguments for route configuration
abortWeb := func() {}
Expand All @@ -119,7 +121,7 @@ func TestConfigureRoutes(t *testing.T) {
}

// Configure routes to API engine
configureRoutes(abortWeb, data, api)
configureRoutes(abortWeb, data, root)

// Start the server
api.ServeHTTP(w, req)
Expand Down
9 changes: 8 additions & 1 deletion pkg/dashboard/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ func (s *Server) StartServer(ctx context.Context, cancel context.CancelFunc) (st
api := NewRouter(cancel, data, s.Debug)
done := s.startBackgroundServer(api, ctx)

return "http://" + s.Address, done, nil
basePath := strings.TrimSpace(os.Getenv("HD_BASE_PATH"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of calculating base path here, make it a field in Server struct and pass the value from main.go. Because this option looks exactly like server start config option. This way we have single place that calculates the option value (main.go) and then others use it from field/variable.

Value of this option would then also go into NewRouter call here.

if basePath == "/" {
basePath = ""
}
if basePath != "" && !strings.HasPrefix(basePath, "/") {
basePath = "/" + basePath
}
return "http://" + s.Address + basePath + "/", done, nil
}

func (s *Server) detectClusterMode(data *objects.DataLayer) error {
Expand Down