Enable Refresh() to be called multiple times

metadata/trustedmetadata/trustedmetadata.go

@@ -57,9 +57,6 @@ func New(rootData []byte) (*TrustedMetadata, error) {
 func (trusted *TrustedMetadata) UpdateRoot(rootData []byte) (*metadata.Metadata[metadata.RootType], error) {
 	log := metadata.GetLogger()
 
-	if trusted.Timestamp != nil {
-		return nil, &metadata.ErrRuntime{Msg: "cannot update root after timestamp"}
-	}
 	log.Info("Updating root")
 	// generate root metadata
 	newRoot, err := metadata.Root().FromBytes(rootData)
@@ -99,9 +96,6 @@ func (trusted *TrustedMetadata) UpdateRoot(rootData []byte) (*metadata.Metadata[
 func (trusted *TrustedMetadata) UpdateTimestamp(timestampData []byte) (*metadata.Metadata[metadata.TimestampType], error) {
 	log := metadata.GetLogger()
 
-	if trusted.Snapshot != nil {
-		return nil, &metadata.ErrRuntime{Msg: "cannot update timestamp after snapshot"}
-	}
 	// client workflow 5.3.10: Make sure final root is not expired.
 	if trusted.Root.Signed.IsExpired(trusted.RefTime) {
 		// no need to check for 5.3.11 (fast forward attack recovery):
@@ -178,9 +172,6 @@ func (trusted *TrustedMetadata) UpdateSnapshot(snapshotData []byte, isTrusted bo
 	if trusted.Timestamp == nil {
 		return nil, &metadata.ErrRuntime{Msg: "cannot update snapshot before timestamp"}
 	}
-	if trusted.Targets[metadata.TARGETS] != nil {
-		return nil, &metadata.ErrRuntime{Msg: "cannot update snapshot after targets"}
-	}
 	log.Info("Updating snapshot")
 
 	// snapshot cannot be loaded if final timestamp is expired

metadata/trustedmetadata/trustedmetadata_test.go

@@ -227,9 +227,10 @@ func TestOutOfOrderOps(t *testing.T) {
 	_, err = trustedSet.UpdateTimestamp(allRoles[metadata.TIMESTAMP])
 	assert.NoError(t, err)
 
-	// Update root after timestamp
+	// Update root after timestamp is now allowed (for multiple Refresh() calls)
+	// but will fail due to version number check (expects v2, has v1)
 	_, err = trustedSet.UpdateRoot(allRoles[metadata.ROOT])
-	assert.ErrorIs(t, err, &metadata.ErrRuntime{Msg: "cannot update root after timestamp"})
+	assert.ErrorIs(t, err, &metadata.ErrBadVersionNumber{Msg: "bad version number, expected 2, got 1"})
 
 	// Update targets before snapshot
 	_, err = trustedSet.UpdateTargets(allRoles[metadata.TARGETS])
@@ -238,9 +239,10 @@ func TestOutOfOrderOps(t *testing.T) {
 	_, err = trustedSet.UpdateSnapshot(allRoles[metadata.SNAPSHOT], false)
 	assert.NoError(t, err)
 
-	// Update timestamp after snapshot
+	// Update timestamp after snapshot is now allowed (for multiple Refresh() calls)
+	// and will succeed since versions are equal
 	_, err = trustedSet.UpdateTimestamp(allRoles[metadata.TIMESTAMP])
-	assert.ErrorIs(t, err, &metadata.ErrRuntime{Msg: "cannot update timestamp after snapshot"})
+	assert.ErrorIs(t, err, &metadata.ErrEqualVersionNumber{Msg: "new timestamp version 1 equals the old one 1"})
 
 	// Update delegated targets before targets
 	_, err = trustedSet.UpdateDelegatedTargets(allRoles["role1"], "role1", metadata.TARGETS)
@@ -249,9 +251,10 @@ func TestOutOfOrderOps(t *testing.T) {
 	_, err = trustedSet.UpdateTargets(allRoles[metadata.TARGETS])
 	assert.NoError(t, err)
 
-	//  Update snapshot after sucessful targets update
+	//  Update snapshot after targets is now allowed (for multiple Refresh() calls)
+	// and will succeed since versions are equal
 	_, err = trustedSet.UpdateSnapshot(allRoles[metadata.SNAPSHOT], false)
-	assert.ErrorIs(t, err, &metadata.ErrRuntime{Msg: "cannot update snapshot after targets"})
+	assert.NoError(t, err)
 
 	_, err = trustedSet.UpdateDelegatedTargets(allRoles["role1"], "role1", metadata.TARGETS)
 	assert.NoError(t, err)

metadata/updater/updater.go

@@ -102,7 +102,9 @@ func New(config *config.UpdaterConfig) (*Updater, error) {
 // Downloads, verifies, and loads metadata for the top-level roles in the
 // specified order (root -> timestamp -> snapshot -> targets) implementing
 // all the checks required in the TUF client workflow.
-// A Refresh() can be done only once during the lifetime of an Updater.
+// Refresh() can be called multiple times during the lifetime of an Updater
+// to ensure that the metadata is up-to-date. Each call will reload the
+// timestamp, snapshot, and targets metadata while preserving the root metadata.
 // If Refresh() has not been explicitly called before the first
 // GetTargetInfo() call, it will be done implicitly at that time.
 // The metadata for delegated roles is not updated by Refresh():
@@ -135,6 +137,9 @@ func (update *Updater) onlineRefresh() error {
 	if err != nil {
 		return err
 	}
+	// Remove the targets entry to allow re-loading during multiple Refresh() calls
+	// while still preventing redundant loads during GetTargetInfo()
+	delete(update.trusted.Targets, metadata.TARGETS)
 	_, err = update.loadTargets(metadata.TARGETS, metadata.ROOT)
 	if err != nil {
 		return err
@@ -313,12 +318,16 @@ func (update *Updater) loadTimestamp() error {
 			if errors.Is(err, &metadata.ErrRepository{}) {
 				// local timestamp is not valid, proceed downloading from remote; note that this error type includes several other subset errors
 				log.Info("Local timestamp is not valid")
+			} else if errors.Is(err, &metadata.ErrEqualVersionNumber{}) {
+				// local timestamp version equals current trusted version, proceed to check remote for updates
+				log.Info("Local timestamp version equals trusted version")
 			} else {
 				// another error
 				return err
 			}
+		} else {
+			log.Info("Local timestamp is valid")
 		}
-		log.Info("Local timestamp is valid")
 		// all okay, local timestamp exists and it is valid, nevertheless proceed with downloading from remote
 	}
 	// load from remote (whether local load succeeded or not)
@@ -368,12 +377,12 @@ func (update *Updater) loadSnapshot() error {
 			}
 		} else {
 			// this means snapshot verification/loading succeeded
-			log.Info("Local snapshot is valid: not downloading new one")
-			return nil
+			log.Info("Local snapshot is valid")
+			// Continue to check remote for potential updates
 		}
 	}
-	// local snapshot does not exist or is invalid, update from remote
-	log.Info("Failed to load local snapshot")
+	// check remote for updates (whether local load succeeded or not)
+	log.Info("Checking remote for snapshot updates")
 	if update.trusted.Timestamp == nil {
 		return fmt.Errorf("trusted timestamp not set")
 	}
@@ -422,7 +431,7 @@ func (update *Updater) loadTargets(roleName, parentName string) (*metadata.Metad
 		log.Info("Local role does not exist", "role", roleName)
 	} else {
 		// successfully read a local targets metadata, so let's try to verify and load it to the trusted metadata set
-		delegatedTargets, err := update.trusted.UpdateDelegatedTargets(data, roleName, parentName)
+		_, err := update.trusted.UpdateDelegatedTargets(data, roleName, parentName)
 		if err != nil {
 			// this means targets verification/loading failed
 			if errors.Is(err, &metadata.ErrRepository{}) {
@@ -434,12 +443,12 @@ func (update *Updater) loadTargets(roleName, parentName string) (*metadata.Metad
 			}
 		} else {
 			// this means targets verification/loading succeeded
-			log.Info("Local role is valid: not downloading new one", "role", roleName)
-			return delegatedTargets, nil
+			log.Info("Local role is valid", "role", roleName)
+			// Continue to check remote for potential updates
 		}
 	}
-	// local "roleName" does not exist or is invalid, update from remote
-	log.Info("Failed to load local role", "role", roleName)
+	// check remote for updates (whether local load succeeded or not)
+	log.Info("Checking remote for role updates", "role", roleName)
 	if update.trusted.Snapshot == nil {
 		return nil, fmt.Errorf("trusted snapshot not set")
 	}

metadata/updater/updater_top_level_update_test.go

@@ -1081,3 +1081,81 @@ func TestTimestampEqVersionsCheck(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, initialTimestampMetadataVer, timestamp.Signed.Meta["snapshot.json"].Version)
 }
+
+func TestMultipleRefreshCalls(t *testing.T) {
+	// Test that Refresh() can be called multiple times on the same Updater instance
+	// and that it successfully updates metadata when changes are available.
+
+	err := loadOrResetTrustedRootMetadata()
+	assert.NoError(t, err)
+
+	updaterConfig, err := loadUpdaterConfig()
+	assert.NoError(t, err)
+
+	// Create an updater and perform first refresh
+	updater := initUpdater(updaterConfig)
+	err = updater.Refresh()
+	assert.NoError(t, err)
+
+	// Verify initial versions
+	assertVersionEquals(t, metadata.TIMESTAMP, 1)
+	assertVersionEquals(t, metadata.SNAPSHOT, 1)
+	assertVersionEquals(t, metadata.TARGETS, 1)
+
+	// Update metadata on the repository
+	simulator.Sim.MDTargets.Signed.Version += 1
+	simulator.Sim.UpdateSnapshot()
+
+	// Call Refresh() again on the same updater instance
+	err = updater.Refresh()
+	assert.NoError(t, err)
+
+	// Verify that metadata was updated
+	assertVersionEquals(t, metadata.TIMESTAMP, 2)
+	assertVersionEquals(t, metadata.SNAPSHOT, 2)
+	assertVersionEquals(t, metadata.TARGETS, 2)
+
+	// Update metadata again
+	simulator.Sim.MDTargets.Signed.Version += 1
+	simulator.Sim.UpdateSnapshot()
+
+	// Call Refresh() a third time
+	err = updater.Refresh()
+	assert.NoError(t, err)
+
+	// Verify that metadata was updated again
+	assertVersionEquals(t, metadata.TIMESTAMP, 3)
+	assertVersionEquals(t, metadata.SNAPSHOT, 3)
+	assertVersionEquals(t, metadata.TARGETS, 3)
+}
+
+func TestMultipleRefreshCallsNoChanges(t *testing.T) {
+	// Test that Refresh() can be called multiple times even when there are no changes
+	// and returns nil (not an error) when everything is already up-to-date.
+
+	err := loadOrResetTrustedRootMetadata()
+	assert.NoError(t, err)
+
+	updaterConfig, err := loadUpdaterConfig()
+	assert.NoError(t, err)
+
+	// Create an updater and perform first refresh
+	updater := initUpdater(updaterConfig)
+	err = updater.Refresh()
+	assert.NoError(t, err)
+
+	// Verify initial versions
+	assertVersionEquals(t, metadata.TIMESTAMP, 1)
+	assertVersionEquals(t, metadata.SNAPSHOT, 1)
+	assertVersionEquals(t, metadata.TARGETS, 1)
+
+	// Call Refresh() again without any changes
+	// Should return nil (no error) since everything is up-to-date
+	err = updater.Refresh()
+	assert.NoError(t, err)
+
+	// Verify versions haven't changed
+	assertVersionEquals(t, metadata.TIMESTAMP, 1)
+	assertVersionEquals(t, metadata.SNAPSHOT, 1)
+	assertVersionEquals(t, metadata.TARGETS, 1)
+}