Skip to content

Commit cca7474

Browse files
pranikumpranikum
committed
Address comments
Signed-off-by: Pranit Kumar <pranikum@amazon.com>
1 parent 3560a0a commit cca7474

File tree

25 files changed

+221
-921
lines changed

25 files changed

+221
-921
lines changed

‎plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -507,62 +507,6 @@ protected S3BlobStore createBlobStore() {
507507
);
508508
}
509509

510-
@Override
511-
protected S3BlobStore createClientSideEncryptedBlobStore() {
512-
serverSideEncryptionType = ServerSideEncryption.AES256.toString();
513-
return new S3BlobStore(
514-
service,
515-
s3AsyncService,
516-
multipartUploadEnabled,
517-
bucket,
518-
bufferSize,
519-
cannedACL,
520-
storageClass,
521-
bulkDeletesSize,
522-
metadata,
523-
asyncUploadUtils,
524-
urgentExecutorBuilder,
525-
priorityExecutorBuilder,
526-
normalExecutorBuilder,
527-
normalPrioritySizeBasedBlockingQ,
528-
lowPrioritySizeBasedBlockingQ,
529-
genericStatsMetricPublisher,
530-
serverSideEncryptionType,
531-
null,
532-
false,
533-
null,
534-
null
535-
);
536-
}
537-
538-
@Override
539-
protected S3BlobStore createServerSideEncryptedBlobStore() {
540-
serverSideEncryptionType = ServerSideEncryption.AWS_KMS.toString();
541-
return new S3BlobStore(
542-
service,
543-
s3AsyncService,
544-
multipartUploadEnabled,
545-
bucket,
546-
bufferSize,
547-
cannedACL,
548-
storageClass,
549-
bulkDeletesSize,
550-
metadata,
551-
asyncUploadUtils,
552-
urgentExecutorBuilder,
553-
priorityExecutorBuilder,
554-
normalExecutorBuilder,
555-
normalPrioritySizeBasedBlockingQ,
556-
lowPrioritySizeBasedBlockingQ,
557-
genericStatsMetricPublisher,
558-
serverSideEncryptionType,
559-
serverSideEncryptionKmsKey,
560-
serverSideEncryptionBucketKey,
561-
serverSideEncryptionEncryptionContext,
562-
expectedBucketOwner
563-
);
564-
}
565-
566510
// only use for testing (S3RepositoryTests)
567511
@Override
568512
protected BlobStore getBlobStore() {

‎plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3RepositoryTests.java‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
package org.opensearch.repositories.s3;
3434

3535
import software.amazon.awssdk.services.s3.S3Client;
36-
import software.amazon.awssdk.services.s3.model.ServerSideEncryption;
3736

3837
import org.opensearch.cluster.metadata.RepositoryMetadata;
3938
import org.opensearch.common.blobstore.BlobStoreException;
@@ -176,39 +175,6 @@ public void testValidateHttpLClientType_Invalid_Values() {
176175
}
177176
}
178177

179-
public void testCreateClientSideEncryptedBlobStore() {
180-
final RepositoryMetadata metadata = new RepositoryMetadata("dummy-repo", "mock", Settings.EMPTY);
181-
try (S3Repository s3Repo = createS3Repo(metadata)) {
182-
// Don't expect any Exception
183-
S3BlobStore blobStore = s3Repo.createClientSideEncryptedBlobStore();
184-
assertNotNull(blobStore);
185-
assertNull(blobStore.serverSideEncryptionKmsKey());
186-
assertEquals(blobStore.serverSideEncryptionType(), ServerSideEncryption.AES256.toString());
187-
assertNull(blobStore.expectedBucketOwner());
188-
assertFalse(blobStore.serverSideEncryptionBucketKey());
189-
}
190-
}
191-
192-
public void testCreateServerSideEncryptedBlobStore() {
193-
Settings settings = Settings.builder()
194-
.put("server_side_encryption_type", ServerSideEncryption.AWS_KMS.toString())
195-
.put("server_side_encryption_kms_key_id", "kms-key-id")
196-
.put("server_side_encryption_bucket_key_enabled", true)
197-
.put("server_side_encryption_encryption_context", "enc-ctx")
198-
.put("expected_bucket_owner", "123456789012")
199-
.build();
200-
201-
final RepositoryMetadata metadata = new RepositoryMetadata("dummy-repo", "mock", settings);
202-
try (S3Repository s3Repo = createS3Repo(metadata)) {
203-
// Don't expect any Exception
204-
S3BlobStore blobStore = s3Repo.createServerSideEncryptedBlobStore();
205-
assertNotNull(blobStore);
206-
assertEquals("kms-key-id", blobStore.serverSideEncryptionKmsKey());
207-
assertEquals("123456789012", blobStore.expectedBucketOwner());
208-
assertTrue(blobStore.serverSideEncryptionBucketKey());
209-
}
210-
}
211-
212178
private S3Repository createS3Repo(RepositoryMetadata metadata) {
213179
return new S3Repository(
214180
metadata,

‎server/src/main/java/org/opensearch/action/admin/cluster/remotestore/metadata/TransportRemoteStoreMetadataAction.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ private Map<String, Map<String, Object>> getSegmentMetadata(
200200
shardId,
201201
indexSettings.getRemoteStorePathStrategy(),
202202
null,
203-
indexSettings.isServerSideEncryptionEnabled()
203+
RemoteStoreUtils.isServerSideEncryptionEnabledIndex(indexSettings.getIndexMetadata())
204204
);
205205

206206
Map<String, RemoteSegmentMetadata> segmentMetadataMapWithFilenames = remoteDirectory.readLatestNMetadataFiles(5);
@@ -260,7 +260,7 @@ private Map<String, Map<String, Object>> getTranslogMetadataFiles(
260260
indexSettings.getRemoteStorePathStrategy(),
261261
new RemoteStoreSettings(clusterService.getSettings(), clusterService.getClusterSettings()),
262262
RemoteStoreUtils.determineTranslogMetadataEnabled(indexMetadata),
263-
indexSettings.isServerSideEncryptionEnabled()
263+
RemoteStoreUtils.isServerSideEncryptionEnabledIndex(indexSettings.getIndexMetadata())
264264
);
265265

266266
Map<String, TranslogTransferMetadata> metadataMap = manager.readLatestNMetadataFiles(5);

‎server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ public Iterator<Setting<?>> settings() {
371371
);
372372

373373
public static final String SETTING_REMOTE_STORE_ENABLED = "index.remote_store.enabled";
374-
public static final String SETTING_REMOTE_STORE_SSE_ENABLED = "index.remote_store.sse.enabled";
374+
// public static final String SETTING_REMOTE_STORE_SSE_ENABLED = "index.remote_store.sse.enabled";
375375
public static final String SETTING_INDEX_APPEND_ONLY_ENABLED = "index.append_only.enabled";
376376

377377
public static final String SETTING_REMOTE_SEGMENT_STORE_REPOSITORY = "index.remote_store.segment.repository";
@@ -415,38 +415,6 @@ public Iterator<Setting<?>> settings() {
415415
Property.Dynamic
416416
);
417417

418-
/**
419-
* Used to specify if the index data should be persisted in the remote store.
420-
*/
421-
public static final Setting<Boolean> INDEX_REMOTE_STORE_SSE_ENABLED_SETTING = Setting.boolSetting(
422-
SETTING_REMOTE_STORE_SSE_ENABLED,
423-
false,
424-
new Setting.Validator<>() {
425-
426-
@Override
427-
public void validate(final Boolean value) {}
428-
429-
@Override
430-
public void validate(final Boolean value, final Map<Setting<?>, Object> settings) {
431-
final Boolean isRemoteStoreEnabled = (Boolean) settings.get(INDEX_REMOTE_STORE_ENABLED_SETTING);
432-
if (!isRemoteStoreEnabled && value) {
433-
throw new IllegalArgumentException(
434-
"Server Side Encryption can be enabled when " + INDEX_REMOTE_STORE_ENABLED_SETTING.getKey() + " is enabled. "
435-
);
436-
}
437-
}
438-
439-
@Override
440-
public Iterator<Setting<?>> settings() {
441-
final List<Setting<?>> settings = List.of(INDEX_REMOTE_STORE_ENABLED_SETTING);
442-
return settings.iterator();
443-
}
444-
},
445-
Property.IndexScope,
446-
Property.PrivateIndex,
447-
Property.Dynamic
448-
);
449-
450418
/**
451419
* Used to specify if the index data should be persisted in the remote store.
452420
*/
@@ -1025,7 +993,7 @@ public Iterator<Setting<?>> settings() {
1025993
public static final String KEY_PRIMARY_TERMS = "primary_terms";
1026994
public static final String REMOTE_STORE_CUSTOM_KEY = "remote_store";
1027995
public static final String TRANSLOG_METADATA_KEY = "translog_metadata";
1028-
public static final String SERVER_SIDE_ENCRYPTION_ENABLED = "server_side_encryption_enabled";
996+
public static final String REMOTE_STORE_SSE_ENABLED_INDEX_KEY = "sse_enabled_index";
1029997
public static final String CONTEXT_KEY = "context";
1030998
public static final String INGESTION_SOURCE_KEY = "ingestion_source";
1031999
public static final String INGESTION_STATUS_KEY = "ingestion_status";

‎server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,8 @@ static Optional<String> validateOverlap(Set<String> requestSettings, Settings co
632632
IndexMetadata buildAndValidateTemporaryIndexMetadata(
633633
final Settings aggregatedIndexSettings,
634634
final CreateIndexClusterStateUpdateRequest request,
635-
final int routingNumShards
635+
final int routingNumShards,
636+
final ClusterState clusterState
636637
) {
637638

638639
final boolean isHiddenAfterTemplates = IndexMetadata.INDEX_HIDDEN_SETTING.get(aggregatedIndexSettings);
@@ -642,7 +643,7 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
642643
tmpImdBuilder.setRoutingNumShards(routingNumShards);
643644
tmpImdBuilder.settings(aggregatedIndexSettings);
644645
tmpImdBuilder.system(isSystem);
645-
addRemoteStoreCustomMetadata(tmpImdBuilder, true);
646+
addRemoteStoreCustomMetadata(tmpImdBuilder, true, clusterState);
646647

647648
if (request.context() != null) {
648649
tmpImdBuilder.context(request.context());
@@ -661,7 +662,9 @@ IndexMetadata buildAndValidateTemporaryIndexMetadata(
661662
* @param tmpImdBuilder index metadata builder.
662663
* @param assertNullOldType flag to verify that the old remote store path type is null
663664
*/
664-
public void addRemoteStoreCustomMetadata(IndexMetadata.Builder tmpImdBuilder, boolean assertNullOldType) {
665+
public void addRemoteStoreCustomMetadata(IndexMetadata.Builder tmpImdBuilder, boolean assertNullOldType, ClusterState clusterState) {
666+
667+
boolean isRestoreFromSnapshot = !assertNullOldType;
665668
if (remoteStoreCustomMetadataResolver == null) {
666669
return;
667670
}
@@ -676,6 +679,27 @@ public void addRemoteStoreCustomMetadata(IndexMetadata.Builder tmpImdBuilder, bo
676679
boolean isTranslogMetadataEnabled = remoteStoreCustomMetadataResolver.isTranslogMetadataEnabled();
677680
remoteCustomData.put(IndexMetadata.TRANSLOG_METADATA_KEY, Boolean.toString(isTranslogMetadataEnabled));
678681

682+
Optional<DiscoveryNode> remoteNode = clusterState.nodes()
683+
.getNodes()
684+
.values()
685+
.stream()
686+
.filter(DiscoveryNode::isRemoteStoreNode)
687+
.findFirst();
688+
689+
String sseEnabledIndex = existingCustomData == null
690+
? null
691+
: existingCustomData.get(IndexMetadata.REMOTE_STORE_SSE_ENABLED_INDEX_KEY);
692+
if (isRestoreFromSnapshot && sseEnabledIndex != null) {
693+
remoteCustomData.put(IndexMetadata.REMOTE_STORE_SSE_ENABLED_INDEX_KEY, sseEnabledIndex);
694+
}
695+
696+
if (remoteNode.isPresent()
697+
&& !isRestoreFromSnapshot
698+
&& RemoteStoreSettings.isServerSideEncryptionRepoEnabled(clusterState.nodes().getMinNodeVersion())
699+
&& RemoteStoreNodeAttribute.isRemoteStoreServerSideEncryptionEnabled(remoteNode.get().getAttributes())) {
700+
remoteCustomData.put(IndexMetadata.REMOTE_STORE_SSE_ENABLED_INDEX_KEY, Boolean.toString(true));
701+
}
702+
679703
// Determine the path type for use using the remoteStorePathResolver.
680704
RemoteStorePathStrategy newPathStrategy = remoteStoreCustomMetadataResolver.getPathStrategy();
681705
remoteCustomData.put(PathType.NAME, newPathStrategy.getType().name());
@@ -730,7 +754,9 @@ private ClusterState applyCreateIndexRequestWithV1Templates(
730754
clusterService.getClusterSettings()
731755
);
732756
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
733-
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(aggregatedIndexSettings, request, routingNumShards);
757+
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(aggregatedIndexSettings, request, routingNumShards, currentState);
758+
759+
// buildServerSideEncryptionSetting(tmpImd, clusterService.getClusterSettings(), false);
734760

735761
return applyCreateIndexWithTemporaryService(
736762
currentState,
@@ -755,6 +781,23 @@ private ClusterState applyCreateIndexRequestWithV1Templates(
755781
);
756782
}
757783

784+
// private void buildServerSideEncryptionSetting(IndexMetadata tmpImd, ClusterSettings clusterSettings, boolean isRestore) {
785+
//
786+
// if (isRestore)
787+
// if (remoteStoreCustomMetadataResolver == null) {
788+
// return;
789+
// }
790+
//
791+
// Map<String, String> remoteCustomMetadata = tmpImd.getCustomData(IndexMetadata.REMOTE_STORE_CUSTOM_KEY);
792+
//
793+
// String sseEnabledIndex = existingCustomData == null ? null : existingCustomData.get(IndexMetadata.SETTING_REMOTE_STORE_SSE_ENABLED);
794+
// if (assertNullOldType || sseEnabledIndex != null) {
795+
//
796+
// }
797+
// remoteCustomData.put(IndexMetadata.SETTING_REMOTE_STORE_SSE_ENABLED, Boolean.toString(true));
798+
//
799+
// }
800+
758801
private ClusterState applyCreateIndexRequestWithV2Template(
759802
final ClusterState currentState,
760803
final CreateIndexClusterStateUpdateRequest request,
@@ -795,7 +838,7 @@ private ClusterState applyCreateIndexRequestWithV2Template(
795838
clusterService.getClusterSettings()
796839
);
797840
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
798-
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(aggregatedIndexSettings, request, routingNumShards);
841+
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(aggregatedIndexSettings, request, routingNumShards, currentState);
799842

800843
return applyCreateIndexWithTemporaryService(
801844
currentState,
@@ -879,7 +922,7 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata(
879922
clusterService.getClusterSettings()
880923
);
881924
final int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetadata);
882-
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(aggregatedIndexSettings, request, routingNumShards);
925+
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(aggregatedIndexSettings, request, routingNumShards, currentState);
883926

884927
return applyCreateIndexWithTemporaryService(
885928
currentState,
@@ -1056,7 +1099,7 @@ static Settings aggregateIndexSettings(
10561099
indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());
10571100

10581101
updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings, combinedTemplateSettings, clusterSettings);
1059-
updateRemoteStoreSettings(indexSettingsBuilder, currentState, clusterSettings, settings, request.index(), false);
1102+
updateRemoteStoreSettings(indexSettingsBuilder, currentState, clusterSettings, settings, request.index());
10601103

10611104
if (sourceMetadata != null) {
10621105
assert request.resizeType() != null;
@@ -1162,8 +1205,7 @@ public static void updateRemoteStoreSettings(
11621205
ClusterState clusterState,
11631206
ClusterSettings clusterSettings,
11641207
Settings nodeSettings,
1165-
String indexName,
1166-
boolean isRestoreFromSnapshot
1208+
String indexName
11671209
) {
11681210
if ((isRemoteDataAttributePresent(nodeSettings)
11691211
&& clusterSettings.get(REMOTE_STORE_COMPATIBILITY_MODE_SETTING).equals(RemoteStoreNodeService.CompatibilityMode.STRICT))
@@ -1179,12 +1221,6 @@ public static void updateRemoteStoreSettings(
11791221

11801222
if (remoteNode.isPresent()) {
11811223
segmentRepo = RemoteStoreNodeAttribute.getSegmentRepoName(remoteNode.get().getAttributes());
1182-
if (!isRestoreFromSnapshot
1183-
&& RemoteStoreSettings.isServerSideEncryptionRepoEnabled(clusterState.nodes().getMinNodeVersion())
1184-
&& RemoteStoreNodeAttribute.isRemoteStoreServerSideEncryptionEnabled(remoteNode.get().getAttributes(), segmentRepo)) {
1185-
settingsBuilder.put(IndexMetadata.SETTING_REMOTE_STORE_SSE_ENABLED, true);
1186-
}
1187-
11881224
translogRepo = RemoteStoreNodeAttribute.getTranslogRepoName(remoteNode.get().getAttributes());
11891225
if (segmentRepo != null) {
11901226
settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true).put(SETTING_REMOTE_SEGMENT_STORE_REPOSITORY, segmentRepo);

‎server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
242242

243243
// Settings for remote store enablement
244244
IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING,
245-
IndexMetadata.INDEX_REMOTE_STORE_SSE_ENABLED_SETTING,
246245
IndexMetadata.INDEX_REMOTE_SEGMENT_STORE_REPOSITORY_SETTING,
247246
IndexMetadata.INDEX_REMOTE_TRANSLOG_REPOSITORY_SETTING,
248247

‎server/src/main/java/org/opensearch/index/IndexService.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import org.opensearch.index.query.QueryShardContext;
8585
import org.opensearch.index.query.SearchIndexNameMatcher;
8686
import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory;
87+
import org.opensearch.index.remote.RemoteStoreUtils;
8788
import org.opensearch.index.seqno.RetentionLeaseSyncer;
8889
import org.opensearch.index.shard.IndexEventListener;
8990
import org.opensearch.index.shard.IndexShard;
@@ -697,12 +698,12 @@ public synchronized IndexShard createShard(
697698
}
698699

699700
remoteDirectory = ((RemoteSegmentStoreDirectoryFactory) remoteDirectoryFactory).newDirectory(
700-
RemoteStoreNodeAttribute.getRemoteStoreSegmentRepo(this.indexSettings.getSettings()),
701+
RemoteStoreNodeAttribute.getRemoteStoreSegmentRepo(this.indexSettings.getNodeSettings()),
701702
this.indexSettings.getUUID(),
702703
shardId,
703704
this.indexSettings.getRemoteStorePathStrategy(),
704705
this.indexSettings.getRemoteStoreSegmentPathPrefix(),
705-
this.indexSettings.isServerSideEncryptionEnabled()
706+
RemoteStoreUtils.isServerSideEncryptionEnabledIndex(this.indexSettings.getIndexMetadata())
706707
);
707708
}
708709
// When an instance of Store is created, a shardlock is created which is released on closing the instance of store.

0 commit comments

Comments
 (0)