Fix after dev review

Author: jreynard-code

api/kubernetes/deploy_via_helm-dev.sh

@@ -458,12 +458,6 @@ image:
   tag: "$API_IMAGE_TAG"
 
 config:
-  workflow:
-    containers:
-      - name : "twingraphImportContainer"
-        imageRegistry: "ghcr.io"
-        imageName: "cosmo-tech/adt-twincache-connector"
-        imageVersion: "0.0.4"
   csm:
     platform:
       argo:

api/src/main/resources/application.yml

@@ -88,6 +88,11 @@ csm:
       scenario-fetch-parameters: cosmo-tech/fetch-scenario-parameters
       send-datawarehouse: cosmo-tech/azure-data-explorer-connector
       scenario-data-upload: cosmo-tech/azure-storage-publish:latest
+    containers:
+      - name: "twingraphImport"
+        imageRegistry: "ghcr.io"
+        imageName: "cosmo-tech/adt-twincache-connector"
+        imageVersion: "0.0.4"
     authorization:
       principal-jwt-claim: "sub"
       tenant-id-jwt-claim: "iss"

scenariorun/src/main/kotlin/com/cosmotech/scenariorun/ContainerFactory.kt

@@ -274,12 +274,11 @@ internal class ContainerFactory(
       nodeLabel: String = NODE_LABEL_DEFAULT
   ): ScenarioRunStartContainers {
 
-    val defaultSizing =
-        if (nodeLabel != null) {
-          (LABEL_SIZING[nodeLabel] ?: BASIC_SIZING)
-        } else {
-          (BASIC_SIZING)
-        }
+    var defaultSizing = BASIC_SIZING
+
+    if (nodeLabel != null) {
+      defaultSizing = LABEL_SIZING[nodeLabel] ?: BASIC_SIZING
+    }
 
     val container =
         buildSimpleContainer(
@@ -324,70 +323,6 @@ internal class ContainerFactory(
         runSizing = nodeSizing.toContainerResourceSizing())
   }
 
-  internal fun getMinimalCommonEnvVars(
-      csmPlatformProperties: CsmPlatformProperties,
-      azureManagedIdentity: Boolean? = null,
-      azureAuthenticationWithCustomerAppRegistration: Boolean? = null,
-  ): MutableMap<String, String> {
-    if (azureManagedIdentity == true && azureAuthenticationWithCustomerAppRegistration == true) {
-      throw IllegalArgumentException(
-          "Don't know which authentication mechanism to use to connect " +
-              "against Azure services. Both azureManagedIdentity and " +
-              "azureAuthenticationWithCustomerAppRegistration cannot be set to true")
-    }
-    val identityEnvVars =
-        if (azureManagedIdentity == true) {
-          mapOf(CSM_AZURE_MANAGED_IDENTITY_VAR to "true")
-        } else if (azureAuthenticationWithCustomerAppRegistration == true) {
-          mapOf(
-              AZURE_TENANT_ID_VAR to
-                  (csmPlatformProperties.azure?.credentials?.customer?.tenantId!!),
-              AZURE_CLIENT_ID_VAR to
-                  (csmPlatformProperties.azure?.credentials?.customer?.clientId!!),
-              AZURE_CLIENT_SECRET_VAR to
-                  (csmPlatformProperties.azure?.credentials?.customer?.clientSecret!!),
-          )
-        } else {
-          mapOf(
-              AZURE_TENANT_ID_VAR to (csmPlatformProperties.azure?.credentials?.core?.tenantId!!),
-              AZURE_CLIENT_ID_VAR to (csmPlatformProperties.azure?.credentials?.core?.clientId!!),
-              AZURE_CLIENT_SECRET_VAR to
-                  (csmPlatformProperties.azure?.credentials?.core?.clientSecret!!),
-          )
-        }
-    val oktaEnvVars: MutableMap<String, String> = mutableMapOf()
-    if (csmPlatformProperties.identityProvider?.code == "okta") {
-      oktaEnvVars.putAll(
-          mapOf(
-              OKTA_CLIENT_ID to (csmPlatformProperties.okta?.clientId!!),
-              OKTA_CLIENT_SECRET to (csmPlatformProperties.okta?.clientSecret!!),
-              OKTA_CLIENT_ISSUER to (csmPlatformProperties.okta?.issuer!!),
-          ))
-    }
-
-    val twinCacheEnvVars: MutableMap<String, String> = mutableMapOf()
-    if (csmPlatformProperties.twincache != null) {
-      val twinCacheInfo = csmPlatformProperties.twincache!!
-      twinCacheEnvVars.putAll(
-          mapOf(
-              TWIN_CACHE_HOST to (twinCacheInfo.host),
-              TWIN_CACHE_PORT to (twinCacheInfo.port),
-              TWIN_CACHE_PASSWORD to (twinCacheInfo.password),
-              TWIN_CACHE_USERNAME to (twinCacheInfo.username),
-          ))
-    }
-    val containerScopes = getContainerScopes(csmPlatformProperties)
-    val commonEnvVars =
-        mapOf(
-            IDENTITY_PROVIDER to (csmPlatformProperties.identityProvider?.code ?: "azure"),
-            API_BASE_URL_VAR to csmPlatformProperties.api.baseUrl,
-            API_BASE_SCOPE_VAR to containerScopes,
-            DATASET_PATH_VAR to DATASET_PATH,
-            PARAMETERS_PATH_VAR to PARAMETERS_PATH,
-        )
-    return (identityEnvVars + commonEnvVars + oktaEnvVars + twinCacheEnvVars).toMutableMap()
-  }
-
   internal fun buildContainersStart(
       scenario: Scenario,
       datasets: List<Dataset>?,
@@ -416,12 +351,11 @@ internal class ContainerFactory(
 
     val runTemplateSizing = template.runSizing?.toSizing()
 
-    val defaultSizing =
-        if (nodeLabel != null) {
-          (LABEL_SIZING[nodeLabel] ?: BASIC_SIZING)
-        } else {
-          (BASIC_SIZING)
-        }
+    var defaultSizing = BASIC_SIZING
+
+    if (nodeLabel != null) {
+      defaultSizing = LABEL_SIZING[nodeLabel] ?: BASIC_SIZING
+    }
 
     val containers =
         buildContainersPipeline(
@@ -1534,14 +1468,8 @@ internal fun getContainerScopes(csmPlatformProperties: CsmPlatformProperties): S
   return "${csmPlatformProperties.azure?.appIdUri}${API_SCOPE_SUFFIX}"
 }
 
-@Suppress("LongMethod")
-internal fun getCommonEnvVars(
+internal fun getMinimalCommonEnvVars(
     csmPlatformProperties: CsmPlatformProperties,
-    csmSimulationId: String,
-    organizationId: String,
-    workspaceId: String,
-    scenarioId: String,
-    workspaceKey: String,
     azureManagedIdentity: Boolean? = null,
     azureAuthenticationWithCustomerAppRegistration: Boolean? = null,
 ): MutableMap<String, String> {
@@ -1590,16 +1518,38 @@ internal fun getCommonEnvVars(
             TWIN_CACHE_USERNAME to (twinCacheInfo.username),
         ))
   }
-
   val containerScopes = getContainerScopes(csmPlatformProperties)
   val commonEnvVars =
       mapOf(
           IDENTITY_PROVIDER to (csmPlatformProperties.identityProvider?.code ?: "azure"),
-          CSM_SIMULATION_ID to csmSimulationId,
           API_BASE_URL_VAR to csmPlatformProperties.api.baseUrl,
           API_BASE_SCOPE_VAR to containerScopes,
           DATASET_PATH_VAR to DATASET_PATH,
           PARAMETERS_PATH_VAR to PARAMETERS_PATH,
+      )
+  return (identityEnvVars + commonEnvVars + oktaEnvVars + twinCacheEnvVars).toMutableMap()
+}
+
+internal fun getCommonEnvVars(
+    csmPlatformProperties: CsmPlatformProperties,
+    csmSimulationId: String,
+    organizationId: String,
+    workspaceId: String,
+    scenarioId: String,
+    workspaceKey: String,
+    azureManagedIdentity: Boolean? = null,
+    azureAuthenticationWithCustomerAppRegistration: Boolean? = null,
+): MutableMap<String, String> {
+
+  val minimalEnvVars =
+      getMinimalCommonEnvVars(
+          csmPlatformProperties,
+          azureManagedIdentity,
+          azureAuthenticationWithCustomerAppRegistration)
+
+  val commonEnvVars =
+      mapOf(
+          CSM_SIMULATION_ID to csmSimulationId,
           AZURE_DATA_EXPLORER_RESOURCE_URI_VAR to
               (csmPlatformProperties.azure?.dataWarehouseCluster?.baseUri ?: ""),
           AZURE_DATA_EXPLORER_RESOURCE_INGEST_URI_VAR to
@@ -1609,7 +1559,7 @@ internal fun getCommonEnvVars(
           PARAMETERS_WORKSPACE_VAR to workspaceId,
           PARAMETERS_SCENARIO_VAR to scenarioId,
       )
-  return (identityEnvVars + commonEnvVars + oktaEnvVars + twinCacheEnvVars).toMutableMap()
+  return (minimalEnvVars + commonEnvVars).toMutableMap()
 }
 
 private fun stackSolutionContainers(

scenariorun/src/main/kotlin/com/cosmotech/scenariorun/azure/ScenarioRunServiceImpl.kt

@@ -35,7 +35,6 @@ import com.cosmotech.scenariorun.CSM_JOB_ID_LABEL_KEY
 import com.cosmotech.scenariorun.ContainerFactory
 import com.cosmotech.scenariorun.SCENARIO_DATA_DOWNLOAD_ARTIFACT_NAME
 import com.cosmotech.scenariorun.api.ScenariorunApiService
-import com.cosmotech.scenariorun.config.ContainerConfig
 import com.cosmotech.scenariorun.domain.RunTemplateParameterValue
 import com.cosmotech.scenariorun.domain.ScenarioRun
 import com.cosmotech.scenariorun.domain.ScenarioRunLogs
@@ -60,7 +59,6 @@ import kotlinx.coroutines.joinAll
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
-import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty
 import org.springframework.context.event.EventListener
 import org.springframework.scheduling.annotation.Async
@@ -82,8 +80,6 @@ internal class ScenarioRunServiceImpl(
     private val azureEventHubsClient: AzureEventHubsClient
 ) : CsmAzureService(), ScenariorunApiService {
 
-  @Autowired private lateinit var containerConfig: ContainerConfig
-
   private fun ScenarioRun.asMapWithAdditionalData(workspaceId: String? = null): Map<String, Any> {
     val scenarioAsMap = this.convertToMap().toMutableMap()
     scenarioAsMap["type"] = "ScenarioRun"
@@ -337,11 +333,11 @@ internal class ScenarioRunServiceImpl(
   @EventListener(TwingraphImportEvent::class)
   fun onTwingraphImportEvent(twingraphImportEvent: TwingraphImportEvent) {
     val twingraphImportContainerList =
-        containerConfig.containers.filter { it.name == "twingraphImportContainer" }
+        csmPlatformProperties.containers.filter { it.name == "twingraphImport" }
 
     if (twingraphImportContainerList.isEmpty()) {
       throw MissingResourceException(
-          "twingraphImportContainer is not found in configuration (workflow.containers.name)",
+          "twingraphImport is not found in configuration (workflow.containers.name)",
           ScenarioRunServiceImpl::class.simpleName,
           "workflow.containers.name")
     }

scenariorun/src/main/kotlin/com/cosmotech/scenariorun/config/ContainerConfig.kt

@@ -5,7 +5,7 @@ package com.cosmotech.twingraph.api
 import com.cosmotech.api.CsmPhoenixService
 import com.cosmotech.api.events.TwingraphImportEvent
 import com.cosmotech.api.rbac.CsmRbac
-import com.cosmotech.api.rbac.PERMISSION_WRITE
+import com.cosmotech.api.rbac.PERMISSION_READ
 import com.cosmotech.organization.api.OrganizationApiService
 import com.cosmotech.organization.azure.getRbac
 import com.cosmotech.api.exceptions.CsmResourceNotFoundException
@@ -38,7 +38,7 @@ class TwingraphServiceImpl(
       twinGraphImport: TwinGraphImport
   ): TwinGraphImportInfo {
     val organization = organizationService.findOrganizationById(organizationId)
-    csmRbac.verify(organization.getRbac(), PERMISSION_WRITE)
+    csmRbac.verify(organization.getRbac(), PERMISSION_READ)
     val requestJobId = this.idGenerator.generate(scope = "graphdataimport", prependPrefix = "gdi-")
     val graphImportEvent =
         TwingraphImportEvent(