From 8f603dba0ec8c44f89b0ae20eded99eaba83cba4 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 13 Mar 2023 10:04:00 +0000 Subject: [PATCH] solve: use comparables instead of reflection in result struct Since go 1.20, interfaces (that can be compared) now implement the comparable interface. The use of reflection in solver/result was a workaround for this limitation, however, we can remove this with the upstream fix. Signed-off-by: Justin Chadwell --- frontend/attestations/sbom/sbom.go | 10 ++++----- frontend/dockerfile/builder/build.go | 2 +- solver/llbsolver/proc/sbom.go | 2 +- solver/result/attestation.go | 8 +++---- solver/result/result.go | 33 +++++++++++++++++----------- 5 files changed, 31 insertions(+), 24 deletions(-) diff --git a/frontend/attestations/sbom/sbom.go b/frontend/attestations/sbom/sbom.go index 113797b21..c8a461d15 100644 --- a/frontend/attestations/sbom/sbom.go +++ b/frontend/attestations/sbom/sbom.go @@ -31,7 +31,7 @@ const ( // build-contexts or multi-stage builds. Handling these separately allows the // scanner to optionally ignore these or to mark them as such in the // attestation. -type Scanner func(ctx context.Context, name string, ref llb.State, extras map[string]llb.State, opts ...llb.ConstraintsOpt) (result.Attestation[llb.State], error) +type Scanner func(ctx context.Context, name string, ref llb.State, extras map[string]llb.State, opts ...llb.ConstraintsOpt) (result.Attestation[*llb.State], error) func CreateSBOMScanner(ctx context.Context, resolver llb.ImageMetaResolver, scanner string) (Scanner, error) { if scanner == "" { @@ -55,7 +55,7 @@ func CreateSBOMScanner(ctx context.Context, resolver llb.ImageMetaResolver, scan return nil, errors.Errorf("scanner %s does not have cmd", scanner) } - return func(ctx context.Context, name string, ref llb.State, extras map[string]llb.State, opts ...llb.ConstraintsOpt) (result.Attestation[llb.State], error) { + return func(ctx context.Context, name string, ref llb.State, extras map[string]llb.State, opts ...llb.ConstraintsOpt) (result.Attestation[*llb.State], error) { var env []string env = append(env, cfg.Config.Env...) env = append(env, "BUILDKIT_SCAN_DESTINATION="+outDir) @@ -86,9 +86,9 @@ func CreateSBOMScanner(ctx context.Context, resolver llb.ImageMetaResolver, scan } stsbom := runscan.AddMount(outDir, llb.Scratch()) - return result.Attestation[llb.State]{ + return result.Attestation[*llb.State]{ Kind: gatewaypb.AttestationKindBundle, - Ref: stsbom, + Ref: &stsbom, Metadata: map[string][]byte{ result.AttestationReasonKey: []byte(result.AttestationReasonSBOM), result.AttestationSBOMCore: []byte(CoreSBOMName), @@ -100,7 +100,7 @@ func CreateSBOMScanner(ctx context.Context, resolver llb.ImageMetaResolver, scan }, nil } -func HasSBOM[T any](res *result.Result[T]) bool { +func HasSBOM[T comparable](res *result.Result[T]) bool { for _, as := range res.Attestations { for _, a := range as { if a.InToto.PredicateType == intoto.PredicateSPDX { diff --git a/frontend/dockerfile/builder/build.go b/frontend/dockerfile/builder/build.go index 565b6fc9e..0edbbaa7f 100644 --- a/frontend/dockerfile/builder/build.go +++ b/frontend/dockerfile/builder/build.go @@ -169,7 +169,7 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) { return err } - attSolve, err := result.ConvertAttestation(&att, func(st llb.State) (client.Reference, error) { + attSolve, err := result.ConvertAttestation(&att, func(st *llb.State) (client.Reference, error) { def, err := st.Marshal(ctx) if err != nil { return nil, err diff --git a/solver/llbsolver/proc/sbom.go b/solver/llbsolver/proc/sbom.go index 2d7e969ba..7f70aba11 100644 --- a/solver/llbsolver/proc/sbom.go +++ b/solver/llbsolver/proc/sbom.go @@ -52,7 +52,7 @@ func SBOMProcessor(scannerRef string, useCache bool) llbsolver.Processor { if err != nil { return nil, err } - attSolve, err := result.ConvertAttestation(&att, func(st llb.State) (solver.ResultProxy, error) { + attSolve, err := result.ConvertAttestation(&att, func(st *llb.State) (solver.ResultProxy, error) { def, err := st.Marshal(ctx) if err != nil { return nil, err diff --git a/solver/result/attestation.go b/solver/result/attestation.go index 77af74da1..2fee27824 100644 --- a/solver/result/attestation.go +++ b/solver/result/attestation.go @@ -1,8 +1,6 @@ package result import ( - "reflect" - pb "github.com/moby/buildkit/frontend/gateway/pb" digest "github.com/opencontainers/go-digest" ) @@ -58,9 +56,11 @@ func FromDigestMap(m map[string]string) []digest.Digest { return ds } -func ConvertAttestation[U any, V any](a *Attestation[U], fn func(U) (V, error)) (*Attestation[V], error) { +func ConvertAttestation[U comparable, V comparable](a *Attestation[U], fn func(U) (V, error)) (*Attestation[V], error) { + var zero U + var ref V - if reflect.ValueOf(a.Ref).IsValid() { + if a.Ref != zero { var err error ref, err = fn(a.Ref) if err != nil { diff --git a/solver/result/result.go b/solver/result/result.go index d5fe2d03c..cfcfe9dcb 100644 --- a/solver/result/result.go +++ b/solver/result/result.go @@ -1,13 +1,12 @@ package result import ( - "reflect" "sync" "github.com/pkg/errors" ) -type Result[T any] struct { +type Result[T comparable] struct { mu sync.Mutex Ref T Refs map[string]T @@ -50,7 +49,8 @@ func (r *Result[T]) SingleRef() (T, error) { r.mu.Lock() defer r.mu.Unlock() - if r.Refs != nil && !reflect.ValueOf(r.Ref).IsValid() { + var zero T + if r.Refs != nil && r.Ref == zero { var t T return t, errors.Errorf("invalid map result") } @@ -77,11 +77,12 @@ func (r *Result[T]) FindRef(key string) (T, bool) { } func (r *Result[T]) EachRef(fn func(T) error) (err error) { - if reflect.ValueOf(r.Ref).IsValid() { + var zero T + if r.Ref != zero { err = fn(r.Ref) } for _, r := range r.Refs { - if reflect.ValueOf(r).IsValid() { + if r != zero { if err1 := fn(r); err1 != nil && err == nil { err = err1 } @@ -89,7 +90,7 @@ func (r *Result[T]) EachRef(fn func(T) error) (err error) { } for _, as := range r.Attestations { for _, a := range as { - if reflect.ValueOf(a.Ref).IsValid() { + if a.Ref != zero { if err1 := fn(a.Ref); err1 != nil && err == nil { err = err1 } @@ -102,8 +103,12 @@ func (r *Result[T]) EachRef(fn func(T) error) (err error) { // EachRef iterates over references in both a and b. // a and b are assumed to be of the same size and map their references // to the same set of keys -func EachRef[U any, V any](a *Result[U], b *Result[V], fn func(U, V) error) (err error) { - if reflect.ValueOf(a.Ref).IsValid() && reflect.ValueOf(b.Ref).IsValid() { +func EachRef[U comparable, V comparable](a *Result[U], b *Result[V], fn func(U, V) error) (err error) { + var ( + zeroU U + zeroV V + ) + if a.Ref != zeroU && b.Ref != zeroV { err = fn(a.Ref, b.Ref) } for k, r := range a.Refs { @@ -111,7 +116,7 @@ func EachRef[U any, V any](a *Result[U], b *Result[V], fn func(U, V) error) (err if !ok { continue } - if reflect.ValueOf(r).IsValid() && reflect.ValueOf(r2).IsValid() { + if r != zeroU && r2 != zeroV { if err1 := fn(r, r2); err1 != nil && err == nil { err = err1 } @@ -127,7 +132,7 @@ func EachRef[U any, V any](a *Result[U], b *Result[V], fn func(U, V) error) (err break } att2 := atts2[i] - if reflect.ValueOf(att.Ref).IsValid() && reflect.ValueOf(att2.Ref).IsValid() { + if att.Ref != zeroU && att2.Ref != zeroV { if err1 := fn(att.Ref, att2.Ref); err1 != nil && err == nil { err = err1 } @@ -137,11 +142,13 @@ func EachRef[U any, V any](a *Result[U], b *Result[V], fn func(U, V) error) (err return err } -func ConvertResult[U any, V any](r *Result[U], fn func(U) (V, error)) (*Result[V], error) { +func ConvertResult[U comparable, V comparable](r *Result[U], fn func(U) (V, error)) (*Result[V], error) { + var zero U + r2 := &Result[V]{} var err error - if reflect.ValueOf(r.Ref).IsValid() { + if r.Ref != zero { r2.Ref, err = fn(r.Ref) if err != nil { return nil, err @@ -152,7 +159,7 @@ func ConvertResult[U any, V any](r *Result[U], fn func(U) (V, error)) (*Result[V r2.Refs = map[string]V{} } for k, r := range r.Refs { - if !reflect.ValueOf(r).IsValid() { + if r == zero { continue } r2.Refs[k], err = fn(r)