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(e2e): provenance registry option for container generator #294

Merged
merged 2 commits into from
Dec 8, 2023
Merged

feat(e2e): provenance registry option for container generator #294

merged 2 commits into from
Dec 8, 2023

Conversation

saisatishkarra
Copy link
Contributor

@saisatishkarra saisatishkarra commented Nov 30, 2023

cc: @laurentsimon Fixes Issue #2981

@saisatishkarra
Copy link
Contributor Author

  1. Added e2e for provenance-registry:
    • pushing image docker.io (swapped from GHCR)
    • pushing provenance to GHCR (Same as original workflow)
    • Set COSIGN_REPOSITORY and additional login command for downloading and verifying the attestation

I noticed that the existing slsa-verifier / workflow takes provenance as ENV variable that is downloaded from cosign. NO changes made to expose any env variable for COSIGN_REPOSITORY in https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.container.default.verify.sh or https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.go.default.verify.sh#L44-L46.

@laurentsimon Let me know if this is not enough. Does Slsa-verifier download / login to a set COSIGN_REPOSITORY / env variable when --provenance-path is not set during verify-image?

@laurentsimon
Copy link
Collaborator

  1. Added e2e for provenance-registry:

    • pushing image docker.io (swapped from GHCR)
    • pushing provenance to GHCR (Same as original workflow)
    • Set COSIGN_REPOSITORY and additional login command for downloading and verifying the attestation

Can we add a last step to delete the image from the registry?

I noticed that the existing slsa-verifier / workflow takes provenance as ENV variable that is downloaded from cosign. NO changes made to expose any env variable for COSIGN_REPOSITORY in https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.container.default.verify.sh or https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.go.default.verify.sh#L44-L46.

@laurentsimon Let me know if this is not enough. Does Slsa-verifier download / login to a set COSIGN_REPOSITORY / env variable when --provenance-path is not set during verify-image?

ah, good catch. slsa-verifier does not handle login by itself. It relies on the user being already logged in. If you've added login for the new provenance registry, it should work, I think.

Copy link
Collaborator

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. Let's add a step to delete image from docker registry and we should be ready to merge. You'll probably need a new job and use if: always() to make sure it always runs

@saisatishkarra
Copy link
Contributor Author

  1. Added e2e for provenance-registry:

    • pushing image docker.io (swapped from GHCR)
    • pushing provenance to GHCR (Same as original workflow)
    • Set COSIGN_REPOSITORY and additional login command for downloading and verifying the attestation

Can we add a last step to delete the image from the registry?

I will work on deleting the image from dockerhub given that the token has enough permissions.

I noticed that the existing slsa-verifier / workflow takes provenance as ENV variable that is downloaded from cosign. NO changes made to expose any env variable for COSIGN_REPOSITORY in https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.container.default.verify.sh or https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.go.default.verify.sh#L44-L46.
@laurentsimon Let me know if this is not enough. Does Slsa-verifier download / login to a set COSIGN_REPOSITORY / env variable when --provenance-path is not set during verify-image?

ah, good catch. slsa-verifier does not handle login by itself. It relies on the user being already logged in. If you've added login for the new provenance registry, it should work, I think.

Additional login added at: https://github.com/slsa-framework/example-package/pull/294/files#diff-6708d79be59bf41aad1275aefcad526256652e711589b301e093c6e0cc7ae10eR144.

@ianlewis
Copy link
Member

ianlewis commented Dec 5, 2023

Can we add a last step to delete the image from the registry?

I will work on deleting the image from dockerhub given that the token has enough permissions.

I'm not sure we really care about deleting the image. None of our other tests really rely on the image being deleted and tests should work ok since we run checks by sha rather than any registry tags. I suppose we're talking about deleting the image contents and not the ghcr 'package' outright?

@ianlewis
Copy link
Member

ianlewis commented Dec 5, 2023

Also, just a note. For the test to work we'll need to set up a package with the image name in the slsa-framework org. Probably only @laurentsimon and I would have access to do that.
https://github.com/orgs/slsa-framework/packages

@laurentsimon
Copy link
Collaborator

laurentsimon commented Dec 6, 2023

Can we add a last step to delete the image from the registry?

I will work on deleting the image from dockerhub given that the token has enough permissions.

I'm not sure we really care about deleting the image. None of our other tests really rely on the image being deleted and tests should work ok since we run checks by sha rather than any registry tags. I suppose we're talking about deleting the image contents and not the ghcr 'package' outright?

My main reason for deleting was that I don't know how docker registry handles blobs, in the sense whether they have size limits or not. If we know that will never be a problem, I'm fine not deleting

@laurentsimon
Copy link
Collaborator

@saisatishkarra I can merge this PR and you can send a folow-up to delete the image (depending on @ianlewis 's question why I asked to delete). Wdut?

@laurentsimon
Copy link
Collaborator

Also, just a note. For the test to work we'll need to set up a package with the image name in the slsa-framework org. Probably only @laurentsimon and I would have access to do that. https://github.com/orgs/slsa-framework/packages

mhh, I did not know that. Could not find an option to setup a new package...

@saisatishkarra
Copy link
Contributor Author

saisatishkarra commented Dec 6, 2023

@saisatishkarra I can merge this PR and you can send a folow-up to delete the image (depending on @ianlewis 's question why I asked to delete). Wdut?

@laurentsimon After some digging seems like docker is deprecating the Advanced Image management API and hub-tool (experimental) doesn't yet support providing password from env / doesn't obey docker login creds, and is pending.

Here are the 3 issues for deleting from docker hub:

docker/roadmap#534
https://docs.docker.com/docker-hub/api/latest/#tag/images/operation/PostNamespacesDeleteImages
docker/hub-tool#198
2 Solution alternatives:

Option 1: Push container image to GHCR and provenance image to dockerhub (i.e. Swap token permissions and registries for storing image and provenance). Leverage https://github.com/marketplace/actions/delete-ghcr-io-package-tag to delete docker container image from GHCR and leave out provenance in a docker registry

Option 2: Push container image to dockerhub and provenance to GHCR. Skip deletion/pruning of images and document reasons.

Clarifications needed:

  • LMK what option to proceed with?
  • If Option 1 is chosen,
    • I would have to refactor the PR to swap registries.
    • Also, can you confirm that the GH_TOKEN (E2E_CONTAINER_TOKEN) has permission to delete the image from GHCR / - What repository to push the provenance in docker hub (Currently Using workflow name as docker repo)?

@saisatishkarra
Copy link
Contributor Author

Also, just a note. For the test to work we'll need to set up a package with the image name in the slsa-framework org. Probably only @laurentsimon and I would have access to do that. https://github.com/orgs/slsa-framework/packages

mhh, I did not know that. Could not find an option to setup a new package...

Let me know what direction to take and if the package has been created?

@ianlewis
Copy link
Member

ianlewis commented Dec 7, 2023

Let me create the package. It's kind of silly but you have to push an empty image using your own token to get it to create the package and only then can you give the repo permission to push...

This shouldn't stop us from merging. The test will just fail initially but that's probably fine.

We don't need it to delete the image. We can handle deletion later if we need to. We would need to address it in our other tests separately anyway.

@ianlewis
Copy link
Member

ianlewis commented Dec 7, 2023

OK, I created the package and set the settings so that example-package has write access. That should allow the test to push to ghcr.
https://github.com/slsa-framework/example-package/pkgs/container/example-package.e2e.container.schedule.main.provenance-registry.slsa3

@saisatishkarra
Copy link
Contributor Author

@laurentsimon / @ianlewis I fixed a few linting issues and modified the config for golangci and yamllint when running locally. Can someone rerun this workflow and fix any other issues for which files aren't changed in this PR to save some back-and-forth iterations?

@saisatishkarra
Copy link
Contributor Author

OK, I created the package and set the settings so that example-package has write access. That should allow the test to push to ghcr. https://github.com/slsa-framework/example-package/pkgs/container/example-package.e2e.container.schedule.main.provenance-registry.slsa3

As a external contributor, i am getting a 404 when clicking the package link (Hoping it was created and exists). Pls make sure to run any workflow / permissions issue to move this forward.

@ianlewis
Copy link
Member

ianlewis commented Dec 8, 2023

OK, I created the package and set the settings so that example-package has write access. That should allow the test to push to ghcr. https://github.com/slsa-framework/example-package/pkgs/container/example-package.e2e.container.schedule.main.provenance-registry.slsa3

As a external contributor, i am getting a 404 when clicking the package link (Hoping it was created and exists). Pls make sure to run any workflow / permissions issue to move this forward.

Ah, right. I forgot that these packages were set as private. I think we made these private since they are only used to make tests work and we don't want folks don't try to download and use them or something.

You shouldn't need access to yourself. The repo just needs access so it should be fine. This repo is a bit funky since it's not easy to fork it and test but you can leave it to us to fix any small test bugs that might crop up after merge.

re: linters: These haven't been fully passing so it's somewhat normal that they aren't passing right now. We need to do a bit of cleanup but haven't had a chance yet.

@ianlewis ianlewis merged commit 28b33c6 into slsa-framework:main Dec 8, 2023
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants