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: set cosign attest predicate type based on Syft output type #1598

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

Nirusu
Copy link
Contributor

@Nirusu Nirusu commented Feb 21, 2023

In #1533, I wanted to remove the hardcoded "custom" predicate type passed to Cosign. This seemed to fix some of the issues I was seeing, though to be honest at this point I don't know exactly why 😅

However, what I did not anticipate is that there's still no way to manually set a predicate type. Generally, I think it makes sense to set the predicate type on the type we are passing as predicate to Cosign.

Thus, here my attempt at this.

Please mind, this is untested so far. Wanted to get general feedback on this first (or if someone can quickly test this and verify the attestation, that'd be cool).

Also, if someone can tell me how I can properly reference the output type in the switch statement rather than just using string comparison based on the user input here without major refactoring, I'd be happy to hear about that :)

@Nirusu Nirusu force-pushed the feat/predicate-based-on-output branch from b65d584 to 8db2182 Compare February 21, 2023 16:21
@kzantow
Copy link
Contributor

kzantow commented Feb 21, 2023

Hi @Nirusu -- could you update to the latest main? It should fix the failing test for you.

@Nirusu Nirusu force-pushed the feat/predicate-based-on-output branch from 8db2182 to 1d2c789 Compare February 21, 2023 17:48
@Nirusu
Copy link
Contributor Author

Nirusu commented Feb 21, 2023

Done.

spiffcs
spiffcs previously approved these changes Feb 22, 2023
@spiffcs spiffcs self-requested a review February 22, 2023 16:17
@spiffcs spiffcs dismissed their stale review February 22, 2023 16:17

wrong PR =( apologies

@spiffcs spiffcs merged commit fa0a9fe into anchore:main Feb 24, 2023
@spiffcs
Copy link
Contributor

spiffcs commented Feb 24, 2023

@Nirusu I tested it and think we can merge as is -- I'll follow this up with maybe a small refactor that uses the correct values for the input types. Had an issue checking out the branch using gh command so no harm in merge and then follow up =)

@kzantow kzantow added the enhancement New feature or request label Mar 2, 2023
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants