merge: #12238

12238: [Backport release-8.2.0] fix(backup/gcs): listing backups when no basePath is set r=oleschoenburg a=oleschoenburg Manual backport of #12221 to fix merge conflicts in the parent pom. Co-authored-by: Ole Schönburg <[email protected]>
camunda · Apr 4, 2023 · 7008175 · 7008175
2 parents 4b1dcf1 + 60674d8
commit 7008175
Show file tree

Hide file tree

Showing 13 changed files with 515 additions and 46 deletions.
diff --git a/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/FileSetManager.java b/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/FileSetManager.java
@@ -28,14 +28,14 @@ final class FileSetManager {
    *
    * <ul>
    *   <li>{@code basePath}
-   *   <li>"contents
+   *   <li>{@code "contents"}
    *   <li>{@code partitionId}
    *   <li>{@code checkpointId}
    *   <li>{@code nodeId}
    *   <li>{@code fileSetName}
    * </ul>
    */
-  private static final String PATH_FORMAT = "%s/contents/%s/%s/%s/%s/";
+  private static final String PATH_FORMAT = "%scontents/%s/%s/%s/%s/";
 
   private final Storage client;
   private final BucketInfo bucketInfo;

diff --git a/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/GcsBackupConfig.java b/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/GcsBackupConfig.java
@@ -27,16 +27,20 @@ private static String requireBucketName(final String bucketName) {
   }
 
   private static String sanitizeBasePath(final String basePath) {
-    if (basePath == null || basePath.isBlank()) {
+    if (basePath == null) {
+      return null;
+    }
+
+    var sanitized = basePath.trim();
+    if (sanitized.isEmpty() || sanitized.equals("/")) {
       return null;
     }
 
     // Remove one leading and one trailing slash if present.
-    String sanitized = basePath;
-    if (basePath.startsWith("/")) {
-      sanitized = basePath.substring(1);
+    while (sanitized.startsWith("/")) {
+      sanitized = sanitized.substring(1);
     }
-    if (basePath.endsWith("/")) {
+    while (sanitized.endsWith("/")) {
       sanitized = sanitized.substring(0, sanitized.length() - 1);
     }
 

diff --git a/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/GcsBackupStore.java b/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/GcsBackupStore.java
@@ -48,7 +48,7 @@ public GcsBackupStore(final GcsBackupConfig config) {
 
   public GcsBackupStore(final GcsBackupConfig config, final Storage client) {
     final var bucketInfo = BucketInfo.of(config.bucketName());
-    final var basePath = Optional.ofNullable(config.basePath()).orElse("");
+    final var basePath = Optional.ofNullable(config.basePath()).map(s -> s + "/").orElse("");
     this.client = client;
     executor = Executors.newWorkStealingPool(4);
     manifestManager = new ManifestManager(client, bucketInfo, basePath);

diff --git a/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/ManifestManager.java b/backup-stores/gcs/src/main/java/io/camunda/zeebe/backup/gcs/ManifestManager.java
@@ -60,7 +60,7 @@ public final class ManifestManager {
    *   <li>{@code "manifests"}
    * </ul>
    */
-  private static final String MANIFESTS_ROOT_PATH_FORMAT = "%s/manifests/";
+  private static final String MANIFESTS_ROOT_PATH_FORMAT = "%smanifests/";
   /**
    * The path format consists of the following elements:
    *

diff --git a/backup-stores/gcs/src/test/java/io/camunda/zeebe/backup/gcs/ConfigTest.java b/backup-stores/gcs/src/test/java/io/camunda/zeebe/backup/gcs/ConfigTest.java
@@ -40,6 +40,20 @@ void shouldRejectEmptyBucketName() {
         .hasMessageContaining("bucketName");
   }
 
+  @Test
+  void shouldAcceptSingleSlashAsBasePath() {
+    // given
+    final var bucketName = "test";
+    final var basePath = "/";
+
+    // when
+    final var config =
+        new GcsBackupConfig.Builder().withBucketName(bucketName).withBasePath(basePath).build();
+
+    // then
+    Assertions.assertThat(config.basePath()).isNull();
+  }
+
   @Test
   void shouldRemoveLeadingSlashesFromBasePath() {
     // given

diff --git a/backup-stores/gcs/src/test/java/io/camunda/zeebe/backup/gcs/GcsBackupStoreIT.java b/backup-stores/gcs/src/test/java/io/camunda/zeebe/backup/gcs/GcsBackupStoreIT.java
@@ -15,50 +15,89 @@
 import org.apache.commons.lang3.RandomStringUtils;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
 import org.testcontainers.junit.jupiter.Container;
 import org.testcontainers.junit.jupiter.Testcontainers;
 
 @Testcontainers
-public class GcsBackupStoreIT implements BackupStoreTestKit {
+public class GcsBackupStoreIT {
   @Container private static final GcsContainer GCS = new GcsContainer();
-  private static final String BUCKET_NAME = RandomStringUtils.randomAlphabetic(10).toLowerCase();
-
-  private GcsBackupStore store;
-
-  @BeforeAll
-  static void createBucket() {
-    final var config =
-        new GcsBackupConfig.Builder()
-            .withBucketName(BUCKET_NAME)
-            .withHost(GCS.externalEndpoint())
-            .withoutAuthentication()
-            .build();
-    try (final var client = GcsBackupStore.buildClient(config)) {
-      client.create(BucketInfo.of(BUCKET_NAME));
-    } catch (final Exception e) {
-      throw new RuntimeException(e);
+
+  @Nested
+  final class WithBasePath implements BackupStoreTestKit {
+    private static final String BUCKET_NAME = RandomStringUtils.randomAlphabetic(10).toLowerCase();
+
+    private GcsBackupStore store;
+
+    @BeforeAll
+    static void createBucket() {
+      final var config =
+          new GcsBackupConfig.Builder()
+              .withBucketName(BUCKET_NAME)
+              .withHost(GCS.externalEndpoint())
+              .withoutAuthentication()
+              .build();
+      try (final var client = GcsBackupStore.buildClient(config)) {
+        client.create(BucketInfo.of(BUCKET_NAME));
+      } catch (final Exception e) {
+        throw new RuntimeException(e);
+      }
     }
-  }
 
-  @BeforeEach
-  void setup() {
-    store =
-        new GcsBackupStore(
-            new GcsBackupConfig.Builder()
-                .withBucketName(BUCKET_NAME)
-                .withBasePath(RandomStringUtils.randomAlphabetic(10).toLowerCase())
-                .withHost(GCS.externalEndpoint())
-                .withoutAuthentication()
-                .build());
-  }
+    @BeforeEach
+    void setup() {
+      store =
+          new GcsBackupStore(
+              new GcsBackupConfig.Builder()
+                  .withBucketName(BUCKET_NAME)
+                  .withBasePath(RandomStringUtils.randomAlphabetic(10).toLowerCase())
+                  .withHost(GCS.externalEndpoint())
+                  .withoutAuthentication()
+                  .build());
+    }
+
+    @Override
+    public BackupStore getStore() {
+      return store;
+    }
 
-  @Override
-  public BackupStore getStore() {
-    return store;
+    @Override
+    public Class<? extends Exception> getBackupInInvalidStateExceptionClass() {
+      return UnexpectedManifestState.class;
+    }
   }
 
-  @Override
-  public Class<? extends Exception> getBackupInInvalidStateExceptionClass() {
-    return UnexpectedManifestState.class;
+  @Nested
+  final class WithoutBasePath implements BackupStoreTestKit {
+
+    private GcsBackupStore store;
+
+    @BeforeEach
+    void setup() throws Exception {
+      final var bucketName = RandomStringUtils.randomAlphabetic(10).toLowerCase();
+
+      final var config =
+          new GcsBackupConfig.Builder()
+              .withBucketName(bucketName)
+              .withHost(GCS.externalEndpoint())
+              .withoutAuthentication()
+              .build();
+
+      try (final var client = GcsBackupStore.buildClient(config)) {
+        client.create(BucketInfo.of(bucketName));
+      }
+
+      store = new GcsBackupStore(config);
+    }
+
+    @Override
+    public BackupStore getStore() {
+      return store;
+    }
+
+    @Override
+    public Class<? extends Exception> getBackupInInvalidStateExceptionClass() {
+      return UnexpectedManifestState.class;
+    }
   }
 }
diff --git a/parent/pom.xml b/parent/pom.xml
@@ -119,6 +119,7 @@
     <version.jnr-posix>3.1.16</version.jnr-posix>
     <version.zpt>8.1.9</version.zpt>
     <version.feign>12.2</version.feign>
+    <version.google-sdk>26.11.0</version.google-sdk>
     <version.awssdk>2.20.33</version.awssdk>
     <version.toxiproxy>2.1.7</version.toxiproxy>
     <version.validation-api>3.0.2</version.validation-api>
@@ -990,6 +991,14 @@
         <version>${version.jnr-posix}</version>
       </dependency>
 
+      <dependency>
+        <groupId>com.google.cloud</groupId>
+        <artifactId>libraries-bom</artifactId>
+        <version>${version.google-sdk}</version>
+        <type>pom</type>
+        <scope>import</scope>
+      </dependency>
+
       <dependency>
         <groupId>software.amazon.awssdk</groupId>
         <artifactId>bom</artifactId>

diff --git a/qa/integration-tests/pom.xml b/qa/integration-tests/pom.xml
@@ -411,6 +411,18 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>io.camunda</groupId>
+      <artifactId>zeebe-backup-store-gcs</artifactId>
+      <scope>test</scope>
+    </dependency>
+
+    <dependency>
+      <groupId>com.google.cloud</groupId>
+      <artifactId>google-cloud-storage</artifactId>
+      <scope>test</scope>
+    </dependency>
+
   </dependencies>
 
   <build>