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.

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

feat: Support configurable base path for server #714

Merged
merged 2 commits into from
Nov 18, 2024
Merged

feat: Support configurable base path for server #714

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
453f0d2
feat: Support configurable base path for server
chrishoage Nov 16, 2024
cd80c43
fixup! feat: Support configurable base path for server
chrishoage Nov 17, 2024
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
2 changes: 2 additions & 0 deletions docs/source/config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Server Configuration
~~~~~~~~~~~~~~~~~~
- ``DAGU_HOST`` (``127.0.0.1``): Server binding host
- ``DAGU_PORT`` (``8080``): Server binding port
- ``DAGU_BASE_PATH`` (``""``): Base path to serve the application
yohamta marked this conversation as resolved.
Show resolved Hide resolved
- ``DAGU_TZ`` (``""``): Server timezone (default: system timezone)
- ``DAGU_CERT_FILE``: SSL certificate file path
- ``DAGU_KEY_FILE``: SSL key file path
Expand Down Expand Up @@ -59,6 +60,7 @@ Create ``admin.yaml`` in ``$HOME/.config/dagu/`` to override default settings. B
# Server Configuration
host: "127.0.0.1" # Web UI hostname
port: 8080 # Web UI port
basePath: "" # Base path to serve the application
tz: "Asia/Tokyo" # Timezone (e.g., "America/New_York")

# Directory Configuration
Expand Down
3 changes: 3 additions & 0 deletions internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type Config struct {
IsAuthToken bool // Enable auth token for API
AuthToken string // Auth token for API
LatestStatusToday bool // Show latest status today or the latest status
BasePath string // Base path for the server
APIBaseURL string // Base URL for API
Debug bool // Enable debug mode (verbose logging)
LogFormat string // Log format
Expand Down Expand Up @@ -168,6 +169,7 @@ func setupViper() error {
viper.SetDefault("host", "127.0.0.1")
viper.SetDefault("port", "8080")
viper.SetDefault("navbarTitle", "Dagu")
viper.SetDefault("basePath", "")
viper.SetDefault("apiBaseURL", "/api/v1")

// Set executable path
Expand Down Expand Up @@ -198,6 +200,7 @@ func bindEnvs() {
_ = viper.BindEnv("logEncodingCharset", "DAGU_LOG_ENCODING_CHARSET")
_ = viper.BindEnv("navbarColor", "DAGU_NAVBAR_COLOR")
_ = viper.BindEnv("navbarTitle", "DAGU_NAVBAR_TITLE")
_ = viper.BindEnv("basePath", "DAGU_BASE_PATH")
_ = viper.BindEnv("apiBaseURL", "DAGU_API_BASE_URL")
_ = viper.BindEnv("tz", "DAGU_TZ")

Expand Down
1 change: 1 addition & 0 deletions internal/frontend/frontend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func New(cfg *config.Config, lg logger.Logger, cli client.Client) *server.Server
AssetsFS: assetsFS,
NavbarColor: cfg.NavbarColor,
NavbarTitle: cfg.NavbarTitle,
BasePath: cfg.BasePath,
APIBaseURL: cfg.APIBaseURL,
TimeZone: cfg.TZ,
}
Expand Down
15 changes: 12 additions & 3 deletions internal/frontend/middleware/global.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package middleware
import (
"context"
"net/http"
"path"
"strings"

"github.com/dagu-org/dagu/internal/logger"
Expand Down Expand Up @@ -72,13 +73,15 @@ var (
authBasic *AuthBasic
authToken *AuthToken
appLogger logger.Logger
basePath string
)

type Options struct {
Handler http.Handler
AuthBasic *AuthBasic
AuthToken *AuthToken
Logger logger.Logger
BasePath string
}

type AuthBasic struct {
Expand All @@ -95,15 +98,21 @@ func Setup(opts *Options) {
authBasic = opts.AuthBasic
authToken = opts.AuthToken
appLogger = opts.Logger
basePath = opts.BasePath
}

func prefixChecker(next http.Handler) http.Handler {
return http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/api") {
next.ServeHTTP(w, r)
if basePath != "" && r.URL.Path == "/" {
http.Redirect(w, r, basePath, http.StatusSeeOther)
return
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the root of the server is hit and we have a base path configured redirect to that base path for convenience.

Copy link
Collaborator

@yohamta yohamta Nov 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I've tested this locally with an nginx reverse proxy (configuration below) but encountered an issue when accessing via the proxy path http://localhost/dagu/. It seems the path prefix check might be the cause. The following code, however, works as expected:

if strings.HasPrefix(r.URL.Path, "/api") {
    next.ServeHTTP(w, r)
} else {
    defaultHandler.ServeHTTP(w, r)
}

Would you mind taking a look when you have a chance?

Test setup

server {
    listen       80;
    server_name  localhost;
    location /dagu/ {
        proxy_pass http://localhost:8080/;
        proxy_set_header Host $host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;
    }
}

~/.dagu/admin.cfg

basePath: /dagu/
port: 8080

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is the trailing slash. I was only testing with /dagu and missed this case.

I have made the BasePath more robust

  • Normalize the value in config.go so that all parts of the app can be sure it has the same shape
  • changed prefixChecker to strip the prefix first, then do the strings.HasPrefix check in a HandlerFunc to not have to worry about the prefix.

I tested with

  • no basepath set at all
  • basepath set as an empty string
  • basePath set as /
  • basePath set as /dagu
  • basePath set as /dagu/
  • basePath set as /dagu/foo

I believe this should be good.

I have pushed up the fix as a !fixup commit to make it easier to review. This can be "squashed" when merging or I can squash it before this is ready to merge.


if strings.HasPrefix(r.URL.Path, path.Join(basePath, "/api")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a better solution to path.Join here? It doesn't feel robust.

It really feels like /api should be a handler which is hung off of http.StripPrefix(basePath, next) such that any route registered under it would be handled by /api

I wasn't sure of the consequences of such a change however, so I just updated the string comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #714 (comment) this has been improved to not rely on path.join for this prefix check

http.StripPrefix(basePath, next).ServeHTTP(w, r)
} else {
defaultHandler.ServeHTTP(w, r)
http.StripPrefix(basePath, defaultHandler).ServeHTTP(w, r)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

http.StripPrefix seemed like the cleanest way to allow all existing routes to work as before.

This is my first time writing Go so if there's a more idiomatic way of doing this let me know!

}
})
}
Expand Down
7 changes: 5 additions & 2 deletions internal/frontend/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type NewServerArgs struct {
// Configuration for the frontend
NavbarColor string
NavbarTitle string
BasePath string
APIBaseURL string
TimeZone string
}
Expand Down Expand Up @@ -92,6 +93,7 @@ func New(params NewServerArgs) *Server {
funcsConfig: funcsConfig{
NavbarColor: params.NavbarColor,
NavbarTitle: params.NavbarTitle,
BasePath: params.BasePath,
APIBaseURL: params.APIBaseURL,
TZ: params.TimeZone,
},
Expand All @@ -110,8 +112,9 @@ func (svr *Server) Shutdown() {

func (svr *Server) Serve(ctx context.Context) (err error) {
middlewareOptions := &pkgmiddleware.Options{
Handler: svr.defaultRoutes(chi.NewRouter()),
Logger: svr.logger,
Handler: svr.defaultRoutes(chi.NewRouter()),
Logger: svr.logger,
BasePath: svr.funcsConfig.BasePath,
}
if svr.authToken != nil {
middlewareOptions.AuthToken = &pkgmiddleware.AuthToken{
Expand Down
7 changes: 6 additions & 1 deletion internal/frontend/server/templates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"bytes"
"io"
"net/http"
"path"
"path/filepath"
"text/template"

Expand Down Expand Up @@ -56,6 +57,7 @@ func (srv *Server) useTemplate(
type funcsConfig struct {
NavbarColor string
NavbarTitle string
BasePath string
APIBaseURL string
TZ string
}
Expand All @@ -78,8 +80,11 @@ func defaultFunctions(cfg funcsConfig) template.FuncMap {
"navbarTitle": func() string {
return cfg.NavbarTitle
},
"basePath": func() string {
return cfg.BasePath
},
"apiURL": func() string {
return cfg.APIBaseURL
return path.Join(cfg.BasePath, cfg.APIBaseURL)
},
"tz": func() string {
return cfg.TZ
Expand Down
3 changes: 2 additions & 1 deletion internal/frontend/templates/base.gohtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@
function getConfig() {
return {
apiURL: "{{ apiURL }}",
basePath: "{{ basePath }}",
title: "{{ navbarTitle }}",
navbarColor: "{{ navbarColor }}",
version: "{{ version }}",
tz: "{{ tz }}",
};
}
</script>
<script defer="defer" src="/assets/bundle.js?v={{ version }}"></script>
<script defer="defer" src="{{ basePath }}/assets/bundle.js?v={{ version }}"></script>
</head>
<body>
{{template "content" .}}
Expand Down
2 changes: 1 addition & 1 deletion ui/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function App({ config }: Props) {
>
<ConfigContext.Provider value={config}>
<UserPreferencesProvider>
<BrowserRouter>
<BrowserRouter basename={config.basePath}>
<Layout {...config}>
<Routes>
<Route path="/" element={<Dashboard />} />
Expand Down
1 change: 1 addition & 0 deletions ui/src/contexts/ConfigContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createContext, useContext } from 'react';

export type Config = {
apiURL: string;
basePath: string;
title: string;
navbarColor: string;
tz: string;
Expand Down
2 changes: 1 addition & 1 deletion ui/webpack.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module.exports = merge(common, {
output: {
filename: 'bundle.js',
path: path.resolve(__dirname, 'dist'),
publicPath: '/assets/',
publicPath: 'auto',
yohamta marked this conversation as resolved.
Show resolved Hide resolved
clean: true,
},
});
Loading