Skip to content

Commit 3c5e969

Browse files
okuminokumin
committed
Address checkstyle errors
1 parent 66957fb commit 3c5e969

11 files changed

Lines changed: 173 additions & 154 deletions

File tree

‎standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/AccessDelegationMode.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
99
*
10-
* http://www.apache.org/licenses/LICENSE-2.0
10+
* http://www.apache.org/licenses/LICENSE-2.0
1111
*
12-
* Unless required by applicable law or agreed to in writing,
13-
* software distributed under the License is distributed on an
14-
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15-
* KIND, either express or implied. See the License for the
16-
* specific language governing permissions and limitations
17-
* under the License.
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
1817
*/
1918

2019
package org.apache.iceberg.rest;

‎standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergVendedCredentialProvider.java‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
99
*
10-
* http://www.apache.org/licenses/LICENSE-2.0
10+
* http://www.apache.org/licenses/LICENSE-2.0
1111
*
12-
* Unless required by applicable law or agreed to in writing,
13-
* software distributed under the License is distributed on an
14-
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15-
* KIND, either express or implied. See the License for the
16-
* specific language governing permissions and limitations
17-
* under the License.
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
1817
*/
1918

2019
package org.apache.iceberg.rest;
@@ -31,7 +30,14 @@
3130
import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils;
3231
import org.apache.hadoop.hive.ql.metadata.HiveException;
3332
import org.apache.hadoop.hive.ql.metadata.HiveUtils;
34-
import org.apache.hadoop.hive.ql.security.authorization.plugin.*;
33+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException;
34+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthorizer;
35+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthzContext;
36+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthzPluginException;
37+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthzSessionContext;
38+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveMetastoreClientFactoryImpl;
39+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveOperationType;
40+
import org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject;
3541
import org.apache.hadoop.security.UserGroupInformation;
3642
import org.apache.iceberg.catalog.TableIdentifier;
3743
import org.apache.iceberg.rest.credentials.Credential;

‎standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestCredentialVendingAws.java‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
99
*
10-
* http://www.apache.org/licenses/LICENSE-2.0
10+
* http://www.apache.org/licenses/LICENSE-2.0
1111
*
12-
* Unless required by applicable law or agreed to in writing,
13-
* software distributed under the License is distributed on an
14-
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15-
* KIND, either express or implied. See the License for the
16-
* specific language governing permissions and limitations
17-
* under the License.
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
1817
*/
1918

2019
package org.apache.iceberg.rest;
@@ -233,11 +232,8 @@ void testReadOnlyUser() throws IOException {
233232
}
234233

235234
var destination = tableLocation + "/credential-vending-it.txt";
236-
Assertions.assertThrows(AccessDeniedException.class, () -> {
237-
try (var output = table.io().newOutputFile(destination).createOrOverwrite()) {
238-
output.write("content".getBytes(StandardCharsets.UTF_8));
239-
}
240-
});
235+
var output = table.io().newOutputFile(destination).createOrOverwrite();
236+
Assertions.assertThrows(AccessDeniedException.class, output::close);
241237
}
242238
}
243239
}

‎standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergVendedCredentialProvider.java‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
99
*
10-
* http://www.apache.org/licenses/LICENSE-2.0
10+
* http://www.apache.org/licenses/LICENSE-2.0
1111
*
12-
* Unless required by applicable law or agreed to in writing,
13-
* software distributed under the License is distributed on an
14-
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15-
* KIND, either express or implied. See the License for the
16-
* specific language governing permissions and limitations
17-
* under the License.
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
1817
*/
1918

2019
package org.apache.iceberg.rest;
@@ -104,8 +103,12 @@ public void testVendWithWritableUser() throws HiveAccessControlException, HiveAu
104103

105104
var inputCaptor = ArgumentCaptor.forClass(List.class);
106105
var outputCaptor = ArgumentCaptor.forClass(List.class);
107-
Mockito.verify(authorizer, Mockito.times(2))
108-
.checkPrivileges(eq(HiveOperationType.QUERY), inputCaptor.capture(), outputCaptor.capture(), any(HiveAuthzContext.class));
106+
Mockito.verify(authorizer, Mockito.times(2)).checkPrivileges(
107+
eq(HiveOperationType.QUERY),
108+
inputCaptor.capture(),
109+
outputCaptor.capture(),
110+
any(HiveAuthzContext.class)
111+
);
109112
assertPrivilegeObjects(List.of(INPUT_OBJECT), inputCaptor.getAllValues().getFirst());
110113
assertPrivilegeObjects(List.of(), inputCaptor.getAllValues().getLast());
111114
assertPrivilegeObjects(List.of(), outputCaptor.getAllValues().getFirst());
@@ -142,8 +145,12 @@ public void testVendWithReadOnlyUser() throws HiveAccessControlException, HiveAu
142145
var operationCaptor = ArgumentCaptor.forClass(HiveOperationType.class);
143146
var inputCaptor = ArgumentCaptor.forClass(List.class);
144147
var outputCaptor = ArgumentCaptor.forClass(List.class);
145-
Mockito.verify(authorizer, Mockito.times(2))
146-
.checkPrivileges(operationCaptor.capture(), inputCaptor.capture(), outputCaptor.capture(), any(HiveAuthzContext.class));
148+
Mockito.verify(authorizer, Mockito.times(2)).checkPrivileges(
149+
operationCaptor.capture(),
150+
inputCaptor.capture(),
151+
outputCaptor.capture(),
152+
any(HiveAuthzContext.class)
153+
);
147154
Assert.assertEquals(List.of(HiveOperationType.QUERY, HiveOperationType.QUERY), operationCaptor.getAllValues());
148155
assertPrivilegeObjects(List.of(INPUT_OBJECT), inputCaptor.getAllValues().getFirst());
149156
assertPrivilegeObjects(List.of(), inputCaptor.getAllValues().getLast());

‎standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/credential/CompositeVendedCredentialProvider.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,30 +58,30 @@ public List<VendedStorageCredential> vend(String username, List<StorageAccessReq
5858
private final List<VendedCredentialProvider> providers;
5959

6060
private static VendedCredentialProvider create(Configuration conf, String providerId) {
61-
final var providerConfigKeyPrefix = "%s.%s".formatted(PROVIDERS_KEY_PREFIX, providerId);
62-
final var classKey = "%s.%s".formatted(providerConfigKeyPrefix, CLASS_KEY);
61+
final var providerConfigKeyPrefix = "%s.%s.".formatted(PROVIDERS_KEY_PREFIX, providerId);
62+
final var classKey = providerConfigKeyPrefix + CLASS_KEY;
6363
final var clazz = conf.getClass(classKey, null, VendedCredentialProvider.class);
6464
if (clazz == null) {
65-
throw new IllegalArgumentException("No vended credential provider class configured for provider ID: " + providerId);
65+
throw new IllegalArgumentException(
66+
"No vended credential provider class configured for provider ID: " + providerId);
6667
}
6768

6869
final VendedCredentialProvider provider;
6970
try {
7071
final var constructor = clazz.getDeclaredConstructor(String.class, Configuration.class);
71-
constructor.setAccessible(true);
7272
provider = constructor.newInstance(providerConfigKeyPrefix, conf);
7373
} catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException e) {
7474
throw new IllegalArgumentException("Failed to instantiate vended credential provider: " + clazz.getName(), e);
7575
}
7676

77-
final var maxCacheSize = conf.getInt("%s.%s".formatted(providerConfigKeyPrefix, CACHE_MAX_SIZE_KEY), 0);
77+
final var maxCacheSize = conf.getInt(providerConfigKeyPrefix + CACHE_MAX_SIZE_KEY, 0);
7878
if (maxCacheSize <= 0) {
7979
LOG.info("Created VendedCredentialProvider, {}, without cache", provider);
8080
return provider;
8181
}
8282

8383
final var maxCacheDuration = Duration.ofNanos(
84-
conf.getTimeDuration("%s.%s".formatted(providerConfigKeyPrefix, CACHE_MAX_DURATION_KEY),
84+
conf.getTimeDuration(providerConfigKeyPrefix + CACHE_MAX_DURATION_KEY,
8585
DEFAULT_MAX_CACHE_DURATION.toNanos(), TimeUnit.NANOSECONDS));
8686
LOG.info("Created VendedCredentialProvider, {}, with caching (capacity={}, duration={}) ", provider, maxCacheSize,
8787
maxCacheDuration);

‎standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/credential/s3/S3Location.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package org.apache.hadoop.hive.metastore.credential.s3;
2020

21+
import org.apache.hadoop.fs.Path;
2122
import software.amazon.awssdk.arns.Arn;
2223

2324
import java.net.URI;
@@ -27,7 +28,7 @@
2728
/**
2829
* An S3 location.
2930
*/
30-
class S3Location {
31+
final class S3Location {
3132
private static final Set<String> SCHEMES = Set.of("s3", "s3a", "s3n");
3233

3334
private final String partition;
@@ -56,7 +57,7 @@ static Optional<S3Location> create(String partition, URI uri) {
5657
if (rawPath == null) {
5758
return Optional.empty();
5859
}
59-
final var path = rawPath.endsWith("/") ? rawPath : rawPath + "/";
60+
final var path = rawPath.endsWith(Path.SEPARATOR) ? rawPath : rawPath + Path.SEPARATOR;
6061
return Optional.of(new S3Location(partition, bucket, path));
6162
}
6263

‎standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/credential/s3/S3VendedCredentialProvider.java‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.hadoop.hive.metastore.credential.VendedCredentialProvider;
2626
import org.apache.hadoop.hive.metastore.credential.VendedStorageCredential;
2727
import software.amazon.awssdk.arns.Arn;
28+
import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
2829
import software.amazon.awssdk.policybuilder.iam.IamConditionOperator;
2930
import software.amazon.awssdk.policybuilder.iam.IamEffect;
3031
import software.amazon.awssdk.policybuilder.iam.IamPolicy;
@@ -62,7 +63,8 @@ public class S3VendedCredentialProvider implements VendedCredentialProvider {
6263
private final StsClient stsClient;
6364

6465
private static StsClient createStsClient(String region) {
65-
final var builder = StsClient.builder();
66+
final var credentialsProvider = DefaultCredentialsProvider.builder().build();
67+
final var builder = StsClient.builder().credentialsProvider(credentialsProvider);
6668
return region == null ? builder.build() : builder.region(Region.of(region)).build();
6769
}
6870

@@ -75,14 +77,14 @@ private static List<String> createPrefixes(String[] prefixes) {
7577

7678
public S3VendedCredentialProvider(String configKeyPrefix, Configuration conf) {
7779
this(
78-
Arn.fromString(Objects.requireNonNull(conf.get("%s.%s".formatted(configKeyPrefix, ROLE_ARN_KEY)))),
79-
conf.get("%s.%s".formatted(configKeyPrefix, EXTERNAL_ID_KEY)),
80-
createPrefixes(conf.getStrings("%s.%s".formatted(configKeyPrefix, PREFIXES_KEY), (String) null)),
80+
Arn.fromString(Objects.requireNonNull(conf.get(configKeyPrefix + ROLE_ARN_KEY))),
81+
conf.get(configKeyPrefix + EXTERNAL_ID_KEY),
82+
createPrefixes(conf.getStrings(configKeyPrefix + PREFIXES_KEY, (String) null)),
8183
(int) Math.min(
8284
Integer.MAX_VALUE,
83-
conf.getTimeDuration("%s.%s".formatted(configKeyPrefix, CREDENTIAL_EXPIRATION_KEY), 3600, TimeUnit.SECONDS)
85+
conf.getTimeDuration(configKeyPrefix + CREDENTIAL_EXPIRATION_KEY, 3600, TimeUnit.SECONDS)
8486
),
85-
createStsClient(conf.get("%s.%s".formatted(configKeyPrefix, REGION_KEY)))
87+
createStsClient(conf.get(configKeyPrefix + REGION_KEY))
8688
);
8789
}
8890

@@ -190,20 +192,22 @@ private IamPolicy buildPolicy(List<StorageAccessRequest> requests) {
190192
bucketLocationBuilder.values().stream().map(IamStatement.Builder::build).forEach(policyBuilder::addStatement);
191193
listBuilder.values().stream().map(IamStatement.Builder::build).forEach(policyBuilder::addStatement);
192194
if (!readResources.isEmpty()) {
193-
final var builder = IamStatement.builder().effect(IamEffect.ALLOW).addAction("s3:GetObject")
194-
.addAction("s3:GetObjectVersion");
195-
readResources.forEach(builder::addResource);
196-
policyBuilder.addStatement(builder.build());
195+
policyBuilder.addStatement(builder -> {
196+
builder.effect(IamEffect.ALLOW).addAction("s3:GetObject").addAction("s3:GetObjectVersion");
197+
readResources.forEach(builder::addResource);
198+
});
197199
}
198200
if (!createResources.isEmpty()) {
199-
final var createBuilder = IamStatement.builder().effect(IamEffect.ALLOW).addAction("s3:PutObject");
200-
createResources.forEach(createBuilder::addResource);
201-
policyBuilder.addStatement(createBuilder.build());
201+
policyBuilder.addStatement(builder -> {
202+
builder.effect(IamEffect.ALLOW).addAction("s3:PutObject");
203+
createResources.forEach(builder::addResource);
204+
});
202205
}
203206
if (!deleteResources.isEmpty()) {
204-
final var deleteBuilder = IamStatement.builder().effect(IamEffect.ALLOW).addAction("s3:DeleteObject");
205-
deleteResources.forEach(deleteBuilder::addResource);
206-
policyBuilder.addStatement(deleteBuilder.build());
207+
policyBuilder.addStatement(builder -> {
208+
builder.effect(IamEffect.ALLOW).addAction("s3:DeleteObject");
209+
deleteResources.forEach(builder::addResource);
210+
});
207211
}
208212
return policyBuilder.build();
209213
}

‎standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/annotation/MetastoreExternalTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@
66
* to you under the Apache License, Version 2.0 (the
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
9-
* <p>
10-
* http://www.apache.org/licenses/LICENSE-2.0
11-
* <p>
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
1212
* Unless required by applicable law or agreed to in writing, software
1313
* distributed under the License is distributed on an "AS IS" BASIS,
1414
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1515
* See the License for the specific language governing permissions and
1616
* limitations under the License.
1717
*/
18+
1819
package org.apache.hadoop.hive.metastore.annotation;
1920

2021
/**

‎standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/credential/s3/TestS3VendedCredentialProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ public void testSupportsWithPrefix() {
7676
new Path("s3a://bucket-b/warehouse/table"),
7777
EnumSet.of(StorageOperation.READ));
7878
Assert.assertFalse(provider.supports(unmatched));
79-
Assert.assertThrows(IllegalArgumentException.class,
80-
() -> provider.vend("test-user", Collections.singletonList(unmatched)));
79+
var requests = Collections.singletonList(unmatched);
80+
Assert.assertThrows(IllegalArgumentException.class, () -> provider.vend("test-user", requests));
8181
}
8282

8383
@Test

‎standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/credential/s3/TestS3VendedCredentialProviderIntegration.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,10 @@ private static S3Client createSessionS3Client(Region region, VendedStorageCreden
8787
private static void deleteObjectIfExists(S3Client s3, String bucket, String key) {
8888
try {
8989
s3.deleteObject(DeleteObjectRequest.builder().bucket(bucket).key(key).build());
90-
} catch (S3Exception ignored) {
90+
} catch (S3Exception e) {
91+
if (e.statusCode() != 404) {
92+
throw e;
93+
}
9194
}
9295
}
9396

0 commit comments

Comments
 (0)