Browse Source

Ensure SnapshotsInProgress is Immutable (#72842)

The user metadata map in `SnapshotsInProgress.Entry` must not be mutable.
This change makes all the collections in the snapshot entry instances
immutable. Unfortunately, the `EncryptedRepository` relies on the fact that
this map is mutable so this commit contains a hack to keep it mutable
for now. It does not contain a proper fix because work will shortly be merged
removing the need for a mutable map in `SnapshotInfo` as well and fixing
the current implementation of `EncryptedRepository` would require adjusting
the `Repository` interface.

closes #72782
Armin Braun 4 years ago
parent
commit
8e564d48a6

+ 3 - 3
server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java

@@ -539,14 +539,14 @@ public class SnapshotsInProgress extends AbstractNamedDiffable<Custom> implement
             this.snapshot = snapshot;
             this.includeGlobalState = includeGlobalState;
             this.partial = partial;
-            this.indices = indices;
-            this.dataStreams = dataStreams;
+            this.indices = Map.copyOf(indices);
+            this.dataStreams = List.copyOf(dataStreams);
             this.featureStates = Collections.unmodifiableList(featureStates);
             this.startTime = startTime;
             this.shards = shards;
             this.repositoryStateId = repositoryStateId;
             this.failure = failure;
-            this.userMetadata = userMetadata;
+            this.userMetadata = userMetadata == null ? null : Map.copyOf(userMetadata);
             this.version = version;
             this.source = source;
             if (source == null) {

+ 3 - 1
server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java

@@ -1306,7 +1306,9 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
                                 entry.partial() ? shardGenerations.totalShards() : entry.shards().size(),
                                 shardFailures,
                                 entry.includeGlobalState(),
-                                entry.userMetadata(),
+                                // TODO: remove this hack making the metadata mutable once
+                                //       https://github.com/elastic/elasticsearch/pull/72776 has been merged
+                                entry.userMetadata() == null ? null : new HashMap<>(entry.userMetadata()),
                                 entry.startTime(),
                                 indexSnapshotDetails);
                         repo.finalizeSnapshot(

+ 1 - 2
x-pack/plugin/repository-encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java

@@ -196,8 +196,7 @@ public class EncryptedRepository extends BlobStoreRepository {
             localRepositoryPasswordSalt,
             localRepositoryPasswordHash
         );
-        // do not wrap in Map.of; we have to be able to modify the map (remove the added entries) when finalizing the snapshot
-        return snapshotUserMetadata;
+        return Map.copyOf(snapshotUserMetadata);
     }
 
     @Override