multi arch build related changes #1699
Conversation
* Fixed build github actions workflow to use existing build configs * fixed directory issue * Fixed db work dir not found issue * enabled all build configurations and added all services options to build * added cache config * added dockerfile to project factory build config * added default dockerfile option * project factory dockerfile fix * Fixed default value not set for workflow * added parallel build workflows * updated workflow * updated workflow * updated workflow * updated workflow * updated workflow * updated workflow * updated workflow
nitish-egov
requested review from
jagankumar-egov,
kavi-egov and
sathishp-eGov
as code owners
WalkthroughThe build pipeline's GitHub Actions workflow was modularized and enhanced with architecture-aware builds, manifest creation, and improved configuration resolution. Several Dockerfile paths in build configurations were updated, a Dockerfile's script copy path was changed, a new build argument was introduced in a service's Dockerfile to support flexible work directory copying, and a base image version was updated in a database-related Dockerfile. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: in the core-services/beneficiary-idgen/pom.xml file, the flyway dependency explicitly specifies vers...Applied to files:
📚 Learning: in the 'health-campaign-services' project, it is acceptable to keep the old mdms endpoint `/egov-mdm...Applied to files:
🪛 Checkov (3.2.334)health-services/project/src/main/resources/db/Dockerfile[LOW] 1-9: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [LOW] 1-9: Ensure that a user for the container has been created (CKV_DOCKER_3) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
health-services/project-factory/Dockerfile (1)
11-23:WORK_DIRis only half-applied – will bloat context and missyarn.lock
- The second
COPYstill usesCOPY . ., pulling the entire repo into the image instead of just the service folder.yarn.lockis no longer copied, so deterministic installs are lost.-# Copy package.json and yarn.lock (if exists) -COPY ${WORK_DIR}/package.json ./ +# Copy package.json + lockfile +COPY ${WORK_DIR}/package.json ./ +COPY ${WORK_DIR}/yarn.lock ./ # optional – won’t fail if absent ... -# Copy the rest of the application code -COPY . . +# Copy the service source only +COPY ${WORK_DIR}/ .This keeps the build context lean and preserves reproducible dependency graphs.
build/build-config.yml (1)
217-323: Trailing spaces all over the YAML – clean up before it bites CI
yamllintis already complaining (e.g. Line 300). Whitespace-only diffs break blame history and can fail stricter pipelines.- dockerfile: "build/maven-java17/Dockerfile" + dockerfile: "build/maven-java17/Dockerfile"Run
yamllint -d "{extends: default, rules: {trailing-spaces: {level: error}}}" build/build-config.ymland trim trailing blanks across the file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yaml(3 hunks)build/build-config.yml(12 hunks)build/maven/Dockerfile(1 hunks)health-services/project-factory/Dockerfile(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: kavi-egov
PR: egovernments/health-campaign-services#1546
File: .github/workflows/build.yaml:92-99
Timestamp: 2025-05-12T09:43:49.129Z
Learning: When reviewing GitHub Actions workflows with Docker builds, remind the user about properly configuring Docker layer caching by adding `cache-from: type=local,src=/tmp/.buildx-cache` and `cache-to: type=local,dest=/tmp/.buildx-cache,mode=max` to the docker/build-push-action step to optimize build performance.
Learnt from: devdatta-egov
PR: egovernments/health-campaign-services#784
File: health-services/resource-estimation-service/src/main/java/org/egov/processor/util/BoundaryUtil.java:33-44
Timestamp: 2024-10-08T20:11:07.772Z
Learning: devdatta-egov prefers using generic exception handlers in methods to potentially handle a broader range of exceptions.
Learnt from: devdatta-egov
PR: egovernments/health-campaign-services#784
File: health-services/resource-estimation-service/src/main/java/org/egov/processor/util/BoundaryUtil.java:33-44
Timestamp: 2024-06-26T19:56:55.913Z
Learning: devdatta-egov prefers using generic exception handlers in methods to potentially handle a broader range of exceptions.
health-services/project-factory/Dockerfile (1)
Learnt from: kavi-egov
PR: #1546
File: .github/workflows/build.yaml:92-99
Timestamp: 2025-05-12T09:43:49.129Z
Learning: When reviewing GitHub Actions workflows with Docker builds, remind the user about properly configuring Docker layer caching by adding cache-from: type=local,src=/tmp/.buildx-cache and cache-to: type=local,dest=/tmp/.buildx-cache,mode=max to the docker/build-push-action step to optimize build performance.
build/build-config.yml (12)
Learnt from: Priyanka-eGov
PR: #934
File: health-services/plan-service/src/main/resources/application.properties:54-54
Timestamp: 2024-10-03T10:08:20.045Z
Learning: In the 'health-campaign-services' project, it is acceptable to keep the old MDMS endpoint /egov-mdms-service/v1/_search in the following files, and updating them is not required:
health-services/referralmanagement/src/main/resources/application.propertieshealth-services/transformer/src/main/resources/application.propertieshealth-services/stock/src/main/resources/application.propertieshealth-services/resource-estimation-service/src/main/resources/application.propertieshealth-services/individual/src/main/resources/application.propertieshealth-services/product/src/main/resources/application.propertieshealth-services/project/src/main/resources/application.propertieshealth-services/household/src/main/resources/application.propertieshealth-services/facility/src/main/resources/application.propertiescore-services/pgr-services/src/main/resources/application.propertiescore-services/egov-hrms/src/main/resources/application.properties- `cor...
Learnt from: holashchand
PR: #1599
File: health-services/individual/src/main/java/org/egov/individual/service/IndividualService.java:158-166
Timestamp: 2025-06-17T08:53:38.146Z
Learning: The individual service in the health-campaign-services project is using Java 17.
Learnt from: Priyanka-eGov
PR: #902
File: health-services/plan-service/src/main/java/digit/service/validator/PlanConfigurationValidator.java:553-553
Timestamp: 2024-10-08T20:11:07.772Z
Learning: The plan-service project is using Java 17.
Learnt from: holashchand
PR: #1599
File: health-services/household/src/main/resources/household-persister.yml:176-189
Timestamp: 2025-06-13T08:53:48.057Z
Learning: The file health-services/household/src/main/resources/household-persister.yml (and similar persister YAMLs in that module) are intended for development use only and do not impact production deployments.
Learnt from: holashchand
PR: #1466
File: health-services/libraries/health-services-models/src/main/java/org/egov/common/models/service/ServiceCriteria.java:42-44
Timestamp: 2025-03-27T05:59:20.332Z
Learning: In the health-campaign-services repository, extra blank lines in the service model classes (such as in ServiceCriteria.java) should be preserved for consistency with the existing service-request service implementation.
Learnt from: kanishq-egov
PR: #847
File: health-services/libraries/health-services-common/pom.xml:11-11
Timestamp: 2024-08-12T06:07:15.114Z
Learning: Not all services in the health campaign services project are required to move to newer versions like 1.0.17-dev-SNAPSHOT, as per kanishq-egov's guidance.
Learnt from: Sreejit-K
PR: #1577
File: core-services/beneficiary-idgen/src/main/resources/application.properties:76-83
Timestamp: 2025-05-29T06:53:25.851Z
Learning: In the health-campaign-services project, configuration properties should follow existing patterns throughout the codebase rather than using environment variable placeholders, to maintain consistency across the project.
Learnt from: shubhang-eGov
PR: #1387
File: health-services/referralmanagement/pom.xml:54-54
Timestamp: 2025-02-11T09:43:42.872Z
Learning: Version updates for health-services-models in referral management service require QA verification to ensure no breaking changes between versions.
Learnt from: Sreejit-K
PR: #1577
File: core-services/beneficiary-idgen/pom.xml:63-66
Timestamp: 2025-05-29T06:56:48.083Z
Learning: In the core-services/beneficiary-idgen/pom.xml file, the Flyway dependency explicitly specifies version 9.22.3 as a known and deliberate addition, overriding Spring Boot's managed dependency version.
Learnt from: tanishi-egov
PR: #1337
File: core-services/egov-hrms/src/main/java/org/egov/hrms/service/EmployeeService.java:144-144
Timestamp: 2025-01-16T11:52:14.029Z
Learning: Changes to employeeService in core-services/egov-hrms should be handled by the HCM team rather than the health campaign services team.
Learnt from: holashchand
PR: #1599
File: core-services/beneficiary-idgen/src/main/resources/application.properties:4-6
Timestamp: 2025-06-17T07:08:22.938Z
Learning: In the eGovernments health-campaign-services project, database credentials in application.properties files are hardcoded as default/development values but are always overridden in production deployments via DevOps Helm charts, which is their standard deployment practice for externalizing secrets.
Learnt from: Sreejit-K
PR: #1577
File: core-services/beneficiary-idgen/src/main/resources/db/Dockerfile:1-9
Timestamp: 2025-05-29T07:17:56.555Z
Learning: The eGovernments (digit) platform has established standards for Dockerfiles that may differ from general Docker security best practices. When reviewing Dockerfiles in digit projects, respect their organizational standards rather than suggesting general security improvements like non-root users or health checks.
.github/workflows/build.yaml (1)
Learnt from: kavi-egov
PR: #1546
File: .github/workflows/build.yaml:92-99
Timestamp: 2025-05-12T09:43:49.129Z
Learning: When reviewing GitHub Actions workflows with Docker builds, remind the user about properly configuring Docker layer caching by adding cache-from: type=local,src=/tmp/.buildx-cache and cache-to: type=local,dest=/tmp/.buildx-cache,mode=max to the docker/build-push-action step to optimize build performance.
🪛 YAMLlint (1.37.1)
build/build-config.yml
[error] 300-300: trailing spaces
(trailing-spaces)
.github/workflows/build.yaml
[warning] 4-4: truthy value should be one of [false, true]
(truthy)
[error] 72-72: too many spaces after colon
(colons)
[error] 75-75: too many spaces after colon
(colons)
[error] 76-76: too many spaces after colon
(colons)
[error] 77-77: too many spaces after colon
(colons)
[error] 78-78: too many spaces after colon
(colons)
[error] 79-79: too many spaces after colon
(colons)
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 100-100: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 107-107: trailing spaces
(trailing-spaces)
[error] 112-112: trailing spaces
(trailing-spaces)
[error] 120-120: trailing spaces
(trailing-spaces)
[error] 128-128: trailing spaces
(trailing-spaces)
[error] 132-132: trailing spaces
(trailing-spaces)
[error] 137-137: trailing spaces
(trailing-spaces)
[error] 141-141: trailing spaces
(trailing-spaces)
[error] 147-147: trailing spaces
(trailing-spaces)
[error] 156-156: trailing spaces
(trailing-spaces)
[error] 164-164: trailing spaces
(trailing-spaces)
[error] 175-175: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.7)
.github/workflows/build.yaml
250-250: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
256-256: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
264-264: the runner of "docker/login-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
342-342: the runner of "docker/login-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
build/maven/Dockerfile (1)
6-9: Startup-script path looks correct – no further action required
COPY build/maven/start.sh ./start.shlines up with the new, version-agnostic layout and is referenced correctly in the runtime stage.
No functional or portability issues spotted here. 👍.github/workflows/build.yaml (2)
241-242: Custom runnerubuntu-24.04-arm– verify availabilityGitHub-hosted runners currently expose
ubuntu-latest,ubuntu-22.04, andubuntu-20.04for both QEMU and native arm64. Ensureubuntu-24.04-armexists in your organisation; otherwise the job will queue forever.
347-360: Duplicate step name & wrong headingTwo steps are both called “Add Summary (Application)”, and the second one actually documents the DB image. Rename for clarity.
- - name: Add Summary (Application) + - name: Add Summary (DB)Likely an incorrect or invalid review comment.
| echo "#### DB Config Summary" >> $GITHUB_STEP_SUMMARY | ||
| echo "Application Work Directory - $DB_WORK_DIR" >> $GITHUB_STEP_SUMMARY | ||
| echo "Image Name - $DB_IMAGE_NAME" >> $GITHUB_STEP_SUMMARY | ||
| echo "Dockerfile Path - $SERVICE_DOCKERFILE" >> $GITHUB_STEP_SUMMARY | ||
There was a problem hiding this comment.
DB summary prints service dockerfile – copy-paste slip
echo "Dockerfile Path - $SERVICE_DOCKERFILE" should reference DB_DOCKERFILE.
-echo "Dockerfile Path - $SERVICE_DOCKERFILE" >> $GITHUB_STEP_SUMMARY
+echo "Dockerfile Path - $DB_DOCKERFILE" >> $GITHUB_STEP_SUMMARY📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "#### DB Config Summary" >> $GITHUB_STEP_SUMMARY | |
| echo "Application Work Directory - $DB_WORK_DIR" >> $GITHUB_STEP_SUMMARY | |
| echo "Image Name - $DB_IMAGE_NAME" >> $GITHUB_STEP_SUMMARY | |
| echo "Dockerfile Path - $SERVICE_DOCKERFILE" >> $GITHUB_STEP_SUMMARY | |
| echo "#### DB Config Summary" >> $GITHUB_STEP_SUMMARY | |
| echo "Application Work Directory - $DB_WORK_DIR" >> $GITHUB_STEP_SUMMARY | |
| echo "Image Name - $DB_IMAGE_NAME" >> $GITHUB_STEP_SUMMARY | |
| echo "Dockerfile Path - $DB_DOCKERFILE" >> $GITHUB_STEP_SUMMARY |
🤖 Prompt for AI Agents
In .github/workflows/build.yaml around lines 176 to 180, the echo statement
printing the Dockerfile path incorrectly uses the variable SERVICE_DOCKERFILE
instead of DB_DOCKERFILE. Update the echo command to reference DB_DOCKERFILE to
correctly display the database Dockerfile path in the summary.
| # Check if repo exists | ||
| HTTP_CODE=$(curl -s -o /dev/null -w '%{http_code}' \ | ||
| -H "Authorization: JWT $TOKEN" \ | ||
| "https://hub.docker.com/v2/repositories/$DOCKER_USERNAME/$SERVICE_NAME/") | ||
| if [ "$REPO_CHECK" -ne 200 ]; then | ||
| if [ "$HTTP_CODE" -ne 200 ]; then | ||
| NEXT_TAG="${BRANCH}-${COMMIT_HASH}" | ||
| else | ||
| EXISTING_TAGS=$(curl -s -H "Authorization: JWT $TOKEN" \ | ||
| "https://hub.docker.com/v2/repositories/$DOCKER_USERNAME/$SERVICE_NAME/tags?page_size=100" \ | ||
| | jq -r '.results[].name') | ||
| LATEST_TAG=$(echo "$EXISTING_TAGS" | grep "^${BRANCH}-${COMMIT_HASH}" || true | sort -V | tail -n 1) | ||
| LATEST_TAG=$(echo "$EXISTING_TAGS" \ | ||
| | grep "^${BRANCH}-${COMMIT_HASH}" || true \ | ||
| | sort -V \ | ||
| | tail -n 1) | ||
| NEXT_TAG="${LATEST_TAG:-${BRANCH}-${COMMIT_HASH}}" | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tag generator does not ensure uniqueness – parallel runs can collide
NEXT_TAG simply reuses the latest tag without incrementing a suffix, so concurrent builds for the same commit/branch overwrite each other.
Consider appending a monotonically-increasing build number (or ${{ github.run_number }}) when the base tag already exists.
-if [ "$HTTP_CODE" -ne 200 ]; then
- NEXT_TAG="${BRANCH}-${COMMIT_HASH}"
-else
- ...
- NEXT_TAG="${LATEST_TAG:-${BRANCH}-${COMMIT_HASH}}"
-fi
+if [ "$HTTP_CODE" -ne 200 ] || [ -z "$LATEST_TAG" ]; then
+ NEXT_TAG="${BRANCH}-${COMMIT_HASH}-0"
+else
+ suffix=$(echo "$LATEST_TAG" | awk -F'-' '{print $NF+1}')
+ NEXT_TAG="${BRANCH}-${COMMIT_HASH}-${suffix}"
+fiCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/build.yaml around lines 208 to 224, the tag generation
logic sets NEXT_TAG to the latest existing tag or a base tag without ensuring
uniqueness, causing parallel builds to overwrite each other's tags. To fix this,
modify the script to append a unique, monotonically-increasing identifier such
as the GitHub run number or a build counter to the base tag when it already
exists, ensuring each build produces a distinct tag and preventing collisions.
| docker buildx build \ | ||
| --platform ${{ matrix.platform }} \ | ||
| --build-arg WORK_DIR=${{ needs.resolve-config.outputs.service_work_dir }} \ | ||
| --file ${{ needs.resolve-config.outputs.service_dockerfile }} \ | ||
| --tag egovio/${{ needs.resolve-config.outputs.service_image_name }}:${{ needs.resolve-config.outputs.tag }}-${{ matrix.arch }} \ | ||
| --cache-from=type=local,src=/tmp/.buildx-cache \ | ||
| --cache-to=type=local,dest=/tmp/.buildx-cache,mode=max \ | ||
| --push \ | ||
| --iidfile digest.txt \ | ||
| . |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Full-repo build context is expensive
docker buildx build ... . sends the whole repository; most services only need ${{ needs.resolve-config.outputs.service_work_dir }}. Reducing context slashes build time and cache misses.
- --iidfile digest.txt \
- .
+ --iidfile digest.txt \
+ "${{ needs.resolve-config.outputs.service_work_dir }}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docker buildx build \ | |
| --platform ${{ matrix.platform }} \ | |
| --build-arg WORK_DIR=${{ needs.resolve-config.outputs.service_work_dir }} \ | |
| --file ${{ needs.resolve-config.outputs.service_dockerfile }} \ | |
| --tag egovio/${{ needs.resolve-config.outputs.service_image_name }}:${{ needs.resolve-config.outputs.tag }}-${{ matrix.arch }} \ | |
| --cache-from=type=local,src=/tmp/.buildx-cache \ | |
| --cache-to=type=local,dest=/tmp/.buildx-cache,mode=max \ | |
| --push \ | |
| --iidfile digest.txt \ | |
| . | |
| docker buildx build \ | |
| --platform ${{ matrix.platform }} \ | |
| --build-arg WORK_DIR=${{ needs.resolve-config.outputs.service_work_dir }} \ | |
| --file ${{ needs.resolve-config.outputs.service_dockerfile }} \ | |
| --tag egovio/${{ needs.resolve-config.outputs.service_image_name }}:${{ needs.resolve-config.outputs.tag }}-${{ matrix.arch }} \ | |
| --cache-from=type=local,src=/tmp/.buildx-cache \ | |
| --cache-to=type=local,dest=/tmp/.buildx-cache,mode=max \ | |
| --push \ | |
| --iidfile digest.txt \ | |
| "${{ needs.resolve-config.outputs.service_work_dir }}" |
🤖 Prompt for AI Agents
In .github/workflows/build.yaml around lines 271 to 280, the docker buildx
command uses the full repository context ('.'), which is inefficient. Change the
build context from '.' to the specific service work directory referenced by ${{
needs.resolve-config.outputs.service_work_dir }} to limit the build context to
only the necessary files, reducing build time and cache misses.
3 participants
Summary by CodeRabbit
New Features
Improvements
Chores