Allow Initialize/NewFromEnv to reinitialize metadata if timestamp exp…

…ired (#893)

* Update initializeTUF to fetch new metadata when timestamp expires

This changes the singleton to only initialize the client once, but on
every invocation, check if the timestamp is expired.

I wrapped all of this in a mutex to prevent concurrent updates (I'm not
sure if this would be safe, I assume the TUF object is not thread-safe).

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* Add concurrent test

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* Use singleton error

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

pkg/tuf/client.go

@@ -58,6 +58,9 @@ var (
 	singletonTUF     *TUF
 	singletonTUFOnce = new(sync.Once)
 	singletonTUFErr  error
+
+	// initMu locks concurrent calls to initializeTUF
+	initMu sync.Mutex
 )
 
 // getRemoteRoot is a var for testing.
@@ -240,6 +243,11 @@ func GetRootStatus(ctx context.Context) (*RootStatus, error) {
 // * forceUpdate: indicates checking the remote for an update, even when the local
 // timestamp.json is up to date.
 func initializeTUF(mirror string, root []byte, embedded fs.FS, forceUpdate bool) (*TUF, error) {
+	initMu.Lock()
+	defer initMu.Unlock()
+
+	// TODO: If a temporary error occurs for a long-running process, this singleton will
+	// never retry
 	singletonTUFOnce.Do(func() {
 		t := &TUF{
 			mirror:   mirror,
@@ -280,24 +288,30 @@ func initializeTUF(mirror string, root []byte, embedded fs.FS, forceUpdate bool)
 			return
 		}
 
-		// We may already have an up-to-date local store! Check to see if it needs to be updated.
-		trustedTimestamp, ok := trustedMeta["timestamp.json"]
-		if ok && !isExpiredTimestamp(trustedTimestamp) && !forceUpdate {
-			// We're golden so stash the TUF object for later use
-			singletonTUF = t
-			return
-		}
+		singletonTUF = t
+	})
+	if singletonTUFErr != nil {
+		return nil, singletonTUFErr
+	}
 
-		// Update if local is not populated or out of date.
-		if err := t.updateMetadataAndDownloadTargets(); err != nil {
-			singletonTUFErr = fmt.Errorf("updating local metadata and targets: %w", err)
-			return
-		}
+	trustedMeta, err := singletonTUF.local.GetMeta()
+	if err != nil {
+		return nil, fmt.Errorf("getting trusted meta: %w", err)
+	}
 
+	// We may already have an up-to-date local store! Check to see if it needs to be updated.
+	trustedTimestamp, ok := trustedMeta["timestamp.json"]
+	if ok && !isExpiredTimestamp(trustedTimestamp) && !forceUpdate {
 		// We're golden so stash the TUF object for later use
-		singletonTUF = t
-	})
-	return singletonTUF, singletonTUFErr
+		return singletonTUF, nil
+	}
+
+	// Update if local is not populated or out of date.
+	if err := singletonTUF.updateMetadataAndDownloadTargets(); err != nil {
+		return nil, fmt.Errorf("updating local metadata and targets: %w", err)
+	}
+
+	return singletonTUF, nil
 }
 
 // TODO: Remove ctx arg.

pkg/tuf/client_test.go

@@ -693,7 +693,7 @@ func updateTufRepo(t *testing.T, td string, r *tuf.Repo, targetData string) {
 	}
 }
 
-func TestConcurrentAccess(t *testing.T) {
+func TestConcurrentAccessNewFromEnv(t *testing.T) {
 	var wg sync.WaitGroup
 
 	for i := 0; i < 20; i++ {
@@ -714,6 +714,24 @@ func TestConcurrentAccess(t *testing.T) {
 	resetForTests()
 }
 
+func TestConcurrentAccessInitialize(t *testing.T) {
+	var wg sync.WaitGroup
+
+	for i := 0; i < 20; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			err := Initialize(context.Background(), DefaultRemoteRoot, nil)
+			if err != nil {
+				t.Errorf("Failed to construct NewFromEnv: %s", err)
+			}
+			time.Sleep(1 * time.Second)
+		}()
+	}
+	wg.Wait()
+	resetForTests()
+}
+
 func TestKeyFormatMigration(t *testing.T) {
 	// Override the expiration time so the test doesn't fail on
 	// expiration.