From 4edb32ff34da9619283add6dc35da5397e1ef796 Mon Sep 17 00:00:00 2001 From: Bob Callaway Date: Fri, 26 Aug 2022 09:40:27 -0400 Subject: [PATCH 1/2] check supportedVersions list rather than directly reading from version map Signed-off-by: Bob Callaway --- cmd/rekor-cli/app/pflag_groups.go | 14 ++- cmd/rekor-cli/app/pflags_test.go | 184 +++++++++++++++++++++++++++++- go.mod | 2 + go.sum | 2 + 4 files changed, 194 insertions(+), 8 deletions(-) diff --git a/cmd/rekor-cli/app/pflag_groups.go b/cmd/rekor-cli/app/pflag_groups.go index aeabc4aec..03c3c4142 100644 --- a/cmd/rekor-cli/app/pflag_groups.go +++ b/cmd/rekor-cli/app/pflag_groups.go @@ -26,6 +26,8 @@ import ( "github.com/sigstore/rekor/pkg/types" "github.com/spf13/cobra" "github.com/spf13/viper" + + "golang.org/x/exp/slices" ) // addFlagToCmd adds the specified command of a specified type to the command's flag set @@ -166,21 +168,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 !slices.Contains(ti.SupportedVersions(), 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") diff --git a/cmd/rekor-cli/app/pflags_test.go b/cmd/rekor-cli/app/pflags_test.go index 6b1faa07b..64887c4aa 100644 --- a/cmd/rekor-cli/app/pflags_test.go +++ b/cmd/rekor-cli/app/pflags_test.go @@ -179,7 +179,7 @@ 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", @@ -187,7 +187,7 @@ func TestArtifactPFlags(t *testing.T) { 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", @@ -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, @@ -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, @@ -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) + } + } +} diff --git a/go.mod b/go.mod index 3553ab3f4..d963462ba 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 25e0dbf88..9b8625a78 100644 --- a/go.sum +++ b/go.sum @@ -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= From 2dd8f31b503645f93a25854ffb3160cec30324d1 Mon Sep 17 00:00:00 2001 From: Bob Callaway Date: Fri, 26 Aug 2022 10:42:31 -0400 Subject: [PATCH 2/2] move check into a type-specific method rather than delegating it to caller Signed-off-by: Bob Callaway --- cmd/rekor-cli/app/pflag_groups.go | 4 +--- pkg/types/types.go | 8 ++++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/rekor-cli/app/pflag_groups.go b/cmd/rekor-cli/app/pflag_groups.go index 03c3c4142..9cf7818e1 100644 --- a/cmd/rekor-cli/app/pflag_groups.go +++ b/cmd/rekor-cli/app/pflag_groups.go @@ -26,8 +26,6 @@ import ( "github.com/sigstore/rekor/pkg/types" "github.com/spf13/cobra" "github.com/spf13/viper" - - "golang.org/x/exp/slices" ) // addFlagToCmd adds the specified command of a specified type to the command's flag set @@ -188,7 +186,7 @@ func ParseTypeFlag(typeStr string) (string, string, error) { case 1: return typeStrings[0], "", nil case 2: - if !slices.Contains(ti.SupportedVersions(), typeStrings[1]) { + 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 diff --git a/pkg/types/types.go b/pkg/types/types.go index b271fd761..256bacd3a 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -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 @@ -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) } @@ -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 {