-
Notifications
You must be signed in to change notification settings - Fork 686
build(actions): AS-771 build internal images for vuln scannings #6144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new GitHub Actions workflow is added to automate building and pushing internal Docker images for security scanning. The workflow supports manual and scheduled triggers, authenticates with Google Cloud, sets up necessary build environments, builds the Docker image using project-specific arguments, and pushes the image to the internal Google Cloud Artifact Registry. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Google Cloud
participant Docker Registry
Developer->>GitHub Actions: Trigger workflow (manual or scheduled)
GitHub Actions->>GitHub Actions: Checkout repository
GitHub Actions->>Google Cloud: Authenticate (Workload Identity/Secrets)
GitHub Actions->>GitHub Actions: Set up Google Cloud SDK and Docker auth
GitHub Actions->>GitHub Actions: Extract build metadata (version, SHA, date)
GitHub Actions->>GitHub Actions: Set up Docker Buildx
GitHub Actions->>Docker Registry: Build and push Docker image with tags
GitHub Actions->>Docker Registry: Generate and push SBOM
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/build-docker-internal.yml (4)
22-23: Add aconcurrencyblock to avoid overlapping weekly buildsA long-running manual build kicked off shortly before the cron job will cause two identical image pushes.
Including a concurrency group (e.g.concurrency: build-internal-image) withcancel-in-progress: trueprevents that race.
35-35: Harden Docker auth step against region typos
gcloud auth configure-docker "${{ env.GCP_LOCATION }}-docker.pkg.dev"fails silently if the location is wrong.
A quickset -euo pipefailor explicitgcloud artifacts repositories list --location="$GCP_LOCATION"check can fail fast and save minutes of CI time.
44-46: Leverage Buildx cache to cut repeated build timeAdding
cache-from/cache-towith the same registry significantly speeds this weekly job with negligible cost.Example:
- name: Build and push internal image uses: docker/build-push-action@v6 with: cache-from: type=registry,ref=${{ env.GCP_LOCATION }}-docker.pkg.dev/.../fiftyone:cache cache-to: type=registry,ref=${{ env.GCP_LOCATION }}-docker.pkg.dev/.../fiftyone:cache,mode=max
54-58: Remove trailing spaces to satisfy YAML-lintLine 55 is flagged by YAML-lint (
trailing-spaces).
Strip the extra space afterscanning.to keep CI green.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-docker-internal.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kevin-dimichel
PR: voxel51/fiftyone#5173
File: .github/workflows/build-docs.yml:54-54
Timestamp: 2024-11-21T20:59:53.855Z
Learning: The `RO_FOT_FG_PAT` token used in `.github/workflows/build-docs.yml` has the necessary permissions to checkout the `voxel51/fiftyone-teams` repository.
.github/workflows/build-docker-internal.yml (1)
Learnt from: kevin-dimichel
PR: voxel51/fiftyone#5173
File: .github/workflows/build-docs.yml:54-54
Timestamp: 2024-11-21T20:59:53.855Z
Learning: The `RO_FOT_FG_PAT` token used in `.github/workflows/build-docs.yml` has the necessary permissions to checkout the `voxel51/fiftyone-teams` repository.
🪛 YAMLlint (1.37.1)
.github/workflows/build-docker-internal.yml
[error] 55-55: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/build-docker-internal.yml (2)
4-6: Cron schedule runs in UTC — confirm it matches the intended 04:05 local timeGitHub Actions interprets cron expressions in UTC.
If the 04:05 schedule is meant to be US-central time (or any other TZ), it will actually fire at a different local hour. Consider adding a comment clarifying the UTC intent or switching to a workflow-dispatch plus a separate, time-zone–aware scheduler.
61-62: Guard against unsafe characters sneaking into the tagIf
fo_versionever contains a pre-release suffix (e.g.1.8.0-beta), Docker will happily accept it, but downstream scanners sometimes choke on multiple dashes. Consider normalising or URL-encoding the tag components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/build-docker-internal.yml (1)
38-43: ParsingVERSIONremains fragile – see previous review
The earlier suggestion to replace thegrep | awkchain with a small Python snippet (orpython -m setup --version) hasn’t been incorporated, so the command will still break on minor formatting changes or single-quotes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-docker-internal.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kevin-dimichel
PR: voxel51/fiftyone#5173
File: .github/workflows/build-docs.yml:54-54
Timestamp: 2024-11-21T20:59:53.855Z
Learning: The `RO_FOT_FG_PAT` token used in `.github/workflows/build-docs.yml` has the necessary permissions to checkout the `voxel51/fiftyone-teams` repository.
.github/workflows/build-docker-internal.yml (1)
Learnt from: kevin-dimichel
PR: voxel51/fiftyone#5173
File: .github/workflows/build-docs.yml:54-54
Timestamp: 2024-11-21T20:59:53.855Z
Learning: The `RO_FOT_FG_PAT` token used in `.github/workflows/build-docs.yml` has the necessary permissions to checkout the `voxel51/fiftyone-teams` repository.
🪛 YAMLlint (1.37.1)
.github/workflows/build-docker-internal.yml
[error] 55-55: trailing spaces
(trailing-spaces)
What changes are proposed in this pull request?
We perform vulnerability assessments on our images. We can perform a weekly build off of
developfor OSS to bring these images under our scanning window.This action will build an internal only image on Thursday mornings (4AM) which we can then include in our vulnerability reports. This action will only build a python3.11, AMD64 image. The multiple python / multiple platform versions are reserved for releases.
Image tags will look like
fiftyone:1.7.0-abc123-python3.11-20250716which indicates this was based on version 1.7 / git hash of abc123, python 3.11, and built on 07/16/2025.How is this patch tested? If it is not, please explain why.
This will be tested via GHA.
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit