Merge pull request #393 from NHSDigital/release/2024-11-06

Release/2024-11-06
NHSDigital · Nov 8, 2024 · ae2bad2 · ae2bad2
2 parents 4dbdf7b + d658f03
commit ae2bad2
Show file tree

Hide file tree

Showing 46 changed files with 535 additions and 269 deletions.
diff --git a/.github/workflows/on-pr-close.yml b/.github/workflows/on-pr-close.yml
@@ -0,0 +1,50 @@
+name: "Workflow: PR Close"
+
+permissions:
+  id-token: write
+  contents: read
+  actions: write
+
+env:
+  TF_CLI_ARGS: -no-color
+  CI_ROLE_NAME: ${{ secrets.CI_ROLE_NAME }}
+  BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
+
+on:
+  pull_request:
+    types: [closed]
+
+jobs:
+  parse-secrets:
+    runs-on: [self-hosted, ci]
+    steps:
+      - id: parse-secrets
+        run: |
+          echo "::add-mask::${{ secrets.CI_ROLE_NAME }}"
+
+  build-base:
+    runs-on: [self-hosted, ci]
+    needs: [parse-secrets]
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ env.BRANCH_NAME }}
+      - uses: ./.github/actions/make/
+        with:
+          command: build
+          save-to-cache: "true"
+          restore-from-cache: "false"
+
+  destroy-pr-workspaces:
+    runs-on: [self-hosted, ci]
+    needs: [parse-secrets, build-base]
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ env.BRANCH_NAME }}
+          fetch-depth: 0
+      - name: Remove PR workspaces
+        uses: ./.github/actions/make/
+        with:
+          command: destroy--redundant-workspaces BRANCH_NAME=origin/${{ env.BRANCH_NAME }} DESTROY_ALL_COMMITS_ON_BRANCH=true KILL_ALL=true TF_CLI_ARGS=${{ env.TF_CLI_ARGS }}
+          requires-aws: true
diff --git a/.github/workflows/pull-requests.yml b/.github/workflows/pull-requests.yml
@@ -218,6 +218,7 @@ jobs:
         test--unit,
         test--feature--local,
         terraform-head-build,
+        apigee--attach-product,
       ]
     runs-on: [self-hosted, ci]
     strategy:
@@ -276,7 +277,13 @@ jobs:
           requires-aws: true
 
   apigee--detach-product:
-    needs: [build-head, test--smoke, apigee--attach-product]
+    needs:
+      [
+        build-head,
+        test--smoke,
+        apigee--attach-product,
+        test--feature--integration,
+      ]
     runs-on: [self-hosted, ci]
     if: ${{ needs.apigee--attach-product.result == 'success' }}
     steps:

diff --git a/.gitignore b/.gitignore
@@ -78,6 +78,7 @@ dist
 openapi
 
 postman-collection.json
+postman-environment.json
 token_cache.json
 cpm.cdx.json
 

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,14 @@
 # Changelog
 
+## 2024-11-06
+- [PI-593] Readme updates
+- [PI-594] More smoke tests
+- [PI-575] Remove implicit OK from responses
+- [PI-558] Add search wrapper
+- [PI-591] Better postman
+- [PI-293] E2E tests with apigee
+- [PI-601] Destroy workspaces on PR close
+
 ## 2024-11-05
 - [PI-585] PR/release CI builds in dev
 

diff --git a/README.md b/README.md
@@ -10,9 +10,12 @@
    3. [AWS SSO Setup](#aws-sso-setup)
    4. [Other helpful commands](#other-helpful-commands)
 2. [Tests](#tests)
-3. [Workflow](#workflow)
-4. [Swagger](#swagger)
-5. [ETL](#etl)
+3. [pytest tests](#pytest-tests)
+4. [End-to-End feature tests](#end-to-end-feature-tests)
+5. [Generate the Feature Test Postman collection](#generate-the-feature-test-postman-collection)
+6. [Workflow](#workflow)
+7. [Swagger](#swagger)
+8. [ETL](#etl)
 
 ---
 
@@ -22,16 +25,20 @@
 
 We use `asdf` to fetch the required versions of prerequisite libraries instead of your system's default version. To get it up and running go to https://asdf-vm.com/guide/getting-started.html. You can check it installed properly by using the command `asdf --version`.
 
-If you are using `pyenv` (you can check by typing `pyenv` and seeing whether it returns a nice list of commands) then you should run:
-
-```
-pyenv install $(cat .python-version)
-```
+However, you will also need to install the `docker engine` separately
 
 Additionally you will need `wget` (doing `which wget` will return blank if not installed). Please Google "how to install wget on my operating system", if you don't already have this installed.
 
+Update any dependencies on your system as required.
+
 Otherwise `asdf` should do the work for you.
 
+### Useful tools
+
+`VScode` is useful and we have a workspace file setup to allow easy integration
+
+`Postman` &/or `Newman` Feature tests create a postman.collection which can be used for manual testing.
+
 ### Project build
 
 Do `make build` every time you would like to pick up and install new local/project dependencies and artifacts. This will always detect changes to:
@@ -220,6 +227,52 @@ The VSCode settings for "Run and Debug" are also set up to run these tests if yo
 `make test--sds--matrix` is used for testing responses match in SDS FHIR between CPM and LDAP. You must provide `SDS_PROD_APIKEY` and `SDS_DEV_APIKEY`. There are 3 optional variables `USE_CPM_PROD`, defaults to `FALSE`, `COMPARISON_ENV`, defaults to `local` and `TEST_COUNT`, defaults to `10` and is the number of requests to make.
 Add `PYTEST_FLAGS='-sv'`.
 
+### End-to-End feature tests
+
+The Feature tests use `behave` (rather than `pytest`) to execute cucumber/gherkin-style end-to-end tests of specific features, in principle
+giving full end-to-end test coverage for API operations.
+
+Executing feature tests locally will give you a good idea whether you have implemented a well-behaved feature whilst in development (i.e. no need to redeploy whilst developing).
+
+Executing feature tests in integration mode will then give you confidence that the feature is ready to deploy in production, but has a much slower development cycle as it will need a full redeploy after codebase or infrastructure changes are implemented.
+
+#### Local
+
+To execute the feature tests entirely locally (executing lambdas directly, and otherwise mocking databases and responses to a high standard) you can do:
+
+```shell
+make test--feature-local
+```
+
+If you would like to pass `behave` flags, e.g. to \[stop after the first failure\]:
+
+```shell
+make test--feature-local BEHAVE_FLAGS="--stop"
+```
+
+#### Integration
+
+To execute the feature tests across the entire stack (including Apigee and AWS) you can do
+
+```shell
+make test--feature-integration
+```
+
+### Generate the Feature Test Postman collection
+
+Our [end-to-end feature tests](#end-to-end-feature-tests) also generate working Postman collections. To generate the Postman collection quickly, without Apigee credentials, you can run the local feature tests. If you would like the Apigee
+credentials generating then you should run the integration feature tests. The generated files are:
+
+- `src/api/tests/feature_tests/postman-collection.json`
+- `src/api/tests/feature_tests/postman-environment.json`
+
+You can drag and drop `postman-collection.json` into the `Collections` tab on Postman,
+and `postman-environment.json` on to the `Environments` tab (remember to activate it). If you generated these
+with the local feature tests, then you will need to manually update the Apigee `baseUrl` and `apiKey` fields
+in the environment (but these are filled out already if generated with the integration feature tests).
+
+💡 **The feature tests are only guaranteed to work out-of-the-box with an empty database**
+
 ## Workflow
 
 In order to create new branches, use the commands listed below. Note that the commands will throw an error if

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-2024.11.05
+2024.11.06
diff --git a/changelog/2024-11-06.md b/changelog/2024-11-06.md
@@ -0,0 +1,7 @@
+- [PI-593] Readme updates
+- [PI-594] More smoke tests
+- [PI-575] Remove implicit OK from responses
+- [PI-558] Add search wrapper
+- [PI-591] Better postman
+- [PI-293] E2E tests with apigee
+- [PI-601] Destroy workspaces on PR close
diff --git a/pyproject.toml b/pyproject.toml
@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "connecting-party-manager"
-version = "2024.11.05"
+version = "2024.11.06"
 description = "Repository for the Connecting Party Manager API and related services"
 authors = ["NHS England"]
 license = "LICENSE.md"

diff --git a/scripts/builder/build.mk b/scripts/builder/build.mk
@@ -6,7 +6,7 @@ POSTMAN_COLLECTION = $(CURDIR)/src/api/tests/feature_tests/postman-collection.js
 TOOL_VERSIONS_COPY = $(TIMESTAMP_DIR)/tool-versions.copy
 POETRY_LOCK = $(CURDIR)/poetry.lock
 INIT_TIMESTAMP = $(CURDIR)/.timestamp/init.timestamp
-SRC_FILES = $(shell find src -type f -name "*.py" -not -path "*/test_*" -not -path "*/fhir/r4/strict_models.py" -not -path "*/fhir/r4/models.py")
+SRC_FILES = $(shell find src -type f -name "*.py" -not -path "*/feature_tests/*" -not -path "*/test_*" -not -path "*/fhir/r4/strict_models.py" -not -path "*/fhir/r4/models.py")
 THIRD_PARTY_DIST = $(CURDIR)/src/layers/third_party/dist
 SWAGGER_DIST = $(CURDIR)/infrastructure/swagger/dist
 SWAGGER_PUBLIC = $(SWAGGER_DIST)/public/swagger.yaml

diff --git a/scripts/infrastructure/apigee.mk b/scripts/infrastructure/apigee.mk
@@ -3,13 +3,14 @@
 APIGEE_CONFIG_PATH = $(CURDIR)/infrastructure/apigee
 APIGEE_TIMESTAMP = $(TIMESTAMP_DIR)/.apigee.stamp
 PROXYGEN_TIMESTAMP = $(TIMESTAMP_DIR)/.proxygen.stamp
+PROXYGEN_PRODUCT_TIMESTAMP = $(TIMESTAMP_DIR)/.proxygen-product.stamp
 
 SWAGGER_APIGEE = $(SWAGGER_DIST)/apigee/swagger.yaml
 WORKSPACE_OUTPUT_JSON = $(CURDIR)/infrastructure/terraform/per_workspace/output.json
 ENVIRONMENT_MAPPING_YAML = $(CURDIR)/infrastructure/apigee/environment_mapping.yaml
 STAGE_MAPPING_YAML = $(CURDIR)/infrastructure/apigee/stage_mapping.yaml
 
-apigee--deploy: $(PROXYGEN_TIMESTAMP)
+apigee--deploy: aws--login $(PROXYGEN_TIMESTAMP)
 
 
 apigee--delete: aws--login
@@ -26,15 +27,7 @@ apigee--delete: aws--login
 apigee--clean:
 	[[ -f $(PROXYGEN_TIMESTAMP) ]] && rm $(PROXYGEN_TIMESTAMP) || :
 
-apigee--attach-product: aws--login
-	WORKSPACE_OUTPUT_JSON=$(WORKSPACE_OUTPUT_JSON) \
-	ENVIRONMENT_MAPPING_YAML=$(ENVIRONMENT_MAPPING_YAML) \
-	STAGE_MAPPING_YAML=$(STAGE_MAPPING_YAML) \
-	APIGEE_CONFIG_PATH=$(APIGEE_CONFIG_PATH) \
-	AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) \
-	AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \
-	AWS_SESSION_TOKEN=$(AWS_SESSION_TOKEN) \
-		bash $(PATH_TO_INFRASTRUCTURE)/apigee/apigee.sh attach_product $(PERSISTENT_ENVIRONMENT_BUILD)
+apigee--attach-product: aws--login $(PROXYGEN_PRODUCT_TIMESTAMP)
 
 apigee--detach-product: aws--login
 	WORKSPACE_OUTPUT_JSON=$(WORKSPACE_OUTPUT_JSON) \
@@ -45,8 +38,10 @@ apigee--detach-product: aws--login
 	AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \
 	AWS_SESSION_TOKEN=$(AWS_SESSION_TOKEN) \
 		bash $(PATH_TO_INFRASTRUCTURE)/apigee/apigee.sh detach_product $(PERSISTENT_ENVIRONMENT_BUILD)
+	[[ -f $(PROXYGEN_PRODUCT_TIMESTAMP) ]] && rm $(PROXYGEN_PRODUCT_TIMESTAMP) || :
 
-$(PROXYGEN_TIMESTAMP): aws--login $(SWAGGER_APIGEE) $(WORKSPACE_OUTPUT_JSON)
+
+$(PROXYGEN_TIMESTAMP): $(SWAGGER_APIGEE) $(WORKSPACE_OUTPUT_JSON)
 	[[ -f $(PROXYGEN_TIMESTAMP) ]] && rm $(PROXYGEN_TIMESTAMP) || :
 
 	WORKSPACE_OUTPUT_JSON=$(WORKSPACE_OUTPUT_JSON) \
@@ -60,3 +55,17 @@ $(PROXYGEN_TIMESTAMP): aws--login $(SWAGGER_APIGEE) $(WORKSPACE_OUTPUT_JSON)
 		bash $(PATH_TO_INFRASTRUCTURE)/apigee/proxygen.sh generate_proxy $(PERSISTENT_ENVIRONMENT_BUILD)
 
 	touch $(PROXYGEN_TIMESTAMP)
+
+
+$(PROXYGEN_PRODUCT_TIMESTAMP): $(PROXYGEN_TIMESTAMP)
+	[[ -f $(PROXYGEN_PRODUCT_TIMESTAMP) ]] && rm $(PROXYGEN_PRODUCT_TIMESTAMP) || :
+
+	WORKSPACE_OUTPUT_JSON=$(WORKSPACE_OUTPUT_JSON) \
+	ENVIRONMENT_MAPPING_YAML=$(ENVIRONMENT_MAPPING_YAML) \
+	STAGE_MAPPING_YAML=$(STAGE_MAPPING_YAML) \
+	APIGEE_CONFIG_PATH=$(APIGEE_CONFIG_PATH) \
+	AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) \
+	AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \
+	AWS_SESSION_TOKEN=$(AWS_SESSION_TOKEN) \
+		bash $(PATH_TO_INFRASTRUCTURE)/apigee/apigee.sh attach_product $(PERSISTENT_ENVIRONMENT_BUILD)
+	touch $(PROXYGEN_PRODUCT_TIMESTAMP)
diff --git a/scripts/infrastructure/apigee/apigee.sh b/scripts/infrastructure/apigee/apigee.sh
@@ -146,17 +146,14 @@ function attach_product(){
 
         token=$(echo "$token_response" | jq -r '.pytest_nhsd_apim_token')
         url=https://api.enterprise.apigee.com/v1/organizations/$_org_name/developers/$email_that_owns_app/apps/$app_name/keys/$client_id
-        echo "$url"
 
-        add_product_response=$(curl -X POST \
+        add_product_response=$(curl --retry 100 -sS -X POST \
             $url \
             -H "Authorization: Bearer $token" \
             -H "Content-type:application/json" \
             -d "{\"apiProducts\": [\"$_product_name\"]}")
 
         status=$(echo "$add_product_response" | jq -r '.status')
-        echo "$status"
-
         if [ -z "$status" ] || [ "$status" != "approved" ]; then
           echo "Failed to add product to the app"
           echo "Response: $add_product_response"
@@ -232,17 +229,14 @@ function detach_product(){
 
         token=$(echo "$token_response" | jq -r '.pytest_nhsd_apim_token')
         url=https://api.enterprise.apigee.com/v1/organizations/$_org_name/developers/$email_that_owns_app/apps/$app_name/keys/$client_id/apiproducts/$_product_name
-        echo "$url"
 
-        detach_product_response=$(curl -X DELETE \
+        detach_product_response=$(curl -sS -X DELETE \
             $url \
             -H "Authorization: Bearer $token" \
             -H "Content-type:application/json" \
             -d "{\"apiProducts\": [\"$_product_name\"]}")
 
         status=$(echo "$detach_product_response" | jq -r '.status')
-        echo "$status"
-
         if [ -z "$status" ] || [ "$status" != "approved" ]; then
           echo "Failed to remove product from the app"
           echo "Response: $detach_product_response"

diff --git a/scripts/infrastructure/apigee/proxygen.sh b/scripts/infrastructure/apigee/proxygen.sh
@@ -4,7 +4,6 @@ PATH_TO_HERE="scripts/infrastructure/apigee"
 APIGEE_DEPLOYMENT_ROLE="NHSDeploymentRole"
 API_NAME="connecting-party-manager"
 PERSISTENT_ENVIRONMENT_BUILD="${2:-false}"
-echo "PERSISTENT_ENVIRONMENT_BUILD is: $PERSISTENT_ENVIRONMENT_BUILD"
 
 
 if [[ -z ${WORKSPACE_OUTPUT_JSON} ]]; then

diff --git a/scripts/infrastructure/destroy/destroy-redundant-workspaces.sh b/scripts/infrastructure/destroy/destroy-redundant-workspaces.sh
@@ -37,12 +37,11 @@ function _destroy_redundant_workspaces() {
     workspaces=$(aws s3 ls "$bucket" --no-paginate | awk '{print $NF}' | sed 's:/$::')
 
     # get JIRA ID from branch name
+    ENVIRONMENT="dev"
     if [[ $BRANCH_NAME =~ feature\/(PI-[0-9]+)[-_] ]]; then
         workspace_id="${BASH_REMATCH[1]}"
-        ENVIRONMENT="ref"
     elif [[ $BRANCH_NAME == *release/* ]]; then
         workspace_id="${BRANCH_NAME##*release/}"
-        ENVIRONMENT="dev"
     fi
     echo "The workspace ID is: $workspace_id"
     echo "Destroying workspaces in: $ENVIRONMENT"

diff --git a/scripts/test/test.mk b/scripts/test/test.mk
@@ -7,6 +7,7 @@ USE_CPM_PROD ?= FALSE
 TEST_COUNT =
 COMPARISON_ENV ?= local
 RUN_SPEEDTEST = ?= FALSE
+PROXYGEN_PRODUCT_TIMESTAMP = $(TIMESTAMP_DIR)/.proxygen-product.stamp
 
 _pytest:
 	AWS_DEFAULT_REGION=$(AWS_DEFAULT_REGION) AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) AWS_SESSION_TOKEN=$(AWS_SESSION_TOKEN) poetry run python -m pytest $(PYTEST_FLAGS) $(_INTERNAL_FLAGS) $(_CACHE_CLEAR)
@@ -32,7 +33,7 @@ test--smoke: aws--login ## Run end-to-end smoke tests (pytest)
 test--%--rerun: ## Rerun failed integration or unit (pytest) tests
 	$(MAKE) test--$* _INTERNAL_FLAGS="--last-failed --last-failed-no-failures none" _CACHE_CLEAR=$(_CACHE_CLEAR)
 
-test--feature--integration: aws--login ## Run integration feature (gherkin) tests
+test--feature--integration: aws--login $(PROXYGEN_PRODUCT_TIMESTAMP) ## Run integration feature (gherkin) tests
 	$(MAKE) _behave _INTERNAL_FLAGS="--define='test_mode=integration' $(_INTERNAL_FLAGS)" AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) AWS_SESSION_TOKEN=$(AWS_SESSION_TOKEN)
 
 test--feature--local: _behave  ## Run local feature (gherkin) tests

diff --git a/src/api/createCpmProduct/tests/test_index.py b/src/api/createCpmProduct/tests/test_index.py
@@ -78,7 +78,6 @@ def test_index(version):
             "Content-Length": str(len(expected_body)),
             "Content-Type": "application/json",
             "Version": version,
-            "Location": "FOO",
         },
     }
     _response_assertions(
@@ -149,7 +148,6 @@ def test_index_no_such_product_team(version):
             "Content-Length": str(len(expected_result)),
             "Content-Type": "application/json",
             "Version": version,
-            "Location": None,
         },
     }
     _response_assertions(

diff --git a/src/api/createProductTeam/tests/v1/test_index_v1.py b/src/api/createProductTeam/tests/v1/test_index_v1.py
@@ -58,7 +58,6 @@ def test_index(version):
             "Content-Length": str(len(expected_body)),
             "Content-Type": "application/json",
             "Version": version,
-            "Location": None,
         },
     }
     _response_assertions(
@@ -107,7 +106,6 @@ def test_index_bad_payload(version):
             "Content-Length": str(len(expected_body)),
             "Content-Type": "application/json",
             "Version": version,
-            "Location": None,
         },
     }
     _response_assertions(
@@ -158,7 +156,6 @@ def test_index(version):
             "Content-Length": str(len(expected_body)),
             "Content-Type": "application/json",
             "Version": version,
-            "Location": None,
         },
     }
     _response_assertions(