Skip to content

Commit 3fa7c30

Browse files
DaveCTurnerDaveCTurner
committed
Migrate to client setting
1 parent 8c617aa commit 3fa7c30

File tree

5 files changed

+50
-26
lines changed

5 files changed

+50
-26
lines changed

‎modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/AbstractRepositoryS3RestTestCase.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,10 @@ private void testNonexistentClient(Boolean readonly) throws Exception {
189189
final var responseObjectPath = ObjectPath.createFromResponse(responseException.getResponse());
190190
assertThat(responseObjectPath.evaluate("error.type"), equalTo("repository_verification_exception"));
191191
assertThat(responseObjectPath.evaluate("error.reason"), containsString("is not accessible on master node"));
192-
assertThat(responseObjectPath.evaluate("error.caused_by.type"), equalTo("illegal_argument_exception"));
193-
assertThat(responseObjectPath.evaluate("error.caused_by.reason"), containsString("Unknown s3 client name"));
192+
assertThat(responseObjectPath.evaluate("error.caused_by.type"), equalTo("repository_exception"));
193+
assertThat(responseObjectPath.evaluate("error.caused_by.reason"), containsString("cannot create blob store"));
194+
assertThat(responseObjectPath.evaluate("error.caused_by.caused_by.type"), equalTo("illegal_argument_exception"));
195+
assertThat(responseObjectPath.evaluate("error.caused_by.caused_by.reason"), containsString("Unknown s3 client name"));
194196
}
195197

196198
public void testNonexistentSnapshot() throws Exception {

‎modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ class S3BlobStore implements BlobStore {
103103

104104
private final TimeValue getRegisterRetryDelay;
105105

106+
private final boolean addPurposeCustomQueryParameter;
107+
106108
S3BlobStore(
107109
S3Service service,
108110
String bucket,
@@ -132,7 +134,7 @@ class S3BlobStore implements BlobStore {
132134
this.bulkDeletionBatchSize = S3Repository.DELETION_BATCH_SIZE_SETTING.get(repositoryMetadata.settings());
133135
this.retryThrottledDeleteBackoffPolicy = retryThrottledDeleteBackoffPolicy;
134136
this.getRegisterRetryDelay = S3Repository.GET_REGISTER_RETRY_DELAY.get(repositoryMetadata.settings());
135-
this.addPurposeCustomQueryParameter = ADD_PURPOSE_CUSTOM_QUERY_PARAMETER_SETTING.get(repositoryMetadata.settings());
137+
this.addPurposeCustomQueryParameter = service.settings(repositoryMetadata).addPurposeCustomQueryParameter;
136138
}
137139

138140
MetricPublisher getMetricPublisher(Operation operation, OperationPurpose purpose) {

‎modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,9 @@ final class S3ClientSettings {
227227
/** Whether chunked encoding should be disabled or not. */
228228
final boolean disableChunkedEncoding;
229229

230+
/** Whether to add the {@code x-purpose} custom query parameter to all requests. */
231+
final boolean addPurposeCustomQueryParameter;
232+
230233
/** Region to use for signing requests or empty string to use default. */
231234
final String region;
232235

@@ -244,6 +247,7 @@ private S3ClientSettings(
244247
int maxRetries,
245248
boolean pathStyleAccess,
246249
boolean disableChunkedEncoding,
250+
boolean addPurposeCustomQueryParameter,
247251
String region
248252
) {
249253
this.credentials = credentials;
@@ -259,6 +263,7 @@ private S3ClientSettings(
259263
this.maxRetries = maxRetries;
260264
this.pathStyleAccess = pathStyleAccess;
261265
this.disableChunkedEncoding = disableChunkedEncoding;
266+
this.addPurposeCustomQueryParameter = addPurposeCustomQueryParameter;
262267
this.region = region;
263268
}
264269

@@ -291,6 +296,11 @@ S3ClientSettings refine(Settings repositorySettings) {
291296
normalizedSettings,
292297
disableChunkedEncoding
293298
);
299+
final boolean newAddPurposeCustomQueryParameter = getRepoSettingOrDefault(
300+
ADD_PURPOSE_CUSTOM_QUERY_PARAMETER,
301+
normalizedSettings,
302+
addPurposeCustomQueryParameter
303+
);
294304
final AwsCredentials newCredentials;
295305
if (checkDeprecatedCredentials(repositorySettings)) {
296306
newCredentials = loadDeprecatedCredentials(repositorySettings);
@@ -309,6 +319,7 @@ S3ClientSettings refine(Settings repositorySettings) {
309319
&& Objects.equals(credentials, newCredentials)
310320
&& newPathStyleAccess == pathStyleAccess
311321
&& newDisableChunkedEncoding == disableChunkedEncoding
322+
&& newAddPurposeCustomQueryParameter == addPurposeCustomQueryParameter
312323
&& Objects.equals(region, newRegion)) {
313324
return this;
314325
}
@@ -326,6 +337,7 @@ S3ClientSettings refine(Settings repositorySettings) {
326337
newMaxRetries,
327338
newPathStyleAccess,
328339
newDisableChunkedEncoding,
340+
newAddPurposeCustomQueryParameter,
329341
newRegion
330342
);
331343
}
@@ -433,6 +445,7 @@ static S3ClientSettings getClientSettings(final Settings settings, final String
433445
getConfigValue(settings, clientName, MAX_RETRIES_SETTING),
434446
getConfigValue(settings, clientName, USE_PATH_STYLE_ACCESS),
435447
getConfigValue(settings, clientName, DISABLE_CHUNKED_ENCODING),
448+
getConfigValue(settings, clientName, ADD_PURPOSE_CUSTOM_QUERY_PARAMETER),
436449
getConfigValue(settings, clientName, REGION)
437450
);
438451
}
@@ -459,6 +472,7 @@ public boolean equals(final Object o) {
459472
&& Objects.equals(proxyUsername, that.proxyUsername)
460473
&& Objects.equals(proxyPassword, that.proxyPassword)
461474
&& Objects.equals(disableChunkedEncoding, that.disableChunkedEncoding)
475+
&& Objects.equals(addPurposeCustomQueryParameter, that.addPurposeCustomQueryParameter)
462476
&& Objects.equals(region, that.region);
463477
}
464478

@@ -477,6 +491,7 @@ public int hashCode() {
477491
maxRetries,
478492
maxConnections,
479493
disableChunkedEncoding,
494+
addPurposeCustomQueryParameter,
480495
region
481496
);
482497
}

‎modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ public List<Setting<?>> getSettings() {
123123
S3ClientSettings.UNUSED_USE_THROTTLE_RETRIES_SETTING,
124124
S3ClientSettings.USE_PATH_STYLE_ACCESS,
125125
S3ClientSettings.UNUSED_SIGNER_OVERRIDE,
126+
S3ClientSettings.ADD_PURPOSE_CUSTOM_QUERY_PARAMETER,
126127
S3ClientSettings.REGION,
127128
S3Service.REPOSITORY_S3_CAS_TTL_SETTING,
128129
S3Service.REPOSITORY_S3_CAS_ANTI_CONTENTION_DELAY_SETTING,

‎modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AddPurposeCustomQueryParameterTests.java

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import java.io.IOException;
3434
import java.util.Collection;
35+
import java.util.List;
3536

3637
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
3738
import static org.hamcrest.Matchers.hasItem;
@@ -47,8 +48,16 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
4748
@Override
4849
protected Settings nodeSettings() {
4950
final var secureSettings = new MockSecureSettings();
50-
secureSettings.setString(S3ClientSettings.ACCESS_KEY_SETTING.getConcreteSettingForNamespace("default").getKey(), "test_access_key");
51-
secureSettings.setString(S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace("default").getKey(), "test_secret_key");
51+
for (final var clientName : List.of("default", "with_purpose", "without_purpose")) {
52+
secureSettings.setString(
53+
S3ClientSettings.ACCESS_KEY_SETTING.getConcreteSettingForNamespace(clientName).getKey(),
54+
randomIdentifier()
55+
);
56+
secureSettings.setString(
57+
S3ClientSettings.SECRET_KEY_SETTING.getConcreteSettingForNamespace(clientName).getKey(),
58+
randomSecretKey()
59+
);
60+
}
5261

5362
return Settings.builder()
5463
.put(super.nodeSettings())
@@ -59,6 +68,9 @@ protected Settings nodeSettings() {
5968
.build();
6069
}
6170

71+
private static final Matcher<Iterable<? super String>> HAS_CUSTOM_QUERY_PARAMETER = hasItem(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE);
72+
private static final Matcher<Iterable<? super String>> NO_CUSTOM_QUERY_PARAMETER = not(HAS_CUSTOM_QUERY_PARAMETER);
73+
6274
public void testCustomQueryParameterConfiguration() throws Throwable {
6375
final var indexName = randomIdentifier();
6476
createIndex(indexName);
@@ -67,29 +79,21 @@ public void testCustomQueryParameterConfiguration() throws Throwable {
6779
final var bucket = randomIdentifier();
6880
final var basePath = randomIdentifier();
6981

70-
runWithFixture(bucket, basePath, null, Settings.EMPTY, not(hasItem(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE)));
71-
runWithFixture(bucket, basePath, "without_purpose", Settings.EMPTY, not(hasItem(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE)));
72-
runWithFixture(bucket, basePath, "with_purpose", Settings.EMPTY, hasItem(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE));
73-
74-
runWithFixture(
75-
bucket,
76-
basePath,
77-
randomFrom("with_purpose", "without_purpose", null), // client name doesn't matter if repository setting specified
78-
Settings.builder().put("add_purpose_custom_query_parameter", false).build(),
79-
not(hasItem(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE))
80-
);
81-
82-
runWithFixture(
83-
bucket,
84-
basePath,
85-
randomFrom("with_purpose", "without_purpose", null), // client name doesn't matter if repository setting specified
86-
Settings.builder().put("add_purpose_custom_query_parameter", true).build(),
87-
hasItem(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE)
88-
);
89-
82+
runCustomQueryParameterTest(bucket, basePath, null, Settings.EMPTY, NO_CUSTOM_QUERY_PARAMETER);
83+
runCustomQueryParameterTest(bucket, basePath, "default", Settings.EMPTY, NO_CUSTOM_QUERY_PARAMETER);
84+
runCustomQueryParameterTest(bucket, basePath, "without_purpose", Settings.EMPTY, NO_CUSTOM_QUERY_PARAMETER);
85+
runCustomQueryParameterTest(bucket, basePath, "with_purpose", Settings.EMPTY, HAS_CUSTOM_QUERY_PARAMETER);
86+
87+
final var falseRepositorySetting = Settings.builder().put("add_purpose_custom_query_parameter", false).build();
88+
final var trueRepositorySetting = Settings.builder().put("add_purpose_custom_query_parameter", true).build();
89+
for (final var clientName : new String[] { null, "default", "with_purpose", "without_purpose" }) {
90+
// client name doesn't matter if repository setting specified
91+
runCustomQueryParameterTest(bucket, basePath, clientName, falseRepositorySetting, NO_CUSTOM_QUERY_PARAMETER);
92+
runCustomQueryParameterTest(bucket, basePath, clientName, trueRepositorySetting, HAS_CUSTOM_QUERY_PARAMETER);
93+
}
9094
}
9195

92-
private void runWithFixture(
96+
private void runCustomQueryParameterTest(
9397
String bucket,
9498
String basePath,
9599
String clientName,

0 commit comments

Comments
 (0)