ci(images): push slurm-operator/-webhook to ECR instead of Docker Hub - #45
Conversation
Moves the 1.0-line container-images workflow off Docker Hub onto this account's ECR (651706779278.dkr.ecr.us-west-2.amazonaws.com), the same registry/role openldap-sync uses: - REGISTRY now points at the ECR URL (docker-bake.hcl tags both the operator and webhook targets off this same variable, so one change covers both images) - Replace the Docker Hub login step with configure-aws-credentials + amazon-ecr-login (github-actions-oidc-role, OIDC) - Move AWS auth ahead of the existing 'refuse to overwrite stable image tags' step, since imagetools inspect now needs registry read access - Broaden that step's missing-tag detection for ECR's error text (ECR reports 'name unknown' / 'does not exist' rather than Docker Hub's 'manifest unknown' when a repo has never been pushed to) Trust policy + ECR repo creation is in togethercomputer/tcloud-infra#6600. Fixes TCL-9443
Broly Security ScanWarning Latest baseline snapshot is stale. Broly is running in PR-only fallback mode until the next scheduled baseline refreshes. This does not block the PR. Note ✅ Clean scan Note Re-scan this PR anytime with
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔵 Needs a closer look
The ECR/OIDC migration depends on external trust configuration and first-run CI validation.
Pull request overview
Moves 1.0 container image publishing from Docker Hub to AWS ECR using GitHub OIDC authentication.
Changes:
- Updates registry and AWS configuration.
- Replaces Docker Hub login with AWS credential and ECR login actions.
- Handles ECR-specific missing-image responses.
File summaries
| File | Summary | Review comment |
|---|---|---|
.github/workflows/container-images-1.0.yaml |
Migrates image publishing and tag verification to ECR. | Nit: rename the Docker Hub-specific step to refer to ECR or image publishing. |
Review details
Suppressed comments (1)
.github/workflows/container-images-1.0.yaml:30
- Now that this workflow publishes to ECR, the existing
Whether to push to Docker Hubstep name is inaccurate and will mislead anyone reading the Actions run. Please rename that step to refer to ECR or to image publishing.
REGISTRY: 651706779278.dkr.ecr.us-west-2.amazonaws.com
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Broly flagged the two new actions as unpinned (blanket policy in this repo requires hash-pinned actions). Pins to the same commit hashes already used for these actions elsewhere in the org (tcloud-infra, tcloud). The AWS_ROLE env var is also flagged as a potential secret (ARN regex match) -- false positive, same non-sensitive role ARN pattern already present unchanged in ~30 other workflows across the org (openldap-sync, tcloud, tcloud-infra, together-cloud-cluster-operator, etc.).
There was a problem hiding this comment.
🔵 Needs a closer look
External OIDC/IAM and ECR prerequisites require first-run validation, and a version annotation nit remains.
Review details
Suppressed comments (1)
.github/workflows/container-images-1.0.yaml:113
- The version annotation is incorrect: commit
7474bc4690e29a8392af63c5b98e7449536d5c3ais theconfigure-aws-credentialsv4.3.1 release, not v4.0.2. Please update the comment or pin the intended v4.0.2 commit so dependency audits do not use stale version information.
uses: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a # v4.0.2
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Moves the 1.0-line
container-images-1.0.yamlworkflow off Docker Hub onto this account's ECR (651706779278.dkr.ecr.us-west-2.amazonaws.com), the same registry/roleopenldap-syncuses.Both images (
slurm-operator,slurm-operator-webhook) are built from this one repo viadocker-bake.hcl'soperator/webhooktargets, both tagged off the sameREGISTRYvariable — so this is a single-file change.Changes
REGISTRYenv now points at the ECR URL instead oftogethercomputer(Docker Hub org)docker/login-action(Docker Hub) withaws-actions/configure-aws-credentials+aws-actions/amazon-ecr-login(OIDC viagithub-actions-oidc-role)docker buildx imagetools inspectnow needs registry read access to check the current tagsDependencies
repo:togethercomputer/slurm-operator:*and theslurm-operator/slurm-operator-webhookECR repositories (Terraform): tcloud-infra#6600Verification
configure-aws-credentials+amazon-ecr-loginworkflows for conventionFixes TCL-9443