Make S3 custom query parameter optional

docs/changelog/128043.yaml

@@ -0,0 +1,19 @@
+pr: 128043
+summary: Make S3 custom query parameter optional
+area: Snapshot/Restore
+type: breaking
+issues: []
+breaking:
+  title: Make S3 custom query parameter optional
+  area: Cluster and node setting
+  details: >-
+    Earlier versions of Elasticsearch would record the purpose of each S3 API
+    call using the `?x-purpose=` custom query parameter. This isn't believed to
+    be necessary outside of the ECH/ECE/ECK/... managed services, and it adds
+    rather a lot to the request logs, so with this change we make the feature
+    optional and disabled by default.
+  impact: >-
+    If you wish to reinstate the old behaviour on a S3 repository, set
+    `s3.client.${CLIENT_NAME}.add_purpose_custom_query_parameter` to `true`
+    for the relevant client.
+  notable: false

modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java

@@ -173,6 +173,7 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
             .put(S3ClientSettings.ENDPOINT_SETTING.getConcreteSettingForNamespace("test").getKey(), httpServerUrl())
             // Disable request throttling because some random values in tests might generate too many failures for the S3 client
             .put(S3ClientSettings.USE_THROTTLE_RETRIES_SETTING.getConcreteSettingForNamespace("test").getKey(), false)
+            .put(S3ClientSettings.ADD_PURPOSE_CUSTOM_QUERY_PARAMETER.getConcreteSettingForNamespace("test").getKey(), "true")
             .put(super.nodeSettings(nodeOrdinal, otherSettings))
             .setSecureSettings(secureSettings);
 
@@ -516,19 +517,13 @@ public void testMultipartUploadCleanup() {
                 blobStore.bucket(),
                 blobStore.blobContainer(repository.basePath().add("test-multipart-upload")).path().buildAsString() + danglingBlobName
             );
-            initiateMultipartUploadRequest.putCustomQueryParameter(
-                S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE,
-                OperationPurpose.SNAPSHOT_DATA.getKey()
-            );
+            blobStore.addPurposeQueryParameter(OperationPurpose.SNAPSHOT_DATA, initiateMultipartUploadRequest);
             final var multipartUploadResult = clientRef.client().initiateMultipartUpload(initiateMultipartUploadRequest);
 
             final var listMultipartUploadsRequest = new ListMultipartUploadsRequest(blobStore.bucket()).withPrefix(
                 repository.basePath().buildAsString()
             );
-            listMultipartUploadsRequest.putCustomQueryParameter(
-                S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE,
-                OperationPurpose.SNAPSHOT_DATA.getKey()
-            );
+            blobStore.addPurposeQueryParameter(OperationPurpose.SNAPSHOT_DATA, listMultipartUploadsRequest);
             assertEquals(
                 List.of(multipartUploadResult.getUploadId()),
                 clientRef.client()

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

@@ -59,6 +59,12 @@ private Request getRegisterRequest(UnaryOperator<Settings> settingsUnaryOperator
                                 .put("canned_acl", "private")
                                 .put("storage_class", "standard")
                                 .put("disable_chunked_encoding", randomBoolean())
+                                .put(
+                                    randomFrom(
+                                        Settings.EMPTY,
+                                        Settings.builder().put("add_purpose_custom_query_parameter", randomBoolean()).build()
+                                    )
+                                )
                                 .build()
                         )
                     )
@@ -183,8 +189,10 @@ private void testNonexistentClient(Boolean readonly) throws Exception {
         final var responseObjectPath = ObjectPath.createFromResponse(responseException.getResponse());
         assertThat(responseObjectPath.evaluate("error.type"), equalTo("repository_verification_exception"));
         assertThat(responseObjectPath.evaluate("error.reason"), containsString("is not accessible on master node"));
-        assertThat(responseObjectPath.evaluate("error.caused_by.type"), equalTo("illegal_argument_exception"));
-        assertThat(responseObjectPath.evaluate("error.caused_by.reason"), containsString("Unknown s3 client name"));
+        assertThat(responseObjectPath.evaluate("error.caused_by.type"), equalTo("repository_exception"));
+        assertThat(responseObjectPath.evaluate("error.caused_by.reason"), containsString("cannot create blob store"));
+        assertThat(responseObjectPath.evaluate("error.caused_by.caused_by.type"), equalTo("illegal_argument_exception"));
+        assertThat(responseObjectPath.evaluate("error.caused_by.caused_by.reason"), containsString("Unknown s3 client name"));
     }
 
     public void testNonexistentSnapshot() throws Exception {

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

@@ -966,7 +966,7 @@ ActionListener<Void> getMultipartUploadCleanupListener(int maxUploads, RefCounti
         try (var clientReference = blobStore.clientReference()) {
             final var bucket = blobStore.bucket();
             final var request = new ListMultipartUploadsRequest(bucket).withPrefix(keyPath).withMaxUploads(maxUploads);
-            request.putCustomQueryParameter(S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE, OperationPurpose.SNAPSHOT_DATA.getKey());
+            blobStore.addPurposeQueryParameter(OperationPurpose.SNAPSHOT_DATA, request);
             final var multipartUploadListing = SocketAccess.doPrivileged(() -> clientReference.client().listMultipartUploads(request));
             final var multipartUploads = multipartUploadListing.getMultipartUploads();
             if (multipartUploads.isEmpty()) {
@@ -1006,10 +1006,7 @@ private ActionListener<Void> newMultipartUploadCleanupListener(
             public void onResponse(Void unused) {
                 try (var clientReference = blobStore.clientReference()) {
                     for (final var abortMultipartUploadRequest : abortMultipartUploadRequests) {
-                        abortMultipartUploadRequest.putCustomQueryParameter(
-                            S3BlobStore.CUSTOM_QUERY_PARAMETER_PURPOSE,
-                            OperationPurpose.SNAPSHOT_DATA.getKey()
-                        );
+                        blobStore.addPurposeQueryParameter(OperationPurpose.SNAPSHOT_DATA, abortMultipartUploadRequest);
                         try {
                             SocketAccess.doPrivilegedVoid(() -> clientReference.client().abortMultipartUpload(abortMultipartUploadRequest));
                             logger.info(

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

@@ -98,6 +98,8 @@ class S3BlobStore implements BlobStore {
 
     private final TimeValue getRegisterRetryDelay;
 
+    private final boolean addPurposeCustomQueryParameter;
+
     S3BlobStore(
         S3Service service,
         String bucket,
@@ -125,6 +127,7 @@ class S3BlobStore implements BlobStore {
         this.bulkDeletionBatchSize = S3Repository.DELETION_BATCH_SIZE_SETTING.get(repositoryMetadata.settings());
         this.retryThrottledDeleteBackoffPolicy = retryThrottledDeleteBackoffPolicy;
         this.getRegisterRetryDelay = S3Repository.GET_REGISTER_RETRY_DELAY.get(repositoryMetadata.settings());
+        this.addPurposeCustomQueryParameter = service.settings(repositoryMetadata).addPurposeCustomQueryParameter;
     }
 
     RequestMetricCollector getMetricCollector(Operation operation, OperationPurpose purpose) {
@@ -594,6 +597,14 @@ static void configureRequestForMetrics(
         OperationPurpose purpose
     ) {
         request.setRequestMetricCollector(blobStore.getMetricCollector(operation, purpose));
-        request.putCustomQueryParameter(CUSTOM_QUERY_PARAMETER_PURPOSE, purpose.getKey());
+        blobStore.addPurposeQueryParameter(purpose, request);
     }
+
+    public void addPurposeQueryParameter(OperationPurpose purpose, AmazonWebServiceRequest request) {
+        if (addPurposeCustomQueryParameter || purpose == OperationPurpose.REPOSITORY_ANALYSIS) {
+            // REPOSITORY_ANALYSIS is a strict check for 100% S3 compatibility, including custom query parameter support, so is always added
+            request.putCustomQueryParameter(CUSTOM_QUERY_PARAMETER_PURPOSE, purpose.getKey());
+        }
+    }
+
 }

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

@@ -173,6 +173,13 @@ final class S3ClientSettings {
         key -> Setting.simpleString(key, Property.NodeScope)
     );
 
+    /** Whether to include the {@code x-purpose} custom query parameter in all requests. */
+    static final Setting.AffixSetting<Boolean> ADD_PURPOSE_CUSTOM_QUERY_PARAMETER = Setting.affixKeySetting(
+        PREFIX,
+        "add_purpose_custom_query_parameter",
+        key -> Setting.boolSetting(key, false, Property.NodeScope)
+    );
+
     /** Credentials to authenticate with s3. */
     final S3BasicCredentials credentials;
 
@@ -217,6 +224,9 @@ final class S3ClientSettings {
     /** Whether chunked encoding should be disabled or not. */
     final boolean disableChunkedEncoding;
 
+    /** Whether to add the {@code x-purpose} custom query parameter to all requests. */
+    final boolean addPurposeCustomQueryParameter;
+
     /** Region to use for signing requests or empty string to use default. */
     final String region;
 
@@ -238,6 +248,7 @@ private S3ClientSettings(
         boolean throttleRetries,
         boolean pathStyleAccess,
         boolean disableChunkedEncoding,
+        boolean addPurposeCustomQueryParameter,
         String region,
         String signerOverride
     ) {
@@ -255,6 +266,7 @@ private S3ClientSettings(
         this.throttleRetries = throttleRetries;
         this.pathStyleAccess = pathStyleAccess;
         this.disableChunkedEncoding = disableChunkedEncoding;
+        this.addPurposeCustomQueryParameter = addPurposeCustomQueryParameter;
         this.region = region;
         this.signerOverride = signerOverride;
     }
@@ -289,6 +301,11 @@ S3ClientSettings refine(Settings repositorySettings) {
             normalizedSettings,
             disableChunkedEncoding
         );
+        final boolean newAddPurposeCustomQueryParameter = getRepoSettingOrDefault(
+            ADD_PURPOSE_CUSTOM_QUERY_PARAMETER,
+            normalizedSettings,
+            addPurposeCustomQueryParameter
+        );
         final S3BasicCredentials newCredentials;
         if (checkDeprecatedCredentials(repositorySettings)) {
             newCredentials = loadDeprecatedCredentials(repositorySettings);
@@ -309,6 +326,7 @@ S3ClientSettings refine(Settings repositorySettings) {
             && Objects.equals(credentials, newCredentials)
             && newPathStyleAccess == pathStyleAccess
             && newDisableChunkedEncoding == disableChunkedEncoding
+            && newAddPurposeCustomQueryParameter == addPurposeCustomQueryParameter
             && Objects.equals(region, newRegion)
             && Objects.equals(signerOverride, newSignerOverride)) {
             return this;
@@ -328,6 +346,7 @@ S3ClientSettings refine(Settings repositorySettings) {
             newThrottleRetries,
             newPathStyleAccess,
             newDisableChunkedEncoding,
+            newAddPurposeCustomQueryParameter,
             newRegion,
             newSignerOverride
         );
@@ -437,6 +456,7 @@ static S3ClientSettings getClientSettings(final Settings settings, final String
                 getConfigValue(settings, clientName, USE_THROTTLE_RETRIES_SETTING),
                 getConfigValue(settings, clientName, USE_PATH_STYLE_ACCESS),
                 getConfigValue(settings, clientName, DISABLE_CHUNKED_ENCODING),
+                getConfigValue(settings, clientName, ADD_PURPOSE_CUSTOM_QUERY_PARAMETER),
                 getConfigValue(settings, clientName, REGION),
                 getConfigValue(settings, clientName, SIGNER_OVERRIDE)
             );
@@ -465,6 +485,7 @@ public boolean equals(final Object o) {
             && Objects.equals(proxyUsername, that.proxyUsername)
             && Objects.equals(proxyPassword, that.proxyPassword)
             && Objects.equals(disableChunkedEncoding, that.disableChunkedEncoding)
+            && Objects.equals(addPurposeCustomQueryParameter, that.addPurposeCustomQueryParameter)
             && Objects.equals(region, that.region)
             && Objects.equals(signerOverride, that.signerOverride);
     }
@@ -485,6 +506,7 @@ public int hashCode() {
             maxConnections,
             throttleRetries,
             disableChunkedEncoding,
+            addPurposeCustomQueryParameter,
             region,
             signerOverride
         );

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

@@ -131,6 +131,7 @@ public List<Setting<?>> getSettings() {
             S3ClientSettings.USE_THROTTLE_RETRIES_SETTING,
             S3ClientSettings.USE_PATH_STYLE_ACCESS,
             S3ClientSettings.SIGNER_OVERRIDE,
+            S3ClientSettings.ADD_PURPOSE_CUSTOM_QUERY_PARAMETER,
             S3ClientSettings.REGION,
             S3Service.REPOSITORY_S3_CAS_TTL_SETTING,
             S3Service.REPOSITORY_S3_CAS_ANTI_CONTENTION_DELAY_SETTING,