Skip to content

Commit 6f226c9

Browse files
Add preservePath option to proxy extension configuration
Signed-off-by: Alex Kattathra Johnson <[email protected]>
1 parent 7829e2c commit 6f226c9

File tree

3 files changed

+143
-10
lines changed

3 files changed

+143
-10
lines changed

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

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

158158
Is the address where the extension backend must be available.
159159

160+
#### `extensions.backend.services.preservePath` (_boolean_)
161+
(optional)
162+
163+
If true, the full url path is preserved when proxying (will not trim /extensions/<extension-name>).
164+
Useful when the backend service is configured to serve content at the `/extensions/<extension-name>` path.
165+
160166
#### `extensions.backend.services.headers` (_list_)
161167

162168
If provided, the headers list will be added on all outgoing requests
@@ -244,6 +250,30 @@ configuration:
244250
└─────────────────┘
245251
```
246252

253+
### Path Preservation with `preservePath`
254+
255+
Some backend services are configured to serve content at a specific base path and may
256+
redirect requests to establish that path context. For these services, you need to use
257+
`preservePath: true` to prevent infinite redirect loops.
258+
259+
Example configuration for Argo Rollouts dashboard:
260+
261+
```yaml
262+
extension.config.rollouts: |
263+
services:
264+
- url: http://argo-rollouts-dashboard.argo-rollouts.svc:3100/extensions/rollouts
265+
preservePath: true
266+
```
267+
268+
**With `preservePath: true`:**
269+
1. Request: `/extensions/rollouts/` → Proxied as: `/extensions/rollouts/`
270+
2. Backend handles the request correctly at its expected path
271+
272+
**Without `preservePath` (causes infinite redirects):**
273+
1. Request: `/extensions/rollouts/` → Proxied as: `/` (path stripped)
274+
2. Backend redirects from `/` to `/extensions/rollouts`
275+
3. Browser follows redirect → infinite loop
276+
247277
### Incoming Request Headers
248278

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

server/extension/extension.go

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,11 @@ type ServiceConfig struct {
182182
// Headers if provided, the headers list will be added on all
183183
// outgoing requests for this service config.
184184
Headers []Header `yaml:"headers"`
185+
186+
// PreservePath if true, will preserve the full original path when proxying
187+
// requests instead of stripping the extension prefix. Default is false to
188+
// maintain backward compatibility.
189+
PreservePath bool `yaml:"preservePath,omitempty"`
185190
}
186191

187192
// Header defines the header to be added in the proxy requests.
@@ -393,15 +398,21 @@ func NewManager(log *log.Entry, namespace string, sg SettingsGetter, ag Applicat
393398
// the Argo CD configmap.
394399
type ExtensionRegistry map[string]ProxyRegistry
395400

401+
// Proxy extends httputil.ReverseProxy with additional configuration
402+
type Proxy struct {
403+
*httputil.ReverseProxy
404+
PreservePath bool
405+
}
406+
396407
// ProxyRegistry is an in memory registry that contains all proxies for a
397408
// given extension. Different extensions will have independent proxy registries.
398409
// This is required to address the use case when one extension is configured with
399410
// multiple backend services in different clusters.
400-
type ProxyRegistry map[ProxyKey]*httputil.ReverseProxy
411+
type ProxyRegistry map[ProxyKey]*Proxy
401412

402413
// NewProxyRegistry will instantiate a new in memory registry for proxies.
403414
func NewProxyRegistry() ProxyRegistry {
404-
r := make(map[ProxyKey]*httputil.ReverseProxy)
415+
r := make(map[ProxyKey]*Proxy)
405416
return r
406417
}
407418

@@ -521,12 +532,12 @@ func validateConfigs(configs *ExtensionConfigs) error {
521532
// NewProxy will instantiate a new reverse proxy based on the provided
522533
// targetURL and config. It will remove sensitive information from the
523534
// incoming request such as the Authorization and Cookie headers.
524-
func NewProxy(targetURL string, headers []Header, config ProxyConfig) (*httputil.ReverseProxy, error) {
535+
func NewProxy(targetURL string, headers []Header, config ProxyConfig, preservePath bool) (*Proxy, error) {
525536
url, err := url.Parse(targetURL)
526537
if err != nil {
527538
return nil, fmt.Errorf("failed to parse proxy URL: %w", err)
528539
}
529-
proxy := &httputil.ReverseProxy{
540+
reverseProxy := &httputil.ReverseProxy{
530541
Transport: newTransport(config),
531542
Director: func(req *http.Request) {
532543
req.Host = url.Host
@@ -540,6 +551,10 @@ func NewProxy(targetURL string, headers []Header, config ProxyConfig) (*httputil
540551
}
541552
},
542553
}
554+
proxy := &Proxy{
555+
ReverseProxy: reverseProxy,
556+
PreservePath: preservePath,
557+
}
543558
return proxy, nil
544559
}
545560

@@ -606,7 +621,7 @@ func (m *Manager) UpdateExtensionRegistry(s *settings.ArgoCDSettings) error {
606621
proxyReg := NewProxyRegistry()
607622
singleBackend := len(ext.Backend.Services) == 1
608623
for _, service := range ext.Backend.Services {
609-
proxy, err := NewProxy(service.URL, service.Headers, ext.Backend.ProxyConfig)
624+
proxy, err := NewProxy(service.URL, service.Headers, ext.Backend.ProxyConfig, service.PreservePath)
610625
if err != nil {
611626
return fmt.Errorf("error creating proxy: %w", err)
612627
}
@@ -627,7 +642,7 @@ func (m *Manager) UpdateExtensionRegistry(s *settings.ArgoCDSettings) error {
627642
func appendProxy(registry ProxyRegistry,
628643
extName string,
629644
service ServiceConfig,
630-
proxy *httputil.ReverseProxy,
645+
proxy *Proxy,
631646
singleBackend bool,
632647
) error {
633648
if singleBackend {
@@ -727,7 +742,7 @@ func (m *Manager) authorize(ctx context.Context, rr *RequestResources, extName s
727742

728743
// findProxy will search the given registry to find the correct proxy to use
729744
// based on the given extName and dest.
730-
func findProxy(registry ProxyRegistry, extName string, dest v1alpha1.ApplicationDestination) (*httputil.ReverseProxy, error) {
745+
func findProxy(registry ProxyRegistry, extName string, dest v1alpha1.ApplicationDestination) (*Proxy, error) {
731746
// First try to find the proxy in the registry just by the extension name.
732747
// This is the simple case for extensions with only one backend service.
733748
key := proxyKey(extName, "", "")
@@ -796,7 +811,7 @@ func (m *Manager) CallExtension() func(http.ResponseWriter, *http.Request) {
796811
userId := m.userGetter.GetUserId(r.Context())
797812
username := m.userGetter.GetUsername(r.Context())
798813
groups := m.userGetter.GetGroups(r.Context())
799-
prepareRequest(r, m.namespace, extName, app, userId, username, groups)
814+
prepareRequest(r, m.namespace, extName, app, userId, username, groups, proxy.PreservePath)
800815
m.log.WithFields(log.Fields{
801816
HeaderArgoCDUserId: userId,
802817
HeaderArgoCDUsername: username,
@@ -831,8 +846,12 @@ func registerMetrics(extName string, metrics httpsnoop.Metrics, extensionMetrics
831846
// - Cluster destination name
832847
// - Cluster destination server
833848
// - Argo CD authenticated username
834-
func prepareRequest(r *http.Request, namespace string, extName string, app *v1alpha1.Application, userId string, username string, groups []string) {
835-
r.URL.Path = strings.TrimPrefix(r.URL.Path, fmt.Sprintf("%s/%s", URLPrefix, extName))
849+
func prepareRequest(r *http.Request, namespace string, extName string, app *v1alpha1.Application, userId string, username string, groups []string, preservePath bool) {
850+
if !preservePath {
851+
// Default behavior: strip the extension prefix from the path
852+
r.URL.Path = strings.TrimPrefix(r.URL.Path, fmt.Sprintf("%s/%s", URLPrefix, extName))
853+
}
854+
836855
r.Header.Set(HeaderArgoCDNamespace, namespace)
837856
if app.Spec.Destination.Name != "" {
838857
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
@@ -737,6 +737,90 @@ func TestCallExtension(t *testing.T) {
737737
require.NotNil(t, resp)
738738
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
739739
})
740+
t.Run("will preserve path when preservePath is true", func(t *testing.T) {
741+
// given
742+
t.Parallel()
743+
f := setup()
744+
withRbac(f, true, true)
745+
withUser(f, "some-user", []string{"group1", "group2"})
746+
747+
clusterURL := "some-url"
748+
app := getApp("", clusterURL, defaultProjectName)
749+
proj := getProjectWithDestinations("project-name", nil, []string{clusterURL})
750+
f.appGetterMock.On("Get", mock.Anything, mock.Anything).Return(app, nil)
751+
withProject(proj, f)
752+
753+
// Create a backend server that echoes the received path
754+
backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
755+
w.Header().Set("X-Received-Path", r.URL.Path)
756+
w.WriteHeader(http.StatusOK)
757+
fmt.Fprint(w, "path:"+r.URL.Path)
758+
}))
759+
defer backendServer.Close()
760+
761+
// Configure extensions with preservePath settings
762+
extensionConfig := fmt.Sprintf(`
763+
extensions:
764+
- name: preserve-path-ext
765+
backend:
766+
services:
767+
- url: %s
768+
preservePath: true
769+
- name: no-preserve-path-ext
770+
backend:
771+
services:
772+
- url: %s
773+
preservePath: false
774+
`, backendServer.URL, backendServer.URL)
775+
776+
withExtensionConfig(extensionConfig, f)
777+
ts := startTestServer(t, f)
778+
defer ts.Close()
779+
780+
test := func(extName string) {
781+
var wg sync.WaitGroup
782+
wg.Add(2)
783+
f.metricsMock.
784+
On("IncExtensionRequestCounter", extName, http.StatusOK).
785+
Run(func(_ mock.Arguments) {
786+
wg.Done()
787+
})
788+
f.metricsMock.
789+
On("ObserveExtensionRequestDuration", extName, mock.Anything).
790+
Run(func(_ mock.Arguments) {
791+
wg.Done()
792+
})
793+
794+
// when
795+
backendPath := "/some/deep/path"
796+
req := newExtensionRequest(t, "GET", fmt.Sprintf("%s/extensions/%s%s", ts.URL, extName, backendPath))
797+
798+
// then
799+
resp, err := http.DefaultClient.Do(req)
800+
require.NoError(t, err)
801+
assert.Equal(t, http.StatusOK, resp.StatusCode)
802+
803+
switch extName {
804+
case "preserve-path-ext":
805+
assert.Equal(t, fmt.Sprintf("/extensions/%s%s", extName, backendPath), resp.Header.Get("X-Received-Path"))
806+
case "no-preserve-path-ext":
807+
assert.Equal(t, backendPath, resp.Header.Get("X-Received-Path"))
808+
}
809+
810+
// waitgroup is necessary to make sure assertions aren't executed before
811+
// the goroutine initiated by extension.CallExtension concludes which would
812+
// lead to flaky test.
813+
wg.Wait()
814+
f.metricsMock.AssertCalled(t, "IncExtensionRequestCounter", extName, http.StatusOK)
815+
f.metricsMock.AssertCalled(t, "ObserveExtensionRequestDuration", extName, mock.Anything)
816+
}
817+
818+
// Test preservePath: true - should keep full path
819+
test("preserve-path-ext")
820+
821+
// Test preservePath: false - should strip extension prefix
822+
test("no-preserve-path-ext")
823+
})
740824
}
741825

742826
func getExtensionConfig(name, url string) string {

0 commit comments

Comments
 (0)