From 49634c4d6a0a5118b2ae0e9960621e93a6aee6fb Mon Sep 17 00:00:00 2001
From: Tamir Kamara <26870601+tamirkamara@users.noreply.github.com>
Date: Mon, 23 Jan 2023 08:35:07 +0000
Subject: [PATCH 1/4] Fix certs shared service and add back e2e test

---
 e2e_tests/test_shared_services.py             |  2 +-
 templates/shared_services/certs/porter.yaml   | 40 ++++++++++++++-----
 .../certs/scripts/letsencrypt.sh              | 13 ------
 .../certs/terraform/appgateway.tf             | 30 --------------
 .../shared_services/certs/terraform/main.tf   |  4 --
 .../certs/terraform/variables.tf              |  8 ----
 6 files changed, 30 insertions(+), 67 deletions(-)

diff --git a/e2e_tests/test_shared_services.py b/e2e_tests/test_shared_services.py
index 372066d338..4809145ef3 100644
--- a/e2e_tests/test_shared_services.py
+++ b/e2e_tests/test_shared_services.py
@@ -137,7 +137,7 @@ async def test_create_shared_service(template_name, verify) -> None:
     )
 
 
-# @pytest.mark.shared_services
+@pytest.mark.shared_services
 @pytest.mark.timeout(45 * 60)
 async def test_create_certs_nexus_shared_service(verify) -> None:
     await disable_and_delete_shared_service_if_exists(strings.NEXUS_SHARED_SERVICE, verify)
diff --git a/templates/shared_services/certs/porter.yaml b/templates/shared_services/certs/porter.yaml
index 134411a50e..eb053dfe34 100755
--- a/templates/shared_services/certs/porter.yaml
+++ b/templates/shared_services/certs/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-certs
-version: 0.3.40
+version: 0.3.42
 description: "An Azure TRE shared service to generate certificates for a specified internal domain using Letsencrypt"
 registry: azuretre
 dockerfile: Dockerfile.tmpl
@@ -56,10 +56,6 @@ install:
       description: "Deploy shared service"
       vars:
         tre_id: ${ bundle.parameters.tre_id }
-        arm_tenant_id: ${ bundle.credentials.azure_tenant_id }
-        arm_client_id: ${ bundle.credentials.azure_client_id }
-        arm_client_secret: ${ bundle.credentials.azure_client_secret }
-        arm_use_msi: ${ bundle.parameters.arm_use_msi }
         domain_prefix: ${ bundle.parameters.domain_prefix }
         cert_name: ${ bundle.parameters.cert_name }
         tre_resource_id: ${ bundle.parameters.id }
@@ -93,6 +89,16 @@ install:
         resource_group_name: ${ bundle.outputs.resource_group_name }
         keyvault_name: ${ bundle.outputs.keyvault_name }
         cert_name: ${ bundle.parameters.cert_name }
+  - az:
+      description: "Stop application gateway"
+      arguments:
+        - network
+        - application-gateway
+        - stop
+      flags:
+        resource-group: ${ bundle.outputs.resource_group_name }
+        name: ${ bundle.outputs.application_gateway_name }
+
 
 upgrade:
   - exec:
@@ -106,10 +112,6 @@ uninstall:
       description: "Tear down shared service"
       vars:
         tre_id: ${ bundle.parameters.tre_id }
-        arm_tenant_id: ${ bundle.credentials.azure_tenant_id }
-        arm_client_id: ${ bundle.credentials.azure_client_id }
-        arm_client_secret: ${ bundle.credentials.azure_client_secret }
-        arm_use_msi: ${ bundle.parameters.arm_use_msi }
         domain_prefix: ${ bundle.parameters.domain_prefix }
         cert_name: ${ bundle.parameters.cert_name }
         tre_resource_id: ${ bundle.parameters.id }
@@ -124,8 +126,6 @@ renew:
       arguments:
         - "output"
       description: "Get Terraform output variables"
-      vars:
-        tre_resource_id: ${ bundle.parameters.id }
       backendConfig:
         resource_group_name: ${ bundle.parameters.tfstate_resource_group_name }
         storage_account_name: ${ bundle.parameters.tfstate_storage_account_name }
@@ -144,6 +144,15 @@ renew:
       flags:
         identity:
         username: ${ bundle.credentials.azure_client_id }
+  - az:
+      description: "Start application gateway"
+      arguments:
+        - network
+        - application-gateway
+        - start
+      flags:
+        resource-group: ${ bundle.outputs.resource_group_name }
+        name: ${ bundle.outputs.application_gateway_name }
   - exec:
       description: "Renew certificate"
       command: bash
@@ -156,3 +165,12 @@ renew:
         resource_group_name: ${ bundle.outputs.resource_group_name }
         keyvault_name: ${ bundle.outputs.keyvault_name }
         cert_name: ${ bundle.parameters.cert_name }
+  - az:
+      description: "Stop application gateway"
+      arguments:
+        - network
+        - application-gateway
+        - stop
+      flags:
+        resource-group: ${ bundle.outputs.resource_group_name }
+        name: ${ bundle.outputs.application_gateway_name }
diff --git a/templates/shared_services/certs/scripts/letsencrypt.sh b/templates/shared_services/certs/scripts/letsencrypt.sh
index 3ffd9db524..4339990f43 100755
--- a/templates/shared_services/certs/scripts/letsencrypt.sh
+++ b/templates/shared_services/certs/scripts/letsencrypt.sh
@@ -46,15 +46,6 @@ done
 # done with processing args and can set this
 set -o nounset
 
-# Start the Application Gateway if stopped
-echo "Checking app gateway status"
-if [[ $(az network application-gateway list --output json --query "[?resourceGroup=='rg-${TRE_ID}'&&name=='agw-certs-${TRE_ID}'&&operationalState=='Stopped'] | length(@)") != 0 ]]; then
-  echo "App gateway stopped. Starting..."
-  az network application-gateway start -g "rg-$TRE_ID" -n "agw-certs-$TRE_ID"
-else
-  echo "App gateway running"
-fi
-
 echo "Checking for index.html file in storage account"
 
 # Create the default index.html page
@@ -142,7 +133,3 @@ az network application-gateway ssl-cert update \
     --gateway-name "${application_gateway_name}" \
     --name 'cert-primary' \
     --key-vault-secret-id "${sid}"
-
-# Stop the app gateway once done to save cost
-echo "Stopping app gateway"
-az network application-gateway stop -g "rg-$TRE_ID" -n "agw-certs-$TRE_ID"
diff --git a/templates/shared_services/certs/terraform/appgateway.tf b/templates/shared_services/certs/terraform/appgateway.tf
index 3b17d5b928..730eddf08e 100644
--- a/templates/shared_services/certs/terraform/appgateway.tf
+++ b/templates/shared_services/certs/terraform/appgateway.tf
@@ -1,25 +1,3 @@
-resource "null_resource" "az_login_sp" {
-  count = var.arm_use_msi == true ? 0 : 1
-  provisioner "local-exec" {
-    command = "az login --service-principal --username ${var.arm_client_id} --password ${var.arm_client_secret} --tenant ${var.arm_tenant_id}"
-  }
-
-  triggers = {
-    timestamp = timestamp()
-  }
-}
-
-resource "null_resource" "az_login_msi" {
-  count = var.arm_use_msi == true ? 1 : 0
-  provisioner "local-exec" {
-    command = "az login --identity -u '${data.azurerm_client_config.current.client_id}'"
-  }
-
-  triggers = {
-    timestamp = timestamp()
-  }
-}
-
 resource "azurerm_public_ip" "appgwpip" {
   name                = "pip-cert-${var.domain_prefix}-${var.tre_id}"
   resource_group_name = data.azurerm_resource_group.rg.name
@@ -185,13 +163,5 @@ resource "azurerm_application_gateway" "agw" {
 
   depends_on = [
     azurerm_key_vault_access_policy.app_gw_managed_identity,
-    null_resource.az_login_sp,
-    null_resource.az_login_msi
   ]
-
-  # Stop app gateway once provisioned to save cost until the generate custom action is invoked (which will start/stop as required)
-  provisioner "local-exec" {
-    command = "az network application-gateway stop -g ${data.azurerm_resource_group.rg.name} -n agw-certs-${var.tre_id}"
-  }
-
 }
diff --git a/templates/shared_services/certs/terraform/main.tf b/templates/shared_services/certs/terraform/main.tf
index c372eb2ba3..373f9143ab 100644
--- a/templates/shared_services/certs/terraform/main.tf
+++ b/templates/shared_services/certs/terraform/main.tf
@@ -5,10 +5,6 @@ terraform {
       source  = "hashicorp/azurerm"
       version = "=3.22.0"
     }
-    null = {
-      source  = "hashicorp/null"
-      version = "=3.1.1"
-    }
   }
 
   backend "azurerm" {}
diff --git a/templates/shared_services/certs/terraform/variables.tf b/templates/shared_services/certs/terraform/variables.tf
index c95259241a..4aff0698f5 100644
--- a/templates/shared_services/certs/terraform/variables.tf
+++ b/templates/shared_services/certs/terraform/variables.tf
@@ -2,14 +2,6 @@ variable "tre_id" {
   type = string
 }
 
-variable "arm_use_msi" {
-  type = bool
-}
-
-variable "arm_tenant_id" {}
-variable "arm_client_id" {}
-variable "arm_client_secret" {}
-
 variable "domain_prefix" {
   type = string
 }

From 778d638f527344d700907532bcc6f0fb8092a0e8 Mon Sep 17 00:00:00 2001
From: Tamir Kamara <26870601+tamirkamara@users.noreply.github.com>
Date: Mon, 23 Jan 2023 12:44:00 +0000
Subject: [PATCH 2/4] x

---
 templates/shared_services/certs/porter.yaml            | 2 +-
 templates/shared_services/certs/scripts/letsencrypt.sh | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/templates/shared_services/certs/porter.yaml b/templates/shared_services/certs/porter.yaml
index eb053dfe34..633cb7de1c 100755
--- a/templates/shared_services/certs/porter.yaml
+++ b/templates/shared_services/certs/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-certs
-version: 0.3.42
+version: 0.3.43
 description: "An Azure TRE shared service to generate certificates for a specified internal domain using Letsencrypt"
 registry: azuretre
 dockerfile: Dockerfile.tmpl
diff --git a/templates/shared_services/certs/scripts/letsencrypt.sh b/templates/shared_services/certs/scripts/letsencrypt.sh
index 4339990f43..92bb4f67f3 100755
--- a/templates/shared_services/certs/scripts/letsencrypt.sh
+++ b/templates/shared_services/certs/scripts/letsencrypt.sh
@@ -114,6 +114,7 @@ openssl pkcs12 -export \
 
 # Save cert and password to KeyVault
 echo "Importing cert to KeyVault ${keyvault_name}"
+set -x
 sid=$(az keyvault certificate import \
     -o json \
     --vault-name "${keyvault_name}" \

From 4616e1e672ba06b919d22641589c0e81ad0b8428 Mon Sep 17 00:00:00 2001
From: Tamir Kamara <26870601+tamirkamara@users.noreply.github.com>
Date: Mon, 23 Jan 2023 16:05:50 +0000
Subject: [PATCH 3/4] certs

---
 e2e_tests/test_shared_services.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/e2e_tests/test_shared_services.py b/e2e_tests/test_shared_services.py
index 4809145ef3..f7bd52ad64 100644
--- a/e2e_tests/test_shared_services.py
+++ b/e2e_tests/test_shared_services.py
@@ -144,7 +144,7 @@ async def test_create_certs_nexus_shared_service(verify) -> None:
     await disable_and_delete_shared_service_if_exists(strings.CERTS_SHARED_SERVICE, verify)
 
     cert_domain = "nexus"
-    cert_name = f"${cert_domain}-ssl"
+    cert_name = "nexus-ssl"
 
     certs_post_payload = {
         "templateName": strings.CERTS_SHARED_SERVICE,

From 5ad972a059ebb535ed0c4e2ddd7abd59a8d36951 Mon Sep 17 00:00:00 2001
From: Tamir Kamara <26870601+tamirkamara@users.noreply.github.com>
Date: Mon, 23 Jan 2023 17:56:47 +0000
Subject: [PATCH 4/4] certs

---
 e2e_tests/test_shared_services.py                      | 2 +-
 templates/shared_services/certs/porter.yaml            | 2 +-
 templates/shared_services/certs/scripts/letsencrypt.sh | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/e2e_tests/test_shared_services.py b/e2e_tests/test_shared_services.py
index f7bd52ad64..0c4302aa51 100644
--- a/e2e_tests/test_shared_services.py
+++ b/e2e_tests/test_shared_services.py
@@ -138,7 +138,7 @@ async def test_create_shared_service(template_name, verify) -> None:
 
 
 @pytest.mark.shared_services
-@pytest.mark.timeout(45 * 60)
+@pytest.mark.timeout(60 * 60)
 async def test_create_certs_nexus_shared_service(verify) -> None:
     await disable_and_delete_shared_service_if_exists(strings.NEXUS_SHARED_SERVICE, verify)
     await disable_and_delete_shared_service_if_exists(strings.CERTS_SHARED_SERVICE, verify)
diff --git a/templates/shared_services/certs/porter.yaml b/templates/shared_services/certs/porter.yaml
index 633cb7de1c..836cd72fa2 100755
--- a/templates/shared_services/certs/porter.yaml
+++ b/templates/shared_services/certs/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-certs
-version: 0.3.43
+version: 0.4.0
 description: "An Azure TRE shared service to generate certificates for a specified internal domain using Letsencrypt"
 registry: azuretre
 dockerfile: Dockerfile.tmpl
diff --git a/templates/shared_services/certs/scripts/letsencrypt.sh b/templates/shared_services/certs/scripts/letsencrypt.sh
index 92bb4f67f3..4339990f43 100755
--- a/templates/shared_services/certs/scripts/letsencrypt.sh
+++ b/templates/shared_services/certs/scripts/letsencrypt.sh
@@ -114,7 +114,6 @@ openssl pkcs12 -export \
 
 # Save cert and password to KeyVault
 echo "Importing cert to KeyVault ${keyvault_name}"
-set -x
 sid=$(az keyvault certificate import \
     -o json \
     --vault-name "${keyvault_name}" \