Skip to content

Commit

Permalink
check supportedVersions list rather than directly reading from versio…
Browse files Browse the repository at this point in the history 
…n map (#1003)

* check supportedVersions list rather than directly reading from version map

Signed-off-by: Bob Callaway <bcallaway@google.com>

* move check into a type-specific method rather than delegating it to caller

Signed-off-by: Bob Callaway <bcallaway@google.com>

Signed-off-by: Bob Callaway <bcallaway@google.com>
  • Loading branch information
@bobcallaway
bobcallaway authored Aug 26, 2022
1 parent 4923f60 commit 557cb32
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 8 deletions.
12 changes: 10 additions & 2 deletions cmd/rekor-cli/app/pflag_groups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,21 +166,29 @@ func CreatePropsFromPflags() *types.ArtifactProperties {
return props
}

//TODO: add tests for this
// ParseTypeFlag validates the requested type (and optional version) are supported
func ParseTypeFlag(typeStr string) (string, string, error) {
// typeStr can come in as:
// type -> use default version for this kind
// type:version_string -> attempt to use specified version string

typeStrings := strings.SplitN(typeStr, ":", 2)
if _, ok := types.TypeMap.Load(typeStrings[0]); !ok {
tf, ok := types.TypeMap.Load(typeStrings[0])
if !ok {
return "", "", fmt.Errorf("unknown type %v", typeStrings[0])
}
ti := tf.(func() types.TypeImpl)()
if ti == nil {
return "", "", fmt.Errorf("type %v is not implemented", typeStrings[0])
}

switch len(typeStrings) {
case 1:
return typeStrings[0], "", nil
case 2:
if !ti.IsSupportedVersion(typeStrings[1]) {
return "", "", fmt.Errorf("type %v does not support version %v", typeStrings[0], typeStrings[1])
}
return typeStrings[0], typeStrings[1], nil
}
return "", "", errors.New("malformed type string")
Expand Down
184 changes: 178 additions & 6 deletions cmd/rekor-cli/app/pflags_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,15 @@ func TestArtifactPFlags(t *testing.T) {
expectValidateSuccess: true,
},
{
caseDesc: "nonexistant local artifact",
caseDesc: "nonexistent local artifact",
artifact: "../../../tests/not_a_file",
signature: "../../../tests/test_file.sig",
publicKey: "../../../tests/test_public_key.key",
expectParseSuccess: false,
expectValidateSuccess: false,
},
{
caseDesc: "nonexistant remote artifact",
caseDesc: "nonexistent remote artifact",
artifact: testServer.URL + "/not_found",
signature: "../../../tests/test_file.sig",
publicKey: "../../../tests/test_public_key.key",
Expand Down Expand Up @@ -531,13 +531,13 @@ func TestSearchPFlags(t *testing.T) {
expectValidateSuccess: true,
},
{
caseDesc: "nonexistant local artifact",
caseDesc: "nonexistent local artifact",
artifact: "../../../tests/not_a_file",
expectParseSuccess: false,
expectValidateSuccess: false,
},
{
caseDesc: "nonexistant remote artifact",
caseDesc: "nonexistent remote artifact",
artifact: testServer.URL + "/not_found",
expectParseSuccess: true,
expectValidateSuccess: true,
Expand All @@ -562,13 +562,13 @@ func TestSearchPFlags(t *testing.T) {
expectValidateSuccess: true,
},
{
caseDesc: "nonexistant local public key",
caseDesc: "nonexistent local public key",
publicKey: "../../../tests/not_a_file",
expectParseSuccess: false,
expectValidateSuccess: false,
},
{
caseDesc: "nonexistant remote public key",
caseDesc: "nonexistent remote public key",
publicKey: testServer.URL + "/not_found",
expectParseSuccess: true,
expectValidateSuccess: true,
Expand Down Expand Up @@ -643,3 +643,175 @@ func TestSearchPFlags(t *testing.T) {
}
}
}

func TestParseTypeFlag(t *testing.T) {
type test struct {
caseDesc string
typeStr string
expectSuccess bool
}

tests := []test{
{
caseDesc: "bogus",
typeStr: "bogus",
expectSuccess: false,
},
{
caseDesc: "rekord",
typeStr: "rekord",
expectSuccess: true,
},
{
caseDesc: "explicit rekord v0.0.1",
typeStr: "rekord:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent rekord v0.0.0",
typeStr: "rekord:0.0.0",
expectSuccess: false,
},
{
caseDesc: "hashedrekord",
typeStr: "hashedrekord",
expectSuccess: true,
},
{
caseDesc: "explicit hashedrekord v0.0.1",
typeStr: "hashedrekord:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent hashedrekord v0.0.0",
typeStr: "hashedrekord:0.0.0",
expectSuccess: false,
},
{
caseDesc: "alpine",
typeStr: "alpine",
expectSuccess: true,
},
{
caseDesc: "explicit alpine v0.0.1",
typeStr: "alpine:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent alpine v0.0.0",
typeStr: "alpine:0.0.0",
expectSuccess: false,
},
{
caseDesc: "cose",
typeStr: "cose",
expectSuccess: true,
},
{
caseDesc: "explicit cose v0.0.1",
typeStr: "cose:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent cose v0.0.0",
typeStr: "cose:0.0.0",
expectSuccess: false,
},
{
caseDesc: "helm",
typeStr: "helm",
expectSuccess: true,
},
{
caseDesc: "explicit helm v0.0.1",
typeStr: "helm:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent helm v0.0.0",
typeStr: "helm:0.0.0",
expectSuccess: false,
},
{
caseDesc: "intoto",
typeStr: "intoto",
expectSuccess: true,
},
{
caseDesc: "explicit intoto v0.0.1",
typeStr: "intoto:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent intoto v0.0.0",
typeStr: "intoto:0.0.0",
expectSuccess: false,
},
{
caseDesc: "jar",
typeStr: "jar",
expectSuccess: true,
},
{
caseDesc: "explicit jar v0.0.1",
typeStr: "jar:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent jar v0.0.0",
typeStr: "jar:0.0.0",
expectSuccess: false,
},
{
caseDesc: "rfc3161",
typeStr: "rfc3161",
expectSuccess: true,
},
{
caseDesc: "explicit rfc3161 v0.0.1",
typeStr: "rfc3161:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent rfc3161 v0.0.0",
typeStr: "rfc3161:0.0.0",
expectSuccess: false,
},
{
caseDesc: "rpm",
typeStr: "rpm",
expectSuccess: true,
},
{
caseDesc: "explicit rpm v0.0.1",
typeStr: "rpm:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent rpm v0.0.0",
typeStr: "rpm:0.0.0",
expectSuccess: false,
},
{
caseDesc: "tuf",
typeStr: "tuf",
expectSuccess: true,
},
{
caseDesc: "explicit tuf v0.0.1",
typeStr: "tuf:0.0.1",
expectSuccess: true,
},
{
caseDesc: "non-existent tuf v0.0.0",
typeStr: "tuf:0.0.0",
expectSuccess: false,
},
}

for _, tc := range tests {
if _, _, err := ParseTypeFlag(tc.typeStr); (err == nil) != tc.expectSuccess {
t.Fatalf("unexpected error parsing type flag in '%v': %v", tc.caseDesc, err)
}
}
}
2 changes: 2 additions & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ require (
sigs.k8s.io/release-utils v0.7.3
)

require golang.org/x/exp v0.0.0-20220823124025-807a23277127

require (
cloud.google.com/go v0.103.0 // indirect
cloud.google.com/go/compute v1.7.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20220823124025-807a23277127 h1:S4NrSKDfihhl3+4jSTgwoIevKxX9p7Iv9x++OEIptDo=
golang.org/x/exp v0.0.0-20220823124025-807a23277127/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand Down
8 changes: 8 additions & 0 deletions pkg/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/sigstore/rekor/pkg/generated/models"
"github.com/sigstore/rekor/pkg/log"
"golang.org/x/exp/slices"
)

// TypeMap stores mapping between type strings and entry constructors
Expand All @@ -42,6 +43,7 @@ type TypeImpl interface {
CreateProposedEntry(context.Context, string, ArtifactProperties) (models.ProposedEntry, error)
DefaultVersion() string
SupportedVersions() []string
IsSupportedVersion(string) bool
UnmarshalEntry(pe models.ProposedEntry) (EntryImpl, error)
}

Expand All @@ -62,10 +64,16 @@ func (rt *RekorType) VersionedUnmarshal(pe models.ProposedEntry, version string)
return entry, entry.Unmarshal(pe)
}

// SupportedVersions returns a list of versions of this type that can be currently entered into the log
func (rt *RekorType) SupportedVersions() []string {
return rt.VersionMap.SupportedVersions()
}

// IsSupportedVersion returns true if the version can be inserted into the log, and false if not
func (rt *RekorType) IsSupportedVersion(proposedVersion string) bool {
return slices.Contains(rt.SupportedVersions(), proposedVersion)
}

// ListImplementedTypes returns a list of all type strings currently known to
// be implemented
func ListImplementedTypes() []string {
Expand Down

0 comments on commit 557cb32

Please sign in to comment.