Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NRL-817 Do not store permissions in artifact #669

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion .github/workflows/persistent-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,15 @@ jobs:
uses: actions/upload-artifact@v4
with:
name: build-artifacts
path: dist/*.zip
path: |
dist/*.zip
!dist/nrlf_permissions.zip

- name: Save NRLF Permissions cache
uses: actions/cache/save@v4
with:
key: ${{ github.run_id }}-nrlf-permissions
path: dist/nrlf_permissions.zip

terraform-plan:
name: Terraform Plan - ${{ inputs.environment }}
Expand Down Expand Up @@ -117,6 +125,13 @@ jobs:
name: build-artifacts
path: dist

- name: Restore NRLF permissions cache
uses: actions/cache/restore@v4
with:
key: ${{ github.run_id }}-nrlf-permissions
path: dist/nrlf_permissions.zip
fail-on-cache-miss: true

- name: Terraform Init
run: |
terraform -chdir=terraform/infrastructure init
Expand Down Expand Up @@ -165,6 +180,13 @@ jobs:
name: build-artifacts
path: dist

- name: Restore NRLF permissions cache
uses: actions/cache/restore@v4
with:
key: ${{ github.run_id }}-nrlf-permissions
path: dist/nrlf_permissions.zip
fail-on-cache-miss: true

- name: Configure Management Credentials
uses: aws-actions/configure-aws-credentials@v4
with:
Expand Down
50 changes: 17 additions & 33 deletions .github/workflows/pr-env-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,6 @@ jobs:
with:
ref: ${{ github.event.pull_request.head.ref }}

- name: Setup asdf cache
uses: actions/cache@v4
with:
path: ~/.asdf
key: ${{ runner.os }}-asdf-${{ hashFiles('**/.tool-versions') }}
restore-keys: |
${{ runner.os }}-asdf-

- name: Install asdf
uses: asdf-vm/actions/[email protected]

Expand Down Expand Up @@ -93,7 +85,15 @@ jobs:
uses: actions/upload-artifact@v4
with:
name: build-artifacts
path: dist/*.zip
path: |
dist/*.zip
!dist/nrlf_permissions.zip

- name: Save NRLF Permissions cache
uses: actions/cache/save@v4
with:
key: ${{ github.run_id }}-nrlf-permissions
path: dist/nrlf_permissions.zip

- name: Add Failure Pull Request Comment
uses: actions/github-script@v7
Expand All @@ -119,14 +119,6 @@ jobs:
with:
ref: ${{ github.event.pull_request.head.ref }}

- name: Setup asdf cache
uses: actions/cache@v4
with:
path: ~/.asdf
key: ${{ runner.os }}-asdf-${{ hashFiles('**/.tool-versions') }}
restore-keys: |
${{ runner.os }}-asdf-

- name: Install asdf
uses: asdf-vm/actions/[email protected]

Expand All @@ -143,6 +135,13 @@ jobs:
name: build-artifacts
path: dist

- name: Restore NRLF permissions cache
uses: actions/cache/restore@v4
with:
key: ${{ github.run_id }}-nrlf-permissions
path: dist/nrlf_permissions.zip
fail-on-cache-miss: true

- name: Retrieve Server Certificates
run: make truststore-pull-server ENV=dev

Expand Down Expand Up @@ -195,14 +194,6 @@ jobs:
with:
ref: ${{ github.event.pull_request.head.ref }}

- name: Setup asdf cache
uses: actions/cache@v4
with:
path: ~/.asdf
key: ${{ runner.os }}-asdf-${{ hashFiles('**/.tool-versions') }}
restore-keys: |
${{ runner.os }}-asdf-

- name: Install asdf and tools
uses: asdf-vm/actions/[email protected]

Expand Down Expand Up @@ -231,6 +222,7 @@ jobs:
run: make test-features-integration TF_WORKSPACE_NAME=${{ needs.set-environment-id.outputs.environment_id }}

performance-test:
if: false
name: Run Performance Tests
needs: [set-environment-id, integration-test]
environment: pull-request
Expand All @@ -242,14 +234,6 @@ jobs:
with:
ref: ${{ github.event.pull_request.head.ref }}

- name: Setup asdf cache
uses: actions/cache@v4
with:
path: ~/.asdf
key: ${{ runner.os }}-asdf-${{ hashFiles('**/.tool-versions') }}
restore-keys: |
${{ runner.os }}-asdf-

- name: Install asdf and tools
uses: asdf-vm/actions/[email protected]

Expand Down
7 changes: 0 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,3 @@ repos:
# language: pygrep
# types: [python]
# exclude: layer/nrlf/nrlf/core/validators.py|layer/psycopg2/.*|mi/.*

- repo: local
hooks:
- id: create_changelog
name: Create changelog from changelog files
entry: changelog/scripts/changelog-pre-commit.sh
language: python
5 changes: 0 additions & 5 deletions .tool-versions
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
awscli 2.15.11
poetry 1.8.2
jq 1.7.1
python 3.12.2
terraform 1.3.4
java zulu-jre-17.42.19
yq 4.35.2
allure 2.27.0
k6 0.50.0
6 changes: 3 additions & 3 deletions behave.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[behave]
#[behave]
# Add the Allure formatter and output directory
format = allure_behave.formatter:AllureFormatter
outfiles = allure-results
# format = allure_behave.formatter:AllureFormatter
# outfiles = allure-results
48 changes: 25 additions & 23 deletions layer/nrlf/core/authoriser.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,41 @@
import json
import sys
import tomllib
from os import path

from botocore.exceptions import ClientError

from nrlf.core.boto import get_s3_client
from nrlf.core.config import Config
from nrlf.core.constants import CUSTODIAN_SEPARATOR, PERMISSIONS_FILENAME
from nrlf.core.logger import LogReference, logger
from nrlf.core.model import ConnectionMetadata


def get_pointer_types(
connection_metadata: ConnectionMetadata, config: Config
) -> list[str]:
ods_code = ".".join(connection_metadata.ods_code)
if connection_metadata.test_pointer_types:
return connection_metadata.test_pointer_types

app_id = connection_metadata.nrl_app_id
ods_code = connection_metadata.ods_code
ods_code_extension = connection_metadata.ods_code_extension
pointer_types = parse_permissions_file(connection_metadata)

if ods_code_extension:
key = f"{app_id}/{ods_code}.{ods_code_extension}.json"
else:
key = f"{app_id}/{ods_code}.json"
if not pointer_types and not connection_metadata.load_test_permissions:
logger.log(LogReference.HANDLER004)
pointer_types = get_pointer_types_from_s3(connection_metadata, config)

return pointer_types


def get_pointer_types_from_s3(
connection_metadata: ConnectionMetadata, config: Config
) -> list[str]:
key = PERMISSIONS_FILENAME
logger.log(LogReference.S3PERMISSIONS001, bucket=config.AUTH_STORE, key=key)
s3_client = get_s3_client()

try:
response = s3_client.get_object(Bucket=config.AUTH_STORE, Key=key)
pointer_types = json.loads(response["Body"].read())
pointer_types = parse_permissions_file(connection_metadata)
logger.log(LogReference.S3PERMISSIONS002, pointer_types=pointer_types)
return pointer_types

Expand All @@ -56,29 +62,25 @@ def get_pointer_types(
raise exc


def parse_permissions_file(
connection_metadata: ConnectionMetadata,
) -> list[str]:
ods_code = ".".join(connection_metadata.ods_code)

def parse_permissions_file(connection_metadata: ConnectionMetadata) -> list[str]:
app_id = connection_metadata.nrl_app_id
ods_code = connection_metadata.ods_code
ods_code_extension = connection_metadata.ods_code_extension

key = ods_code
if ods_code_extension:
key = f"{app_id}/{ods_code}.{ods_code_extension}.json"
else:
key = f"{app_id}/{ods_code}.json"
key += CUSTODIAN_SEPARATOR + ods_code_extension

file_path = f"/opt/python/nrlf_permissions/{key}"
file_path = f"/opt/python/nrlf_permissions/{PERMISSIONS_FILENAME}"

if connection_metadata.is_test_event:
file_path = path.abspath(f"layer/test_permissions/{key}")
if connection_metadata.load_test_permissions:
file_path = path.abspath(f"layer/test_permissions/{PERMISSIONS_FILENAME}")

pointer_types = []
try:
with open(file_path) as file:
pointer_types = json.load(file)
with open(file_path, "rb") as file:
data = tomllib.load(file)
pointer_types = data[app_id][ods_code]
except Exception as exc:
logger.log(
LogReference.S3PERMISSIONS005,
Expand Down
3 changes: 2 additions & 1 deletion layer/nrlf/core/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Source(Enum):
PERMISSION_AUDIT_DATES_FROM_PAYLOAD = "audit-dates-from-payload"
PERMISSION_SUPERSEDE_IGNORE_DELETE_FAIL = "supersede-ignore-delete-fail"
PERMISSION_ALLOW_ALL_POINTER_TYPES = "allow-all-pointer-types"
PERMISSIONS_FILENAME = "permissions.toml"


PRODUCER_URL_PATH = "/producer/FHIR/R4/DocumentReference"
Expand All @@ -49,7 +50,7 @@ class PointerTypes(Enum):

@staticmethod
def list():
return list(map(lambda type: type.value, PointerTypes))
return [pointer.value for pointer in PointerTypes]


class Categories(Enum):
Expand Down
8 changes: 2 additions & 6 deletions layer/nrlf/core/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from aws_lambda_powertools.utilities.typing import LambdaContext
from pydantic import BaseModel

from nrlf.core.authoriser import get_pointer_types, parse_permissions_file
from nrlf.core.authoriser import get_pointer_types
from nrlf.core.codes import SpineErrorConcept
from nrlf.core.config import Config
from nrlf.core.constants import PERMISSION_ALLOW_ALL_POINTER_TYPES, PointerTypes
Expand Down Expand Up @@ -73,12 +73,8 @@ def load_connection_metadata(headers: Dict[str, str], config: Config):
return metadata

logger.log(LogReference.HANDLER004b)
pointer_types = parse_permissions_file(metadata)
if not pointer_types and not metadata.is_test_event:
logger.log(LogReference.HANDLER004)
pointer_types = get_pointer_types(metadata, config)

metadata.pointer_types = pointer_types
metadata.pointer_types = get_pointer_types(metadata, config)

return metadata

Expand Down
7 changes: 6 additions & 1 deletion layer/nrlf/core/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,16 @@ class ClientRpDetails(BaseModel):

class ConnectionMetadata(BaseModel):
pointer_types: list[str] = Field(alias="nrl.pointer-types", default_factory=list)
test_pointer_types: list[str] = Field(
alias="nrl.test-pointer-types", default_factory=list
)
ods_code: str = Field(alias="nrl.ods-code")
ods_code_extension: str | None = Field(alias="nrl.ods-code-extension", default=None)
nrl_permissions: list[str] = Field(alias="nrl.permissions", default_factory=list)
nrl_app_id: str = Field(alias="nrl.app-id")
is_test_event: bool = Field(alias="nrl.test-event", default=False)
load_test_permissions: bool = Field(
alias="nrl.load-test-permissions", default=False
)
client_rp_details: ClientRpDetails

@property
Expand Down
2 changes: 1 addition & 1 deletion layer/nrlf/tests/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create_headers(
"nrl.ods-code": ods_code,
"nrl.permissions": nrl_permissions,
"nrl.app-id": nrl_app_id,
"nrl.test-event": True,
"nrl.load-test-permissions": True,
}
),
"nhsd-client-rp-details": json.dumps(
Expand Down
1 change: 0 additions & 1 deletion layer/test_permissions/Y05868-TestApp-12345678/RQI.json

This file was deleted.

This file was deleted.

1 change: 0 additions & 1 deletion layer/test_permissions/Y05868-TestApp-12345678/X26.json

This file was deleted.

6 changes: 0 additions & 6 deletions layer/test_permissions/Y05868-TestApp-12345678/Y05868.json

This file was deleted.

10 changes: 10 additions & 0 deletions layer/test_permissions/permissions.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[Y05868-TestApp-12345678]
RQI = ["http://snomed.info/sct|736253001"]
TestCode = ["http://snomed.info/sct|736253001"]
X26 = ["http://snomed.info/sct|861421000000109"]
Y05868 = [
"http://snomed.info/sct|736253001",
"http://snomed.info/sct|736253002",
"http://snomed.info/sct|1363501000000100",
"http://snomed.info/sct|861421000000109"
]
Loading