Replace sigv4-enabled with security in Iceberg

Author: ebyhr

docs/src/main/sphinx/object-storage/metastores.md

@@ -517,8 +517,6 @@ following properties:
     Defaults to `false`.
 * - `iceberg.rest-catalog.view-endpoints-enabled`
   - Enable view endpoints. Defaults to `true`.
-* - `iceberg.rest-catalog.sigv4-enabled`
-  - Enable AWS Signature version 4 (SigV4). Defaults to `false`.
 * - `iceberg.rest-catalog.signing-name`
   - AWS SigV4 signing service name. Defaults to `execute-api`.
 * - `iceberg.rest-catalog.case-insensitive-name-matching`

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogConfig.java

@@ -27,13 +27,17 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
 
-@DefunctConfig("iceberg.rest-catalog.parent-namespace")
+@DefunctConfig({
+        "iceberg.rest-catalog.parent-namespace",
+        "iceberg.rest-catalog.sigv4-enabled",
+})
 public class IcebergRestCatalogConfig
 {
     public enum Security
     {
         NONE,
         OAUTH2,
+        SIGV4,
     }
 
     public enum SessionType
@@ -51,7 +55,6 @@ public enum SessionType
     private Duration sessionTimeout = new Duration(CatalogProperties.AUTH_SESSION_TIMEOUT_MS_DEFAULT, MILLISECONDS);
     private boolean vendedCredentialsEnabled;
     private boolean viewEndpointsEnabled = true;
-    private boolean sigV4Enabled;
     private boolean caseInsensitiveNameMatching;
     private Duration caseInsensitiveNameMatchingCacheTtl = new Duration(1, MINUTES);
 
@@ -179,19 +182,6 @@ public IcebergRestCatalogConfig setViewEndpointsEnabled(boolean viewEndpointsEna
         return this;
     }
 
-    public boolean isSigV4Enabled()
-    {
-        return sigV4Enabled;
-    }
-
-    @Config("iceberg.rest-catalog.sigv4-enabled")
-    @ConfigDescription("Enable AWS Signature version 4 (SigV4)")
-    public IcebergRestCatalogConfig setSigV4Enabled(boolean sigV4Enabled)
-    {
-        this.sigV4Enabled = sigV4Enabled;
-        return this;
-    }
-
     public boolean isCaseInsensitiveNameMatching()
     {
         return caseInsensitiveNameMatching;

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogModule.java

@@ -13,7 +13,6 @@
  */
 package io.trino.plugin.iceberg.catalog.rest;
 
-import com.google.common.collect.ImmutableMap;
 import com.google.inject.Binder;
 import com.google.inject.Scopes;
 import io.airlift.configuration.AbstractConfigurationAwareModule;
@@ -42,12 +41,9 @@ protected void setup(Binder binder)
                 new NoneSecurityModule()));
         install(conditionalModule(
                 IcebergRestCatalogConfig.class,
-                IcebergRestCatalogConfig::isSigV4Enabled,
-                internalBinder -> {
-                    configBinder(internalBinder).bindConfig(IcebergRestCatalogSigV4Config.class);
-                    internalBinder.bind(AwsProperties.class).to(SigV4AwsProperties.class).in(Scopes.SINGLETON);
-                },
-                internalBinder -> internalBinder.bind(AwsProperties.class).toInstance(ImmutableMap::of)));
+                config -> config.getSecurity() == Security.SIGV4,
+                new SigV4SecurityModule(),
+                new NoneSecurityModule()));
 
         binder.bind(TrinoCatalogFactory.class).to(TrinoIcebergRestCatalogFactory.class).in(Scopes.SINGLETON);
         newOptionalBinder(binder, IcebergFileSystemFactory.class).setBinding().to(IcebergRestCatalogFileSystemFactory.class).in(Scopes.SINGLETON);

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/SigV4AwsProperties.java

@@ -36,7 +36,7 @@
 import static org.apache.iceberg.aws.AwsProperties.REST_SIGNING_NAME;
 
 public class SigV4AwsProperties
-        implements AwsProperties
+        implements SecurityProperties
 {
     // Copy of `org.apache.iceberg.aws.AwsClientProperties.CLIENT_CREDENTIAL_PROVIDER_PREFIX` https://github.com/apache/iceberg/blob/ab6fc83ec0269736355a0a89c51e44e822264da8/aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java#L69
     private static final String CLIENT_CREDENTIAL_PROVIDER_PREFIX = "client.credentials-provider.";

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/SigV4SecurityModule.java

@@ -0,0 +1,30 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.trino.plugin.iceberg.catalog.rest;
+
+import com.google.inject.Binder;
+import io.airlift.configuration.AbstractConfigurationAwareModule;
+
+import static io.airlift.configuration.ConfigBinder.configBinder;
+
+public class SigV4SecurityModule
+        extends AbstractConfigurationAwareModule
+{
+    @Override
+    protected void setup(Binder binder)
+    {
+        configBinder(binder).bindConfig(IcebergRestCatalogSigV4Config.class);
+        binder.bind(SecurityProperties.class).to(SigV4AwsProperties.class);
+    }
+}

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoIcebergRestCatalogFactory.java

@@ -63,7 +63,6 @@ public class TrinoIcebergRestCatalogFactory
     private final boolean vendedCredentialsEnabled;
     private final boolean viewEndpointsEnabled;
     private final SecurityProperties securityProperties;
-    private final AwsProperties awsProperties;
     private final boolean uniqueTableLocation;
     private final TypeManager typeManager;
     private final boolean caseInsensitiveNameMatching;
@@ -79,7 +78,6 @@ public TrinoIcebergRestCatalogFactory(
             CatalogName catalogName,
             IcebergRestCatalogConfig restConfig,
             SecurityProperties securityProperties,
-            AwsProperties awsProperties,
             IcebergConfig icebergConfig,
             TypeManager typeManager,
             NodeVersion nodeVersion)
@@ -97,7 +95,6 @@ public TrinoIcebergRestCatalogFactory(
         this.vendedCredentialsEnabled = restConfig.isVendedCredentialsEnabled();
         this.viewEndpointsEnabled = restConfig.isViewEndpointsEnabled();
         this.securityProperties = requireNonNull(securityProperties, "securityProperties is null");
-        this.awsProperties = requireNonNull(awsProperties, "awsProperties is null");
         requireNonNull(icebergConfig, "icebergConfig is null");
         this.uniqueTableLocation = icebergConfig.isUniqueTableLocation();
         this.typeManager = requireNonNull(typeManager, "typeManager is null");
@@ -126,7 +123,6 @@ public synchronized TrinoCatalog create(ConnectorIdentity identity)
             properties.put("trino-version", trinoVersion);
             properties.put(AUTH_SESSION_TIMEOUT_MS, String.valueOf(sessionTimeout.toMillis()));
             properties.putAll(securityProperties.get());
-            properties.putAll(awsProperties.get());
 
             if (vendedCredentialsEnabled) {
                 properties.put("header.X-Iceberg-Access-Delegation", "vended-credentials");

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestIcebergRestCatalogConfig.java

@@ -41,7 +41,6 @@ public void testDefaults()
                 .setSecurity(IcebergRestCatalogConfig.Security.NONE)
                 .setVendedCredentialsEnabled(false)
                 .setViewEndpointsEnabled(true)
-                .setSigV4Enabled(false)
                 .setCaseInsensitiveNameMatching(false)
                 .setCaseInsensitiveNameMatchingCacheTtl(new Duration(1, MINUTES)));
     }
@@ -59,7 +58,6 @@ public void testExplicitPropertyMappings()
                 .put("iceberg.rest-catalog.session-timeout", "100ms")
                 .put("iceberg.rest-catalog.vended-credentials-enabled", "true")
                 .put("iceberg.rest-catalog.view-endpoints-enabled", "false")
-                .put("iceberg.rest-catalog.sigv4-enabled", "true")
                 .put("iceberg.rest-catalog.case-insensitive-name-matching", "true")
                 .put("iceberg.rest-catalog.case-insensitive-name-matching.cache-ttl", "3m")
                 .buildOrThrow();
@@ -74,7 +72,6 @@ public void testExplicitPropertyMappings()
                 .setSecurity(IcebergRestCatalogConfig.Security.OAUTH2)
                 .setVendedCredentialsEnabled(true)
                 .setViewEndpointsEnabled(false)
-                .setSigV4Enabled(true)
                 .setCaseInsensitiveNameMatching(true)
                 .setCaseInsensitiveNameMatchingCacheTtl(new Duration(3, MINUTES));
 

plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/rest/TestIcebergS3TablesConnectorSmokeTest.java

@@ -71,7 +71,7 @@ protected QueryRunner createQueryRunner()
                 .addIcebergProperty("iceberg.rest-catalog.uri", "https://glue.%s.amazonaws.com/iceberg".formatted(AWS_REGION))
                 .addIcebergProperty("iceberg.rest-catalog.warehouse", "s3tablescatalog/" + S3_TABLES_BUCKET)
                 .addIcebergProperty("iceberg.rest-catalog.view-endpoints-enabled", "false")
-                .addIcebergProperty("iceberg.rest-catalog.sigv4-enabled", "true")
+                .addIcebergProperty("iceberg.rest-catalog.security", "sigv4")
                 .addIcebergProperty("iceberg.rest-catalog.signing-name", "glue")
                 .addIcebergProperty("iceberg.writer-sort-buffer-size", "1MB")
                 .addIcebergProperty("iceberg.allowed-extra-properties", "write.metadata.delete-after-commit.enabled,write.metadata.previous-versions-max")