CCM-12777: remove component quarantine bucket#931
Conversation
This reverts commit bffe6e9. Co-authored-by: Copilot <copilot@github.com>
…ps://github.com/NHSDigital/nhs-notify-web-template-management into feature/ccm-12777-remove-comp-quarantine-bucket
| module.s3bucket_quarantine.id | ||
| ] | ||
| objectKey = [ | ||
| { prefix = "${var.environment}/proofs/" }, |
There was a problem hiding this comment.
outside of the scope of this PR (i didn't get round to reviewing the previous one) but is var.environment okay as a prefix when we sometimes share environment names between app and sandbox?
There was a problem hiding this comment.
I did consider adding in the component type to the hierarchy. But given it's only dev environments this applies to and that realistically you'll only be using sbx or app at any given time I didn't think it needed an additional prefix.
But it does mean that files uploaded the via sbx or app will trigger both app and sbx lambdas. So maybe there is something to be said there? What do you think?
There was a problem hiding this comment.
Hm... if it does trigger both lambdas then maybe it will cause an issue...
Let say both the app and sbx lambda both attempt to move the file from the bucket then they'll be fighting over it.
There was a problem hiding this comment.
i'm not a huge fan of environments being able to affect each other, i think the fact that uploading files on an sbx environment can cause files to appear on an app environment is a problem, even if you're not using both at the same time, you'll return to your app environment to find it has a bunch of stuff from when you were using sbx which doesn't seem ideal. if you really like just having the environment as the prefix, i think we'll need to add a check before deploying any app/sbx that there's no other app/sbx environment sharing the name, and get everyone to rename their sbx environments, but just using CSI as the prefix makes sense to me.
There was a problem hiding this comment.
Yeah I agree - thanks for highlighting. I'll update the PR.
There was a problem hiding this comment.
Updated to use local.csi
bhansell1
dismissed stale reviews from andykay-nhs, sidnhs, and jamesthompson26-nhs
via
d2cd402
bhansell1
force-pushed
the
feature/ccm-12777-remove-comp-quarantine-bucket
branch
from
d2cd402 to
2b1cff1
Compare
…emplate-management into feature/ccm-12777-remove-comp-quarantine-bucket
5 participants
Description
envto usingcsi. This is because using justenvthenappandsbxenvironment will read from the same bucket and clashContext
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.