Verify build fix with prisma field encryption inclusion #65
Conversation
This is a temp fix to see if this will fix the noted buidl issue. prisma field encryption variables seems to be needed during the build process and currently fails without that value being set A short term solution is to populate it is build time, but this needs to be refactored as this presents a securoty issue
WalkthroughThis pull request introduces two new environment variables for field encryption in Prisma within the Changes
Possibly related PRs
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .env.production.example (1 hunks)
- .github/workflows/run-tests.yml (2 hunks)
Additional context used
Gitleaks
.env.production.example
25-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (2)
.github/workflows/run-tests.yml (2)
11-11: LGTM!The addition of the
fix/production-build-prisma-field-encryptionbranch to thepull_requesttrigger aligns with the PR objective to verify the build fix. This change ensures that the workflow runs for pull requests targeting this branch.
31-66: Great addition to the CI/CD pipeline!The new
build-and-pushjob is a valuable enhancement to the workflow. It automates the process of building and pushing Docker images based on the branch being used, which facilitates better version control and deployment strategies.The conditional logic based on the branch name ensures that the appropriate image tag is used, and the use of the
docker/build-push-actionand secrets for DockerHub authentication follows recommended practices.This addition will reduce manual effort and potential errors in the deployment process.
The branch name wasnt being returned when creating PRs, this is only returned when pushed into main
.github/workflows/run-tests.yml
Outdated
| uses: docker/build-push-action@v5 | ||
| with: | ||
| platforms: linux/amd64,linux/arm64 | ||
| push: true |
There was a problem hiding this comment.
Should we not check if it is nit on pull request before we push the image?
There was a problem hiding this comment.
@jacob-khoza-symb Yup thats correct, I still need to make some updates on this action script.
The current changes are just intended for testing purposes to have an image build and sent to dockerhub to verify the build works as intended (without having to merge into master multiple times)
Side note: The entire GHA needs a bit of refactoring and improvement
To test and build the image on branches to verify image build succeed Only push to Dockerhub on main branch build
To fix a few issues with the build pipeline. Also to attempt to speed up the build process
…o fix/production-build-prisma-field-encryption
…te-docker-production-image-build
Update docker production image build
Also removed the temp change for trigger the build/push on a feature branch (This was purely added for testing without merging into main)
The docker build test doesnt need multiplatform builds. if it succeeds on one, thats should be fine
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .github/workflows/run-tests.yml (0 hunks)
- .github/workflows/tests-build-push.yml (2 hunks)
- docker/production/Dockerfile (3 hunks)
Files not reviewed due to no reviewable changes (1)
- .github/workflows/run-tests.yml
Additional context used
actionlint
.github/workflows/tests-build-push.yml
42-42: property "get_branch" is not defined in object type {}
(expression)
50-50: property "get_branch" is not defined in object type {}
(expression)
57-57: property "get_branch" is not defined in object type {}
(expression)
Additional comments not posted (8)
docker/production/Dockerfile (3)
6-6: LGTM!The addition of
python3,make, andg++packages to theapk addcommand in thedepsstage is a good change. These are common build tools and dependencies that are likely needed for compiling native modules or other build tasks.Installing these packages in the
depsstage helps keep the final image size small.
23-23: Great optimization!Updating the build command to use
npm run build --turbois an excellent change. The--turboflag suggests the project is leveraging TurboRepo for optimized builds.TurboRepo can significantly improve build performance and efficiency, especially for larger projects. This change is likely to lead to faster build times and an improved developer experience, without introducing any bugs or regressions.
45-46: Improved clarity and consistency!Reformatting the environment variable declarations for
PORTandHOSTNAMEto use the=syntax instead of spaces is a good change. This is a more standard practice in Dockerfiles and improves the clarity and consistency of the environment variable definitions.While this change doesn't alter the functionality, it enhances the readability and maintainability of the Dockerfile.
.github/workflows/tests-build-push.yml (5)
1-1: LGTM!The new workflow name accurately reflects the inclusion of the testing step, aligning with the PR objective.
4-9: LGTM!The updated triggering events ensure that the workflow runs on both
pushandpull_requestevents for themainbranch, which is a sensible configuration.
12-27: LGTM!The new
testjob is a valuable addition to the workflow, ensuring that tests are run before building and pushing the Docker image. The job configuration and steps are well-structured and appropriate for the purpose of verifying the build fix.
50-50: Fix the undefinedget_branchproperty.Similar to the previous comment, the static analysis tool has identified that the
get_branchproperty is not defined in the object type. This could cause the workflow to fail when running on themainbranch.To fix this issue, define the
get_branchstep as suggested in the previous comment and update the condition to use theget_branchoutput:if: ${{ steps.get_branch.outputs.branch_name == 'main' }}Tools
actionlint
50-50: property "get_branch" is not defined in object type {}
(expression)
57-57: Fix the undefinedget_branchproperty.Once again, the static analysis tool has identified that the
get_branchproperty is not defined in the object type. This could cause the workflow to fail when running on themainbranch.To fix this issue, define the
get_branchstep as suggested in the previous comments and update the condition to use theget_branchoutput:if: ${{ steps.get_branch.outputs.branch_name == 'main' }}Tools
actionlint
57-57: property "get_branch" is not defined in object type {}
(expression)
BMartinos
changed the title
This is a temp fix to see if this will fix the noted buidl issue. prisma field encryption variables seems to be needed during the build process and currently fails without that value being set A short term solution is to populate it is build time, but this needs to be refactored as this presents a securoty issue
Summary by CodeRabbit
New Features
Chores