Skip to content

Commit 7d0b1de

Browse files
Add preservePath option to proxy extension configuration
Signed-off-by: Alex Kattathra Johnson <[email protected]>
1 parent 1db5f2e commit 7d0b1de

File tree

3 files changed

+144
-10
lines changed

3 files changed

+144
-10
lines changed

docs/developer-guide/extensions/proxy-extensions.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,12 @@ Defines a list with backend url by cluster.
150150

151151
Is the address where the extension backend must be available.
152152

153+
#### `extensions.backend.services.preservePath` (*boolean*)
154+
(optional)
155+
156+
If true, the full url path is preserved when proxying (will not trim /extensions/<extension-name>).
157+
Useful when the backend service is configured to serve content at the `/extensions/<extension-name>` path.
158+
153159
#### `extensions.backend.services.headers` (*list*)
154160

155161
If provided, the headers list will be added on all outgoing requests
@@ -232,6 +238,30 @@ configuration:
232238
└─────────────────┘
233239
```
234240

241+
### Path Preservation with `preservePath`
242+
243+
Some backend services are configured to serve content at a specific base path and may
244+
redirect requests to establish that path context. For these services, you need to use
245+
`preservePath: true` to prevent infinite redirect loops.
246+
247+
Example configuration for Argo Rollouts dashboard:
248+
249+
```yaml
250+
extension.config.rollouts: |
251+
services:
252+
- url: http://argo-rollouts-dashboard.argo-rollouts.svc:3100/extensions/rollouts
253+
preservePath: true
254+
```
255+
256+
**With `preservePath: true`:**
257+
1. Request: `/extensions/rollouts/` → Proxied as: `/extensions/rollouts/`
258+
2. Backend handles the request correctly at its expected path
259+
260+
**Without `preservePath` (causes infinite redirects):**
261+
1. Request: `/extensions/rollouts/` → Proxied as: `/` (path stripped)
262+
2. Backend redirects from `/` to `/extensions/rollouts`
263+
3. Browser follows redirect → infinite loop
264+
235265
### Incoming Request Headers
236266

237267
Note that Argo CD API Server requires additional HTTP headers to be

server/extension/extension.go

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ type ServiceConfig struct {
178178
// Headers if provided, the headers list will be added on all
179179
// outgoing requests for this service config.
180180
Headers []Header `yaml:"headers"`
181+
182+
// PreservePath if true, will preserve the full original path when proxying
183+
// requests instead of stripping the extension prefix. Default is false to
184+
// maintain backward compatibility.
185+
PreservePath bool `yaml:"preservePath,omitempty"`
181186
}
182187

183188
// Header defines the header to be added in the proxy requests.
@@ -383,15 +388,21 @@ func NewManager(log *log.Entry, namespace string, sg SettingsGetter, ag Applicat
383388
// the Argo CD configmap.
384389
type ExtensionRegistry map[string]ProxyRegistry
385390

391+
// Proxy extends httputil.ReverseProxy with additional configuration
392+
type Proxy struct {
393+
*httputil.ReverseProxy
394+
PreservePath bool
395+
}
396+
386397
// ProxyRegistry is an in memory registry that contains all proxies for a
387398
// given extension. Different extensions will have independent proxy registries.
388399
// This is required to address the use case when one extension is configured with
389400
// multiple backend services in different clusters.
390-
type ProxyRegistry map[ProxyKey]*httputil.ReverseProxy
401+
type ProxyRegistry map[ProxyKey]*Proxy
391402

392403
// NewProxyRegistry will instantiate a new in memory registry for proxies.
393404
func NewProxyRegistry() ProxyRegistry {
394-
r := make(map[ProxyKey]*httputil.ReverseProxy)
405+
r := make(map[ProxyKey]*Proxy)
395406
return r
396407
}
397408

@@ -511,12 +522,12 @@ func validateConfigs(configs *ExtensionConfigs) error {
511522
// NewProxy will instantiate a new reverse proxy based on the provided
512523
// targetURL and config. It will remove sensitive information from the
513524
// incoming request such as the Authorization and Cookie headers.
514-
func NewProxy(targetURL string, headers []Header, config ProxyConfig) (*httputil.ReverseProxy, error) {
525+
func NewProxy(targetURL string, headers []Header, config ProxyConfig, preservePath bool) (*Proxy, error) {
515526
url, err := url.Parse(targetURL)
516527
if err != nil {
517528
return nil, fmt.Errorf("failed to parse proxy URL: %w", err)
518529
}
519-
proxy := &httputil.ReverseProxy{
530+
reverseProxy := &httputil.ReverseProxy{
520531
Transport: newTransport(config),
521532
Director: func(req *http.Request) {
522533
req.Host = url.Host
@@ -530,6 +541,10 @@ func NewProxy(targetURL string, headers []Header, config ProxyConfig) (*httputil
530541
}
531542
},
532543
}
544+
proxy := &Proxy{
545+
ReverseProxy: reverseProxy,
546+
PreservePath: preservePath,
547+
}
533548
return proxy, nil
534549
}
535550

@@ -596,7 +611,7 @@ func (m *Manager) UpdateExtensionRegistry(s *settings.ArgoCDSettings) error {
596611
proxyReg := NewProxyRegistry()
597612
singleBackend := len(ext.Backend.Services) == 1
598613
for _, service := range ext.Backend.Services {
599-
proxy, err := NewProxy(service.URL, service.Headers, ext.Backend.ProxyConfig)
614+
proxy, err := NewProxy(service.URL, service.Headers, ext.Backend.ProxyConfig, service.PreservePath)
600615
if err != nil {
601616
return fmt.Errorf("error creating proxy: %w", err)
602617
}
@@ -617,7 +632,7 @@ func (m *Manager) UpdateExtensionRegistry(s *settings.ArgoCDSettings) error {
617632
func appendProxy(registry ProxyRegistry,
618633
extName string,
619634
service ServiceConfig,
620-
proxy *httputil.ReverseProxy,
635+
proxy *Proxy,
621636
singleBackend bool,
622637
) error {
623638
if singleBackend {
@@ -717,7 +732,7 @@ func (m *Manager) authorize(ctx context.Context, rr *RequestResources, extName s
717732

718733
// findProxy will search the given registry to find the correct proxy to use
719734
// based on the given extName and dest.
720-
func findProxy(registry ProxyRegistry, extName string, dest v1alpha1.ApplicationDestination) (*httputil.ReverseProxy, error) {
735+
func findProxy(registry ProxyRegistry, extName string, dest v1alpha1.ApplicationDestination) (*Proxy, error) {
721736
// First try to find the proxy in the registry just by the extension name.
722737
// This is the simple case for extensions with only one backend service.
723738
key := proxyKey(extName, "", "")
@@ -785,7 +800,7 @@ func (m *Manager) CallExtension() func(http.ResponseWriter, *http.Request) {
785800

786801
user := m.userGetter.GetUser(r.Context())
787802
groups := m.userGetter.GetGroups(r.Context())
788-
prepareRequest(r, m.namespace, extName, app, user, groups)
803+
prepareRequest(r, m.namespace, extName, app, user, groups, proxy.PreservePath)
789804
m.log.WithFields(log.Fields{
790805
HeaderArgoCDUsername: user,
791806
HeaderArgoCDGroups: strings.Join(groups, ","),
@@ -819,8 +834,13 @@ func registerMetrics(extName string, metrics httpsnoop.Metrics, extensionMetrics
819834
// - Cluster destination name
820835
// - Cluster destination server
821836
// - Argo CD authenticated username
822-
func prepareRequest(r *http.Request, namespace string, extName string, app *v1alpha1.Application, username string, groups []string) {
823-
r.URL.Path = strings.TrimPrefix(r.URL.Path, fmt.Sprintf("%s/%s", URLPrefix, extName))
837+
func prepareRequest(r *http.Request, namespace string, extName string, app *v1alpha1.Application, username string, groups []string, preservePath bool) {
838+
if !preservePath {
839+
// Default behavior: strip the extension prefix from the path
840+
r.URL.Path = strings.TrimPrefix(r.URL.Path, fmt.Sprintf("%s/%s", URLPrefix, extName))
841+
}
842+
// If preservePath is true, keep the original path as-is
843+
824844
r.Header.Set(HeaderArgoCDNamespace, namespace)
825845
if app.Spec.Destination.Name != "" {
826846
r.Header.Set(HeaderArgoCDTargetClusterName, app.Spec.Destination.Name)

server/extension/extension_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,90 @@ func TestCallExtension(t *testing.T) {
735735
require.NotNil(t, resp)
736736
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
737737
})
738+
t.Run("will preserve path when preservePath is true", func(t *testing.T) {
739+
// given
740+
t.Parallel()
741+
f := setup()
742+
withRbac(f, true, true)
743+
withUser(f, "some-user", []string{"group1", "group2"})
744+
745+
clusterURL := "some-url"
746+
app := getApp("", clusterURL, defaultProjectName)
747+
proj := getProjectWithDestinations("project-name", nil, []string{clusterURL})
748+
f.appGetterMock.On("Get", mock.Anything, mock.Anything).Return(app, nil)
749+
withProject(proj, f)
750+
751+
// Create a backend server that echoes the received path
752+
backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
753+
w.Header().Set("X-Received-Path", r.URL.Path)
754+
w.WriteHeader(http.StatusOK)
755+
fmt.Fprint(w, "path:"+r.URL.Path)
756+
}))
757+
defer backendServer.Close()
758+
759+
// Configure extensions with preservePath settings
760+
extensionConfig := fmt.Sprintf(`
761+
extensions:
762+
- name: preserve-path-ext
763+
backend:
764+
services:
765+
- url: %s
766+
preservePath: true
767+
- name: no-preserve-path-ext
768+
backend:
769+
services:
770+
- url: %s
771+
preservePath: false
772+
`, backendServer.URL, backendServer.URL)
773+
774+
withExtensionConfig(extensionConfig, f)
775+
ts := startTestServer(t, f)
776+
defer ts.Close()
777+
778+
test := func(extName string) {
779+
var wg sync.WaitGroup
780+
wg.Add(2)
781+
f.metricsMock.
782+
On("IncExtensionRequestCounter", extName, http.StatusOK).
783+
Run(func(_ mock.Arguments) {
784+
wg.Done()
785+
})
786+
f.metricsMock.
787+
On("ObserveExtensionRequestDuration", extName, mock.Anything).
788+
Run(func(_ mock.Arguments) {
789+
wg.Done()
790+
})
791+
792+
// when
793+
backendPath := "/some/deep/path"
794+
req := newExtensionRequest(t, "GET", fmt.Sprintf("%s/extensions/%s%s", ts.URL, extName, backendPath))
795+
796+
// then
797+
resp, err := http.DefaultClient.Do(req)
798+
require.NoError(t, err)
799+
assert.Equal(t, http.StatusOK, resp.StatusCode)
800+
801+
switch extName {
802+
case "preserve-path-ext":
803+
assert.Equal(t, fmt.Sprintf("/extensions/%s%s", extName, backendPath), resp.Header.Get("X-Received-Path"))
804+
case "no-preserve-path-ext":
805+
assert.Equal(t, backendPath, resp.Header.Get("X-Received-Path"))
806+
}
807+
808+
// waitgroup is necessary to make sure assertions aren't executed before
809+
// the goroutine initiated by extension.CallExtension concludes which would
810+
// lead to flaky test.
811+
wg.Wait()
812+
f.metricsMock.AssertCalled(t, "IncExtensionRequestCounter", extName, http.StatusOK)
813+
f.metricsMock.AssertCalled(t, "ObserveExtensionRequestDuration", extName, mock.Anything)
814+
}
815+
816+
// Test preservePath: true - should keep full path
817+
test("preserve-path-ext")
818+
819+
// Test preservePath: false - should strip extension prefix
820+
test("no-preserve-path-ext")
821+
})
738822
}
739823

740824
func getExtensionConfig(name, url string) string {

0 commit comments

Comments
 (0)