Skip to content

VED-867: Change Secret Location#1282

Closed
Akol125 wants to merge 3 commits intostaging/VED-16-mns-vacc-event-notificationsfrom
VED-876-change-secret-location
Closed

VED-867: Change Secret Location#1282
Akol125 wants to merge 3 commits intostaging/VED-16-mns-vacc-event-notificationsfrom
VED-876-change-secret-location

Conversation

@Akol125
Copy link
Copy Markdown
Contributor

@Akol125 Akol125 commented Mar 11, 2026

Summary

  • Routine Change
    Change secret manager location in codebase to reflect the changes made to AWS Secret Manager private Key.

Add any other relevant notes or explanations here. Remove this line if you have nothing to add.

Reviews Required

  • Dev
  • Test
  • Tech Author
  • Product Owner

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all of the acceptance criteria of the ticket.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • If there were changes that are outside of the regular release processes e.g. account infrastructure to setup, manual setup for external API integrations, secrets to set, then I have checked that the developer has flagged this to the Tech Lead as release steps.
  • I have checked that no Personal Identifiable Data (PID) is logged as part of the changes.

@github-actions
Copy link
Copy Markdown
Contributor

This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket:

VED-876

@Akol125 Akol125 changed the base branch from master to staging/VED-16-mns-vacc-event-notifications March 11, 2026 14:57
self.secret_name = (
f"imms/pds/{environment}/jwt-secrets"
f"imms/outbound/{environment}/jwt-secrets"
if service == Service.PDS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct - isnt environment int only, not dev etc?
Please correct me if I'm wrong, I thought we have the same secret name for int and dev?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we have set the environment variables for all our external api clients in terraform tf.vars and it would either only be int for dev and preprod environment or prod for prod environment. so this {environment} here is passed as a parameter that is derived whenever AppRestrictedAuth is instantiated either through mns or pds.

@Akol125 Akol125 changed the title Ved 876 change secret location VED-867: Change Secret Location Mar 11, 2026
Comment thread lambdas/shared/src/common/api_clients/authentication.py Outdated
Copy link
Copy Markdown
Contributor

@dlzhry2nhs dlzhry2nhs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we have done so for the new INT app (as we established this morning), but have we done the same for the PROD one?

I would like us to NOT merge this until it the pre-requisite is done for both apps so this can be released at any time. Otherwise this will mean we will have to cut around it on master.

Additionally, this work is not intrinsically part of the MNS work, so it should not point at the staging branch. It should point at the master branch. If we bundle this up with the MNS work, it will mean that we then add another dependency on our old PDS and MNS permissions being replicated to the new app.

It might be better to leave flexibility on the MNS work to use either the new or old apps (in case the process for the old app takes a while).

Can disregard the above comment. Decision seems to be to use the new app for MNS Publish and in parallel get the existing permissions of the old app added to the new one. No problem with this, just need to be mindful to ensure we are progressing those tasks so the work does not get blocked.

I am happy with the changes.

@dlzhry2nhs dlzhry2nhs self-requested a review March 12, 2026 09:19
@dlzhry2nhs dlzhry2nhs dismissed their stale review March 12, 2026 09:19

Dismissing my review. I will no longer be responsible for approving or requesting changes on PRs, as my last day is tomorrow.

@sonarqubecloud
Copy link
Copy Markdown

@dlzhry2nhs
Copy link
Copy Markdown
Contributor

Closing as changes were covered off by: #1278

@dlzhry2nhs dlzhry2nhs closed this Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants