CCM-17012: Enable e2e test on PR envs (#566)

* test pr proxy e2e tests

* point to internal branch workflow

* actually point to internal branch workflow for build proxies and run e2e on pr too

* try to fix tests

* Fix tests

* Fix pact tests

* create-letter-batch logs letterIds, e2e read and wait for queue

* env template cleanup

* env template cleanup 2

* smoking out failed tests

* add markers

* npm install

* All tests run

* ci(tests): pass targetAccountGroup through acceptance composite actions

* infra: set shorter letter queue visibility timeout for dev group

* component tests: centralize GET /letters retry handling and update specs

* e2e: map account IDs by target group and block production test execution

* update env template

* env specific lambda runtime env vars

* bump

* revert lockfile

* test with internal branch

* remove internal ref

* address copilot comments

---------

Co-authored-by: Mark Slowey <113013138+masl2@users.noreply.github.com>

Author: francisco-videira-nhs

.env.template

@@ -1,15 +1,26 @@
-PR_NUMBER=prxx # remove if needs to run against main
-GITHUB_TOKEN= # Your github Personal Access Token (PAT)
+# Your github Personal Access Token (PAT)
+GITHUB_TOKEN=
 
+# Apigee proxy name to be used for test execution
+# nhs-notify-supplier--internal-dev--nhs-notify-supplier-PR-XX
+PROXY_NAME=
 
-# The variables below are used for End to End tests
-PROXY_NAME= # information about the proxy name can be found in the tests/e2e-tests/README.md
+# APIM env to run e2e tests against, other options are: ref, int, prod
+API_ENVIRONMENT=internal-dev
 
+# Used for component and e2e tests
+# Account group controls default account id mapping for tests.
+# If omitted, tests default to dev:
+# TARGET_ACCOUNT_GROUP=nhs-notify-supplier-api-dev
+# Mapping used by tests:
+# nhs-notify-supplier-api-dev -> 820178564574
+# nhs-notify-supplier-api-nonprod -> 885964308133
+# nhs-notify-supplier-api-prod -> blocked (tests are intentionally disabled for prod)
+TARGET_ACCOUNT_GROUP=nhs-notify-supplier-api-dev
 
-
-# * nhs-notify-supplier--internal-dev--nhs-notify-supplier
-# * nhs-notify-supplier--internal-dev--nhs-notify-supplier-PR-XX
-# * nhs-notify-supplier--ref--nhs-notify-supplier -- ref env
+# Resource namespace used to resolve AWS resource names for tests (main, pr123)
+# remove if needs to run against main
+TARGET_ENVIRONMENT=prxx
 
 # API Keys
 # ========
@@ -25,9 +36,9 @@ export STATUS_ENDPOINT_API_KEY=xxx
 
 # Private Keys
 # ============
-# private key used to generate authentication for tests ran against the internal-dev and internal-qa
-export NON_PROD_PRIVATE_KEY=xxx # path to the private key file
-# private key used to generate authentication for tests ran against the int environment
+# private key path used to generate authentication for tests ran against the internal-dev and internal-qa
+export NON_PROD_PRIVATE_KEY=xxx
+# private key path used to generate authentication for tests ran against the int environment
 export INTEGRATION_PRIVATE_KEY=xxx
-# private key used to generate authentication for tests ran against the prod environment
+# private key path used to generate authentication for tests ran against the prod environment
 export PRODUCTION_PRIVATE_KEY=xxx

…ns/acceptance-tests-component/action.yml …s/acceptance-tests-components/action.yml.github/actions/acceptance-tests-component/action.yml renamed to .github/actions/acceptance-tests-components/action.yml .github/actions/acceptance-tests-component/action.yml renamed to .github/actions/acceptance-tests-components/action.yml

@@ -14,6 +14,10 @@ inputs:
     description: Name of the component under test
     required: true
 
+  targetAccountGroup:
+    description: Name of the account group under test (e.g. nhs-notify-supplier-api-dev)
+    required: true
+
 runs:
   using: "composite"
 
@@ -39,5 +43,6 @@ runs:
       shell: bash
       env:
         TARGET_ENVIRONMENT: ${{ inputs.targetEnvironment }}
+        TARGET_ACCOUNT_GROUP: ${{ inputs.targetAccountGroup }}
       run: |
         make test-${{ inputs.testType }}

.github/actions/acceptance-tests-e2e/action.yml

@@ -5,6 +5,9 @@ inputs:
   targetEnvironment:
     description: Name of the environment under test
     required: true
+  targetAccountGroup:
+    description: Name of the account group under test
+    required: true
 
 runs:
   using: "composite"
@@ -65,6 +68,7 @@ runs:
       env:
         TARGET_ENVIRONMENT: ${{ inputs.targetEnvironment }}
         PR_NUMBER: ${{ steps.set_pr_number.outputs.pr_number }}
+        TARGET_ACCOUNT_GROUP: ${{ inputs.targetAccountGroup }}
       run: |
         echo "$DEV_E2E_KEYS_PRIVATE" > "${GITHUB_WORKSPACE}/internal-dev-test-1.pem"
         chmod 600 "${GITHUB_WORKSPACE}/internal-dev-test-1.pem"

.github/actions/acceptance-tests/action.yml

@@ -11,8 +11,7 @@ inputs:
     required: true
 
   targetAccountGroup:
-    description: Name of the account group under test
-    default: nhs-notify-template-management-dev
+    description: Name of the account group under test (e.g. nhs-notify-supplier-api-dev)
     required: true
 
   targetComponent:
@@ -24,16 +23,18 @@ runs:
 
   steps:
 
-    - name: Run component tests
+    - name: Run components tests (sandbox and component tests)
       if: ${{ inputs.testType != 'e2e' }}
-      uses: ./.github/actions/acceptance-tests-component
+      uses: ./.github/actions/acceptance-tests-components
       with:
         testType: ${{ inputs.testType }}
         targetEnvironment: ${{ inputs.targetEnvironment }}
         targetComponent: ${{ inputs.targetComponent }}
+        targetAccountGroup: ${{ inputs.targetAccountGroup }}
 
     - name: Run e2e tests
-      if: ${{ inputs.testType == 'e2e' && inputs.targetEnvironment == 'main' }}
+      if: ${{ inputs.testType == 'e2e' }}
       uses: ./.github/actions/acceptance-tests-e2e
       with:
         targetEnvironment: ${{ inputs.targetEnvironment }}
+        targetAccountGroup: ${{ inputs.targetAccountGroup }}

.github/workflows/deploy-dynamic-env-proxy.yaml

@@ -29,7 +29,8 @@ jobs:
 
       - name: Resolve nodejs version
         id: toolversions
-        run: echo "nodejs_version=$(grep '^nodejs\s' .tool-versions | cut -f2 -d' ')" >>
+        run:
+          echo "nodejs_version=$(grep '^nodejs\s' .tool-versions | cut -f2 -d' ')" >>
           "$GITHUB_OUTPUT"
 
       - name: "Check if pull request exists for this branch and set

.github/workflows/stage-3-build.yaml

@@ -189,6 +189,7 @@ jobs:
             --terraformAction "apply" \
             --overrideProjectName "nhs" \
             --overrideRoleName "nhs-main-acct-supplier-api-github-deploy"
+
   populate-config:
     name: "Populate Supplier Config"
     runs-on: ubuntu-latest

Makefile

@@ -2,6 +2,7 @@
 # the project as automated steps to be executed on locally and in the CD pipeline.
 
 include scripts/init.mk
+-include .env # Load environment variables from .env file if it exists
 
 # ==============================================================================
 
@@ -130,11 +131,14 @@ ${VERBOSE}.SILENT: \
 # E2E Test commands #
 #####################
 
+# https://pytest-xdist.readthedocs.io/en/stable/known-limitations.html#output-stdout-and-stderr-from-workers means pytest won't print to stdout even with -s
+PYTEST_WORKERS := 4 # set to 0 to see stdout/stderr when debugging e2e tests
+
 TEST_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \
 	STATUS_ENDPOINT_API_KEY="$(STATUS_ENDPOINT_API_KEY)" \
 	PYTHONPATH=. poetry run pytest --disable-warnings -vv \
 	--color=yes \
-	-n 4 \
+		-n $(PYTEST_WORKERS) \
 	--api-name=nhs-notify-supplier \
 	--proxy-name="$(PROXY_NAME)" \
 	-s \
@@ -145,7 +149,6 @@ TEST_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \
 	--only-rerun 'AssertionError: Unexpected 502' \
 	--junitxml=test-report.xml
 
-
 .internal-dev-test:
 	@cd tests/e2e-tests && \
 	$(TEST_CMD) \
@@ -161,7 +164,7 @@ TEST_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \
 PROD_CMD := APIGEE_ACCESS_TOKEN="$(APIGEE_ACCESS_TOKEN)" \
 	PYTHONPATH=. poetry run pytest --disable-warnings -vv \
 	--color=yes \
-	-n 4 \
+	-n $(PYTEST_WORKERS) \
 	--api-name=nhs-notify-supplier \
 	--proxy-name="$(PROXY_NAME)" \
 	-s \

infrastructure/terraform/components/api/README.md

@@ -19,6 +19,7 @@ No requirements.
 | <a name="input_csoc_log_forwarding"></a> [csoc\_log\_forwarding](#input\_csoc\_log\_forwarding) | Enable forwarding of API Gateway logs to CSOC | `bool` | `true` | no |
 | <a name="input_default_tags"></a> [default\_tags](#input\_default\_tags) | A map of default tags to apply to all taggable resources within the component | `map(string)` | `{}` | no |
 | <a name="input_disable_gateway_execute_endpoint"></a> [disable\_gateway\_execute\_endpoint](#input\_disable\_gateway\_execute\_endpoint) | Disable the execution endpoint for the API Gateway | `bool` | `true` | no |
+| <a name="input_download_url_ttl_seconds"></a> [download\_url\_ttl\_seconds](#input\_download\_url\_ttl\_seconds) | TTL in seconds for generated download URLs | `number` | `60` | no |
 | <a name="input_enable_alarms"></a> [enable\_alarms](#input\_enable\_alarms) | Enable CloudWatch alarms for this deployed environment | `bool` | `true` | no |
 | <a name="input_enable_api_data_trace"></a> [enable\_api\_data\_trace](#input\_enable\_api\_data\_trace) | Enable API Gateway data trace logging | `bool` | `false` | no |
 | <a name="input_enable_backups"></a> [enable\_backups](#input\_enable\_backups) | Enable backups | `bool` | `false` | no |
@@ -33,14 +34,18 @@ No requirements.
 | <a name="input_eventpub_data_plane_bus_arn"></a> [eventpub\_data\_plane\_bus\_arn](#input\_eventpub\_data\_plane\_bus\_arn) | ARN of the EventBridge data plane bus for eventpub | `string` | `""` | no |
 | <a name="input_force_destroy"></a> [force\_destroy](#input\_force\_destroy) | Flag to force deletion of S3 buckets | `bool` | `false` | no |
 | <a name="input_force_lambda_code_deploy"></a> [force\_lambda\_code\_deploy](#input\_force\_lambda\_code\_deploy) | If the lambda package in s3 has the same commit id tag as the terraform build branch, the lambda will not update automatically. Set to True if making changes to Lambda code from on the same commit for example during development | `bool` | `false` | no |
-| <a name="input_group"></a> [group](#input\_group) | The group variables are being inherited from (often synonmous with account short-name) | `string` | n/a | yes |
+| <a name="input_group"></a> [group](#input\_group) | The account group short-name | `string` | n/a | yes |
 | <a name="input_kms_deletion_window"></a> [kms\_deletion\_window](#input\_kms\_deletion\_window) | When a kms key is deleted, how long should it wait in the pending deletion state? | `string` | `"30"` | no |
 | <a name="input_letter_event_source"></a> [letter\_event\_source](#input\_letter\_event\_source) | Source value to use for the letter status event updates | `string` | `"/data-plane/supplier-api/nhs-supplier-api-prod/main/update-status"` | no |
+| <a name="input_letter_queue_ttl_hours"></a> [letter\_queue\_ttl\_hours](#input\_letter\_queue\_ttl\_hours) | TTL in hours for letter queue records | `number` | `168` | no |
+| <a name="input_letter_queue_visibility_timeout"></a> [letter\_queue\_visibility\_timeout](#input\_letter\_queue\_visibility\_timeout) | Visibility timeout in seconds for processing queued letter updates | `number` | `300` | no |
 | <a name="input_letter_table_ttl_hours"></a> [letter\_table\_ttl\_hours](#input\_letter\_table\_ttl\_hours) | Number of hours to set as TTL on letters table | `number` | `24` | no |
+| <a name="input_letter_ttl_hours"></a> [letter\_ttl\_hours](#input\_letter\_ttl\_hours) | TTL in hours for letter records | `number` | `12960` | no |
 | <a name="input_log_level"></a> [log\_level](#input\_log\_level) | The log level to be used in lambda functions within the component. Any log with a lower severity than the configured value will not be logged: https://docs.python.org/3/library/logging.html#levels | `string` | `"INFO"` | no |
 | <a name="input_log_retention_in_days"></a> [log\_retention\_in\_days](#input\_log\_retention\_in\_days) | The retention period in days for the Cloudwatch Logs events to be retained, default of 0 is indefinite | `number` | `0` | no |
 | <a name="input_manually_configure_mtls_truststore"></a> [manually\_configure\_mtls\_truststore](#input\_manually\_configure\_mtls\_truststore) | Manually manage the truststore used for API Gateway mTLS (e.g. for prod environment) | `bool` | `false` | no |
 | <a name="input_max_get_limit"></a> [max\_get\_limit](#input\_max\_get\_limit) | Default limit to apply to GET requests that support pagination | `number` | `2500` | no |
+| <a name="input_mi_ttl_hours"></a> [mi\_ttl\_hours](#input\_mi\_ttl\_hours) | TTL in hours for MI records | `number` | `2160` | no |
 | <a name="input_parent_acct_environment"></a> [parent\_acct\_environment](#input\_parent\_acct\_environment) | Name of the environment responsible for the acct resources used, affects things like DNS zone. Useful for named dev environments | `string` | `"main"` | no |
 | <a name="input_project"></a> [project](#input\_project) | The name of the tfscaffold project | `string` | n/a | yes |
 | <a name="input_region"></a> [region](#input\_region) | The AWS Region | `string` | n/a | yes |

infrastructure/terraform/components/api/locals.tf

@@ -23,15 +23,15 @@ locals {
 
   common_lambda_env_vars = {
     APIM_CORRELATION_HEADER         = "nhsd-correlation-id",
-    DOWNLOAD_URL_TTL_SECONDS        = 60
-    EVENT_SOURCE                    = "/data-plane/supplier-api/${var.group}/${var.environment}/letters"
-    LETTER_TTL_HOURS                = 12960, # 18 months * 30 days * 24 hours
+    DOWNLOAD_URL_TTL_SECONDS        = var.download_url_ttl_seconds,
+    EVENT_SOURCE                    = "/data-plane/supplier-api/${var.group}/${var.environment}/letters",
+    LETTER_TTL_HOURS                = var.letter_ttl_hours,
     LETTER_QUEUE_TABLE_NAME         = aws_dynamodb_table.letter_queue.name,
-    LETTER_QUEUE_TTL_HOURS          = 168  # 7 days * 24 hours
-    LETTER_QUEUE_VISIBILITY_TIMEOUT = 300, # 5 minutes * 60 seconds
+    LETTER_QUEUE_TTL_HOURS          = var.letter_queue_ttl_hours,
+    LETTER_QUEUE_VISIBILITY_TIMEOUT = var.letter_queue_visibility_timeout,
     LETTERS_TABLE_NAME              = aws_dynamodb_table.letters.name,
     MI_TABLE_NAME                   = aws_dynamodb_table.mi.name,
-    MI_TTL_HOURS                    = 2160 # 90 days * 24 hours
+    MI_TTL_HOURS                    = var.mi_ttl_hours,
     SNS_TOPIC_ARN                   = "${module.eventsub.sns_topic.arn}",
     SUPPLIER_CONFIG_TABLE_NAME      = aws_dynamodb_table.supplier-configuration.name,
     SUPPLIER_QUOTAS_TABLE_NAME      = aws_dynamodb_table.supplier-quotas.name,

infrastructure/terraform/components/api/variables.tf

@@ -24,7 +24,7 @@ variable "region" {
 
 variable "group" {
   type        = string
-  description = "The group variables are being inherited from (often synonmous with account short-name)"
+  description = "The account group short-name"
 }
 
 ##
@@ -111,6 +111,36 @@ variable "max_get_limit" {
   default     = 2500
 }
 
+variable "download_url_ttl_seconds" {
+  type        = number
+  description = "TTL in seconds for generated download URLs"
+  default     = 60
+}
+
+variable "letter_ttl_hours" {
+  type        = number
+  description = "TTL in hours for letter records"
+  default     = 12960
+}
+
+variable "letter_queue_ttl_hours" {
+  type        = number
+  description = "TTL in hours for letter queue records"
+  default     = 168
+}
+
+variable "letter_queue_visibility_timeout" {
+  type        = number
+  description = "Visibility timeout in seconds for processing queued letter updates"
+  default     = 300
+}
+
+variable "mi_ttl_hours" {
+  type        = number
+  description = "TTL in hours for MI records"
+  default     = 2160
+}
+
 variable "parent_acct_environment" {
   type        = string
   description = "Name of the environment responsible for the acct resources used, affects things like DNS zone. Useful for named dev environments"