Skip to content

Commit 526d290

Browse files
committed
Merge remote-tracking branch 'origin/main' into codex/issue-8487-license-spdx-compliance
Signed-off-by: Jonah Kowall <jkowall@kowall.net> # Conflicts: # examples/reverse-proxy/docker-compose.yml
2 parents 86b331b + 486ea50 commit 526d290

18 files changed

Lines changed: 939 additions & 69 deletions

File tree

.github/workflows/ci-e2e-all.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,6 @@ jobs:
4545

4646
tailsampling:
4747
uses: ./.github/workflows/ci-e2e-tailsampling.yml
48+
49+
ui-reverse-proxy:
50+
uses: ./.github/workflows/ci-e2e-ui-reverse-proxy.yml
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Copyright (c) 2026 The Jaeger Authors.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
# E2E integration tests for Jaeger UI served behind a reverse proxy (ADR-009).
5+
# Validates three use cases in a single job:
6+
#
7+
# UC-1: proxy forwards the URL prefix unchanged to Jaeger
8+
# (exercises the existing examples/reverse-proxy/ setup)
9+
# UC-2: single Jaeger pod served under two different external prefixes
10+
# simultaneously, with the proxy stripping the prefix on one path
11+
# UC-3: proxy rewrites an external prefix to a different internal prefix
12+
# (the case that motivated ADR-009 / issue #5157)
13+
14+
name: CIT UI Reverse Proxy
15+
16+
on:
17+
workflow_call:
18+
19+
# See https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions
20+
permissions:
21+
contents: read
22+
23+
jobs:
24+
ui-reverse-proxy:
25+
runs-on: ubuntu-latest
26+
27+
steps:
28+
- name: Harden Runner
29+
uses: step-security/harden-runner@a5ad31d6a139d249332a2605b85202e8c0b78450 # v2.19.1
30+
with:
31+
egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
32+
33+
- uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0
34+
with:
35+
submodules: true
36+
37+
- name: Fetch git tags
38+
run: git fetch --prune --unshallow --tags
39+
40+
- uses: ./.github/actions/setup-go
41+
with:
42+
go-version: 1.26.x
43+
44+
- name: Setup Node.js version
45+
uses: ./.github/actions/setup-node.js
46+
47+
- name: Run UI reverse-proxy integration tests (UC-1, UC-2, UC-3)
48+
run: bash scripts/e2e/ui-reverse-proxy.sh

cmd/jaeger/internal/extension/jaegerquery/config.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
package jaegerquery
55

66
import (
7+
"fmt"
8+
"path"
9+
"strings"
10+
711
"github.com/asaskevich/govalidator"
812
"go.opentelemetry.io/collector/confmap/xconfmap"
913

@@ -29,6 +33,23 @@ type Storage struct {
2933
}
3034

3135
func (cfg *Config) Validate() error {
36+
// Normalize BasePath once so all downstream consumers see a clean value.
37+
bp := cfg.BasePath
38+
if bp != "" && bp != "/" {
39+
if !strings.HasPrefix(bp, "/") {
40+
return fmt.Errorf("invalid base_path %q: must start with '/'", bp)
41+
}
42+
// path.Clean collapses duplicate slashes and resolves . / .. segments,
43+
// producing a canonical path without a trailing slash.
44+
clean := path.Clean(bp)
45+
if clean != bp && clean+"/" != bp {
46+
// The value had duplicate slashes or traversal segments beyond a
47+
// single trailing slash — reject it so callers don't silently get
48+
// a different path than they intended.
49+
return fmt.Errorf("invalid base_path %q: must not contain dot segments, path traversal, or duplicate slashes (normalized: %q)", bp, clean)
50+
}
51+
cfg.BasePath = clean
52+
}
3253
_, err := govalidator.ValidateStruct(cfg)
3354
return err
3455
}

cmd/jaeger/internal/extension/jaegerquery/config_test.go

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@ import (
88

99
"github.com/stretchr/testify/assert"
1010
"github.com/stretchr/testify/require"
11+
12+
queryapp "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerquery/internal"
1113
)
1214

1315
func Test_Validate(t *testing.T) {
1416
tests := []struct {
15-
name string
16-
config *Config
17-
expectedErr string
17+
name string
18+
config *Config
19+
expectedErr string
20+
expectedBasePath string
1821
}{
1922
{
2023
name: "Empty config",
@@ -30,15 +33,59 @@ func Test_Validate(t *testing.T) {
3033
},
3134
expectedErr: "",
3235
},
36+
{
37+
name: "BasePath no leading slash",
38+
config: &Config{
39+
QueryOptions: queryapp.QueryOptions{BasePath: "no-leading-slash"},
40+
Storage: Storage{TracesPrimary: "some-storage"},
41+
},
42+
expectedErr: "invalid base_path",
43+
},
44+
{
45+
name: "BasePath trailing slash normalized",
46+
config: &Config{
47+
QueryOptions: queryapp.QueryOptions{BasePath: "/jaeger/"},
48+
Storage: Storage{TracesPrimary: "some-storage"},
49+
},
50+
expectedErr: "",
51+
expectedBasePath: "/jaeger",
52+
},
53+
{
54+
name: "BasePath duplicate slashes rejected",
55+
config: &Config{
56+
QueryOptions: queryapp.QueryOptions{BasePath: "/jaeger//foo"},
57+
Storage: Storage{TracesPrimary: "some-storage"},
58+
},
59+
expectedErr: "invalid base_path",
60+
},
61+
{
62+
name: "BasePath double leading slash rejected",
63+
config: &Config{
64+
QueryOptions: queryapp.QueryOptions{BasePath: "//jaeger"},
65+
Storage: Storage{TracesPrimary: "some-storage"},
66+
},
67+
expectedErr: "invalid base_path",
68+
},
69+
{
70+
name: "BasePath path traversal rejected",
71+
config: &Config{
72+
QueryOptions: queryapp.QueryOptions{BasePath: "/jaeger/../other"},
73+
Storage: Storage{TracesPrimary: "some-storage"},
74+
},
75+
expectedErr: "invalid base_path",
76+
},
3377
}
3478

3579
for _, tt := range tests {
3680
t.Run(tt.name, func(t *testing.T) {
3781
err := tt.config.Validate()
3882
if tt.expectedErr == "" {
3983
require.NoError(t, err)
84+
if tt.expectedBasePath != "" {
85+
assert.Equal(t, tt.expectedBasePath, tt.config.BasePath)
86+
}
4087
} else {
41-
assert.Equal(t, tt.expectedErr, err.Error())
88+
assert.ErrorContains(t, err, tt.expectedErr)
4289
}
4390
})
4491
}

cmd/jaeger/internal/extension/jaegerquery/internal/static_handler.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ var (
3030
configJsPattern = regexp.MustCompile(`(?im)^\s*//\s*JAEGER_CONFIG_JS.*\n.*`)
3131
versionPattern = regexp.MustCompile("JAEGER_VERSION *= *DEFAULT_VERSION;")
3232
compabilityPattern = regexp.MustCompile("JAEGER_STORAGE_CAPABILITIES *= *DEFAULT_STORAGE_CAPABILITIES;")
33-
basePathPattern = regexp.MustCompile(`<base href="/"`) // Note: tag is not closed
3433
)
3534

3635
// RegisterStaticHandler adds handler for static assets to the router.
@@ -119,16 +118,8 @@ func (sH *StaticAssetsHandler) loadAndEnrichIndexHTML(open func(string) (http.Fi
119118
versionJSON, _ := json.Marshal(version.Get())
120119
versionString := fmt.Sprintf("JAEGER_VERSION = %s;", string(versionJSON))
121120
indexBytes = versionPattern.ReplaceAll(indexBytes, []byte(versionString))
122-
// replace base path
123-
if sH.options.BasePath == "" {
124-
sH.options.BasePath = "/"
125-
}
126-
if sH.options.BasePath != "/" {
127-
if !strings.HasPrefix(sH.options.BasePath, "/") || strings.HasSuffix(sH.options.BasePath, "/") {
128-
return nil, fmt.Errorf("invalid base path '%s'. Must start but not end with a slash '/', e.g. '/jaeger/ui'", sH.options.BasePath)
129-
}
130-
indexBytes = basePathPattern.ReplaceAll(indexBytes, fmt.Appendf(nil, `<base href="%s/"`, sH.options.BasePath))
131-
}
121+
// The <base href> is no longer injected here. The UI detects its own mount-point
122+
// prefix at page-load time via an inline script in index.html (see ADR-009).
132123

133124
return indexBytes, nil
134125
}

cmd/jaeger/internal/extension/jaegerquery/internal/static_handler_test.go

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,13 @@ func TestRegisterStaticHandler(t *testing.T) {
6565
archiveStorage bool // archive storage enabled?
6666
metricsStorage bool // metrics storage enabled?
6767
logAccess bool
68-
expectedBaseHTML string // substring to match in the home page
6968
UIConfigPath string // path to UI config
7069
expectedUIConfig string // expected UI config
7170
expectedStorageCapabilities string // expected storage capabilities
7271
}{
7372
{
7473
basePath: "",
7574
baseURL: "/",
76-
expectedBaseHTML: `<base href="/"`,
7775
archiveStorage: false,
7876
logAccess: true,
7977
UIConfigPath: "",
@@ -84,15 +82,13 @@ func TestRegisterStaticHandler(t *testing.T) {
8482
basePath: "/",
8583
baseURL: "/",
8684
archiveStorage: false,
87-
expectedBaseHTML: `<base href="/"`,
8885
UIConfigPath: "fixture/ui-config.json",
8986
expectedUIConfig: `JAEGER_CONFIG = {"x":"y"};`,
9087
expectedStorageCapabilities: `JAEGER_STORAGE_CAPABILITIES = {"archiveStorage":false,"metricsStorage":false};`,
9188
},
9289
{
9390
basePath: "/jaeger",
9491
baseURL: "/jaeger/",
95-
expectedBaseHTML: `<base href="/jaeger/"`,
9692
subroute: true,
9793
archiveStorage: true,
9894
UIConfigPath: "fixture/ui-config.js",
@@ -102,7 +98,6 @@ func TestRegisterStaticHandler(t *testing.T) {
10298
{
10399
basePath: "/metrics",
104100
baseURL: "/metrics/",
105-
expectedBaseHTML: `<base href="/metrics/"`,
106101
subroute: true,
107102
metricsStorage: true,
108103
UIConfigPath: "fixture/ui-config.js",
@@ -149,7 +144,12 @@ func TestRegisterStaticHandler(t *testing.T) {
149144
assert.Contains(t, html, testCase.expectedUIConfig, "actual: %v", html)
150145
assert.Contains(t, html, testCase.expectedStorageCapabilities, "actual: %v", html)
151146
assert.Contains(t, html, `JAEGER_VERSION = {"gitCommit":"","gitVersion":"dev","buildDate":""};`, "actual: %v", html)
152-
assert.Contains(t, html, testCase.expectedBaseHTML, "actual: %v", html)
147+
// Verify the inline base-path script marker is present and the backend
148+
// did not rewrite <base href> to a path-specific value (ADR-009).
149+
assert.Contains(t, html, `data-inject-target="BASE_URL"`, "<base> tag must carry data-inject-target marker for client-side base-path detection")
150+
if testCase.basePath != "" && testCase.basePath != "/" {
151+
assert.NotContains(t, html, `<base href="`+testCase.baseURL+`"`, "backend must not inject path-specific <base href>")
152+
}
153153

154154
asset := httpGet("static/asset.txt")
155155
assert.Contains(t, asset, "some asset", "actual: %v", asset)
@@ -170,17 +170,6 @@ func TestNewStaticAssetsHandlerErrors(t *testing.T) {
170170
Logger: zap.NewNop(),
171171
})
172172
require.Error(t, err)
173-
174-
for _, base := range []string{"x", "x/", "/x/"} {
175-
_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{
176-
UIConfig: UIConfig{
177-
ConfigFile: "fixture/ui-config.json",
178-
},
179-
BasePath: base,
180-
Logger: zap.NewNop(),
181-
})
182-
assert.ErrorContainsf(t, err, "invalid base path", "basePath=%s", base)
183-
}
184173
}
185174

186175
func TestHotReloadUIConfig(t *testing.T) {

0 commit comments

Comments
 (0)