remoteconfig: make rc client a singleton (#2297)

Make the remote config client a singleton. Tracing and profiling will then be able to use it in addition to ASM.
It's a requirement to have a single shared instance of the remote config client per tracer.

internal/appsec/appsec.go

@@ -14,9 +14,8 @@ import (
 
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
-	"gopkg.in/DataDog/dd-trace-go.v1/internal/remoteconfig"
 
-	"github.com/DataDog/go-libddwaf"
+	waf "github.com/DataDog/go-libddwaf"
 )
 
 // Enabled returns true when AppSec is up and running. Meaning that the appsec build tag is enabled, the env var
@@ -66,7 +65,9 @@ func Start(opts ...StartOption) {
 
 	// Start the remote configuration client
 	log.Debug("appsec: starting the remote configuration client")
-	appsec.startRC()
+	if err := appsec.startRC(); err != nil {
+		log.Error("appsec: Remote config: disabled due to an instanciation error: %v", err)
+	}
 
 	if !set {
 		// AppSec is not enforced by the env var and can be enabled through remote config
@@ -114,23 +115,13 @@ func setActiveAppSec(a *appsec) {
 type appsec struct {
 	cfg       *Config
 	limiter   *TokenTicker
-	rc        *remoteconfig.Client
 	wafHandle *wafHandle
 	started   bool
 }
 
 func newAppSec(cfg *Config) *appsec {
-	var client *remoteconfig.Client
-	var err error
-	if cfg.rc != nil {
-		client, err = remoteconfig.NewClient(*cfg.rc)
-	}
-	if err != nil {
-		log.Error("appsec: Remote config: disabled due to a client creation error: %v", err)
-	}
 	return &appsec{
 		cfg: cfg,
-		rc:  client,
 	}
 }
 

internal/appsec/remoteconfig.go

@@ -287,95 +287,109 @@ func mergeRulesDataEntries(entries1, entries2 []rc.ASMDataRuleDataEntry) []rc.AS
 	return entries
 }
 
-func (a *appsec) startRC() {
-	if a.rc != nil {
-		a.rc.Start()
+func (a *appsec) startRC() error {
+	if a.cfg.rc != nil {
+		return remoteconfig.Start(*a.cfg.rc)
 	}
+	return nil
 }
 
 func (a *appsec) stopRC() {
-	if a.rc != nil {
-		a.rc.Stop()
+	if a.cfg.rc != nil {
+		remoteconfig.Stop()
 	}
 }
 
 func (a *appsec) registerRCProduct(p string) error {
-	if a.rc == nil {
+	if a.cfg.rc == nil {
 		return fmt.Errorf("no valid remote configuration client")
 	}
-	a.cfg.rc.Products[p] = struct{}{}
-	a.rc.RegisterProduct(p)
-	return nil
-}
-
-func (a *appsec) unregisterRCProduct(p string) error {
-	if a.rc == nil {
-		return fmt.Errorf("no valid remote configuration client")
-	}
-	delete(a.cfg.rc.Products, p)
-	a.rc.UnregisterProduct(p)
-	return nil
+	return remoteconfig.RegisterProduct(p)
 }
 
 func (a *appsec) registerRCCapability(c remoteconfig.Capability) error {
-	a.cfg.rc.Capabilities[c] = struct{}{}
-	if a.rc == nil {
+	if a.cfg.rc == nil {
 		return fmt.Errorf("no valid remote configuration client")
 	}
-	a.rc.RegisterCapability(c)
-	return nil
+	return remoteconfig.RegisterCapability(c)
 }
 
-func (a *appsec) unregisterRCCapability(c remoteconfig.Capability) {
-	if a.rc == nil {
+func (a *appsec) unregisterRCCapability(c remoteconfig.Capability) error {
+	if a.cfg.rc == nil {
 		log.Debug("appsec: Remote config: no valid remote configuration client")
-		return
+		return nil
 	}
-	delete(a.cfg.rc.Capabilities, c)
-	a.rc.UnregisterCapability(c)
+	return remoteconfig.UnregisterCapability(c)
 }
 
 func (a *appsec) enableRemoteActivation() error {
-	if a.rc == nil {
+	if a.cfg.rc == nil {
 		return fmt.Errorf("no valid remote configuration client")
 	}
-	a.registerRCProduct(rc.ProductASMFeatures)
-	a.registerRCCapability(remoteconfig.ASMActivation)
-	a.rc.RegisterCallback(a.onRemoteActivation)
-	return nil
+	err := a.registerRCProduct(rc.ProductASMFeatures)
+	if err != nil {
+		return err
+	}
+	err = a.registerRCCapability(remoteconfig.ASMActivation)
+	if err != nil {
+		return err
+	}
+	return remoteconfig.RegisterCallback(a.onRemoteActivation)
 }
 
 func (a *appsec) enableRCBlocking() {
-	if a.rc == nil {
+	if a.cfg.rc == nil {
 		log.Debug("appsec: Remote config: no valid remote configuration client")
 		return
 	}
 
-	a.registerRCProduct(rc.ProductASM)
-	a.registerRCProduct(rc.ProductASMDD)
-	a.registerRCProduct(rc.ProductASMData)
-	a.rc.RegisterCallback(a.onRCRulesUpdate)
+	products := []string{rc.ProductASM, rc.ProductASMDD, rc.ProductASMData}
+	for _, p := range products {
+		if err := a.registerRCProduct(p); err != nil {
+			log.Debug("appsec: Remote config: couldn't register product %s: %v", p, err)
+		}
+	}
+
+	if err := remoteconfig.RegisterCallback(a.onRCRulesUpdate); err != nil {
+		log.Debug("appsec: Remote config: couldn't register callback: %v", err)
+	}
 
 	if _, isSet := os.LookupEnv(rulesEnvVar); !isSet {
-		a.registerRCCapability(remoteconfig.ASMUserBlocking)
-		a.registerRCCapability(remoteconfig.ASMRequestBlocking)
-		a.registerRCCapability(remoteconfig.ASMIPBlocking)
-		a.registerRCCapability(remoteconfig.ASMDDRules)
-		a.registerRCCapability(remoteconfig.ASMExclusions)
-		a.registerRCCapability(remoteconfig.ASMCustomRules)
-		a.registerRCCapability(remoteconfig.ASMCustomBlockingResponse)
+		caps := []remoteconfig.Capability{
+			remoteconfig.ASMUserBlocking,
+			remoteconfig.ASMRequestBlocking,
+			remoteconfig.ASMIPBlocking,
+			remoteconfig.ASMDDRules,
+			remoteconfig.ASMExclusions,
+			remoteconfig.ASMCustomRules,
+			remoteconfig.ASMCustomBlockingResponse,
+		}
+		for _, c := range caps {
+			if err := a.registerRCCapability(c); err != nil {
+				log.Debug("appsec: Remote config: couldn't register capability %v: %v", c, err)
+			}
+		}
 	}
 }
 
 func (a *appsec) disableRCBlocking() {
-	if a.rc == nil {
+	if a.cfg.rc == nil {
 		return
 	}
-	a.unregisterRCCapability(remoteconfig.ASMDDRules)
-	a.unregisterRCCapability(remoteconfig.ASMExclusions)
-	a.unregisterRCCapability(remoteconfig.ASMIPBlocking)
-	a.unregisterRCCapability(remoteconfig.ASMRequestBlocking)
-	a.unregisterRCCapability(remoteconfig.ASMUserBlocking)
-	a.unregisterRCCapability(remoteconfig.ASMCustomRules)
-	a.rc.UnregisterCallback(a.onRCRulesUpdate)
+	caps := []remoteconfig.Capability{
+		remoteconfig.ASMDDRules,
+		remoteconfig.ASMExclusions,
+		remoteconfig.ASMIPBlocking,
+		remoteconfig.ASMRequestBlocking,
+		remoteconfig.ASMUserBlocking,
+		remoteconfig.ASMCustomRules,
+	}
+	for _, c := range caps {
+		if err := a.unregisterRCCapability(c); err != nil {
+			log.Debug("appsec: Remote config: couldn't unregister capability %v: %v", c, err)
+		}
+	}
+	if err := remoteconfig.UnregisterCallback(a.onRCRulesUpdate); err != nil {
+		log.Debug("appsec: Remote config: couldn't unregister callback: %v", err)
+	}
 }

internal/appsec/remoteconfig_test.go

@@ -33,6 +33,8 @@ func TestASMFeaturesCallback(t *testing.T) {
 	cfg, err := newConfig()
 	require.NoError(t, err)
 	a := newAppSec(cfg)
+	err = a.startRC()
+	require.NoError(t, err)
 
 	t.Setenv(enabledEnvVar, "")
 	os.Unsetenv(enabledEnvVar)
@@ -333,22 +335,27 @@ func TestRemoteActivationScenarios(t *testing.T) {
 
 		require.NotNil(t, activeAppSec)
 		require.False(t, Enabled())
-		client := activeAppSec.rc
-		require.NotNil(t, client)
-		require.Contains(t, client.Capabilities, remoteconfig.ASMActivation)
-		require.Contains(t, client.Products, rc.ProductASMFeatures)
+		found, err := remoteconfig.HasCapability(remoteconfig.ASMActivation)
+		require.NoError(t, err)
+		require.True(t, found)
+		found, err = remoteconfig.HasProduct(rc.ProductASMFeatures)
+		require.NoError(t, err)
+		require.True(t, found)
 	})
 
 	t.Run("DD_APPSEC_ENABLED=true", func(t *testing.T) {
 		t.Setenv(enabledEnvVar, "true")
+		remoteconfig.Reset()
 		Start(WithRCConfig(remoteconfig.DefaultClientConfig()))
 		defer Stop()
 
 		require.True(t, Enabled())
-		client := activeAppSec.rc
-		require.NotNil(t, client)
-		require.NotContains(t, client.Capabilities, remoteconfig.ASMActivation)
-		require.NotContains(t, client.Products, rc.ProductASMFeatures)
+		found, err := remoteconfig.HasCapability(remoteconfig.ASMActivation)
+		require.NoError(t, err)
+		require.False(t, found)
+		found, err = remoteconfig.HasProduct(rc.ProductASMFeatures)
+		require.NoError(t, err)
+		require.False(t, found)
 	})
 
 	t.Run("DD_APPSEC_ENABLED=false", func(t *testing.T) {
@@ -397,11 +404,10 @@ func TestCapabilities(t *testing.T) {
 			if !Enabled() && activeAppSec == nil {
 				t.Skip()
 			}
-			require.NotNil(t, activeAppSec.rc)
-			require.Len(t, activeAppSec.rc.Capabilities, len(tc.expected))
 			for _, cap := range tc.expected {
-				_, contained := activeAppSec.rc.Capabilities[cap]
-				require.True(t, contained)
+				found, err := remoteconfig.HasCapability(cap)
+				require.NoError(t, err)
+				require.True(t, found)
 			}
 		})
 	}

internal/remoteconfig/config.go

@@ -30,8 +30,6 @@ type ClientConfig struct {
 	Env string
 	// The time interval between two client polls to the agent for updates
 	PollInterval time.Duration
-	// The products this client is interested in
-	Products map[string]struct{}
 	// The tracer's runtime id
 	RuntimeID string
 	// The name of the user's application
@@ -40,17 +38,13 @@ type ClientConfig struct {
 	TracerVersion string
 	// The base TUF root metadata file
 	TUFRoot string
-	// The capabilities of the client
-	Capabilities map[Capability]struct{}
 	// HTTP is the HTTP client used to receive config updates
 	HTTP *http.Client
 }
 
 // DefaultClientConfig returns the default remote config client configuration
 func DefaultClientConfig() ClientConfig {
 	return ClientConfig{
-		Capabilities:  map[Capability]struct{}{},
-		Products:      map[string]struct{}{},
 		Env:           os.Getenv("DD_ENV"),
 		HTTP:          &http.Client{Timeout: 10 * time.Second},
 		PollInterval:  pollIntervalFromEnv(),