merge in Bob's changes

Signed-off-by: Priya Wadhwa <priya@chainguard.dev>

cmd/rekor-cli/app/upload.go

@@ -31,6 +31,7 @@ import (
 
 	"github.com/sigstore/rekor/cmd/rekor-cli/app/format"
 	"github.com/sigstore/rekor/pkg/client"
+	gen_client "github.com/sigstore/rekor/pkg/generated/client"
 	"github.com/sigstore/rekor/pkg/generated/client/entries"
 	"github.com/sigstore/rekor/pkg/generated/models"
 	"github.com/sigstore/rekor/pkg/log"
@@ -72,12 +73,9 @@ var uploadCmd = &cobra.Command{
 		if err != nil {
 			return nil, err
 		}
-		params := entries.NewCreateLogEntryParams()
-		params.SetTimeout(viper.GetDuration("timeout"))
+		var entry models.ProposedEntry
 
 		entryStr := viper.GetString("entry")
-		var resp *entries.CreateLogEntryCreated
-
 		if entryStr != "" {
 			var entryReader io.Reader
 			entryURL, err := url.Parse(entryStr)
@@ -95,62 +93,34 @@ var uploadCmd = &cobra.Command{
 					return nil, fmt.Errorf("error processing entry file: %w", err)
 				}
 			}
-			entry, err := models.UnmarshalProposedEntry(entryReader, runtime.JSONConsumer())
+			entry, err = models.UnmarshalProposedEntry(entryReader, runtime.JSONConsumer())
 			if err != nil {
 				return nil, fmt.Errorf("error parsing entry file: %w", err)
 			}
-			params.SetProposedEntry(entry)
-			r, err := rekorClient.Entries.CreateLogEntry(params)
-			if err != nil {
-				switch e := err.(type) {
-				case *entries.CreateLogEntryConflict:
-					return &uploadCmdOutput{
-						Location:      e.Location.String(),
-						AlreadyExists: true,
-					}, nil
-				default:
-					return nil, err
-				}
-			}
-			resp = r
 		} else {
-			// Start with the default entry and work your way down
-			typeStr, _, err := ParseTypeFlag(viper.GetString("type"))
-			if err != nil {
-				return nil, err
-			}
-			supportedVersions, err := GetSupportedVersions(viper.GetString("type"))
+			typeStr, versionStr, err := ParseTypeFlag(viper.GetString("type"))
 			if err != nil {
 				return nil, err
 			}
+
 			props := CreatePropsFromPflags()
 
-			for _, sv := range supportedVersions {
-				log.CliLogger.Infof("Trying to upload entry of type %s at version %s", typeStr, sv)
-				entry, err := types.NewProposedEntry(context.Background(), typeStr, sv, *props)
-				if err != nil {
-					return nil, fmt.Errorf("error: %w", err)
-				}
-				params.SetProposedEntry(entry)
-				r, err := rekorClient.Entries.CreateLogEntry(params)
-				if err == nil {
-					resp = r
-					break
-				}
-				switch e := err.(type) {
-				case *entries.CreateLogEntryConflict:
-					return &uploadCmdOutput{
-						Location:      e.Location.String(),
-						AlreadyExists: true,
-					}, nil
-				default:
-					log.CliLogger.Warnf("Failed to upload entry, will try with another supported version if one exists: %v", err)
-				}
+			entry, err = types.NewProposedEntry(context.Background(), typeStr, versionStr, *props)
+			if err != nil {
+				return nil, fmt.Errorf("error: %w", err)
 			}
 		}
-
-		if resp == nil {
-			return nil, fmt.Errorf("no valid entry was created")
+		resp, err := tryUpload(rekorClient, entry)
+		if err != nil {
+			switch e := err.(type) {
+			case *entries.CreateLogEntryConflict:
+				return &uploadCmdOutput{
+					Location:      e.Location.String(),
+					AlreadyExists: true,
+				}, nil
+			default:
+				return nil, err
+			}
 		}
 
 		var newIndex int64
@@ -187,6 +157,29 @@ var uploadCmd = &cobra.Command{
 	}),
 }
 
+func tryUpload(rekorClient *gen_client.Rekor, entry models.ProposedEntry) (*entries.CreateLogEntryCreated, error) {
+	params := entries.NewCreateLogEntryParams()
+	params.SetTimeout(viper.GetDuration("timeout"))
+	if pei, ok := entry.(types.ProposedEntryIterator); ok {
+		params.SetProposedEntry(pei.Get())
+	} else {
+		params.SetProposedEntry(entry)
+	}
+	resp, err := rekorClient.Entries.CreateLogEntry(params)
+	if err != nil {
+		if e, ok := err.(*entries.CreateLogEntryBadRequest); ok {
+			if pei, ok := entry.(types.ProposedEntryIterator); ok {
+				if pei.HasNext() {
+					log.CliLogger.Errorf("failed to upload entry: %v", e)
+					return tryUpload(rekorClient, pei.GetNext())
+				}
+			}
+		}
+		return nil, err
+	}
+	return resp, nil
+}
+
 func init() {
 	initializePFlagMap()
 	if err := addArtifactPFlags(uploadCmd); err != nil {

pkg/types/entries.go

@@ -45,6 +45,14 @@ type EntryWithAttestationImpl interface {
 	AttestationKeyValue() (string, []byte) // returns the key to be used when storing the attestation as well as the attestation itself
 }
 
+// ProposedEntryIterator is an iterator over a list of proposed entries
+type ProposedEntryIterator interface {
+	models.ProposedEntry
+	HasNext() bool
+	Get() models.ProposedEntry
+	GetNext() models.ProposedEntry
+}
+
 // EntryFactory describes a factory function that can generate structs for a specific versioned type
 type EntryFactory func() EntryImpl
 

pkg/types/intoto/intoto.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/sigstore/rekor/pkg/generated/models"
+	"github.com/sigstore/rekor/pkg/log"
 	"github.com/sigstore/rekor/pkg/types"
 	"golang.org/x/exp/slices"
 )
@@ -60,9 +61,41 @@ func (it BaseIntotoType) UnmarshalEntry(pe models.ProposedEntry) (types.EntryImp
 }
 
 func (it *BaseIntotoType) CreateProposedEntry(ctx context.Context, version string, props types.ArtifactProperties) (models.ProposedEntry, error) {
+	var head ProposedIntotoEntryIterator
+	var next *ProposedIntotoEntryIterator
 	if version == "" {
+		// get default version as head of list
 		version = it.DefaultVersion()
+		ei, err := it.VersionedUnmarshal(nil, version)
+		if err != nil {
+			return nil, fmt.Errorf("fetching default Intoto version implementation: %w", err)
+		}
+		pe, err := ei.CreateFromArtifactProperties(ctx, props)
+		if err != nil {
+			return nil, fmt.Errorf("creating default Intoto entry: %w", err)
+		}
+		head.ProposedEntry = pe
+		next = &head
+		for _, v := range it.SupportedVersions() {
+			if v == it.DefaultVersion() {
+				continue
+			}
+			ei, err := it.VersionedUnmarshal(nil, v)
+			if err != nil {
+				log.Logger.Errorf("fetching Intoto version (%v) implementation: %w", v, err)
+				continue
+			}
+			versionedPE, err := ei.CreateFromArtifactProperties(ctx, props)
+			if err != nil {
+				log.Logger.Errorf("error creating Intoto entry of version (%v): %w", v, err)
+				continue
+			}
+			next.next = &ProposedIntotoEntryIterator{versionedPE, nil}
+			next = next.next.(*ProposedIntotoEntryIterator)
+		}
+		return head, nil
 	}
+
 	ei, err := it.VersionedUnmarshal(nil, version)
 	if err != nil {
 		return nil, fmt.Errorf("fetching Intoto version implementation: %w", err)
@@ -74,9 +107,7 @@ func (it BaseIntotoType) DefaultVersion() string {
 	return "0.0.2"
 }
 
-// SupportedVersions returns the supported versions for this type;
-// it deliberately omits 0.0.1 from the list of supported versions as that
-// version did not persist signatures inside the log entry
+// SupportedVersions returns the supported versions for this type in the order of preference
 func (it BaseIntotoType) SupportedVersions() []string {
 	return []string{"0.0.2", "0.0.1"}
 }
@@ -85,3 +116,20 @@ func (it BaseIntotoType) SupportedVersions() []string {
 func (it *BaseIntotoType) IsSupportedVersion(proposedVersion string) bool {
 	return slices.Contains(it.SupportedVersions(), proposedVersion)
 }
+
+type ProposedIntotoEntryIterator struct {
+	models.ProposedEntry
+	next models.ProposedEntry
+}
+
+func (p ProposedIntotoEntryIterator) HasNext() bool {
+	return p.next != nil
+}
+
+func (p ProposedIntotoEntryIterator) GetNext() models.ProposedEntry {
+	return p.next
+}
+
+func (p ProposedIntotoEntryIterator) Get() models.ProposedEntry {
+	return p.ProposedEntry
+}

tests/e2e_test.go

@@ -33,19 +33,16 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"reflect"
-	"runtime"
 	"strconv"
 	"strings"
 	"testing"
 	"time"
 
 	"golang.org/x/sync/errgroup"
-	"sigs.k8s.io/release-utils/version"
 
 	"github.com/cyberphone/json-canonicalization/go/src/webpki.org/jsoncanonicalizer"
 	"github.com/go-openapi/strfmt"
@@ -524,6 +521,7 @@ func TestIntoto(t *testing.T) {
 	write(t, string(eb), attestationPath)
 	write(t, ecdsaPub, pubKeyPath)
 
+	// If we do it twice, it should already exist
 	out := runCli(t, "upload", "--artifact", attestationPath, "--type", "intoto", "--public-key", pubKeyPath)
 	outputContains(t, out, "Created entry at")
 	uuid := getUUIDFromUploadOutput(t, out)
@@ -653,6 +651,7 @@ func TestIntotoMultiSig(t *testing.T) {
 	write(t, ecdsaPub, ecdsapubKeyPath)
 	write(t, pubKey, rsapubKeyPath)
 
+	// If we do it twice, it should already exist
 	out := runCli(t, "upload", "--artifact", attestationPath, "--type", "intoto", "--public-key", ecdsapubKeyPath, "--public-key", rsapubKeyPath)
 	outputContains(t, out, "Created entry at")
 	uuid := getUUIDFromUploadOutput(t, out)
@@ -696,6 +695,7 @@ func TestIntotoMultiSig(t *testing.T) {
 
 }
 
+/*
 func TestIntotoBlockV001(t *testing.T) {
 	td := t.TempDir()
 	attestationPath := filepath.Join(td, "attestation.json")
@@ -789,6 +789,7 @@ func TestIntotoBlockV001(t *testing.T) {
 		t.Fatalf("failed inserting v0.0.1 entry: %v", err)
 	}
 }
+*/
 
 func TestTimestampArtifact(t *testing.T) {
 	var out string