From e3e8c276c6aae6c62dc58dd4c4497ad6ea601af7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 11 Jun 2024 18:03:22 +0000 Subject: [PATCH] fix(migrator): GCS to SQL migrator APPLICATION_PERMISSION objectType fix (#1466) (#1471) * fix(migrator): GCS to SQL migrator APPLICATION_PERMISSION fix/refactor * fix(migrator): GCS to SQL migrator APPLICATION_PERMISSION fix/refactor (cherry picked from commit 3ab3bfcb4648e437ec26a3453de5a18496989d8a) Co-authored-by: Christos Arvanitis --- .../spinnaker/front50/model/ObjectType.java | 27 +++++++++++++++--- .../spinnaker/front50/config/GcsConfig.java | 7 +++-- .../front50/model/GcsStorageService.kt | 17 ++++++----- .../front50/model/TestGcsStorageService.kt | 28 +++++++++++++++++-- 4 files changed, 63 insertions(+), 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 index bc234b067..0d9aed2b8 100644 --- 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 clazz; public final String group; public final String defaultMetadataFilename; + public final String gcsMetadataFilename; ObjectType(Class clazz, String group, String defaultMetadataFilename) { + this(clazz, group, defaultMetadataFilename, null); + } + + ObjectType( + Class 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 index 0d1d14533..74d4dd1e9 100644 --- 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 index 16f795a5b..adcdff6ac 100644 --- 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 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 index e48647257..dba4aba30 100644 --- 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,6 +64,7 @@ import strikt.assertions.isNotNull import strikt.assertions.isSuccess import strikt.assertions.isTrue import strikt.assertions.startsWith +import strikt.assertions.endsWith class GcsStorageServiceTest { @@ -71,7 +72,7 @@ class GcsStorageServiceTest { 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,14 +300,19 @@ 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 = storageService.listObjectVersions(ObjectType.APPLICATION, "plumpstuff", 100).toList() - + val permVersions: List = + storageService.listObjectVersions(ObjectType.APPLICATION_PERMISSION, "plumpstuff", 100).toList() expectThat(versions).hasSize(3) expectThat(versions[0].name).isEqualToIgnoringCase("version3") expectThat(versions[0].updateTs).isEqualTo("333") @@ -309,12 +320,22 @@ class GcsStorageServiceTest { 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" })