-
Notifications
You must be signed in to change notification settings - Fork 0
push signed artifacts to new connector registry #34
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
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (5)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds a Go module declaration and dependency; introduces protobuf schemas defining artifact manifests (Manifest, Asset, Image) and a Stable message with timestamps for immutable release metadata and per-asset SBOM/signature/certificate references. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
8ec90c8 to
6e5818a
Compare
| - amd64 | ||
| - arm64 | ||
| tags: | ||
| - "baton_lambda_support" |
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.
Consolidated docker template for GoReleaser >= 2.12 with optional lambda build and signing
Do we actually want to bundle in the lambda infra into order containers? Its an extra 10-15MB IIRC and highly specialized for our lambda deployments.
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.
These are still separate images, see lines 21 and 41 below. I've consolidated the goreleaser template so that they're in one file (and one GitHub job).
Note, the amd64 and arm64 arch are now bundled into a single image for GHCR (corresponding to image from line 21).
I'm wondering if the baton_lambda_support tag here is correct tho for the build step
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.
Lets fix this in a different PR
3e8b1ee to
4c6f3df
Compare
.github/workflows/release.yaml
Outdated
| msi: | ||
| required: false | ||
| type: boolean | ||
| default: false |
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.
When do we not want MSIs?
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.
im going to remove this flag since i'm not sure yet about this, plus this functionality didnt make it into this PR anyways
| run: | | ||
| set -euo pipefail | ||
| cd dist | ||
| cosign sign-blob --yes "manifest.json" \ |
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.
Did we verify the signatures?
| RESPONSE=$(aws lambda invoke \ | ||
| --function-name "${{ github.event.repository.name }}-releases" \ | ||
| --payload "{\"repository\":\"${{ github.event.repository.full_name }}\", \"tag\":\"${{ inputs.tag }}\"}" \ | ||
| --function-name "${{ github.event.repository.owner.login }}-${{ github.event.repository.name }}-artifact-releases" \ |
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.
Is this correct?
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.
yes. basically it was wrong, but we weren'tm using it anyways. the lambda just hardcodes conductorone
|
This PR is hard to review for correctness, but thats easy to verify: TODOS:
|
2ab69dd to
c74e9cf
Compare
cmd/extract-images/main.go
Outdated
| } | ||
|
|
||
| if !foundIndex { | ||
| content, _ := os.ReadFile(digestFile) |
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.
^
Updated connector release workflow for signing artifacts and pushing to the new c1 connector registry. The registry will provide a central, consistent way to distribute binaries and container images to customers across all public and private connectors.
This PR makes a number of changes and and will necessitate a new major release version bump.
For a connector repo to upgrade to the new workflow:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.