From 62b660b739cbabee9471e4a5cdf5741a9174f1a3 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 7 May 2021 10:05:02 +0200 Subject: [PATCH] Ensure SnapshotsInProgress is Immutable 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 --- .../java/org/elasticsearch/cluster/SnapshotsInProgress.java | 6 +++--- .../java/org/elasticsearch/snapshots/SnapshotsService.java | 4 +++- .../repositories/encrypted/EncryptedRepository.java | 3 +-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java index f368ece2e8908..b515aaec47000 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java @@ -539,14 +539,14 @@ private Entry(Snapshot snapshot, boolean includeGlobalState, boolean partial, St 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) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 10d270e8ceffa..f077169919449 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1306,7 +1306,9 @@ private void finalizeSnapshotEntry(SnapshotsInProgress.Entry entry, Metadata met 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( diff --git a/x-pack/plugin/repository-encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java b/x-pack/plugin/repository-encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java index ccc341b345e23..86e3101e93132 100644 --- a/x-pack/plugin/repository-encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java +++ b/x-pack/plugin/repository-encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java @@ -197,8 +197,7 @@ public Map adaptUserMetadata(Map userMetadata) { 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