feat: add reusable build-and-push-to-ECR workflow#17
Draft
ashwinimanoj-habuild wants to merge 8 commits into
Draft
feat: add reusable build-and-push-to-ECR workflow#17ashwinimanoj-habuild wants to merge 8 commits into
ashwinimanoj-habuild wants to merge 8 commits into
Conversation
Centralizes Docker build + ECR push for all services. Tags images with git SHA (for K8s/ArgoCD) and latest (for ECS backwards compat). Supports single-service repos, mono-repos with multiple services, and worker services from the same repo via configurable inputs. Includes: OIDC auth, SDK_TOKEN for private npm, submodules, ClickUp notifications, and input validation.
Anshulhabuild
left a comment
There was a problem hiding this comment.
Summary
This pull request introduces a reusable GitHub Actions workflow that builds a Docker image and pushes it to Amazon ECR. It supports a variety of repo configurations (single-service, mono-repo, and workers) and includes optional notifications to ClickUp. Overall, the workflow is designed to be flexible and parameter-driven, allowing the configuration of build arguments, Dockerfile location, and AWS credentials dynamically.
✅ Good Practices
- The workflow provides clear usage examples and detailed inline documentation at the top, making it easy for other developers to integrate and understand.
- Input validation is implemented early in the workflow (e.g., verifying that the service name matches a regex and that the environment is one of a fixed set), which helps catch configuration mistakes early.
- The use of AWS OIDC for credential configuration (via aws-actions/configure-aws-credentials) promotes secure, short-lived credentials.
- The workflow leverages Docker tagging best practices by tagging images with both the Git SHA (for reproducibility) and
latest(for ease of deployment). - The conditional steps, such as configuring npm for private packages and sending notifications to ClickUp, are well-contained with clear conditions.
🚨 Critical Issues
- None of the steps appear to create significant security vulnerabilities. However, it is crucial to ensure that any token used (e.g., SDK_TOKEN, CLICKUP_TOKEN) is properly masked and that the credentials are rotated regularly.
- The direct use of secrets in build arguments (like passing SDK_TOKEN into the Docker build) should be carefully monitored to avoid inadvertently exposing these secrets in logs if not handled securely within the Dockerfile.
⚠️ Suggestions
- Consider adding error handling or more informative logging for the Docker build step, so that failures in building or pushing images are easier to diagnose.
- In the "Notify ClickUp" step, if the curl command fails, a final log or alert could be helpful even if the error is deemed non-blocking.
- The determination of the Dockerfile location using an inline expression might be abstracted into a separate script or step to improve clarity, especially for maintainers not already familiar with GitHub Actions expressions.
📝 Minor Issues
- The environment variable mappings in the steps are clear, but adding additional inline comments for variables (like ECR_REPO and SHA_TAG) might improve readability for new contributors.
- The regex for the service name only allows lowercase letters and hyphens; ensure this aligns with how service names are defined across the project.
- The condition for checking submodules uses a ternary-style pattern in the inputs, which might be simplified or more explicitly documented to avoid misunderstandings.
- Consider using double quotes for all environment variable usages in shell scripts to further guard against potential word splitting or globbing issues.
Questions
- Is there a specific reason for the default
notify-clickupinput being set to true? Under what circumstances might a user want to disable this, aside from leaving the CLICKUP_TOKEN secret empty? - Are there plans to include additional error handling if the Docker push commands fail, or is the current level of logging deemed sufficient for troubleshooting?
- For the dynamic Dockerfile determination in the "Set image metadata" step, would it be beneficial to validate that the provided Dockerfile path exists before proceeding with the build?
These comments should guide further refinements and potential discussion points. Overall, the changes improve the CI/CD process in a clear and flexible manner.
Security fixes:
- H-1: Replace raw docker build with docker/build-push-action, eliminating
shell injection via extra-build-args (now a proper multiline input)
- H-2: Move inputs.dockerfile to env var, add path validation regex
- H-3: SDK_TOKEN no longer exposed in image layers (build-push-action
handles build-args without persisting in layer metadata)
- H-5: Pin all actions to immutable SHA digests
- M-1: Use jq for JSON construction in ClickUp notifications
- M-3: Validate aws-account-id is exactly 12 digits
- M-5: Add top-level permissions: {} for least-privilege
Architecture fixes:
- H-6: BuildKit with GHA cache for fast cached builds
- H-7: Auto-create ECR repo if it doesn't exist (with scan-on-push)
- H-8: Use full 40-char SHA as canonical tag (short SHA kept as alias)
- M-6: Remove unused node-version input
- M-7: Use official aws-actions/amazon-ecr-login action
- L-1: Default use-submodules to false
- L-5: Add OCI labels for build provenance
Operations fixes:
- H-9: Add timeout-minutes: 30
- H-10: Add concurrency group to prevent tag races
- H-11: Atomic push via build-push-action (single manifest operation)
- H-12: Add failure notification to ClickUp
- M-9: Skip build if SHA tag already exists in ECR
- L-4: Add set -euo pipefail to shell steps
Move validation, ECR repo management, metadata computation, and ClickUp notifications into scripts/build-push/. The workflow YAML is now declarative — it wires inputs, secrets, and script calls. Scripts are testable locally, lintable with shellcheck, and produce clean diffs independent of YAML formatting.
Replace the reusable workflow (workflow_call) with a composite action.
Scripts are now accessible via $GITHUB_ACTION_PATH — no need to
checkout the .github repo separately.
Composite actions run inside the caller's job (same runner), so
outputs and files are shared naturally. Callers can add steps
after the build (e.g., ECS task definition update).
Structure:
.github/actions/build-push-ecr/
action.yml # inputs, outputs, steps
scripts/
validate.sh # input validation
set-metadata.sh # ECR repo URI, tags
ensure-ecr-repo.sh # auto-create ECR repo
check-existing-image.sh # skip if SHA already in ECR
notify-clickup.sh # success/failure via jq
Caller syntax changes from:
uses: habuildserver/.github/.github/workflows/build-push-ecr.yaml@main
to:
uses: habuildserver/.github/.github/actions/build-push-ecr@main
The composite action builds whatever Dockerfile it's handed — it cannot verify the resulting image is actually K8s-deployable. Add a concise section naming the four baseline constraints services must meet: 1. No env secrets baked into image layers (.dockerignore gate) 2. Runs cleanly with readOnlyRootFilesystem: true 3. No build-time migrations or integration tests 4. SHA tags are authoritative; :latest is ECS-compat only Points at the tactical runbook in habuild-k8s-gitops for the Dockerfile / .dockerignore templates and the strategic plan doc in infra-plans for the rollout sequencing. Motivated by the chat-service rollout on 2026-04-19 which surfaced all three build-time/runtime conflicts when the existing ECS-era Dockerfile (shipping .env.* into /app via COPY . ., running prisma migrate at build time) met the archetype's readOnlyRootFilesystem + CSI subPath invariants. Capturing the constraints in the build-action README so the next team onboarding discovers them before a failed sync, not during it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
latestfor backwards compatibility with ECSaws-account-idis a required input since it differs per environmentStructure
Features
Usage
Migration path
Each service replaces its inline build+push steps with a single composite action call. ECS deploys keep working (latest tag), while K8s/ArgoCD uses the SHA tag.