From 405bf6ee3ad46ed515cbd2023f66f50e39b84ed0 Mon Sep 17 00:00:00 2001 From: Kyle Ames Date: Tue, 17 Dec 2024 16:05:35 -0500 Subject: [PATCH 1/6] [RC] Support configuring the core RC service with JWT auth Private Action Runners are now leverging the core RC service in order to receive remote config updates. They require a different authentication scheme than Datadog API keys. This allows for the core service to be configured to use a JWT for authentication to the backend. It is not configurable via the command line or an agent YAML file, as it is only used programmatically from within the Datadog Private Action Runner. There are no other use cases. --- .../rcservice/rcserviceimpl/rcservice.go | 2 ++ pkg/config/remote/api/http.go | 14 +++++++++++++- pkg/config/remote/service/service.go | 14 +++++++++++++- pkg/config/remote/service/service_test.go | 4 ++++ pkg/config/remote/service/util.go | 9 ++++++++- pkg/config/remote/service/util_test.go | 3 ++- 6 files changed, 42 insertions(+), 4 deletions(-) diff --git a/comp/remote-config/rcservice/rcserviceimpl/rcservice.go b/comp/remote-config/rcservice/rcserviceimpl/rcservice.go index 48226f0dcb916..34b5475f28760 100644 --- a/comp/remote-config/rcservice/rcserviceimpl/rcservice.go +++ b/comp/remote-config/rcservice/rcserviceimpl/rcservice.go @@ -75,6 +75,8 @@ func newRemoteConfigService(deps dependencies) (rcservice.Component, error) { options := []remoteconfig.Option{ remoteconfig.WithAPIKey(apiKey), + // KEA: Temp For Testing Out Impl in Staging + remoteconfig.WithPARJWT("eyJhbGciOiJFUzI1NiIsImN0eSI6IkpXVCIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MzUyOTU4NjEsIm9yZ0lkIjoyLCJydW5uZXJJZCI6InJ1bm5lci1YUzI2ZWFUVkZaM21KWDNMWktDWXhRIn0.0ZSfsRyZdKzHG_wYjyJAI9zPG7t-ZPhW7U9x0vqMs10cOO_RCbwJCafmZt3LJlCImk3K5wuODEs6RQBQYbCwOQ"), remoteconfig.WithTraceAgentEnv(traceAgentEnv), remoteconfig.WithConfigRootOverride(deps.Cfg.GetString("site"), deps.Cfg.GetString("remote_configuration.config_root")), remoteconfig.WithDirectorRootOverride(deps.Cfg.GetString("site"), deps.Cfg.GetString("remote_configuration.director_root")), diff --git a/pkg/config/remote/api/http.go b/pkg/config/remote/api/http.go index 62f7872a2e671..7f42a2b56ca59 100644 --- a/pkg/config/remote/api/http.go +++ b/pkg/config/remote/api/http.go @@ -46,11 +46,13 @@ type API interface { Fetch(context.Context, *pbgo.LatestConfigsRequest) (*pbgo.LatestConfigsResponse, error) FetchOrgData(context.Context) (*pbgo.OrgDataResponse, error) FetchOrgStatus(context.Context) (*pbgo.OrgStatusResponse, error) + UpdatePARJWT(string) } // Auth defines the possible Authentication data to access the RC backend type Auth struct { APIKey string + PARJWT string AppKey string UseAppKey bool } @@ -66,11 +68,17 @@ type HTTPClient struct { func NewHTTPClient(auth Auth, cfg model.Reader, baseURL *url.URL) (*HTTPClient, error) { header := http.Header{ "Content-Type": []string{"application/x-protobuf"}, - "DD-Api-Key": []string{auth.APIKey}, + } + if auth.PARJWT != "" { + header["DD-PAR-JWT"] = []string{auth.PARJWT} + } + if auth.APIKey != "" { + header["DD-Api-Key"] = []string{auth.APIKey} } if auth.UseAppKey { header["DD-Application-Key"] = []string{auth.AppKey} } + transport := httputils.CreateHTTPTransport(cfg) // Set the keep-alive timeout to 30s instead of the default 90s, so the http RC client is not closed by the backend transport.IdleConnTimeout = 30 * time.Second @@ -216,6 +224,10 @@ func (c *HTTPClient) FetchOrgStatus(ctx context.Context) (*pbgo.OrgStatusRespons return response, err } +func (c *HTTPClient) UpdatePARJWT(jwt string) { + c.header.Set("DD-PAR-JWT", jwt) +} + func checkStatusCode(resp *http.Response) error { // Specific case: authentication method is wrong // we want to be descriptive about what can be done diff --git a/pkg/config/remote/service/service.go b/pkg/config/remote/service/service.go index a379ac5b387ec..ff045f79a431c 100644 --- a/pkg/config/remote/service/service.go +++ b/pkg/config/remote/service/service.go @@ -248,6 +248,7 @@ type options struct { site string rcKey string apiKey string + parJWT string traceAgentEnv string databaseFileName string databaseFilePath string @@ -264,6 +265,7 @@ type options struct { var defaultOptions = options{ rcKey: "", apiKey: "", + parJWT: "", traceAgentEnv: "", databaseFileName: "remote-config.db", databaseFilePath: "", @@ -355,6 +357,11 @@ func WithAPIKey(apiKey string) func(s *options) { return func(s *options) { s.apiKey = apiKey } } +// WithPARJWT sets the JWT for the private action runner +func WithPARJWT(jwt string) func(s *options) { + return func(s *options) { s.parJWT = jwt } +} + // WithClientCacheBypassLimit validates and sets the service client cache bypass limit func WithClientCacheBypassLimit(limit int, cfgPath string) func(s *options) { if limit < minCacheBypassLimit || limit > maxCacheBypassLimit { @@ -415,7 +422,7 @@ func NewService(cfg model.Reader, rcType, baseRawURL, hostname string, tagsGette backoffPolicy := backoff.NewExpBackoffPolicy(minBackoffFactor, baseBackoffTime, options.maxBackoff.Seconds(), recoveryInterval, recoveryReset) - authKeys, err := getRemoteConfigAuthKeys(options.apiKey, options.rcKey) + authKeys, err := getRemoteConfigAuthKeys(options.apiKey, options.rcKey, options.parJWT) if err != nil { return nil, err } @@ -532,6 +539,11 @@ func (s *CoreAgentService) Start() { }() } +// UpdatePARJWT updates the stored JWT for HTTP API calls +func (s *CoreAgentService) UpdatePARJWT(jwt string) { + s.api.UpdatePARJWT(jwt) +} + func startWithAgentPollLoop(s *CoreAgentService) { err := s.refresh() if err != nil { diff --git a/pkg/config/remote/service/service_test.go b/pkg/config/remote/service/service_test.go index 40d8302db3192..8e36efe6b3402 100644 --- a/pkg/config/remote/service/service_test.go +++ b/pkg/config/remote/service/service_test.go @@ -69,6 +69,10 @@ func (m *mockAPI) FetchOrgStatus(ctx context.Context) (*pbgo.OrgStatusResponse, return args.Get(0).(*pbgo.OrgStatusResponse), args.Error(1) } +func (m *mockAPI) UpdatePARJWT(jwt string) { + m.Called(jwt) +} + type mockUptane struct { mock.Mock } diff --git a/pkg/config/remote/service/util.go b/pkg/config/remote/service/util.go index b0d541e964f54..4f91ed6ceb48b 100644 --- a/pkg/config/remote/service/util.go +++ b/pkg/config/remote/service/util.go @@ -139,6 +139,8 @@ func openCacheDB(path string, agentVersion string, apiKey string) (*bbolt.DB, er type remoteConfigAuthKeys struct { apiKey string + parJWT string + rcKeySet bool rcKey *msgpgo.RemoteConfigKey } @@ -146,6 +148,7 @@ type remoteConfigAuthKeys struct { func (k *remoteConfigAuthKeys) apiAuth() api.Auth { auth := api.Auth{ APIKey: k.apiKey, + PARJWT: k.parJWT, } if k.rcKeySet { auth.UseAppKey = true @@ -154,12 +157,15 @@ func (k *remoteConfigAuthKeys) apiAuth() api.Auth { return auth } -func getRemoteConfigAuthKeys(apiKey string, rcKey string) (remoteConfigAuthKeys, error) { +func getRemoteConfigAuthKeys(apiKey string, rcKey string, parJWT string) (remoteConfigAuthKeys, error) { if rcKey == "" { return remoteConfigAuthKeys{ apiKey: apiKey, + parJWT: parJWT, }, nil } + + // Legacy auth with RC specific keys rcKey = strings.TrimPrefix(rcKey, "DDRCM_") encoding := base32.StdEncoding.WithPadding(base32.NoPadding) rawKey, err := encoding.DecodeString(rcKey) @@ -176,6 +182,7 @@ func getRemoteConfigAuthKeys(apiKey string, rcKey string) (remoteConfigAuthKeys, } return remoteConfigAuthKeys{ apiKey: apiKey, + parJWT: parJWT, rcKeySet: true, rcKey: &key, }, nil diff --git a/pkg/config/remote/service/util_test.go b/pkg/config/remote/service/util_test.go index ba4b8eaff1f89..2eb4cabcaedcd 100644 --- a/pkg/config/remote/service/util_test.go +++ b/pkg/config/remote/service/util_test.go @@ -27,6 +27,7 @@ func TestAuthKeys(t *testing.T) { tests := []struct { rcKey string apiKey string + parJWT string err bool output remoteConfigAuthKeys }{ @@ -45,7 +46,7 @@ func TestAuthKeys(t *testing.T) { } for _, test := range tests { t.Run(fmt.Sprintf("%s|%s", test.apiKey, test.rcKey), func(tt *testing.T) { - output, err := getRemoteConfigAuthKeys(test.apiKey, test.rcKey) + output, err := getRemoteConfigAuthKeys(test.apiKey, test.rcKey, test.parJWT) if test.err { assert.Error(tt, err) } else { From 22e07c89ebb556d72c7b2a0434b3ef9ed0c1089f Mon Sep 17 00:00:00 2001 From: Kyle Ames Date: Fri, 20 Dec 2024 11:49:45 -0500 Subject: [PATCH 2/6] remove testing stuff --- comp/remote-config/rcservice/rcserviceimpl/rcservice.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/comp/remote-config/rcservice/rcserviceimpl/rcservice.go b/comp/remote-config/rcservice/rcserviceimpl/rcservice.go index 34b5475f28760..48226f0dcb916 100644 --- a/comp/remote-config/rcservice/rcserviceimpl/rcservice.go +++ b/comp/remote-config/rcservice/rcserviceimpl/rcservice.go @@ -75,8 +75,6 @@ func newRemoteConfigService(deps dependencies) (rcservice.Component, error) { options := []remoteconfig.Option{ remoteconfig.WithAPIKey(apiKey), - // KEA: Temp For Testing Out Impl in Staging - remoteconfig.WithPARJWT("eyJhbGciOiJFUzI1NiIsImN0eSI6IkpXVCIsInR5cCI6IkpXVCJ9.eyJleHAiOjE3MzUyOTU4NjEsIm9yZ0lkIjoyLCJydW5uZXJJZCI6InJ1bm5lci1YUzI2ZWFUVkZaM21KWDNMWktDWXhRIn0.0ZSfsRyZdKzHG_wYjyJAI9zPG7t-ZPhW7U9x0vqMs10cOO_RCbwJCafmZt3LJlCImk3K5wuODEs6RQBQYbCwOQ"), remoteconfig.WithTraceAgentEnv(traceAgentEnv), remoteconfig.WithConfigRootOverride(deps.Cfg.GetString("site"), deps.Cfg.GetString("remote_configuration.config_root")), remoteconfig.WithDirectorRootOverride(deps.Cfg.GetString("site"), deps.Cfg.GetString("remote_configuration.director_root")), From 6ff7b9e2acef20ecafa65012f77789f4ad09bd02 Mon Sep 17 00:00:00 2001 From: Kyle Ames Date: Fri, 20 Dec 2024 14:18:09 -0500 Subject: [PATCH 3/6] protect against concurrent access --- pkg/config/remote/api/http.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/pkg/config/remote/api/http.go b/pkg/config/remote/api/http.go index 7f42a2b56ca59..db010e16691f1 100644 --- a/pkg/config/remote/api/http.go +++ b/pkg/config/remote/api/http.go @@ -13,6 +13,7 @@ import ( "io" "net/http" "net/url" + "sync" "time" "google.golang.org/protobuf/proto" @@ -61,7 +62,9 @@ type Auth struct { type HTTPClient struct { baseURL string client *http.Client - header http.Header + + headerLock sync.RWMutex + header http.Header } // NewHTTPClient returns a new HTTP configuration client @@ -112,7 +115,10 @@ func (c *HTTPClient) Fetch(ctx context.Context, request *pbgo.LatestConfigsReque if err != nil { return nil, fmt.Errorf("failed to create org data request: %w", err) } + + c.headerLock.RLock() req.Header = c.header + c.headerLock.Unlock() resp, err := c.client.Do(req) if err != nil { @@ -158,7 +164,10 @@ func (c *HTTPClient) FetchOrgData(ctx context.Context) (*pbgo.OrgDataResponse, e if err != nil { return nil, fmt.Errorf("failed to create org data request: %w", err) } + + c.headerLock.RLock() req.Header = c.header + c.headerLock.Unlock() resp, err := c.client.Do(req) if err != nil { @@ -195,7 +204,10 @@ func (c *HTTPClient) FetchOrgStatus(ctx context.Context) (*pbgo.OrgStatusRespons if err != nil { return nil, fmt.Errorf("failed to create org data request: %w", err) } + + c.headerLock.RLock() req.Header = c.header + c.headerLock.Unlock() resp, err := c.client.Do(req) if err != nil { @@ -225,7 +237,9 @@ func (c *HTTPClient) FetchOrgStatus(ctx context.Context) (*pbgo.OrgStatusRespons } func (c *HTTPClient) UpdatePARJWT(jwt string) { + c.headerLock.Lock() c.header.Set("DD-PAR-JWT", jwt) + c.headerLock.Unlock() } func checkStatusCode(resp *http.Response) error { From d1620b9d6050db863415be0411ac87fa9aae2b6e Mon Sep 17 00:00:00 2001 From: Kyle Ames Date: Fri, 20 Dec 2024 15:00:14 -0500 Subject: [PATCH 4/6] add tests to option -> auth processing --- pkg/config/remote/service/util_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/config/remote/service/util_test.go b/pkg/config/remote/service/util_test.go index 2eb4cabcaedcd..7b2883f134974 100644 --- a/pkg/config/remote/service/util_test.go +++ b/pkg/config/remote/service/util_test.go @@ -43,6 +43,13 @@ func TestAuthKeys(t *testing.T) { {rcKey: generateKey(t, 2, "datadoghq.com", ""), err: true}, {rcKey: generateKey(t, 2, "", "app_Key"), err: true}, {rcKey: generateKey(t, 0, "datadoghq.com", "app_Key"), err: true}, + {parJWT: "myJWT", err: false, output: remoteConfigAuthKeys{ + parJWT: "myJWT", + }}, + {parJWT: "myJWT", apiKey: "myAPIKey", err: false, output: remoteConfigAuthKeys{ + parJWT: "myJWT", + apiKey: "myAPIKey", + }}, } for _, test := range tests { t.Run(fmt.Sprintf("%s|%s", test.apiKey, test.rcKey), func(tt *testing.T) { From 10f494f63e9e06b4bab098b774bf02a09987b774 Mon Sep 17 00:00:00 2001 From: Kyle Ames Date: Fri, 20 Dec 2024 15:15:06 -0500 Subject: [PATCH 5/6] linter comments --- pkg/config/remote/api/http.go | 2 ++ pkg/config/remote/service/service.go | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/config/remote/api/http.go b/pkg/config/remote/api/http.go index db010e16691f1..9fc1ddf241d63 100644 --- a/pkg/config/remote/api/http.go +++ b/pkg/config/remote/api/http.go @@ -236,6 +236,8 @@ func (c *HTTPClient) FetchOrgStatus(ctx context.Context) (*pbgo.OrgStatusRespons return response, err } +// UpdatePARJWT allows for dynamic setting of a Private Action Runners JWT +// Token for authentication to the RC backend. func (c *HTTPClient) UpdatePARJWT(jwt string) { c.headerLock.Lock() c.header.Set("DD-PAR-JWT", jwt) diff --git a/pkg/config/remote/service/service.go b/pkg/config/remote/service/service.go index ff045f79a431c..0d41b688c253f 100644 --- a/pkg/config/remote/service/service.go +++ b/pkg/config/remote/service/service.go @@ -539,7 +539,8 @@ func (s *CoreAgentService) Start() { }() } -// UpdatePARJWT updates the stored JWT for HTTP API calls +// UpdatePARJWT updates the stored JWT for Private Action Runners +// for authentication to the remote config backend. func (s *CoreAgentService) UpdatePARJWT(jwt string) { s.api.UpdatePARJWT(jwt) } From 791f4485c17676fce8deab2c92a942c0be40e4d5 Mon Sep 17 00:00:00 2001 From: Dario Meloni Date: Mon, 23 Dec 2024 10:04:12 +0100 Subject: [PATCH 6/6] Changes to internal headers have no side effects Changing c.headers with the update call would change also the contents in req.Header. To avoid this possible race condition the headers are copied into the request. --- pkg/config/remote/api/http.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/config/remote/api/http.go b/pkg/config/remote/api/http.go index 9fc1ddf241d63..bdf13e5acd0ea 100644 --- a/pkg/config/remote/api/http.go +++ b/pkg/config/remote/api/http.go @@ -116,9 +116,7 @@ func (c *HTTPClient) Fetch(ctx context.Context, request *pbgo.LatestConfigsReque return nil, fmt.Errorf("failed to create org data request: %w", err) } - c.headerLock.RLock() - req.Header = c.header - c.headerLock.Unlock() + c.addHeaders(req) resp, err := c.client.Do(req) if err != nil { @@ -165,9 +163,7 @@ func (c *HTTPClient) FetchOrgData(ctx context.Context) (*pbgo.OrgDataResponse, e return nil, fmt.Errorf("failed to create org data request: %w", err) } - c.headerLock.RLock() - req.Header = c.header - c.headerLock.Unlock() + c.addHeaders(req) resp, err := c.client.Do(req) if err != nil { @@ -205,9 +201,7 @@ func (c *HTTPClient) FetchOrgStatus(ctx context.Context) (*pbgo.OrgStatusRespons return nil, fmt.Errorf("failed to create org data request: %w", err) } - c.headerLock.RLock() - req.Header = c.header - c.headerLock.Unlock() + c.addHeaders(req) resp, err := c.client.Do(req) if err != nil { @@ -244,6 +238,14 @@ func (c *HTTPClient) UpdatePARJWT(jwt string) { c.headerLock.Unlock() } +func (c *HTTPClient) addHeaders(req *http.Request) { + c.headerLock.RLock() + for k, v := range c.header { + req.Header[k] = v + } + c.headerLock.RUnlock() +} + func checkStatusCode(resp *http.Response) error { // Specific case: authentication method is wrong // we want to be descriptive about what can be done