feat: add image option for service version inferring#11
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SBOM upload flow to target a revised API endpoint shape (moving service version from the URL path into query parameters) and adds support for associating uploads with a container image reference (via --image and environment variables) in addition to, or instead of, a service version.
Changes:
- Add
--imagesupport (includingIMAGE/BIFROST_IMAGEenv var fallbacks) and allow uploads with either service version or image (or both). - Update the API upload endpoint path and send
version/imageas query parameters. - Extend CLI/API tests and update README usage examples accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates documentation to describe image-reference uploads and examples. |
| internal/bifrost/sbom_upload.go | Accepts image via flags/env and passes it into the API client; relaxes validation to require version-or-image. |
| internal/bifrost/options.go | Adds the --image flag and updates help text for version/image requirements. |
| internal/bifrost/cli_test.go | Adds CLI-level tests for image flag/env behavior and missing version+image validation. |
| internal/bifrost/api.go | Changes upload URL path and adds version/image query params; extends API constructor to include image. |
| internal/bifrost/api_test.go | Updates existing tests for new endpoint/query behavior and adds coverage for image-only uploads and validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
konrader
left a comment
There was a problem hiding this comment.
Looks good, just a minor comment about skip support for slightly confusing BIFROST_IMAGE env var.
| fl.StringVar(&opts.service, "service", "", "Name of the service") | ||
| fl.StringVar(&opts.serviceVersion, "service-version", "", "Version of the service") | ||
| fl.StringVar(&opts.serviceVersion, "service-version", "", "Service version for the uploaded SBOM (or SERVICE_VERSION environment variable); required unless an image is provided") | ||
| fl.StringVar(&opts.image, "image", "", "Container image reference for the uploaded SBOM (or IMAGE/BIFROST_IMAGE environment variables); required unless a service version is provided") |
There was a problem hiding this comment.
Let's just support IMAGE env var, since BIFROST_IMAGE sounds like pointing out some image from us.
| if image := os.Getenv("IMAGE"); image != "" { | ||
| opts.image = image | ||
| } | ||
| if image := os.Getenv("BIFROST_IMAGE"); image != "" { |
There was a problem hiding this comment.
Skip supporting this.
Or we should support BIFROST_ prefix on all env vars.
| | `--api-key` | Yes | `BIFROST_API_KEY` | Bifrost API key used for authentication. | | ||
| | `--service` | Yes | `SERVICE` | Name of the service. | | ||
| | `--service-version` | Conditional | `SERVICE_VERSION` | Service version for the uploaded SBOM. Required unless an image is provided. | | ||
| | `--image` | Conditional | `IMAGE`, `BIFROST_IMAGE` | Container image reference for the uploaded SBOM. Required unless a service version is provided. | |
# Conflicts: # internal/bifrost/api.go # internal/bifrost/api_test.go # internal/bifrost/sbom_upload.go
Updated the SBOM upload path to use the new integration api endpoint and also adds support for using image references for setting version name