Merge branch 'develop' of github.com:NHSDigital/NRLF into feature/eema1-NRL-753-capabilityDeployFix

eesa456 · eesa456 · commit 78d318ebd1d9 · 2025-07-09T14:48:28.000+01:00
diff --git a/.github/workflows/pr-env-destroy.yml b/.github/workflows/pr-env-destroy.yml
@@ -4,6 +4,7 @@ run-name: "Destroy PR Environment - #${{ github.event.pull_request.number }} (${
 on:
   pull_request:
     types: [closed]
+  workflow_dispatch:
 
 concurrency:
   group: environment-${{ github.event.pull_request.number }}
@@ -73,9 +74,11 @@ jobs:
       - name: Terraform Init
         run: |
           terraform -chdir=terraform/infrastructure init
-          terraform -chdir=terraform/infrastructure workspace new ${{ needs.set-environment-id.outputs.environment_id }} || \
           terraform -chdir=terraform/infrastructure workspace select ${{ needs.set-environment-id.outputs.environment_id }}
 
+      - name: Build artifacts
+        run: make build get-s3-perms
+
       - name: Terraform Destroy
         run: |
           terraform -chdir=terraform/infrastructure destroy \
diff --git a/api/consumer/searchDocumentReference/tests/test_search_document_reference_consumer.py b/api/consumer/searchDocumentReference/tests/test_search_document_reference_consumer.py
@@ -1,4 +1,5 @@
 import json
+from unittest.mock import patch
 
 from moto import mock_aws
 
@@ -720,7 +721,10 @@ def test_search_document_reference_invalid_category(
 
 @mock_aws
 @mock_repository
-def test_search_document_reference_invalid_json(repository: DocumentPointerRepository):
+@patch("api.consumer.searchDocumentReference.search_document_reference.logger")
+def test_search_document_reference_invalid_json(
+    mock_logger, repository: DocumentPointerRepository
+):
     doc_ref = load_document_reference("Y05868-736253002-Valid")
     doc_pointer = DocumentPointer.from_document_reference(doc_ref)
     repository.create(doc_pointer)
@@ -783,3 +787,7 @@ def test_search_document_reference_invalid_json(repository: DocumentPointerRepos
             {"resource": expected_operation_outcome},
         ],
     }
+
+    assert any(
+        call[0][0].name == "CONSEARCH005" for call in mock_logger.log.call_args_list
+    )
diff --git a/api/consumer/searchPostDocumentReference/tests/test_search_post_document_reference_consumer.py b/api/consumer/searchPostDocumentReference/tests/test_search_post_document_reference_consumer.py
@@ -1,4 +1,5 @@
 import json
+from unittest.mock import patch
 
 from moto import mock_aws
 
@@ -480,8 +481,9 @@ def test_search_document_reference_invalid_category(
 
 @mock_aws
 @mock_repository
+@patch("api.consumer.searchPostDocumentReference.search_post_document_reference.logger")
 def test_search_post_document_reference_invalid_json_adds_operation_outcome(
-    repository: DocumentPointerRepository,
+    mock_logger, repository: DocumentPointerRepository
 ):
     doc_ref = load_document_reference("Y05868-736253002-Valid")
     doc_pointer = DocumentPointer.from_document_reference(doc_ref)
@@ -547,3 +549,7 @@ def test_search_post_document_reference_invalid_json_adds_operation_outcome(
             {"resource": expected_operation_outcome},
         ],
     }
+
+    assert any(
+        call[0][0].name == "CONPOSTSEARCH005" for call in mock_logger.log.call_args_list
+    )
diff --git a/api/producer/searchDocumentReference/tests/test_search_document_reference_producer.py b/api/producer/searchDocumentReference/tests/test_search_document_reference_producer.py
@@ -1,4 +1,5 @@
 import json
+from unittest.mock import patch
 
 from moto import mock_aws
 
@@ -457,7 +458,10 @@ def test_search_document_reference_filters_by_pointer_types(
 
 @mock_aws
 @mock_repository
-def test_search_document_reference_invalid_json(repository: DocumentPointerRepository):
+@patch("api.producer.searchDocumentReference.search_document_reference.logger")
+def test_search_document_reference_invalid_json(
+    mock_logger, repository: DocumentPointerRepository
+):
     doc_ref = load_document_reference("Y05868-736253002-Valid")
     doc_pointer = DocumentPointer.from_document_reference(doc_ref)
     repository.create(doc_pointer)
@@ -514,3 +518,7 @@ def test_search_document_reference_invalid_json(repository: DocumentPointerRepos
             {"resource": expected_operation_outcome},
         ],
     }
+
+    assert any(
+        call[0][0].name == "PROSEARCH005" for call in mock_logger.log.call_args_list
+    )
diff --git a/api/producer/searchPostDocumentReference/tests/test_search_post_document_reference_producer.py b/api/producer/searchPostDocumentReference/tests/test_search_post_document_reference_producer.py
@@ -1,4 +1,5 @@
 import json
+from unittest.mock import patch
 
 from moto import mock_aws
 
@@ -474,7 +475,9 @@ def test_search_document_reference_filters_by_pointer_types(
 
 @mock_aws
 @mock_repository
+@patch("api.producer.searchPostDocumentReference.search_post_document_reference.logger")
 def test_search_post_document_reference_invalid_json_adds_operation_outcome(
+    mock_logger,
     repository: DocumentPointerRepository,
 ):
     doc_ref = load_document_reference("Y05868-736253002-Valid")
@@ -535,3 +538,7 @@ def test_search_post_document_reference_invalid_json_adds_operation_outcome(
             {"resource": expected_operation_outcome},
         ],
     }
+
+    assert any(
+        call[0][0].name == "PROPOSTSEARCH005" for call in mock_logger.log.call_args_list
+    )
diff --git a/layer/nrlf/core/log_references.py b/layer/nrlf/core/log_references.py
@@ -7,6 +7,14 @@ class _Reference:
     level: str
     message: str
 
+    # WARNING: This makes all _Reference objects unequal unless they are the same instance.
+    # It's fine for our use case, but be cautious if you compare _Reference objects.
+    def __eq__(self, other):
+        return self is other
+
+    def __hash__(self):
+        return id(self)
+
 
 class LogReference(Enum):
     # Request Handler Logs
diff --git a/terraform/account-wide-infrastructure/dev/secrets.tf b/terraform/account-wide-infrastructure/dev/secrets.tf
@@ -43,7 +43,6 @@ resource "aws_secretsmanager_secret" "devsandbox_environment_configuration" {
 }
 
 resource "aws_secretsmanager_secret" "powerbi_gw_instance_admin_pwd" {
-  count       = var.enable_reporting && var.enable_powerbi_auto_push ? 1 : 0
   name        = "${local.project}--dev-powerbi-gw-instance-admin-pwd"
   description = "Admin password for the PowerBI Gateway EC2 instance"
 }
diff --git a/terraform/account-wide-infrastructure/prod/ec2.tf b/terraform/account-wide-infrastructure/prod/ec2.tf
@@ -12,7 +12,7 @@ module "vpc" {
 module "powerbi_gw_instance" {
   count              = var.enable_reporting && var.enable_powerbi_auto_push ? 1 : 0
   source             = "../modules/powerbi-gw-ec2"
-  use_custom_ami     = false
+  use_custom_ami     = var.use_powerbi_gw_custom_ami
   instance_type      = var.powerbi_gw_instance_type
   name_prefix        = "nhsd-nrlf--prod-powerbi-gw"
   target_bucket_arn  = module.prod-glue.target_bucket_arn
diff --git a/terraform/account-wide-infrastructure/prod/secrets.tf b/terraform/account-wide-infrastructure/prod/secrets.tf
@@ -23,7 +23,6 @@ resource "aws_secretsmanager_secret" "prod_environment_configuration" {
 }
 
 resource "aws_secretsmanager_secret" "powerbi_gw_instance_admin_pwd" {
-  count       = var.enable_reporting && var.enable_powerbi_auto_push ? 1 : 0
   name        = "${local.project}--prod-powerbi-gw-instance-admin-pwd"
   description = "Admin password for the PowerBI Gateway EC2 instance"
 }
diff --git a/terraform/account-wide-infrastructure/prod/vars.tf b/terraform/account-wide-infrastructure/prod/vars.tf
@@ -54,7 +54,7 @@ variable "instance_type" {
 variable "enable_powerbi_auto_push" {
   type        = bool
   description = "Enable automatic pushing of info into PowerBI"
-  default     = false
+  default     = true
 }
 
 variable "powerbi_gw_instance_type" {
@@ -66,5 +66,5 @@ variable "powerbi_gw_instance_type" {
 variable "use_powerbi_gw_custom_ami" {
   type        = bool
   description = "Use custom image for PowerBI GW instance"
-  default     = false
+  default     = true
 }
diff --git a/terraform/account-wide-infrastructure/test/secrets.tf b/terraform/account-wide-infrastructure/test/secrets.tf
@@ -108,7 +108,6 @@ resource "aws_secretsmanager_secret" "ref_environment_configuration" {
 # PowerBI secrets
 #
 resource "aws_secretsmanager_secret" "powerbi_gw_instance_admin_pwd" {
-  count       = var.enable_reporting && var.enable_powerbi_auto_push ? 1 : 0
   name        = "${local.project}--test-powerbi-gw-instance-admin-pwd"
   description = "Admin password for the PowerBI Gateway EC2 instance"
 }
diff --git a/terraform/infrastructure/api_gateway.tf b/terraform/infrastructure/api_gateway.tf
@@ -10,6 +10,10 @@ module "consumer__gateway" {
     method_readDocumentReference       = "arn:aws:apigateway:eu-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:eu-west-2:${local.aws_account_id}:function:${substr("${local.prefix}--api--consumer--readDocumentReference", 0, 64)}/invocations"
     method_status                      = "arn:aws:apigateway:eu-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:eu-west-2:${local.aws_account_id}:function:${substr("${local.prefix}--api--consumer--status", 0, 64)}/invocations"
   }
+  endpoint_allowed_methods = {
+    "/DocumentReference"      = "GET",
+    "/DocumentReference/{id}" = "GET"
+  }
   kms_key_id                   = module.kms__cloudwatch.kms_arn
   domain                       = local.apis.domain
   path                         = local.apis.consumer.path
@@ -35,7 +39,10 @@ module "producer__gateway" {
     method_deleteDocumentReference     = "arn:aws:apigateway:eu-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:eu-west-2:${local.aws_account_id}:function:${substr("${local.prefix}--api--producer--deleteDocumentReference", 0, 64)}/invocations"
     method_status                      = "arn:aws:apigateway:eu-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:eu-west-2:${local.aws_account_id}:function:${substr("${local.prefix}--api--producer--status", 0, 64)}/invocations"
   }
-
+  endpoint_allowed_methods = {
+    "/DocumentReference"      = "GET,POST",
+    "/DocumentReference/{id}" = "GET,PUT,DELETE"
+  }
   kms_key_id                   = module.kms__cloudwatch.kms_arn
   domain                       = local.apis.domain
   path                         = local.apis.producer.path
diff --git a/terraform/infrastructure/consumer.tftpl b/terraform/infrastructure/consumer.tftpl
@@ -63,7 +63,7 @@
           {
             "type": "DocumentReference",
             "profile": "http://hl7.org/fhir/R4/documentreference.html",
-            "documentation": "Additional business rules apply to constrain patient, organisation and type/category content.",
+            "documentation": "Additional business rules apply to constrain patient, organisation and type/category content. Note that HEAD requests are not supported.",
             "interaction": [
               {
                 "code": "read",
diff --git a/terraform/infrastructure/modules/api_gateway/api_gateway.tf b/terraform/infrastructure/modules/api_gateway/api_gateway.tf
@@ -52,9 +52,11 @@ resource "aws_api_gateway_deployment" "api_gateway_deployment" {
   rest_api_id = aws_api_gateway_rest_api.api_gateway_rest_api.id
 
   triggers = {
-    redeployment    = sha1(jsonencode(aws_api_gateway_rest_api.api_gateway_rest_api.body))
-    resource_change = "${md5(file("${path.module}/api_gateway.tf"))}"
-    capabilities    = sha1(var.capability_statement_content)
+    redeployment              = sha1(jsonencode(aws_api_gateway_rest_api.api_gateway_rest_api.body))
+    resource_change           = "${md5(file("${path.module}/api_gateway.tf"))}"
+    capabilities              = sha1(var.capability_statement_content)
+    head_responses_change     = md5(file("${path.module}/head_responses.tf"))
+    parent_api_gateway_change = md5(file("${path.module}/../../api_gateway.tf"))
   }
 
   lifecycle {
@@ -67,7 +69,8 @@ resource "aws_api_gateway_deployment" "api_gateway_deployment" {
     aws_api_gateway_method.capability,
     aws_api_gateway_integration.capability,
     aws_api_gateway_method_response.capability_200,
-    aws_api_gateway_integration_response.capability
+    aws_api_gateway_integration_response.capability,
+    aws_api_gateway_integration_response.head_integration_response
   ]
 }
 
diff --git a/terraform/infrastructure/modules/api_gateway/head_responses.tf b/terraform/infrastructure/modules/api_gateway/head_responses.tf
@@ -0,0 +1,67 @@
+# Define a reusable map of allowed methods for each path
+data "aws_api_gateway_resource" "resource" {
+  for_each    = var.endpoint_allowed_methods
+  rest_api_id = aws_api_gateway_rest_api.api_gateway_rest_api.id
+  path        = each.key
+
+  depends_on = [aws_api_gateway_rest_api.api_gateway_rest_api]
+}
+
+# Add HEAD method to each resource with 405 response
+resource "aws_api_gateway_method" "head_method" {
+  for_each = var.endpoint_allowed_methods
+
+  rest_api_id   = aws_api_gateway_rest_api.api_gateway_rest_api.id
+  resource_id   = data.aws_api_gateway_resource.resource[each.key].id
+  http_method   = "HEAD"
+  authorization = "NONE"
+}
+
+resource "aws_api_gateway_integration" "head_integration" {
+  for_each = var.endpoint_allowed_methods
+
+  rest_api_id          = aws_api_gateway_rest_api.api_gateway_rest_api.id
+  resource_id          = data.aws_api_gateway_resource.resource[each.key].id
+  http_method          = aws_api_gateway_method.head_method[each.key].http_method
+  type                 = "MOCK"
+  passthrough_behavior = "WHEN_NO_TEMPLATES"
+
+  request_templates = {
+    "application/json"      = <<-EOF
+      { "statusCode": 405 }
+    EOF
+    "application/json+fhir" = <<-EOF
+      { "statusCode": 405 }
+    EOF
+    "application/fhir+json" = <<-EOF
+      { "statusCode": 405 }
+    EOF
+  }
+}
+
+resource "aws_api_gateway_method_response" "head_method_response" {
+  for_each = var.endpoint_allowed_methods
+
+  rest_api_id = aws_api_gateway_rest_api.api_gateway_rest_api.id
+  resource_id = data.aws_api_gateway_resource.resource[each.key].id
+  http_method = aws_api_gateway_method.head_method[each.key].http_method
+  status_code = "405"
+
+  response_parameters = {
+    "method.response.header.Allow" = true
+  }
+}
+
+resource "aws_api_gateway_integration_response" "head_integration_response" {
+  for_each = var.endpoint_allowed_methods
+
+  rest_api_id       = aws_api_gateway_rest_api.api_gateway_rest_api.id
+  resource_id       = data.aws_api_gateway_resource.resource[each.key].id
+  http_method       = aws_api_gateway_method.head_method[each.key].http_method
+  status_code       = aws_api_gateway_method_response.head_method_response[each.key].status_code
+  selection_pattern = "" # default catch-all
+
+  response_parameters = {
+    "method.response.header.Allow" = "'${each.value}'"
+  }
+}
diff --git a/terraform/infrastructure/modules/api_gateway/vars.tf b/terraform/infrastructure/modules/api_gateway/vars.tf
@@ -22,3 +22,7 @@ variable "retention" {
   default = 90
   type    = number
 }
+
+variable "endpoint_allowed_methods" {
+  type = map(string)
+}
diff --git a/terraform/infrastructure/producer.tftpl b/terraform/infrastructure/producer.tftpl
@@ -62,7 +62,7 @@
           {
             "type": "DocumentReference",
             "profile": "http://hl7.org/fhir/R4/documentreference.html",
-            "documentation": "Additional business rules apply to validate patient, organisation and type/category content.",
+            "documentation": "Additional business rules apply to validate patient, organisation and type/category content. Note that HEAD requests are not supported.",
             "interaction": [
               {
                 "code": "read",
diff --git a/tests/features/consumer/headOnGetEndpoint.feature b/tests/features/consumer/headOnGetEndpoint.feature
@@ -0,0 +1,66 @@
+Feature: Consumer - HEAD Requests
+
+  Scenario Outline: DocumentReference with HEAD fails (Content-Type: <content_type>)
+    Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API
+    When consumer 'RX898' sends HEAD request to 'DocumentReference' endpoint with headers:
+      | header       | value          |
+      | Content-Type | <content_type> |
+    Then the response status code is 405
+    And the response has an empty body
+    And the Allow header is 'GET'
+    And the Content-Length header is '0'
+
+    Examples:
+      | content_type          |
+      | application/json      |
+      | application/json+fhir |
+      | application/fhir+json |
+
+  Scenario Outline: DocumentReference/{id} with HEAD fails (Content-Type: <content_type>)
+    Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API
+    When consumer 'RX898' sends HEAD request to 'DocumentReference/random-id' endpoint with headers:
+      | header       | value          |
+      | Content-Type | <content_type> |
+    Then the response status code is 405
+    And the response has an empty body
+    And the Allow header is 'GET'
+    And the Content-Length header is '0'
+
+    Examples:
+      | content_type          |
+      | application/json      |
+      | application/json+fhir |
+      | application/fhir+json |
+
+  Scenario Outline: DocumentReference with HEAD fails with 415 with unsupported (Content-Type: <content_type>)
+    Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API
+    When consumer 'RX898' sends HEAD request to 'DocumentReference' endpoint with headers:
+      | header       | value          |
+      | Content-Type | <content_type> |
+    Then the response status code is 415
+    And the Content-Type header is 'application/json'
+    And the Content-Length header is '0'
+    And the Allow header is not present
+
+    Examples:
+      | content_type             |
+      | application/notsupported |
+      | application/helloworld   |
+      | application/harold       |
+
+  Scenario Outline: DocumentReference/{id} with HEAD fails (Content-Type: <content_type>)
+    Given the application 'DataShare' (ID 'z00z-y11y-x22x') is registered to access the API
+    When consumer 'RX898' sends HEAD request to 'DocumentReference/random-id' endpoint with headers:
+      | header       | value          |
+      | Content-Type | <content_type> |
+    Then the response status code is 415
+    And the response has an empty body
+    And the Content-Type header is 'application/json'
+    And the Content-Length header is '0'
+    And the Allow header is not present
+
+    Examples:
+      | content_type             |
+      | application/notsupported |
+      | application/helloworld   |
+      | application/harold       |
diff --git a/tests/features/producer/headOnGetEndpoint.feature b/tests/features/producer/headOnGetEndpoint.feature
diff --git a/tests/features/steps/2_request.py b/tests/features/steps/2_request.py
diff --git a/tests/features/steps/3_assert.py b/tests/features/steps/3_assert.py
diff --git a/tests/utilities/api_clients.py b/tests/utilities/api_clients.py

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,6 @@ resource "aws_secretsmanager_secret" "devsandbox_environment_configuration" {`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`resource "aws_secretsmanager_secret" "powerbi_gw_instance_admin_pwd" {`
`46`		`- count = var.enable_reporting && var.enable_powerbi_auto_push ? 1 : 0`
`47`	`46`	`name = "${local.project}--dev-powerbi-gw-instance-admin-pwd"`
`48`	`47`	`description = "Admin password for the PowerBI Gateway EC2 instance"`
`49`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,6 @@ resource "aws_secretsmanager_secret" "prod_environment_configuration" {`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`resource "aws_secretsmanager_secret" "powerbi_gw_instance_admin_pwd" {`
`26`		`- count = var.enable_reporting && var.enable_powerbi_auto_push ? 1 : 0`
`27`	`26`	`name = "${local.project}--prod-powerbi-gw-instance-admin-pwd"`
`28`	`27`	`description = "Admin password for the PowerBI Gateway EC2 instance"`
`29`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ variable "instance_type" {`
`54`	`54`	`variable "enable_powerbi_auto_push" {`
`55`	`55`	`type = bool`
`56`	`56`	`description = "Enable automatic pushing of info into PowerBI"`
`57`		`- default = false`
	`57`	`+ default = true`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`variable "powerbi_gw_instance_type" {`
`@@ -66,5 +66,5 @@ variable "powerbi_gw_instance_type" {`
`66`	`66`	`variable "use_powerbi_gw_custom_ami" {`
`67`	`67`	`type = bool`
`68`	`68`	`description = "Use custom image for PowerBI GW instance"`
`69`		`- default = false`
	`69`	`+ default = true`
`70`	`70`	`}`