Merge branch 'VED-887-Slack-alerting-API-errors' of https://github.com/NHSDigital/immunisation-fhir-api into VED-887-Slack-alerting-API-errors

Author: Akol125

.github/workflows/deploy-backend.yml

@@ -83,7 +83,7 @@ jobs:
         run: make plan-ci
 
       - name: Save Terraform Plan
-        uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4
+        uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f
         with:
           name: ${{ env.ENVIRONMENT }}-${{ env.SUB_ENVIRONMENT }}-tfplan
           path: infrastructure/instance/tfplan

.github/workflows/quality-checks.yml

@@ -247,7 +247,7 @@ jobs:
           fi
 
       - name: SonarCloud Scan
-        uses: SonarSource/sonarqube-scan-action@fd88b7d7ccbaefd23d8f36f73b59db7a3d246602
+        uses: SonarSource/sonarqube-scan-action@a31c9398be7ace6bbfaf30c0bd5d415f843d45e9
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

infrastructure/instance/environments/prod/blue/variables.tfvars

@@ -5,3 +5,4 @@ pds_environment                   = "prod"
 error_alarm_notifications_enabled = true
 create_mesh_processor             = true
 has_sub_environment_scope         = false
+dspp_kms_key_alias                = "nhsd-dspp-core-prod-extended-attributes-gdp-key"

infrastructure/instance/environments/prod/green/variables.tfvars

@@ -5,3 +5,4 @@ pds_environment                   = "prod"
 error_alarm_notifications_enabled = true
 create_mesh_processor             = true
 has_sub_environment_scope         = false
+dspp_kms_key_alias                = "nhsd-dspp-core-prod-extended-attributes-gdp-key"

infrastructure/instance/file_name_processor.tf

@@ -253,13 +253,44 @@ resource "aws_iam_policy" "filenameprocessor_dynamo_access_policy" {
   })
 }
 
+# Kms policy setup on filenameprocessor lambda for dps cross account bucket access
+resource "aws_iam_policy" "filenameprocessor_dps_extended_attribute_kms_policy" {
+  name        = "${local.short_prefix}-filenameproc-dps-kms-policy"
+  description = "Allow Lambda to use DPS KMS key for SSE-KMS encrypted S3 bucket access"
+
+  policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [
+      {
+        Effect = "Allow",
+        Action = [
+          "kms:Decrypt",
+          "kms:GenerateDataKey",
+          "kms:DescribeKey"
+        ],
+        Resource = "arn:aws:kms:eu-west-2:${var.dspp_core_account_id}:key/*",
+        "Condition" = {
+          "ForAnyValue:StringEquals" = {
+            "kms:ResourceAliases" = "alias/${var.dspp_kms_key_alias}"
+          }
+        }
+      }
+    ]
+  })
+}
 
 # Attach the execution policy to the Lambda role
 resource "aws_iam_role_policy_attachment" "filenameprocessor_lambda_exec_policy_attachment" {
   role       = aws_iam_role.filenameprocessor_lambda_exec_role.name
   policy_arn = aws_iam_policy.filenameprocessor_lambda_exec_policy.arn
 }
 
+#Attach the dps kms policy to the Lambda role
+resource "aws_iam_role_policy_attachment" "filenameprocessor_lambda_dps_kms_ea_policy_attachment" {
+  role       = aws_iam_role.filenameprocessor_lambda_exec_role.name
+  policy_arn = aws_iam_policy.filenameprocessor_dps_extended_attribute_kms_policy.arn
+}
+
 # Attach the SQS policy to the Lambda role
 resource "aws_iam_role_policy_attachment" "filenameprocessor_lambda_sqs_policy_attachment" {
   role       = aws_iam_role.filenameprocessor_lambda_exec_role.name

infrastructure/instance/variables.tf

@@ -10,6 +10,12 @@ variable "csoc_account_id" {
   default = "693466633220"
 }
 
+variable "dspp_kms_key_alias" {
+  description = "Alias name of the DPS KMS key allowed for SSE-KMS encryption"
+  type        = string
+  default     = "nhsd-dspp-core-ref-extended-attributes-gdp-key"
+}
+
 variable "create_mesh_processor" {
   default = false
 }

lambdas/backend/src/controller/fhir_controller.py

@@ -36,6 +36,7 @@
 from service.fhir_service import FhirService, get_service_url
 
 IMMUNIZATION_ENV = os.getenv("IMMUNIZATION_ENV")
+IMMUNIZATION_BASE_PATH = os.getenv("IMMUNIZATION_BASE_PATH")
 
 
 def make_controller(
@@ -51,7 +52,7 @@ def make_controller(
 
 class FhirController:
     _IMMUNIZATION_ID_PATTERN = r"^[A-Za-z0-9\-.]{1,64}$"
-    _API_SERVICE_URL = get_service_url()
+    _API_SERVICE_URL = get_service_url(IMMUNIZATION_ENV, IMMUNIZATION_BASE_PATH)
 
     def __init__(
         self,

lambdas/backend/src/service/constants.py

@@ -0,0 +1,4 @@
+"""Constants for the fhir_service layer"""
+
+DEFAULT_BASE_PATH = "immunisation-fhir-api/FHIR/R4"
+PR_ENV_PREFIX = "pr-"

lambdas/backend/src/service/fhir_service.py

@@ -2,7 +2,6 @@
 import datetime
 import logging
 import os
-import urllib.parse
 import uuid
 from typing import Any, Optional, cast
 from uuid import uuid4
@@ -43,11 +42,10 @@
     validate_identifiers_match,
     validate_resource_versions_match,
 )
-from controller.constants import IMMUNIZATION_TARGET_LEGACY_KEY_NAME, ImmunizationSearchParameterName
-from controller.parameter_parser import PATIENT_IDENTIFIER_SYSTEM
 from filter import Filter
 from models.errors import UnauthorizedVaxError
 from repository.fhir_repository import ImmunizationRepository
+from service.search_url_helper import create_url_for_bundle_link, get_service_url
 
 logging.basicConfig(level="INFO")
 logger = logging.getLogger()
@@ -59,24 +57,6 @@
 IMMUNIZATION_VALIDATOR = ImmunizationValidator()
 
 
-def get_service_url(
-    service_env: str = IMMUNIZATION_ENV,
-    service_base_path: str = IMMUNIZATION_BASE_PATH,
-) -> str:
-    if not service_base_path:
-        service_base_path = "immunisation-fhir-api/FHIR/R4"
-
-    non_prod = ["internal-dev", "int", "sandbox"]
-    if service_env in non_prod:
-        subdomain = f"{service_env}."
-    elif service_env == "prod":
-        subdomain = ""
-    else:
-        subdomain = "internal-dev."
-
-    return f"https://{subdomain}api.service.nhs.uk/{service_base_path}"
-
-
 class FhirService:
     _DATA_MISSING_DATE_TIME_ERROR_MSG = (
         "Data quality issue - immunisation with ID %s was found containing no occurrenceDateTime"
@@ -98,7 +78,7 @@ def get_immunization_by_identifier(
         """
         Get an Immunization by its ID. Returns a FHIR Bundle containing the search results.
         """
-        base_url = f"{get_service_url()}/Immunization"
+        base_url = f"{get_service_url(IMMUNIZATION_ENV, IMMUNIZATION_BASE_PATH)}/Immunization"
         resource, resource_metadata = self.immunization_repo.get_immunization_by_identifier(identifier)
 
         if not resource:
@@ -251,7 +231,7 @@ def search_immunizations(
             BundleEntry(
                 resource=Immunization.parse_obj(imms),
                 search=BundleEntrySearch(mode="match"),
-                fullUrl=f"{get_service_url()}/Immunization/{imms['id']}",
+                fullUrl=f"{get_service_url(IMMUNIZATION_ENV, IMMUNIZATION_BASE_PATH)}/Immunization/{imms['id']}",
             )
             for imms in processed_resources
         ]
@@ -288,7 +268,15 @@ def search_immunizations(
             link=[
                 BundleLink(
                     relation="self",
-                    url=self.create_url_for_bundle_link(permitted_vacc_types, nhs_number, date_from, date_to, include),
+                    url=create_url_for_bundle_link(
+                        permitted_vacc_types,
+                        nhs_number,
+                        date_from,
+                        date_to,
+                        include,
+                        IMMUNIZATION_ENV,
+                        IMMUNIZATION_BASE_PATH,
+                    ),
                 )
             ],
             total=len(processed_resources),
@@ -408,29 +396,3 @@ def process_patient_for_bundle(patient: dict):
             new_patient["id"] = new_patient["identifier"][0].get("value")
 
         return new_patient
-
-    @staticmethod
-    def create_url_for_bundle_link(
-        immunization_targets: set[str],
-        patient_nhs_number: str,
-        date_from: Optional[datetime.date],
-        date_to: Optional[datetime.date],
-        include: Optional[str],
-    ) -> str:
-        """Creates url for the searchset Bundle Link."""
-        params = {
-            # Temporarily maintaining this for backwards compatibility with imms history, but we should remove it
-            IMMUNIZATION_TARGET_LEGACY_KEY_NAME: ",".join(immunization_targets),
-            ImmunizationSearchParameterName.IMMUNIZATION_TARGET: ",".join(immunization_targets),
-            ImmunizationSearchParameterName.PATIENT_IDENTIFIER: f"{PATIENT_IDENTIFIER_SYSTEM}|{patient_nhs_number}",
-        }
-
-        if date_from:
-            params[ImmunizationSearchParameterName.DATE_FROM] = date_from.isoformat()
-        if date_to:
-            params[ImmunizationSearchParameterName.DATE_TO] = date_to.isoformat()
-        if include:
-            params[ImmunizationSearchParameterName.INCLUDE] = include
-
-        query = urllib.parse.urlencode(params)
-        return f"{get_service_url()}/Immunization?{query}"

lambdas/backend/src/service/search_url_helper.py

@@ -0,0 +1,60 @@
+"""Module containing helper functions for the constructions of Immunisation FHIR API search URLs"""
+
+import datetime
+import urllib.parse
+from typing import Optional
+
+from controller.constants import IMMUNIZATION_TARGET_LEGACY_KEY_NAME, ImmunizationSearchParameterName
+from controller.parameter_parser import PATIENT_IDENTIFIER_SYSTEM
+from service.constants import DEFAULT_BASE_PATH, PR_ENV_PREFIX
+
+
+def get_service_url(service_env: Optional[str], service_base_path: Optional[str]) -> str:
+    """Sets the service URL based on service parameters derived from env vars. PR environments use internal-dev while
+    we also default to this environment. The only other exceptions are preprod which maps to the Apigee int environment
+    and prod which does not have a subdomain."""
+    if not service_base_path:
+        service_base_path = DEFAULT_BASE_PATH
+
+    if service_env is None or is_pr_env(service_env):
+        subdomain = "internal-dev."
+    elif service_env == "preprod":
+        subdomain = "int."
+    elif service_env == "prod":
+        subdomain = ""
+    else:
+        subdomain = f"{service_env}."
+
+    return f"https://{subdomain}api.service.nhs.uk/{service_base_path}"
+
+
+def is_pr_env(service_env: Optional[str]) -> bool:
+    return service_env is not None and service_env.startswith(PR_ENV_PREFIX)
+
+
+def create_url_for_bundle_link(
+    immunization_targets: set[str],
+    patient_nhs_number: str,
+    date_from: Optional[datetime.date],
+    date_to: Optional[datetime.date],
+    include: Optional[str],
+    service_env: Optional[str],
+    service_base_path: Optional[str],
+) -> str:
+    """Creates url for the searchset Bundle Link."""
+    params = {
+        # Temporarily maintaining this for backwards compatibility with imms history, but we should remove it
+        IMMUNIZATION_TARGET_LEGACY_KEY_NAME: ",".join(immunization_targets),
+        ImmunizationSearchParameterName.IMMUNIZATION_TARGET: ",".join(immunization_targets),
+        ImmunizationSearchParameterName.PATIENT_IDENTIFIER: f"{PATIENT_IDENTIFIER_SYSTEM}|{patient_nhs_number}",
+    }
+
+    if date_from:
+        params[ImmunizationSearchParameterName.DATE_FROM] = date_from.isoformat()
+    if date_to:
+        params[ImmunizationSearchParameterName.DATE_TO] = date_to.isoformat()
+    if include:
+        params[ImmunizationSearchParameterName.INCLUDE] = include
+
+    query = urllib.parse.urlencode(params)
+    return f"{get_service_url(service_env, service_base_path)}/Immunization?{query}"