Skip to content

Conversation

notdurson
Copy link
Contributor

@notdurson notdurson commented Dec 12, 2024

what

  • This change adds a step to the container build workflow which signs the resulting images and stores the attestation/signature for review by users.

why

tests

  • local cosign run was successful.

references

@notdurson notdurson requested review from a team as code owners December 12, 2024 20:59
@notdurson notdurson requested review from GenPage, nitrocode and X-Guardian and removed request for a team December 12, 2024 20:59
@dosubot dosubot bot added the docker Pull requests that update Docker code label Dec 12, 2024
@jamengual jamengual changed the title add image attestation workflow step fix: add image attestation workflow step Dec 12, 2024
@bmbeverst
Copy link

Looks like this needs some additional permissions per step one of the action's docs.

@notdurson
Copy link
Contributor Author

I can't see the vuln that Fossa's complaining about. @nitrocode can you take a look?

@jamengual
Copy link
Contributor

image

@nitrocode
Copy link
Member

Thanks for the contribution!

Could you test this out in your fork by merging your signing-and-sbom branch into your fork's main branch? That should trigger a push and then we can then look at the released image to see if it successfully added image attestation.

@chenrui333 chenrui333 added the feature New functionality/enhancement label Dec 15, 2024
@chenrui333
Copy link
Member

the vulnerbility issue should be fixed per #5165

chenrui333
chenrui333 previously approved these changes Dec 15, 2024
Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 15, 2024
@chenrui333 chenrui333 changed the title fix: add image attestation workflow step feat: add image attestation workflow step Dec 15, 2024
@notdurson
Copy link
Contributor Author

@nitrocode @chenrui333 Friends, thank you for your patience. We conducted some additional tests and were able to successfully sign/attest to an image in my fork.

I would appreciate it if you could review the most recent commit here, and give it a maintainer-grade 👍 if you're okay with it.

@notdurson
Copy link
Contributor Author

notdurson commented Dec 18, 2024

@jamengual ok - it's still failing, but that's expected. it's because we're hitting this issue in the attest-build-provenance action - tl;dr, the action won't run if it's run as part of a merge from a fork, the merge has to be completed from a branch in the same repo.

in my fork, the attestation works on the latest build. are you comfortable approving a merge with this in mind?

@GenPage
Copy link
Member

GenPage commented Dec 19, 2024

The image building is expected to fail because the github token changes are not merged to main yet.

@GenPage
Copy link
Member

GenPage commented Dec 19, 2024

@notdurson question, I saw we were gating the attest and push on the global PUSH var. Why was it removed?

remove conditional for image attestation step

@notdurson
Copy link
Contributor Author

@GenPage I removed it because it was causing the attestation step to skip during test builds. I can include dependency updates in tests in the future, in order to trigger a push; however, this might fail due to my lack of member/maintainer permission. It'll also slow down the process.

Would you like me to add the gate back, or are you okay with leaving it out?

@GenPage
Copy link
Member

GenPage commented Dec 19, 2024

@notdurson Ah, so it was for testing. Our main image has a gate for pushing where we want to test builds and not push (on PRs). Let's add it back for consistency unless I misunderstand the attestation point. It should only be required for images pushed not solely built correct?

@plygrnd
Copy link

plygrnd commented Dec 19, 2024

@GenPage Sounds good, incoming!

@notdurson
Copy link
Contributor Author

@GenPage @jamengual apologies for delay. I cleaned the tree and force-pushed the relevant changes.

@GenPage GenPage merged commit 91574ef into runatlantis:main Dec 19, 2024
33 checks passed
@notdurson notdurson deleted the signing-and-sbom branch December 19, 2024 21:37
@notdurson notdurson mentioned this pull request Dec 20, 2024
1 task
terakoya76 pushed a commit to terakoya76/atlantis that referenced this pull request Dec 31, 2024
kvanzuijlen pushed a commit to kvanzuijlen/atlantis that referenced this pull request Jan 4, 2025
lukaspj pushed a commit to lukaspj/atlantis that referenced this pull request Jan 8, 2025
Signed-off-by: Dan Urson <[email protected]>
Signed-off-by: Lukas Peter Aldershaab <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Pull requests that update Docker code feature New functionality/enhancement github-actions lgtm This PR has been approved by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Start signing Atlantis containers and providing signatures/SBOMs for new releases
8 participants