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

Replace RawAttestation with Cryptolib Attestation #519

Merged
merged 4 commits into from
Jun 1, 2020

Conversation

acamadeo
Copy link
Contributor

Replace all instances of RawAttestation with cryptolib Attestation. Includes documentation fixes for functions and structs.

acamadeo added 2 commits May 29, 2020 11:50
Migrate metadata_test and cache_test

Migrate grafeas.go

Update most tests in review package

Remove tests for generic and unknown signature types

[ISSUE-506] Remove unnecessary error check

Leftover from grafeas#511. Also see the discussion [here](grafeas#514 (comment)).

Disclaimer: I was not able to test this locally now, but reading the code and the aforementioned PR, I believe this change is correct.

Cleanup base64

Clean up comments

Modify helpers

Last revisions
Copy link

@alexcope alexcope left a comment

Choose a reason for hiding this comment

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

Go readability lgtm after comments are resolved.

pkg/kritis/metadata/containeranalysis/cache_test.go Outdated Show resolved Hide resolved
pkg/kritis/metadata/metadata.go Outdated Show resolved Hide resolved
@ooq
Copy link
Contributor

ooq commented Jun 1, 2020

Feel free to ignore the cloud build error for now.

@pull-request-size pull-request-size bot added size/XL and removed size/L labels Jun 1, 2020
@acamadeo acamadeo merged commit 270a291 into grafeas:master Jun 1, 2020
Copy link
Contributor

@nenaddedic nenaddedic left a comment

Choose a reason for hiding this comment

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

Just small stuff.

}
ras = append(ras, ra)
atts = append(atts, att)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest returning []cryptolib.Attestation{att} directly. Also below.

func (avt *AttestorValidatingTransport) fetchAttestations(image string) ([]*cryptolib.Attestation, error) {
atts := []*cryptolib.Attestation{}
rawAtts, err := avt.Client.Attestations(image, &avt.Attestor)
func (avt *AttestorValidatingTransport) fetchAttestations(image string) ([]cryptolib.Attestation, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In L154, you can just call avt.Client.Attestations directly and get rid of fetchAttestations() altogether.

@acamadeo acamadeo deleted the replace-rawatt branch June 10, 2020 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants