fix(migrator): GCS to SQL migrator APPLICATION_PERMISSION objectType …

…fix (#1466) (#1473) * fix(migrator): GCS to SQL migrator APPLICATION_PERMISSION fix/refactor * fix(migrator): GCS to SQL migrator APPLICATION_PERMISSION fix/refactor (cherry picked from commit 3ab3bfc) Co-authored-by: Christos Arvanitis <[email protected]>
spinnaker · Jun 11, 2024 · 121849e · 121849e
1 parent 25bc224
commit 121849e
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 16 deletions.
diff --git a/front50-core/src/main/java/com/netflix/spinnaker/front50/model/ObjectType.java b/front50-core/src/main/java/com/netflix/spinnaker/front50/model/ObjectType.java
@@ -37,11 +37,13 @@ public enum ObjectType {
       PipelineTemplate.class, "pipeline-templates", "pipeline-template-metadata.json"),
   NOTIFICATION(Notification.class, "notifications", "notification-metadata.json"),
   SERVICE_ACCOUNT(ServiceAccount.class, "serviceAccounts", "serviceAccount-metadata.json"),
-  APPLICATION(Application.class, "applications", "application-metadata.json"),
-  // TODO(ewiseblatt) Add migration logic to allow GCS to use application-permission.json like the
-  // other providers.
+
+  APPLICATION(Application.class, "applications", "application-metadata.json", "specification.json"),
   APPLICATION_PERMISSION(
-      Application.Permission.class, "applications", "application-permission.json"),
+      Application.Permission.class,
+      "applications",
+      "application-permission.json",
+      "permission.json"),
   SNAPSHOT(Snapshot.class, "snapshots", "snapshot.json"),
   ENTITY_TAGS(EntityTags.class, "tags", "entity-tags-metadata.json"),
   DELIVERY(Delivery.class, "delivery", "delivery-metadata.json"),
@@ -52,10 +54,27 @@ public enum ObjectType {
   public final Class<? extends Timestamped> clazz;
   public final String group;
   public final String defaultMetadataFilename;
+  public final String gcsMetadataFilename;
 
   ObjectType(Class<? extends Timestamped> clazz, String group, String defaultMetadataFilename) {
+    this(clazz, group, defaultMetadataFilename, null);
+  }
+
+  ObjectType(
+      Class<? extends Timestamped> clazz,
+      String group,
+      String defaultMetadataFilename,
+      String gcsMetadataFilename) {
     this.clazz = clazz;
     this.group = group;
     this.defaultMetadataFilename = defaultMetadataFilename;
+    this.gcsMetadataFilename = gcsMetadataFilename;
+  }
+
+  public String getDefaultMetadataFilename(boolean useGcsMetadataFilename) {
+    if (useGcsMetadataFilename && this.gcsMetadataFilename != null) {
+      return this.gcsMetadataFilename;
+    }
+    return this.defaultMetadataFilename;
   }
 }
diff --git a/front50-gcs/src/main/java/com/netflix/spinnaker/front50/config/GcsConfig.java b/front50-gcs/src/main/java/com/netflix/spinnaker/front50/config/GcsConfig.java
@@ -33,6 +33,7 @@
 import com.netflix.spinnaker.front50.model.DefaultObjectKeyLoader;
 import com.netflix.spinnaker.front50.model.GcsStorageService;
 import com.netflix.spinnaker.front50.model.ObjectKeyLoader;
+import com.netflix.spinnaker.front50.model.ObjectType;
 import com.netflix.spinnaker.front50.model.application.ApplicationPermissionDAO;
 import com.netflix.spinnaker.front50.model.application.DefaultApplicationPermissionDAO;
 import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry;
@@ -58,8 +59,10 @@ public class GcsConfig {
 
   private static final Logger log = LoggerFactory.getLogger(GcsConfig.class);
 
-  private static final String DATA_FILENAME = "specification.json";
-  private static final String APPLICATION_PERMISSION_DATA_FILENAME = "permission.json";
+  private static final String DATA_FILENAME =
+      ObjectType.APPLICATION.getDefaultMetadataFilename(true);
+  private static final String APPLICATION_PERMISSION_DATA_FILENAME =
+      ObjectType.APPLICATION_PERMISSION.getDefaultMetadataFilename(true);
 
   @Bean
   public GcsStorageService defaultGoogleCloudStorageService(

diff --git a/front50-gcs/src/main/kotlin/com/netflix/spinnaker/front50/model/GcsStorageService.kt b/front50-gcs/src/main/kotlin/com/netflix/spinnaker/front50/model/GcsStorageService.kt
@@ -127,12 +127,15 @@ class GcsStorageService(
 
     try {
       val rootDirectory = daoRoot(objectType)
+
       storage.list(bucketName, BlobListOption.prefix("$rootDirectory/"))
         .iterateAll()
         .forEach { blob ->
-          val objectKey = getObjectKey(blob, rootDirectory)
-          if (objectKey != null) {
-            results.put(objectKey, blob.updateTime)
+          if (blob.name.endsWith("/"+ objectType.getDefaultMetadataFilename(true))) {
+            val objectKey = getObjectKey(blob, rootDirectory, objectType.getDefaultMetadataFilename(true))
+            if (objectKey != null) {
+              results.put(objectKey, blob.updateTime)
+            }
           }
         }
     } catch (e: Exception) {
@@ -142,11 +145,11 @@ class GcsStorageService(
     return results.build()
   }
 
-  private fun getObjectKey(blob: Blob, rootDirectory: String): String? {
+  private fun getObjectKey(blob: Blob, rootDirectory: String, defaultMetadataKey: String): String? {
     val name = blob.name
-    return if (!name.startsWith("$rootDirectory/") || !name.endsWith("/$dataFilename")) {
+    return if (!name.startsWith("$rootDirectory/") || !name.endsWith("/$defaultMetadataKey")) {
       null
-    } else name.substring(rootDirectory.length + 1, name.length - dataFilename.length - 1)
+    } else name.substring(rootDirectory.length + 1, name.length - defaultMetadataKey.length - 1)
   }
 
   override fun <T : Timestamped> listObjectVersions(
@@ -280,7 +283,7 @@ class GcsStorageService(
   }
 
   private fun pathForKey(objectType: ObjectType, key: String): String {
-    return "${daoRoot(objectType)}/$key/$dataFilename"
+    return "${daoRoot(objectType)}/$key/"+ objectType.getDefaultMetadataFilename(true)
   }
 
   private fun lastModifiedBlobId(objectType: ObjectType): BlobId {

diff --git a/front50-gcs/src/test/kotlin/com/netflix/spinnaker/front50/model/TestGcsStorageService.kt b/front50-gcs/src/test/kotlin/com/netflix/spinnaker/front50/model/TestGcsStorageService.kt
@@ -64,14 +64,15 @@ import strikt.assertions.isNotNull
 import strikt.assertions.isSuccess
 import strikt.assertions.isTrue
 import strikt.assertions.startsWith
+import strikt.assertions.endsWith
 
 class GcsStorageServiceTest {
 
   companion object {
     private const val BUCKET_NAME = "myBucket"
     private const val BUCKET_LOCATION = "bucketLocation"
     private const val BASE_PATH = "my/base/path"
-    private const val DATA_FILENAME = "my-file.txt"
+    private val DATA_FILENAME = ObjectType.APPLICATION.getDefaultMetadataFilename(true);
   }
 
   private lateinit var gcs: Storage
@@ -230,6 +231,7 @@ class GcsStorageServiceTest {
 
     expectThat(blob).isNotNull()
     expectThat(blob!!.contentType).isEqualTo("application/json")
+    expectThat(blob.name).endsWith("/$DATA_FILENAME")
   }
 
   @Test
@@ -255,10 +257,12 @@ class GcsStorageServiceTest {
     storageService.storeObject(ObjectType.APPLICATION, "app1", Application())
     storageService.storeObject(ObjectType.APPLICATION, "app2", Application())
     storageService.storeObject(ObjectType.APPLICATION, "app3", Application())
-
+    storageService.storeObject(ObjectType.APPLICATION_PERMISSION,"app3",Application.Permission())
     val keys = storageService.listObjectKeys(ObjectType.APPLICATION)
+    val keysWithPermissions = storageService.listObjectKeys(ObjectType.APPLICATION_PERMISSION)
 
     expectThat(keys).containsKeys("app1", "app2", "app3")
+    expectThat(keysWithPermissions).containsKeys("app3")
   }
 
   @Test
@@ -269,6 +273,8 @@ class GcsStorageServiceTest {
     storageService.storeObject(ObjectType.APPLICATION, "app3", Application())
     storageService.storeObject(ObjectType.DELIVERY, "delivery", Delivery())
     storageService.storeObject(ObjectType.PIPELINE, "pipeline", Pipeline())
+    storageService.storeObject(ObjectType.APPLICATION_PERMISSION,"app4",Application.Permission())
+
 
     val keys = storageService.listObjectKeys(ObjectType.APPLICATION)
 
@@ -294,27 +300,42 @@ class GcsStorageServiceTest {
 
     clock.setEpochMilli(111L)
     storageService.storeObject(ObjectType.APPLICATION, "plumpstuff", Application().apply { name = "version1" })
+    storageService.storeObject(ObjectType.APPLICATION_PERMISSION, "plumpstuff", Application.Permission().apply { name = "versionPerm1" })
+
     clock.setEpochMilli(222L)
     storageService.storeObject(ObjectType.APPLICATION, "plumpstuff", Application().apply { name = "version2" })
+    storageService.storeObject(ObjectType.APPLICATION_PERMISSION, "plumpstuff", Application.Permission().apply { name = "versionPerm2" })
     clock.setEpochMilli(333L)
     storageService.storeObject(ObjectType.APPLICATION, "plumpstuff", Application().apply { name = "version3" })
+    storageService.storeObject(ObjectType.APPLICATION_PERMISSION, "plumpstuff", Application.Permission().apply { name = "versionPerm3" })
 
     val versions: List<Application> =
       storageService.listObjectVersions<Application>(ObjectType.APPLICATION, "plumpstuff", 100).toList()
-
+    val permVersions: List<Application.Permission> =
+      storageService.listObjectVersions<Application.Permission>(ObjectType.APPLICATION_PERMISSION, "plumpstuff", 100).toList()
     expectThat(versions).hasSize(3)
     expectThat(versions[0].name).isEqualToIgnoringCase("version3")
     expectThat(versions[0].updateTs).isEqualTo("333")
     expectThat(versions[1].name).isEqualToIgnoringCase("version2")
     expectThat(versions[1].updateTs).isEqualTo("222")
     expectThat(versions[2].name).isEqualToIgnoringCase("version1")
     expectThat(versions[2].updateTs).isEqualTo("111")
+
+    expectThat(permVersions).hasSize(3)
+    expectThat(permVersions[0].name).isEqualToIgnoringCase("versionPerm3")
+    expectThat(permVersions[0].lastModified).isEqualTo(333)
+    expectThat(permVersions[1].name).isEqualToIgnoringCase("versionPerm2")
+    expectThat(permVersions[1].lastModified).isEqualTo(222)
+    expectThat(permVersions[2].name).isEqualToIgnoringCase("versionPerm1")
+    expectThat(permVersions[2].lastModified).isEqualTo(111)
   }
 
   @Test
   fun `listObjectVersions ignores similar filenames`() {
 
     writeFile("$BASE_PATH/${ObjectType.APPLICATION.group}/plumpstuff/$DATA_FILENAME", """{ "name": "the good one" }""")
+    writeFile("$BASE_PATH/${ObjectType.APPLICATION.group}/plumpstuff/" +
+      ObjectType.APPLICATION_PERMISSION.getDefaultMetadataFilename(true), """{ "name": "the good one but permissions file" }""")
     writeFile("$BASE_PATH/${ObjectType.APPLICATION.group}/plumpstuff/unknownFilename.txt", """{}""")
     writeFile("$BASE_PATH${ObjectType.APPLICATION.group}/plumpstuff/$DATA_FILENAME", """{}""")
     writeFile("$BASE_PATH/${ObjectType.APPLICATION.group}plumpstuff/$DATA_FILENAME", """{}""")
@@ -333,6 +354,7 @@ class GcsStorageServiceTest {
     storageService.storeObject(ObjectType.APPLICATION, "app1", Application().apply { name = "app1v1" })
     storageService.storeObject(ObjectType.APPLICATION, "app1", Application().apply { name = "app1v2" })
     storageService.storeObject(ObjectType.APPLICATION, "app1", Application().apply { name = "app1v3" })
+    storageService.storeObject(ObjectType.APPLICATION_PERMISSION, "app1", Application.Permission().apply { name = "perm1" })
     storageService.storeObject(ObjectType.APPLICATION, "app2", Application().apply { name = "app2" })
     storageService.storeObject(ObjectType.APPLICATION, "app3", Application().apply { name = "app3" })
     storageService.storeObject(ObjectType.PIPELINE, "app1", Application().apply { name = "app1" })