feat(extensions): Automatically apply extension configs without resta…

…rting API-Server (argoproj#15574)

* feat: auto configure extensions

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* feat: auto-reload extension configs without restarting api-server

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* clean unused gorilla mux

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* update docs

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* Address review comments

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* Add more test cases

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* refactoring to reduce unnecessary function

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* Add log

Signed-off-by: Leonardo Luz Almeida <[email protected]>

* fix bugs found during manual tests

Signed-off-by: Leonardo Luz Almeida <[email protected]>

---------

Signed-off-by: Leonardo Luz Almeida <[email protected]>
Signed-off-by: Jimmy Neville <[email protected]>

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

@@ -32,7 +32,7 @@ data:
 Once the proxy extension is enabled, it can be configured in the main
 Argo CD configmap ([argocd-cm][2]).
 
-The example below demonstrate all possible configurations available
+The example below demonstrates all possible configurations available
 for proxy extensions:
 
 ```yaml
@@ -60,9 +60,11 @@ data:
             server: https://some-cluster
 ```
 
-If a the configuration is changed, Argo CD Server will need to be
-restarted as the proxy handlers are only registered once during the
-initialization of the server.
+Note: There is no need to restart Argo CD Server after modifiying the
+`extension.config` entry in Argo CD configmap. Changes will be
+automatically applied. A new proxy registry will be built making
+all new incoming extensions requests (`<argocd-host>/extensions/*`) to
+respect the new configuration.
 
 Every configuration entry is explained below:
 

go.mod

@@ -42,7 +42,6 @@ require (
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/google/uuid v1.3.0
 	github.com/gorilla/handlers v1.5.1
-	github.com/gorilla/mux v1.8.0
 	github.com/gorilla/websocket v1.5.0
 	github.com/gosimple/slug v1.13.1
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
@@ -88,6 +87,7 @@ require (
 	google.golang.org/protobuf v1.31.0
 	gopkg.in/square/go-jose.v2 v2.6.0
 	gopkg.in/yaml.v2 v2.4.0
+	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.24.2
 	k8s.io/apiextensions-apiserver v0.24.2
 	k8s.io/apimachinery v0.24.2
@@ -274,7 +274,6 @@ require (
 	gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/cli-runtime v0.24.2 // indirect
 	k8s.io/component-base v0.24.2 // indirect
 	k8s.io/component-helpers v0.24.2 // indirect

go.sum

@@ -1277,7 +1277,6 @@ github.com/gorilla/handlers v1.5.1/go.mod h1:t8XrUpc4KVXb7HGyJ4/cEnwQiaxrX/hz1Zv
 github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
 github.com/gorilla/mux v1.7.4/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
-github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
 github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
 github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
 github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=

server/extension/extension.go

@@ -12,9 +12,8 @@ import (
 	"strings"
 	"time"
 
-	"github.com/gorilla/mux"
 	log "github.com/sirupsen/logrus"
-	"sigs.k8s.io/yaml"
+	"gopkg.in/yaml.v3"
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	applisters "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1"
@@ -126,15 +125,15 @@ func getAppName(appHeader string) (string, string, error) {
 // ExtensionConfigs defines the configurations for all extensions
 // retrieved from Argo CD configmap (argocd-cm).
 type ExtensionConfigs struct {
-	Extensions []ExtensionConfig `json:"extensions"`
+	Extensions []ExtensionConfig `yaml:"extensions"`
 }
 
 // ExtensionConfig defines the configuration for one extension.
 type ExtensionConfig struct {
 	// Name defines the endpoint that will be used to register
 	// the extension route. Mandatory field.
-	Name    string        `json:"name"`
-	Backend BackendConfig `json:"backend"`
+	Name    string        `yaml:"name"`
+	Backend BackendConfig `yaml:"backend"`
 }
 
 // BackendConfig defines the backend service configurations that will
@@ -144,30 +143,30 @@ type ExtensionConfig struct {
 // service.
 type BackendConfig struct {
 	ProxyConfig
-	Services []ServiceConfig `json:"services"`
+	Services []ServiceConfig `yaml:"services"`
 }
 
 // ServiceConfig provides the configuration for a backend service.
 type ServiceConfig struct {
 	// URL is the address where the extension backend must be available.
 	// Mandatory field.
-	URL string `json:"url"`
+	URL string `yaml:"url"`
 
 	// Cluster if provided, will have to match the application
 	// destination name to have requests properly forwarded to this
 	// service URL.
-	Cluster *ClusterConfig `json:"cluster,omitempty"`
+	Cluster *ClusterConfig `yaml:"cluster,omitempty"`
 
 	// Headers if provided, the headers list will be added on all
 	// outgoing requests for this service config.
-	Headers []Header `json:"headers"`
+	Headers []Header `yaml:"headers"`
 }
 
 // Header defines the header to be added in the proxy requests.
 type Header struct {
 	// Name defines the name of the header. It is a mandatory field if
 	// a header is provided.
-	Name string `json:"name"`
+	Name string `yaml:"name"`
 	// Value defines the value of the header. The actual value can be
 	// provided as verbatim or as a reference to an Argo CD secret key.
 	// In order to provide it as a reference, it is necessary to prefix
@@ -176,15 +175,15 @@ type Header struct {
 	//   value: '$some.argocd.secret.key'
 	// In the example above, the value will be replaced with the one from
 	// the argocd-secret with key 'some.argocd.secret.key'.
-	Value string `json:"value"`
+	Value string `yaml:"value"`
 }
 
 type ClusterConfig struct {
 	// Server specifies the URL of the target cluster's Kubernetes control plane API. This must be set if Name is not set.
-	Server string `json:"server"`
+	Server string `yaml:"server"`
 
 	// Name is an alternate way of specifying the target cluster by its symbolic name. This must be set if Server is not set.
-	Name string `json:"name"`
+	Name string `yaml:"name"`
 }
 
 // ProxyConfig allows configuring connection behaviour between Argo CD
@@ -193,24 +192,24 @@ type ProxyConfig struct {
 	// ConnectionTimeout is the maximum amount of time a dial to
 	// the extension server will wait for a connect to complete.
 	// Default: 2 seconds
-	ConnectionTimeout time.Duration `json:"connectionTimeout"`
+	ConnectionTimeout time.Duration `yaml:"connectionTimeout"`
 
 	// KeepAlive specifies the interval between keep-alive probes
 	// for an active network connection between the API server and
 	// the extension server.
 	// Default: 15 seconds
-	KeepAlive time.Duration `json:"keepAlive"`
+	KeepAlive time.Duration `yaml:"keepAlive"`
 
 	// IdleConnectionTimeout is the maximum amount of time an idle
 	// (keep-alive) connection between the API server and the extension
 	// server will remain idle before closing itself.
 	// Default: 60 seconds
-	IdleConnectionTimeout time.Duration `json:"idleConnectionTimeout"`
+	IdleConnectionTimeout time.Duration `yaml:"idleConnectionTimeout"`
 
 	// MaxIdleConnections controls the maximum number of idle (keep-alive)
 	// connections between the API server and the extension server.
 	// Default: 30
-	MaxIdleConnections int `json:"maxIdleConnections"`
+	MaxIdleConnections int `yaml:"maxIdleConnections"`
 }
 
 // SettingsGetter defines the contract to retrieve Argo CD Settings.
@@ -300,6 +299,7 @@ type Manager struct {
 	application ApplicationGetter
 	project     ProjectGetter
 	rbac        RbacEnforcer
+	registry    ExtensionRegistry
 }
 
 // NewManager will initialize a new manager.
@@ -313,6 +313,11 @@ func NewManager(log *log.Entry, sg SettingsGetter, ag ApplicationGetter, pg Proj
 	}
 }
 
+// ExtensionRegistry is an in memory registry that contains contains all
+// proxies for all extensions. The key is the extension name defined in
+// the Argo CD configmap.
+type ExtensionRegistry map[string]ProxyRegistry
+
 // ProxyRegistry is an in memory registry that contains all proxies for a
 // given extension. Different extensions will have independent proxy registries.
 // This is required to address the use case when one extension is configured with
@@ -344,6 +349,10 @@ func proxyKey(extName, cName, cServer string) ProxyKey {
 }
 
 func parseAndValidateConfig(s *settings.ArgoCDSettings) (*ExtensionConfigs, error) {
+	if s.ExtensionConfig == "" {
+		return nil, fmt.Errorf("no extensions configurations found")
+	}
+
 	extConfigMap := map[string]interface{}{}
 	err := yaml.Unmarshal([]byte(s.ExtensionConfig), &extConfigMap)
 	if err != nil {
@@ -383,6 +392,9 @@ func validateConfigs(configs *ExtensionConfigs) error {
 		}
 		exts[ext.Name] = struct{}{}
 		svcTotal := len(ext.Backend.Services)
+		if svcTotal == 0 {
+			return fmt.Errorf("no backend service configured for extension %s", ext.Name)
+		}
 		for _, svc := range ext.Backend.Services {
 			if svc.URL == "" {
 				return fmt.Errorf("extensions.backend.services.url must be configured")
@@ -465,25 +477,47 @@ func applyProxyConfigDefaults(c *ProxyConfig) {
 	}
 }
 
-// RegisterHandlers will retrieve all configured extensions
-// and register the respective http handlers in the given
-// router.
-func (m *Manager) RegisterHandlers(r *mux.Router) error {
-	m.log.Info("Registering extension handlers...")
+// RegisterExtensions will retrieve all extensions configurations
+// and update the extension registry.
+func (m *Manager) RegisterExtensions() error {
 	settings, err := m.settings.Get()
 	if err != nil {
 		return fmt.Errorf("error getting settings: %s", err)
 	}
-
-	if settings.ExtensionConfig == "" {
-		return fmt.Errorf("No extensions configurations found")
+	err = m.UpdateExtensionRegistry(settings)
+	if err != nil {
+		return fmt.Errorf("error updating extension registry: %s", err)
 	}
+	return nil
+}
 
-	extConfigs, err := parseAndValidateConfig(settings)
+// UpdateExtensionRegistry will first parse and validate the extensions
+// configurations from the given settings. If no errors are found, it will
+// iterate over the given configurations building a new extension registry.
+// At the end, it will update the manager with the newly created registry.
+func (m *Manager) UpdateExtensionRegistry(s *settings.ArgoCDSettings) error {
+	extConfigs, err := parseAndValidateConfig(s)
 	if err != nil {
 		return fmt.Errorf("error parsing extension config: %s", err)
 	}
-	return m.registerExtensions(r, extConfigs)
+	extReg := make(map[string]ProxyRegistry)
+	for _, ext := range extConfigs.Extensions {
+		proxyReg := NewProxyRegistry()
+		singleBackend := len(ext.Backend.Services) == 1
+		for _, service := range ext.Backend.Services {
+			proxy, err := NewProxy(service.URL, service.Headers, ext.Backend.ProxyConfig)
+			if err != nil {
+				return fmt.Errorf("error creating proxy: %s", err)
+			}
+			err = appendProxy(proxyReg, ext.Name, service, proxy, singleBackend)
+			if err != nil {
+				return fmt.Errorf("error appending proxy: %s", err)
+			}
+		}
+		extReg[ext.Name] = proxyReg
+	}
+	m.registry = extReg
+	return nil
 }
 
 // appendProxy will append the given proxy in the given registry. Will use
@@ -525,31 +559,6 @@ func appendProxy(registry ProxyRegistry,
 	return nil
 }
 
-// registerExtensions will iterate over the given extConfigs and register
-// http handlers for every extension. It also registers a list extensions
-// handler under the "/extensions/" endpoint.
-func (m *Manager) registerExtensions(r *mux.Router, extConfigs *ExtensionConfigs) error {
-	extRouter := r.PathPrefix(fmt.Sprintf("%s/", URLPrefix)).Subrouter()
-	for _, ext := range extConfigs.Extensions {
-		registry := NewProxyRegistry()
-		singleBackend := len(ext.Backend.Services) == 1
-		for _, service := range ext.Backend.Services {
-			proxy, err := NewProxy(service.URL, service.Headers, ext.Backend.ProxyConfig)
-			if err != nil {
-				return fmt.Errorf("error creating proxy: %s", err)
-			}
-			err = appendProxy(registry, ext.Name, service, proxy, singleBackend)
-			if err != nil {
-				return fmt.Errorf("error appending proxy: %s", err)
-			}
-		}
-		m.log.Infof("Registering handler for %s/%s...", URLPrefix, ext.Name)
-		extRouter.PathPrefix(fmt.Sprintf("/%s/", ext.Name)).
-			HandlerFunc(m.CallExtension(ext.Name, registry))
-	}
-	return nil
-}
-
 // authorize will enforce rbac rules are satified for the given RequestResources.
 // The following validations are executed:
 //   - enforce the subject has permission to read application/project provided
@@ -624,10 +633,29 @@ func findProxy(registry ProxyRegistry, extName string, dest v1alpha1.Application
 	return nil, fmt.Errorf("no proxy found for extension %q", extName)
 }
 
+// ProxyRegistry returns the proxy registry associated for the given
+// extension name.
+func (m *Manager) ProxyRegistry(name string) (ProxyRegistry, bool) {
+	pReg, found := m.registry[name]
+	return pReg, found
+}
+
 // CallExtension returns a handler func responsible for forwarding requests to the
 // extension service. The request will be sanitized by removing sensitive headers.
-func (m *Manager) CallExtension(extName string, registry ProxyRegistry) func(http.ResponseWriter, *http.Request) {
+func (m *Manager) CallExtension() func(http.ResponseWriter, *http.Request) {
 	return func(w http.ResponseWriter, r *http.Request) {
+		segments := strings.Split(strings.TrimPrefix(r.URL.Path, "/"), "/")
+		if segments[0] != "extensions" {
+			http.Error(w, fmt.Sprintf("Invalid URL: first segment must be %s", URLPrefix), http.StatusBadRequest)
+			return
+		}
+		extName := segments[1]
+		if extName == "" {
+			http.Error(w, "Invalid URL: extension name must be provided", http.StatusBadRequest)
+			return
+		}
+		extName = strings.ReplaceAll(extName, "\n", "")
+		extName = strings.ReplaceAll(extName, "\r", "")
 		reqResources, err := ValidateHeaders(r)
 		if err != nil {
 			http.Error(w, fmt.Sprintf("Invalid headers: %s", err), http.StatusBadRequest)
@@ -640,7 +668,13 @@ func (m *Manager) CallExtension(extName string, registry ProxyRegistry) func(htt
 			return
 		}
 
-		proxy, err := findProxy(registry, extName, app.Spec.Destination)
+		proxyRegistry, ok := m.ProxyRegistry(extName)
+		if !ok {
+			m.log.Warnf("proxy extension warning: attempt to call unregistered extension: %s", extName)
+			http.Error(w, "Extension not found", http.StatusNotFound)
+			return
+		}
+		proxy, err := findProxy(proxyRegistry, extName, app.Spec.Destination)
 		if err != nil {
 			m.log.Errorf("findProxy error: %s", err)
 			http.Error(w, "invalid extension", http.StatusBadRequest)