From b84a1ea2fd03d368397d877a9d200d1630505cdd Mon Sep 17 00:00:00 2001 From: rbehjati Date: Tue, 31 Jan 2023 12:57:01 +0000 Subject: [PATCH] Improve the code to better match Golang style & lints (#219) * Update function names to better match Golang style * Refactor ProvenanceIRVerifier::Verify to reduce its cyclomatic complexity --- internal/common/common.go | 73 ++++++++++++++----------- internal/endorser/endorser.go | 20 ++++--- internal/verifier/verifier.go | 100 ++++++++++++++++++---------------- 3 files changed, 103 insertions(+), 90 deletions(-) diff --git a/internal/common/common.go b/internal/common/common.go index 490b51e0..7391dd05 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -105,16 +105,52 @@ func NewProvenanceIR(binarySHA256Digest string, buildType string, binaryName str return provenance } -// GetBinarySHA256Digest gets the binary sha256 digest. -func (p *ProvenanceIR) GetBinarySHA256Digest() string { +// BinarySHA256Digest returns the binary sha256 digest. +func (p *ProvenanceIR) BinarySHA256Digest() string { return p.binarySHA256Digest } -// GetBinaryName returns the binary name. -func (p *ProvenanceIR) GetBinaryName() string { +// BinaryName returns the binary name. +func (p *ProvenanceIR) BinaryName() string { return p.binaryName } +// BuildType returns the buildType. +func (p *ProvenanceIR) BuildType() string { + return p.buildType +} + +// BuildCmd return the build cmd, or an error if the build cmd has not been set. +func (p *ProvenanceIR) BuildCmd() ([]string, error) { + if !p.HasBuildCmd() { + return nil, fmt.Errorf("provenance does not have a build cmd") + } + return *p.buildCmd, nil +} + +// RepoURIs returns repo URIs in the provenance. +func (p *ProvenanceIR) RepoURIs() []string { + return *p.repoURIs +} + +// BuilderImageSHA256Digest returns the builder image sha256 digest, or an +// error if the builder image sha256 digest has not been set. +func (p *ProvenanceIR) BuilderImageSHA256Digest() (string, error) { + if !p.HasBuilderImageSHA256Digest() { + return "", fmt.Errorf("provenance does not have a builder image SHA256 digest") + } + return *p.builderImageSHA256Digest, nil +} + +// TrustedBuilder returns the builder image sha256 digest, or an error if the +// trusted builder has not been set. +func (p *ProvenanceIR) TrustedBuilder() (string, error) { + if !p.HasTrustedBuilder() { + return "", fmt.Errorf("provenance does not have a trusted builder") + } + return *p.trustedBuilder, nil +} + // WithBuildCmd sets the build cmd when creating a new ProvenanceIR. func WithBuildCmd(buildCmd []string) func(p *ProvenanceIR) { return func(p *ProvenanceIR) { @@ -127,14 +163,6 @@ func (p *ProvenanceIR) HasBuildCmd() bool { return p.buildCmd != nil } -// GetBuildCmd gets the build cmd. Returns an error if the build cmd has not been set. -func (p *ProvenanceIR) GetBuildCmd() ([]string, error) { - if !p.HasBuildCmd() { - return nil, fmt.Errorf("provenance does not have a build cmd") - } - return *p.buildCmd, nil -} - // WithBuilderImageSHA256Digest sets the builder image sha256 digest when creating a new ProvenanceIR. func WithBuilderImageSHA256Digest(builderImageSHA256Digest string) func(p *ProvenanceIR) { return func(p *ProvenanceIR) { @@ -147,14 +175,6 @@ func (p *ProvenanceIR) HasBuilderImageSHA256Digest() bool { return p.builderImageSHA256Digest != nil } -// GetBuilderImageSHA256Digest gets the builder image sha256 digest. Returns an error if the builder image sha256 digest has not been set. -func (p *ProvenanceIR) GetBuilderImageSHA256Digest() (string, error) { - if !p.HasBuilderImageSHA256Digest() { - return "", fmt.Errorf("provenance does not have a builder image SHA256 digest") - } - return *p.builderImageSHA256Digest, nil -} - // WithRepoURIs sets repo URIs referenced in the provenance when creating a new ProvenanceIR. func WithRepoURIs(repoURIs []string) func(p *ProvenanceIR) { return func(p *ProvenanceIR) { @@ -167,11 +187,6 @@ func (p *ProvenanceIR) HasRepoURIs() bool { return p.repoURIs != nil } -// GetRepoURIs gets references to a repo in the provenance. There is no guarantee to get all the references to any repo. -func (p *ProvenanceIR) GetRepoURIs() []string { - return *p.repoURIs -} - // WithTrustedBuilder sets the trusted builder when creating a new ProvenanceIR. func WithTrustedBuilder(trustedBuilder string) func(p *ProvenanceIR) { return func(p *ProvenanceIR) { @@ -184,14 +199,6 @@ func (p *ProvenanceIR) HasTrustedBuilder() bool { return p.trustedBuilder != nil } -// GetTrustedBuilder gets the builder image sha256 digest. Returns an error if the trusted builder has not been set. -func (p *ProvenanceIR) GetTrustedBuilder() (string, error) { - if !p.HasTrustedBuilder() { - return "", fmt.Errorf("provenance does not have a trusted builder") - } - return *p.trustedBuilder, nil -} - // FromValidatedProvenance maps a validated provenance to ProvenanceIR by checking the provenance's // predicate and build type. // diff --git a/internal/endorser/endorser.go b/internal/endorser/endorser.go index 5997a509..40e8bec3 100644 --- a/internal/endorser/endorser.go +++ b/internal/endorser/endorser.go @@ -80,8 +80,8 @@ func verifyAndSummarizeProvenances(referenceValues *common.ReferenceValues, prov } verifiedProvenances := amber.VerifiedProvenanceSet{ - BinaryDigest: provenanceIRs[0].GetBinarySHA256Digest(), - BinaryName: provenanceIRs[0].GetBinaryName(), + BinaryDigest: provenanceIRs[0].BinarySHA256Digest(), + BinaryName: provenanceIRs[0].BinaryName(), Provenances: provenancesData, } @@ -109,28 +109,28 @@ func verifyConsistency(provenanceIRs []common.ProvenanceIR) error { var errs error // get the binary digest and binary name of the first provenance as reference - refBinaryDigest := provenanceIRs[0].GetBinarySHA256Digest() - refBinaryName := provenanceIRs[0].GetBinaryName() + refBinaryDigest := provenanceIRs[0].BinarySHA256Digest() + refBinaryName := provenanceIRs[0].BinaryName() // verify that all remaining provenances have the same binary digest and binary name. for ind := 1; ind < len(provenanceIRs); ind++ { - if provenanceIRs[ind].GetBinarySHA256Digest() != refBinaryDigest { + if provenanceIRs[ind].BinarySHA256Digest() != refBinaryDigest { multierr.AppendInto(&errs, fmt.Errorf("provenances are not consistent: unexpected binary SHA256 digest in provenance #%d; got %q, want %q", ind, - provenanceIRs[ind].GetBinarySHA256Digest(), refBinaryDigest)) + provenanceIRs[ind].BinarySHA256Digest(), refBinaryDigest)) } - if provenanceIRs[ind].GetBinaryName() != refBinaryName { + if provenanceIRs[ind].BinaryName() != refBinaryName { multierr.AppendInto(&errs, fmt.Errorf("provenances are not consistent: unexpected subject name in provenance #%d; got %q, want %q", ind, - provenanceIRs[ind].GetBinaryName(), refBinaryName)) + provenanceIRs[ind].BinaryName(), refBinaryName)) } } return errs } -// LoadProvenance loads a number of provenance from the give URIs. Returns an +// LoadProvenances loads a number of provenance from the give URIs. Returns an // array of ParsedProvenance instances, or an error if loading or parsing any // of the provenances fails. See LoadProvenance for more details. func LoadProvenances(provenanceURIs []string) ([]ParsedProvenance, error) { @@ -174,6 +174,8 @@ func LoadProvenance(provenanceURI string) (*ParsedProvenance, error) { }, nil } +// GetProvenanceBytes fetches provenance bytes from the give URI. Supported URI +// schemes are "http", "https", and "file". Only local files are supported. func GetProvenanceBytes(provenanceURI string) ([]byte, error) { uri, err := url.Parse(provenanceURI) if err != nil { diff --git a/internal/verifier/verifier.go b/internal/verifier/verifier.go index 814d66c3..c8421b4c 100644 --- a/internal/verifier/verifier.go +++ b/internal/verifier/verifier.go @@ -106,116 +106,120 @@ type ProvenanceIRVerifier struct { // Verify verifies an instance of ProvenanceIRVerifier by comparing its Got and Want fields. // Verify checks fields, which (i) are set in Got, i.e., GotHasX is true, and (ii) are set in Want. -// -//nolint:cyclop -func (verifier *ProvenanceIRVerifier) Verify() error { +func (v *ProvenanceIRVerifier) Verify() error { var errs error - // Verify BinarySHA256 Digest. Every reference value contains a binary digest. - if verifier.Want.BinarySHA256Digests != nil { - if err := verifyBinarySHA256Digest(verifier.Want, verifier.Got); err != nil { - multierr.AppendInto(&errs, fmt.Errorf("failed to verify binary SHA256 digest: %v", err)) - } + if err := v.verifyBinarySHA256Digest(); err != nil { + multierr.AppendInto(&errs, fmt.Errorf("failed to verify binary SHA256 digest: %v", err)) } // Verify HasBuildCmd. - if verifier.Got.HasBuildCmd() && verifier.Want.WantBuildCmds { - multierr.AppendInto(&errs, verifyHasNotEmptyBuildCmd(verifier.Got)) - } + multierr.AppendInto(&errs, v.verifyBuildCmd()) // Verify BuilderImageDigest. - if verifier.Got.HasBuilderImageSHA256Digest() && verifier.Want.BuilderImageSHA256Digests != nil { - if err := verifyBuilderImageDigest(verifier.Want, verifier.Got); err != nil { - multierr.AppendInto(&errs, fmt.Errorf("failed to verify builder image digests: %v", err)) - } + if err := v.verifyBuilderImageDigest(); err != nil { + multierr.AppendInto(&errs, fmt.Errorf("failed to verify builder image digests: %v", err)) } // Verify RepoURIs. - if verifier.Got.HasRepoURIs() && verifier.Want.RepoURI != "" { - multierr.AppendInto(&errs, verifyRepoURIs(verifier.Want, verifier.Got)) - } + multierr.AppendInto(&errs, v.verifyRepoURIs()) // Verify TrustedBuilder. - if verifier.Got.HasTrustedBuilder() && verifier.Want.TrustedBuilders != nil { - if err := verifyTrustedBuilder(verifier.Want, verifier.Got); err != nil { - multierr.AppendInto(&errs, fmt.Errorf("failed to verify trusted builder: %v", err)) - } + if err := v.verifyTrustedBuilder(); err != nil { + multierr.AppendInto(&errs, fmt.Errorf("failed to verify trusted builder: %v", err)) } return errs } -// verifyBinarySHA256Digest verifies that the binary SHA256 in this provenance is contained in the given reference binary SHA256 digests (in want). -func verifyBinarySHA256Digest(want *common.ReferenceValues, got *common.ProvenanceIR) error { - gotBinarySHA256Digest := got.GetBinarySHA256Digest() - - if want.BinarySHA256Digests == nil { - return fmt.Errorf("no reference binary SHA256 digests given") +// verifyBinarySHA256Digest verifies that the binary SHA256 Got in this +// verifier is among the Wanted digests. +func (v *ProvenanceIRVerifier) verifyBinarySHA256Digest() error { + if v.Want.BinarySHA256Digests == nil { + return nil } - for _, wantBinarySHA256Digest := range want.BinarySHA256Digests { + gotBinarySHA256Digest := v.Got.BinarySHA256Digest() + + for _, wantBinarySHA256Digest := range v.Want.BinarySHA256Digests { if wantBinarySHA256Digest == gotBinarySHA256Digest { return nil } } return fmt.Errorf("the reference binary SHA256 digests (%v) do not contain the actual binary SHA256 digest (%v)", - want.BinarySHA256Digests, + v.Want.BinarySHA256Digests, gotBinarySHA256Digest) } -// verifyHasNotEmptyBuildCmd verifies that the build cmd is not empty. -func verifyHasNotEmptyBuildCmd(got *common.ProvenanceIR) error { - if buildCmd, err := got.GetBuildCmd(); err != nil || len(buildCmd) == 0 { - return fmt.Errorf("no build cmd found") +// verifyBuildCmd verifies the build cmd. Returns an error if a build command is +// needed in the Want reference values, but is not present in the Got provenance. +func (v *ProvenanceIRVerifier) verifyBuildCmd() error { + if v.Got.HasBuildCmd() && v.Want.WantBuildCmds { + if buildCmd, err := v.Got.BuildCmd(); err != nil || len(buildCmd) == 0 { + return fmt.Errorf("no build cmd found") + } } return nil } -// verifyBuilderImageDigest verifies that the given builder image digest matches a builder image digest in the reference values. -func verifyBuilderImageDigest(want *common.ReferenceValues, got *common.ProvenanceIR) error { - gotBuilderImageDigest, err := got.GetBuilderImageSHA256Digest() +// verifyBuilderImageDigest verifies that the builder image digest in the Got +// provenance matches a builder image digest in the Want reference values. +func (v *ProvenanceIRVerifier) verifyBuilderImageDigest() error { + if !v.Got.HasBuilderImageSHA256Digest() || v.Want.BuilderImageSHA256Digests == nil { + return nil + } + + gotBuilderImageDigest, err := v.Got.BuilderImageSHA256Digest() if err != nil { return fmt.Errorf("no builder image digest set") } - for _, wantBuilderImageSHA256Digest := range want.BuilderImageSHA256Digests { + for _, wantBuilderImageSHA256Digest := range v.Want.BuilderImageSHA256Digests { if wantBuilderImageSHA256Digest == gotBuilderImageDigest { return nil } } return fmt.Errorf("the reference builder image digests (%v) do not contain the actual builder image digest (%v)", - want.BuilderImageSHA256Digests, + v.Want.BuilderImageSHA256Digests, gotBuilderImageDigest) } -// verifyRepoURIs verifies that the references to URIs in the provenance point to the repo URI given in the reference values. -func verifyRepoURIs(want *common.ReferenceValues, got *common.ProvenanceIR) error { +// verifyRepoURIs verifies that the references to URIs in the Got provenance +// match the repo URI in the Want reference values. +func (v *ProvenanceIRVerifier) verifyRepoURIs() error { var errs error - for _, gotRepoURI := range got.GetRepoURIs() { + if !v.Got.HasRepoURIs() || v.Want.RepoURI == "" { + return nil + } + + for _, gotRepoURI := range v.Got.RepoURIs() { // We want the want.RepoURI be contained in every repo uri from the provenance. - if !strings.Contains(gotRepoURI, want.RepoURI) { - multierr.AppendInto(&errs, fmt.Errorf("the URI from the provenance (%v) does not contain the repo URI (%v)", gotRepoURI, want.RepoURI)) + if !strings.Contains(gotRepoURI, v.Want.RepoURI) { + multierr.AppendInto(&errs, fmt.Errorf("the URI from the provenance (%v) does not contain the repo URI (%v)", gotRepoURI, v.Want.RepoURI)) } } return errs } // verifyTrustedBuilder verifies that the given trusted builder matches a trusted builder in the reference values. -func verifyTrustedBuilder(want *common.ReferenceValues, got *common.ProvenanceIR) error { - gotTrustedBuilder, err := got.GetTrustedBuilder() +func (v *ProvenanceIRVerifier) verifyTrustedBuilder() error { + if !v.Got.HasTrustedBuilder() || v.Want.TrustedBuilders == nil { + return nil + } + gotTrustedBuilder, err := v.Got.TrustedBuilder() if err != nil { return fmt.Errorf("no trusted builder set") } - for _, wantTrustedBuilder := range want.TrustedBuilders { + for _, wantTrustedBuilder := range v.Want.TrustedBuilders { if wantTrustedBuilder == gotTrustedBuilder { return nil } } return fmt.Errorf("the reference trusted builders (%v) do not contain the actual trusted builder (%v)", - want.TrustedBuilders, + v.Want.TrustedBuilders, gotTrustedBuilder) }