Merge branch 'stable-3.4' into stable-3.5
* stable-3.4:
Make DynamoDBRefDatebase class implement ExtendedGlobalRefDatabase
computeAndPut should return false instead throwing exception
Consume global-refdb directly from source
Bump global-refdb to v3.4.8.6
Add AWS credentials profile name parameter
Change-Id: I31d43d8ae625178aa1ff7a620efda486d5048371
diff --git a/BUILD b/BUILD
index 72ae9c1..1dbc913 100644
--- a/BUILD
+++ b/BUILD
@@ -17,6 +17,7 @@
],
resources = glob(["src/main/resources/**/*"]),
deps = [
+ ":global-refdb-neverlink",
"@amazon-aws-core//jar",
"@amazon-dynamodb//jar",
"@amazon-regions//jar",
@@ -25,7 +26,6 @@
"@aws-java-sdk-core//jar",
"@aws-java-sdk-dynamodb//jar",
"@dynamodb-lock-client//jar",
- "@global-refdb//jar",
"@jackson-annotations//jar",
"@jackson-core//jar",
"@jackson-databind//jar",
@@ -36,6 +36,7 @@
junit_tests(
name = "aws-dynamodb-refdb_tests",
+ timeout = "long",
srcs = glob(["src/test/java/**/*.java"]),
resources = glob(["src/test/resources/**/*"]),
tags = ["aws-dynamodb-refdb"],
@@ -50,6 +51,7 @@
visibility = ["//visibility:public"],
exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":aws-dynamodb-refdb__plugin",
+ "//plugins/global-refdb",
"@amazon-regions//jar",
"@testcontainers//jar",
"@docker-java-api//jar",
@@ -58,6 +60,11 @@
"@jna//jar",
"@testcontainer-localstack//jar",
"@aws-java-sdk-dynamodb//jar",
- "@global-refdb//jar",
],
)
+
+java_library(
+ name = "global-refdb-neverlink",
+ neverlink = 1,
+ exports = ["//plugins/global-refdb"],
+)
diff --git a/Jenkinsfile b/Jenkinsfile
index 48cce50..b1692ff 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -1,2 +1,3 @@
pluginPipeline(formatCheckId: 'gerritforge:plugins-aws-dynamodb-refdb-code-style',
- buildCheckId: 'gerritforge:plugins-aws-dynamodb-refdb-build-test')
+ buildCheckId: 'gerritforge:plugins-aws-dynamodb-refdb-build-test',
+ extraModules: [ 'global-refdb' ])
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index c3f2ea5..c3aa3e8 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -119,9 +119,3 @@
artifact = "net.java.dev.jna:jna:5.5.0",
sha1 = "0e0845217c4907822403912ad6828d8e0b256208",
)
-
- maven_jar(
- name = "global-refdb",
- artifact = "com.gerritforge:global-refdb:3.3.2.1",
- sha1 = "00b6b0f39b3c8fc280a19d91fb0681954ebccd02",
- )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/AmazonDynamoDBProvider.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/AmazonDynamoDBProvider.java
index 76ec8c0..eb13649 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/AmazonDynamoDBProvider.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/AmazonDynamoDBProvider.java
@@ -14,7 +14,9 @@
package com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb;
+import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.auth.DefaultAWSCredentialsProviderChain;
+import com.amazonaws.auth.profile.ProfileCredentialsProvider;
import com.amazonaws.client.builder.AwsClientBuilder;
import com.amazonaws.regions.DefaultAwsRegionProviderChain;
import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
@@ -48,6 +50,13 @@
endpoint ->
builder.withEndpointConfiguration(
new AwsClientBuilder.EndpointConfiguration(endpoint.toASCIIString(), region)));
- return builder.withCredentials(new DefaultAWSCredentialsProviderChain()).build();
+ return builder.withCredentials(getCredentialsProvider()).build();
+ }
+
+ private AWSCredentialsProvider getCredentialsProvider() {
+ return configuration
+ .getAwsConfigurationProfileName()
+ .<AWSCredentialsProvider>map(ProfileCredentialsProvider::new)
+ .orElse(new DefaultAWSCredentialsProviderChain());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Configuration.java
index 791e1c7..01e2f4f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/Configuration.java
@@ -34,6 +34,7 @@
private final Optional<URI> endpoint;
private final String refsDbTableName;
private final String locksTableName;
+ private final Optional<String> awsConfigurationProfileName;
@Inject
Configuration(PluginConfigFactory configFactory, @PluginName String pluginName) {
@@ -44,12 +45,14 @@
// TODO: add prefix
this.refsDbTableName = pluginConfig.getString("refsDbTableName", DEFAULT_REFS_DB_TABLE_NAME);
this.locksTableName = pluginConfig.getString("locksTableName", DEFAULT_LOCKS_TABLE_NAME);
+ this.awsConfigurationProfileName = Optional.ofNullable(pluginConfig.getString("profileName"));
logger.atInfo().log(
- "dynamodb-refdb configuration: refsDbTableName: %s|locksTableName:%s%s%s",
+ "dynamodb-refdb configuration: refsDbTableName: %s|locksTableName:%s%s%s%s",
refsDbTableName,
locksTableName,
region.map(r -> String.format("|region: %s", r.id())).orElse(""),
- endpoint.map(e -> String.format("|endpoint: %s", e.toASCIIString())).orElse(""));
+ endpoint.map(e -> String.format("|endpoint: %s", e.toASCIIString())).orElse(""),
+ awsConfigurationProfileName.map(p -> String.format("|profile: %s", p)).orElse(""));
}
Optional<Region> getRegion() {
@@ -73,4 +76,8 @@
String getLocksTableName() {
return locksTableName;
}
+
+ Optional<String> getAwsConfigurationProfileName() {
+ return awsConfigurationProfileName;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
index 44822e9..933fd3f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabase.java
@@ -18,17 +18,20 @@
import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
import com.amazonaws.services.dynamodbv2.AmazonDynamoDBLockClient;
import com.amazonaws.services.dynamodbv2.LockItem;
+import com.amazonaws.services.dynamodbv2.model.AttributeAction;
import com.amazonaws.services.dynamodbv2.model.AttributeValue;
+import com.amazonaws.services.dynamodbv2.model.AttributeValueUpdate;
import com.amazonaws.services.dynamodbv2.model.ConditionalCheckFailedException;
import com.amazonaws.services.dynamodbv2.model.GetItemResult;
import com.amazonaws.services.dynamodbv2.model.LockNotGrantedException;
import com.amazonaws.services.dynamodbv2.model.UpdateItemRequest;
-import com.gerritforge.gerrit.globalrefdb.GlobalRefDatabase;
+import com.gerritforge.gerrit.globalrefdb.ExtendedGlobalRefDatabase;
import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException;
import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError;
import com.google.common.collect.ImmutableMap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
import com.google.inject.Inject;
import java.util.Optional;
import javax.inject.Singleton;
@@ -36,7 +39,7 @@
import org.eclipse.jgit.lib.Ref;
@Singleton
-public class DynamoDBRefDatabase implements GlobalRefDatabase {
+public class DynamoDBRefDatabase implements ExtendedGlobalRefDatabase {
public static final String REF_DB_PRIMARY_KEY = "refPath";
public static final String REF_DB_VALUE_KEY = "refValue";
@@ -132,11 +135,10 @@
project.get(), currValueForPath, newValueForPath);
return true;
} catch (ConditionalCheckFailedException e) {
- throw new GlobalRefDbSystemError(
- String.format(
- "Conditional Check Failure when updating refPath %s. expected: %s New: %s",
- refPath, currValueForPath, newValueForPath),
- e);
+ logger.atWarning().withCause(e).log(
+ "Conditional Check Failure when updating refPath %s. expected: %s New: %s",
+ refPath, currValueForPath, newValueForPath);
+ return false;
} catch (Exception e) {
throw new GlobalRefDbSystemError(
String.format(
@@ -147,6 +149,29 @@
}
@Override
+ public <T> void put(NameKey project, String refName, T value) throws GlobalRefDbSystemError {
+ String refPath = pathFor(project, refName);
+ String refValue =
+ Optional.ofNullable(value).map(Object::toString).orElse(ObjectId.zeroId().getName());
+ try {
+ dynamoDBClient.updateItem(
+ configuration.getRefsDbTableName(),
+ ImmutableMap.of(REF_DB_PRIMARY_KEY, new AttributeValue(refPath)),
+ ImmutableMap.of(
+ REF_DB_VALUE_KEY,
+ new AttributeValueUpdate(new AttributeValue(refValue), AttributeAction.PUT)));
+ logger.atFine().log(
+ "Updated path for project %s, path %s, value: %s", project.get(), refPath, refValue);
+ } catch (Exception e) {
+ throw new GlobalRefDbSystemError(
+ String.format(
+ "Error updating path for project %s, path %s. value: %s",
+ project.get(), refPath, refValue),
+ e);
+ }
+ }
+
+ @Override
public AutoCloseable lockRef(Project.NameKey project, String refName)
throws GlobalRefDbLockException {
String refPath = pathFor(project, refName);
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index ca94b64..0d871cc 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -10,6 +10,7 @@
endpoint = http://localhost:4566
locksTableName = lockTable
refsDbTableName = refsDb
+ profileName = aws-dynamodb-refdb
```
`plugin.aws-dynamodb-refdb.region`
@@ -33,4 +34,11 @@
: Optional. The name of the dynamoDB table used to store git refs and their
associated sha1.
+`plugin.aws-dynamodb-refdb.profileName`
+: Optional. The name of the aws configuration and credentials profile used to
+connect to the DynamoDb. See [Configuration and credential file settings](https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html)
+Default: When not specified credentials are provided via the Default Credentials
+Provider Chain, as explained [here](https://docs.aws.amazon.com/sdk-for-java/v1/developer-guide/credentials.html)
+
+
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ConfigurationTest.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ConfigurationTest.java
index 5195c01..6b961d7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ConfigurationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/ConfigurationTest.java
@@ -101,6 +101,26 @@
}
@Test
+ public void shouldReadEmptyAwsConfigurationProfileNameByDefault() {
+ when(pluginConfigFactoryMock.getFromGerritConfig(PLUGIN_NAME))
+ .thenReturn(pluginConfig.asPluginConfig());
+
+ Configuration configuration = new Configuration(pluginConfigFactoryMock, PLUGIN_NAME);
+ assertThat(configuration.getAwsConfigurationProfileName().isPresent()).isFalse();
+ }
+
+ @Test
+ public void shouldReadConfiguredAwsConfigurationProfileName() {
+ String profileName = "aws-dynamodb-refdb";
+ pluginConfig.setString("profileName", profileName);
+ when(pluginConfigFactoryMock.getFromGerritConfig(PLUGIN_NAME))
+ .thenReturn(pluginConfig.asPluginConfig());
+
+ Configuration configuration = new Configuration(pluginConfigFactoryMock, PLUGIN_NAME);
+ assertThat(configuration.getAwsConfigurationProfileName().get()).isEqualTo(profileName);
+ }
+
+ @Test
public void shouldReadEmptyRegionByDefault() {
when(pluginConfigFactoryMock.getFromGerritConfig(PLUGIN_NAME))
.thenReturn(pluginConfig.asPluginConfig());
diff --git a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
index ba9b81d..38b6dc8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/validation/dfsrefdb/dynamodb/DynamoDBRefDatabaseIT.java
@@ -16,19 +16,11 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
-import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.Configuration.DEFAULT_LOCKS_TABLE_NAME;
import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.Configuration.DEFAULT_REFS_DB_TABLE_NAME;
-import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.DynamoDBRefDatabase.REF_DB_PRIMARY_KEY;
-import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.DynamoDBRefDatabase.REF_DB_VALUE_KEY;
-import static com.googlesource.gerrit.plugins.validation.dfsrefdb.dynamodb.DynamoDBRefDatabase.pathFor;
import static org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB;
import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
-import com.amazonaws.services.dynamodbv2.model.AttributeValue;
-import com.amazonaws.services.dynamodbv2.model.PutItemRequest;
-import com.gerritforge.gerrit.globalrefdb.GlobalRefDbSystemError;
-import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.WaitUtil;
@@ -169,6 +161,39 @@
}
@Test
+ public void putShouldBeSuccessfulWhenNoPreviousValueForRefExists() {
+ String refName = "refs/changes/01/01/meta";
+ String newRefValue = "533d3ccf8a650fb26380faa732921a2c74924d5c";
+
+ dynamoDBRefDatabase().put(project, refName, newRefValue);
+ Optional<String> result = dynamoDBRefDatabase().get(project, refName, String.class);
+ assertThat(result).hasValue(newRefValue);
+ }
+
+ @Test
+ public void putShouldSuccessfullyUpdateRemovedRef() {
+ String refName = "refs/changes/01/01/meta";
+ String newRefValue = null;
+
+ dynamoDBRefDatabase().put(project, refName, newRefValue);
+ Optional<String> result = dynamoDBRefDatabase().get(project, refName, String.class);
+ assertThat(result).hasValue(ObjectId.zeroId().getName());
+ }
+
+ @Test
+ public void putShouldBeSuccessfulWhenUpdatingRef() {
+ String refName = "refs/changes/01/01/meta";
+ String oldValue = "123";
+ String newValue = "345";
+ dynamoDBRefDatabase().put(project, refName, oldValue);
+
+ dynamoDBRefDatabase().put(project, refName, newValue);
+
+ Optional<String> result = dynamoDBRefDatabase().get(project, refName, String.class);
+ assertThat(result).hasValue(newValue);
+ }
+
+ @Test
public void compareAndPutShouldSuccessfullyUpdateRemovedRef() throws Exception {
String refName = "refs/changes/01/01/meta";
String currentRefValue = "533d3ccf8a650fb26380faa732921a2c74924d5c";
@@ -182,7 +207,7 @@
}
@Test
- public void compareAndPutShouldThrowWhenStoredRefIsNotExpected() {
+ public void compareAndPutShouldReturnFalseWhenStoredRefIsNotExpected() {
String refName = "refs/changes/01/01/meta";
String currentRefValue = "533d3ccf8a650fb26380faa732921a2c74924d5c";
String newRefValue = "9f6f2963cf44505428c61b935ff1ca65372cf28c";
@@ -190,20 +215,10 @@
Ref expectedRef = refOf(refName, expectedRefValue);
createRefInDynamoDB(project, refName, currentRefValue);
-
- GlobalRefDbSystemError thrown =
- assertThrows(
- GlobalRefDbSystemError.class,
- () ->
- dynamoDBRefDatabase()
- .compareAndPut(project, expectedRef, ObjectId.fromString(newRefValue)));
- assertThat(thrown)
- .hasMessageThat()
- .containsMatch(
- "Conditional Check Failure when updating refPath.*expected:.*"
- + expectedRefValue
- + " New:.*"
- + newRefValue);
+ assertThat(
+ dynamoDBRefDatabase()
+ .compareAndPut(project, expectedRef, ObjectId.fromString(newRefValue)))
+ .isFalse();
}
@Test
@@ -235,16 +250,7 @@
}
private void createRefInDynamoDB(Project.NameKey project, String refPath, String refValue) {
- dynamoDBClient()
- .putItem(
- new PutItemRequest()
- .withTableName(DEFAULT_REFS_DB_TABLE_NAME)
- .withItem(
- ImmutableMap.of(
- REF_DB_PRIMARY_KEY,
- new AttributeValue(pathFor(project, refPath)),
- REF_DB_VALUE_KEY,
- new AttributeValue(refValue))));
+ dynamoDBRefDatabase().put(project, refPath, refValue);
}
private Ref refOf(String refName, @Nullable String objectIdSha1) {
