Skip to content
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

feat: add image attestation workflow step #5158

Merged
merged 3 commits into from
Dec 19, 2024

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

Signed-off-by: Dan Urson <dan.urson@cresta.ai>
@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
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