Check in role manager dependencies #493
Conversation
lorenyu
requested review from
charles-nava,
jamesbursa,
rocketnova and
shawnvanderjagt
There was a problem hiding this comment.
LGTM
In the long term, I think this is less ideal because we're checking in specific versions of these packages. Do we already have a process for updating these?
I still have it on my radar to investigate alternative approaches to handling the role manager. In the meantime, I think this is a good move.
|
@rocketnova good point. some quick ideas:
thoughts on any of those? |
|
@jamesbursa also had the idea to use one of the pre-existing lambda layers that seems to have pg8000 installed already by default |
|
@rocketnova I forgot this PR existed. It looks like we decided back in December to check in role manager dependencies, but I never merged this. That would allow us to:
Thoughts? |
|
@lorenyu I also forgot that we already made a decision on this. Let's move forward with this as the short-term solution and keep our eye on a long-term solution that will allow us to remove this. |
- Remove role manager dependencies from source control - Add role manager archive to source control - Add make command for building role manager archive - Add documentation on how to update role manager ## Context The “check in role manager dependencies” change in [PR 493](#493) does not function as desired. The role manager lambda still shows a diff on the source_code_hash on a clean checkout of the repo. This change removes the archive_file data source altogether, and checks in the zip archive directly into the codebase. Benefits include: - We no longer need to check in the python dependencies into source control, which is a ton of files including binary files - There’s nothing nondeterministic for project teams, the archive file is exactly the zip package that gets deployed to lambda The downside is that updating the role manager is more manual. So this change also adds a `make infra-module-database-role-manager-archive` to rebuild the role manager package and adds instructions for doing that.
Ticket
n/a
Changes
Context for reviewers
A number of people were confused by the fact that the terraform plan is never clean due to the triggers_replace = timestamp(). There were also concerns about the dependency on pip3. This change adds the role manager dependency to the source code, which eliminates the need for the local-exec provisioner step and also removes the dependency on pip3. The cost is that there is now some vendor code in the /modules/database/role_manager/vendor folder.
Testing
Developed and tested in navapbc/platform-test#71