backport(remote-config): Backport 13382 & 13517 to 7.39.x (#13528)

* [RCM-132] Send back fetch error to backend (#13282) * Send back fetch error to backend * Fix test * (rcm) improve error reporting (#13382) * [RCM-432] fix(remote-config): Clean remote state on error & use fixed fork of go-tuf (#13517) * [RCM-432] fix(TUF): Clean remote state on TUF error * fix(go-tuf): Use fork of go-tuf for the time being. * refactor(uptane): Refactor the Update methods Co-authored-by: Paul <[email protected]> Co-authored-by: Arthur Bellal <[email protected]>
DataDog · Sep 16, 2022 · a645f67 · a645f67
1 parent ce9091f
commit a645f67
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 26 deletions.
diff --git a/go.mod b/go.mod
@@ -23,6 +23,7 @@ replace (
 	github.com/lxn/walk => github.com/lxn/walk v0.0.0-20180521183810-02935bac0ab8
 	github.com/mholt/archiver => github.com/mholt/archiver v2.0.1-0.20171012052341-26cf5bb32d07+incompatible
 	github.com/spf13/cast => github.com/DataDog/cast v1.3.1-0.20190301154711-1ee8c8bd14a3
+	github.com/theupdateframework/go-tuf => github.com/DataDog/go-tuf v0.3.0--fix-localmeta
 	github.com/ugorji/go => github.com/ugorji/go v1.1.7
 )
 

diff --git a/go.sum b/go.sum
diff --git a/pkg/config/remote/service/service.go b/pkg/config/remote/service/service.go
@@ -228,23 +228,13 @@ func (s *Service) refresh() error {
 	previousState, err := s.uptane.TUFVersionState()
 	if err != nil {
 		log.Warnf("could not get previous TUF version state: %v", err)
-		if s.lastUpdateErr != nil {
-			s.lastUpdateErr = fmt.Errorf("%v: %v", err, s.lastUpdateErr)
-		} else {
-			s.lastUpdateErr = err
-		}
 	}
 	if s.forceRefresh() || err != nil {
 		previousState = uptane.TUFVersions{}
 	}
 	clientState, err := s.getClientState()
 	if err != nil {
 		log.Warnf("could not get previous backend client state: %v", err)
-		if s.lastUpdateErr != nil {
-			s.lastUpdateErr = fmt.Errorf("%v: %v", err, s.lastUpdateErr)
-		} else {
-			s.lastUpdateErr = err
-		}
 	}
 	request := buildLatestConfigsRequest(s.hostname, previousState, activeClients, s.products, s.newProducts, s.lastUpdateErr, clientState)
 	s.Unlock()
@@ -254,12 +244,13 @@ func (s *Service) refresh() error {
 	s.lastUpdateErr = nil
 	if err != nil {
 		s.backoffErrorCount = s.backoffPolicy.IncError(s.backoffErrorCount)
+		s.lastUpdateErr = fmt.Errorf("api: %v", err)
 		return err
 	}
 	err = s.uptane.Update(response)
 	if err != nil {
 		s.backoffErrorCount = s.backoffPolicy.IncError(s.backoffErrorCount)
-		s.lastUpdateErr = err
+		s.lastUpdateErr = fmt.Errorf("tuf: %v", err)
 		return err
 	}
 	s.firstUpdate = false

diff --git a/pkg/config/remote/service/service_test.go b/pkg/config/remote/service/service_test.go
@@ -34,7 +34,7 @@ type mockAPI struct {
 }
 
 const (
-	jsonDecodingError = "unexpected end of JSON input"
+	httpError = "api: simulated HTTP error"
 )
 
 func (m *mockAPI) Fetch(ctx context.Context, request *pbgo.LatestConfigsRequest) (*pbgo.LatestConfigsResponse, error) {
@@ -127,8 +127,6 @@ func TestServiceBackoffFailure(t *testing.T) {
 		CurrentDirectorRootVersion:   0,
 		Products:                     []string{},
 		NewProducts:                  []string{},
-		HasError:                     true,
-		Error:                        jsonDecodingError,
 	}).Return(lastConfigResponse, errors.New("simulated HTTP error"))
 	uptaneClient.On("TUFVersionState").Return(uptane.TUFVersions{}, nil)
 	uptaneClient.On("Update", lastConfigResponse).Return(nil)
@@ -143,6 +141,22 @@ func TestServiceBackoffFailure(t *testing.T) {
 	err := service.refresh()
 	assert.NotNil(t, err)
 
+	// Sending the http error too
+	api.On("Fetch", mock.Anything, &pbgo.LatestConfigsRequest{
+		Hostname:                     service.hostname,
+		AgentVersion:                 version.AgentVersion,
+		CurrentConfigSnapshotVersion: 0,
+		CurrentConfigRootVersion:     0,
+		CurrentDirectorRootVersion:   0,
+		Products:                     []string{},
+		NewProducts:                  []string{},
+		HasError:                     true,
+		Error:                        httpError,
+	}).Return(lastConfigResponse, errors.New("simulated HTTP error"))
+	uptaneClient.On("TUFVersionState").Return(uptane.TUFVersions{}, nil)
+	uptaneClient.On("Update", lastConfigResponse).Return(nil)
+	uptaneClient.On("TargetsCustom").Return([]byte{}, nil)
+
 	// We should be tracking an error now. With the default backoff config, our refresh interval
 	// should be somewhere in the range of 1 + [30,60], so [31,61]
 	assert.Equal(t, service.backoffErrorCount, 1)
@@ -188,8 +202,6 @@ func TestServiceBackoffFailureRecovery(t *testing.T) {
 		CurrentDirectorRootVersion:   0,
 		Products:                     []string{},
 		NewProducts:                  []string{},
-		HasError:                     true,
-		Error:                        jsonDecodingError,
 	}).Return(lastConfigResponse, nil)
 	uptaneClient.On("TUFVersionState").Return(uptane.TUFVersions{}, nil)
 	uptaneClient.On("Update", lastConfigResponse).Return(nil)
@@ -316,8 +328,6 @@ func TestService(t *testing.T) {
 		CurrentDirectorRootVersion:   0,
 		Products:                     []string{},
 		NewProducts:                  []string{},
-		HasError:                     true,
-		Error:                        jsonDecodingError,
 	}).Return(lastConfigResponse, nil)
 	uptaneClient.On("TUFVersionState").Return(uptane.TUFVersions{}, nil)
 	uptaneClient.On("Update", lastConfigResponse).Return(nil)

diff --git a/pkg/config/remote/service/util.go b/pkg/config/remote/service/util.go
@@ -90,6 +90,9 @@ type targetsCustom struct {
 }
 
 func parseTargetsCustom(rawTargetsCustom []byte) (targetsCustom, error) {
+	if len(rawTargetsCustom) == 0 {
+		return targetsCustom{}, nil
+	}
 	var custom targetsCustom
 	err := json.Unmarshal(rawTargetsCustom, &custom)
 	if err != nil {

diff --git a/pkg/config/remote/uptane/client.go b/pkg/config/remote/uptane/client.go
@@ -69,7 +69,7 @@ func NewClient(cacheDB *bbolt.DB, cacheKey string, orgID int64) (*Client, error)
 	return c, nil
 }
 
-// Update updates the uptane client
+// Update updates the uptane client and rollbacks in case of error
 func (c *Client) Update(response *pbgo.LatestConfigsResponse) error {
 	c.Lock()
 	defer c.Unlock()
@@ -79,20 +79,28 @@ func (c *Client) Update(response *pbgo.LatestConfigsResponse) error {
 	// the defer is present to be sure a transaction is never left behind.
 	defer c.transactionalStore.rollback()
 
-	err := c.updateRepos(response)
+	err := c.update(response)
 	if err != nil {
+		c.configRemoteStore = newRemoteStoreConfig(c.targetStore)
+		c.directorRemoteStore = newRemoteStoreDirector(c.targetStore)
+		c.configTUFClient = client.NewClient(c.configLocalStore, c.configRemoteStore)
+		c.directorTUFClient = client.NewClient(c.directorLocalStore, c.directorRemoteStore)
 		return err
 	}
-	err = c.pruneTargetFiles()
+	return c.transactionalStore.commit()
+}
+
+// update updates the uptane client
+func (c *Client) update(response *pbgo.LatestConfigsResponse) error {
+	err := c.updateRepos(response)
 	if err != nil {
 		return err
 	}
-	err = c.verify()
+	err = c.pruneTargetFiles()
 	if err != nil {
 		return err
 	}
-
-	return c.transactionalStore.commit()
+	return c.verify()
 }
 
 // TargetsCustom returns the current targets custom of this uptane client