Repository: sigstore/rekor
Pull request #1004 (2022-08-26 15:24:40)

enable blocking specific pluggable type versions from being inserted into the log
This will be leveraged by #973 to block v0.0.1 versions of the intoto type from being inserted into the future

@pxp928

Signed-off-by: Bob Callaway <bcallaway@google.com>

<!--
Thanks for opening a pull request! Please do not just delete this text.  The three fields below are mandatory.

Please remember to:
- This PR requires an issue. If it is a new feature, the issue should proceed the PR and will have allowed sufficent time for discussions to take place. Pleases use
issue tags such as "Closes #XYZ" or "Resolves sigstore/repo-name#XYZ".
  [documentation](https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
- ensure your commits are signed-off, as sigstore uses the [DCO](https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin) using `git commit -s`, or `git commit -s --amend` if you want to amend already existing commits
- lastly, ensure there are no merge commits!

Thank you :)
-->

#### Summary
<!--
 Explain the **motivation** for making this change. What existing problem does the pull request solve? How can reviewers test this PR?
-->

#### Release Note
<!--
Add a release note for each of the following conditions:

* Config changes (additions, deletions, updates)
* API additions—new endpoint, new response fields, or newly accepted request parameters
* Database changes (any)
* Websocket additions or changes
* Anything noteworthy to an administrator running private sigstore instances (err on the side of over-communicating)
* New features and improvements, including behavioural changes, UI changes and CLI changes
* Bug fixes and fixes of previous known issues
* Deprecation warnings, breaking changes, or compatibility notes

If no release notes are required write NONE. Use past-tense.

-->

#### Documentation
<!--

Does this change require an update to documentation? How will users implement your new feature?

Please reference a PR within https://docs.sigstore.dev

-->

codecov-commenter: # [Codecov](https://codecov.io/gh/sigstore/rekor/pull/1004?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore) Report
> Merging [#1004](https://codecov.io/gh/sigstore/rekor/pull/1004?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore) (77dcdec) into [main](https://codecov.io/gh/sigstore/rekor/commit/87c1978f5d8f1c1278a587a99f31c4fc987b1f1b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore) (87c1978) will **increase** coverage by `0.05%`.
> The diff coverage is `28.57%`.

```diff
@@            Coverage Diff             @@
##             main    #1004      +/-   ##
==========================================
+ Coverage   40.94%   41.00%   +0.05%     
==========================================
  Files          69       69              
  Lines        6496     6517      +21     
==========================================
+ Hits         2660     2672      +12     
- Misses       3554     3563       +9     
  Partials      282      282              
```


| [Impacted Files](https://codecov.io/gh/sigstore/rekor/pull/1004?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore) | Coverage Δ | |
|---|---|---|
| [cmd/rekor-cli/app/get.go](https://codecov.io/gh/sigstore/rekor/pull/1004/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore#diff-Y21kL3Jla29yLWNsaS9hcHAvZ2V0Lmdv) | `2.77% <0.00%> (ø)` | |
| [pkg/api/entries.go](https://codecov.io/gh/sigstore/rekor/pull/1004/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore#diff-cGtnL2FwaS9lbnRyaWVzLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/types/types.go](https://codecov.io/gh/sigstore/rekor/pull/1004/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore#diff-cGtnL3R5cGVzL3R5cGVzLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/types/entries.go](https://codecov.io/gh/sigstore/rekor/pull/1004/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore#diff-cGtnL3R5cGVzL2VudHJpZXMuZ28=) | `5.08% <15.38%> (-1.17%)` | :arrow_down: |
| [cmd/rekor-cli/app/pflag\_groups.go](https://codecov.io/gh/sigstore/rekor/pull/1004/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore#diff-Y21kL3Jla29yLWNsaS9hcHAvcGZsYWdfZ3JvdXBzLmdv) | `89.47% <66.66%> (+2.27%)` | :arrow_up: |
| [pkg/types/rekord/v0.0.1/entry.go](https://codecov.io/gh/sigstore/rekor/pull/1004/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore#diff-cGtnL3R5cGVzL3Jla29yZC92MC4wLjEvZW50cnkuZ28=) | `49.00% <0.00%> (+0.66%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sigstore)


Patch:
diff --git a/cmd/rekor-cli/app/get.go b/cmd/rekor-cli/app/get.go
index 440b10a..b75f8a5 100644
--- a/cmd/rekor-cli/app/get.go
+++ b/cmd/rekor-cli/app/get.go
@@ -159,7 +159,7 @@ func parseEntry(uuid string, e models.LogEntryAnon) (interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
-	eimpl, err := types.NewEntry(pe)
+	eimpl, err := types.UnmarshalEntry(pe)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/api/entries.go b/pkg/api/entries.go
index 3225ed6..dac3be0 100644
--- a/pkg/api/entries.go
+++ b/pkg/api/entries.go
@@ -101,7 +101,7 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC
 		if err != nil {
 			return nil, err
 		}
-		eimpl, err := types.NewEntry(pe)
+		eimpl, err := types.UnmarshalEntry(pe)
 		if err != nil {
 			return nil, err
 		}
@@ -161,7 +161,7 @@ func GetLogEntryByIndexHandler(params entries.GetLogEntryByIndexParams) middlewa
 
 func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middleware.Responder) {
 	ctx := params.HTTPRequest.Context()
-	entry, err := types.NewEntry(params.ProposedEntry)
+	entry, err := types.CreateVersionedEntry(params.ProposedEntry)
 	if err != nil {
 		return nil, handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf(validationError, err))
 	}
@@ -341,7 +341,7 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
 		for _, e := range params.Entry.Entries() {
 			e := e // https://golang.org/doc/faq#closures_and_goroutines
 			g.Go(func() error {
-				entry, err := types.NewEntry(e)
+				entry, err := types.UnmarshalEntry(e)
 				if err != nil {
 					return err
 				}
diff --git a/pkg/types/README.md b/pkg/types/README.md
index f77f240..81246c4 100644
--- a/pkg/types/README.md
+++ b/pkg/types/README.md
@@ -8,6 +8,10 @@ Rekor supports pluggable types (aka different schemas) for entries stored in the
 
 - Alpine Packages [schema](alpine/alpine_schema.json)
   - Versions: 0.0.1
+- COSE Envelopes [schema](cose/cose_schema.json)
+  - Versions: 0.0.1
+- HashedRekord [schema](hashedrekord/hashedrekord_schema.json)
+  - Versions: 0.0.1
 - Helm Provenance Files [schema](helm/helm_schema.json)
   - Versions: 0.0.1
 - In-Toto Attestations [schema](intoto/intoto_schema.json)
@@ -20,7 +24,7 @@ Rekor supports pluggable types (aka different schemas) for entries stored in the
   - Versions: 0.0.1
 - RPM Packages [schema](rpm/rpm_schema.json)
   - Versions: 0.0.1
-- COSE Envelopes [schema](cose/cose_schema.json)
+- TUF Metadata [schema](tuf/tuf_schema.json)
   - Versions: 0.0.1
 
 Refer to [Rekor docs](https://docs.sigstore.dev/rekor/pluggable-types) for adding support for new types.
diff --git a/pkg/types/entries.go b/pkg/types/entries.go
index 529ede0..cda7f27 100644
--- a/pkg/types/entries.go
+++ b/pkg/types/entries.go
@@ -59,8 +59,28 @@ func NewProposedEntry(ctx context.Context, kind, version string, props ArtifactP
 	return nil, fmt.Errorf("could not create entry for kind '%v'", kind)
 }
 
-// NewEntry returns the specific instance for the type and version specified in the doc
-func NewEntry(pe models.ProposedEntry) (EntryImpl, error) {
+// CreateVersionedEntry returns the specific instance for the type and version specified in the doc
+// This method should be used on the insertion flow, which validates that the specific version proposed
+// is permitted to be entered into the log.
+func CreateVersionedEntry(pe models.ProposedEntry) (EntryImpl, error) {
+	ei, err := UnmarshalEntry(pe)
+	if err != nil {
+		return nil, err
+	}
+	kind := pe.Kind()
+	if tf, found := TypeMap.Load(kind); found {
+		if !tf.(func() TypeImpl)().IsSupportedVersion(ei.APIVersion()) {
+			return nil, fmt.Errorf("entry kind '%v' does not support inserting entries of version '%v'", kind, ei.APIVersion())
+		}
+	}
+
+	return ei, nil
+}
+
+// UnmarshalEntry returns the specific instance for the type and version specified in the doc
+// This method does not check for whether the version of the entry could be currently inserted into the log,
+// and is useful when dealing with entries that have been persisted to the log.
+func UnmarshalEntry(pe models.ProposedEntry) (EntryImpl, error) {
 	if pe == nil {
 		return nil, errors.New("proposed entry cannot be nil")
 	}
@@ -73,7 +93,7 @@ func NewEntry(pe models.ProposedEntry) (EntryImpl, error) {
 		}
 		return t.UnmarshalEntry(pe)
 	}
-	return nil, fmt.Errorf("could not create entry for kind '%v'", kind)
+	return nil, fmt.Errorf("could not unmarshal entry for kind '%v'", kind)
 }
 
 // DecodeEntry maps the (abstract) input structure into the specific entry implementation class;


Test patch:
diff --git a/pkg/types/alpine/v0.0.1/entry_test.go b/pkg/types/alpine/v0.0.1/entry_test.go
index d53e0f3..153e748 100644
--- a/pkg/types/alpine/v0.0.1/entry_test.go
+++ b/pkg/types/alpine/v0.0.1/entry_test.go
@@ -157,7 +157,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/hashedrekord/v0.0.1/entry_test.go b/pkg/types/hashedrekord/v0.0.1/entry_test.go
index 99b1177..72a55d6 100644
--- a/pkg/types/hashedrekord/v0.0.1/entry_test.go
+++ b/pkg/types/hashedrekord/v0.0.1/entry_test.go
@@ -285,7 +285,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/helm/v0.0.1/entry_test.go b/pkg/types/helm/v0.0.1/entry_test.go
index 655e7b5..69eb38f 100644
--- a/pkg/types/helm/v0.0.1/entry_test.go
+++ b/pkg/types/helm/v0.0.1/entry_test.go
@@ -188,7 +188,7 @@ func TestCrossFieldValidation(t *testing.T) {
 				if err != nil {
 					t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 				}
-				if _, err := types.NewEntry(pe); err != nil {
+				if _, err := types.UnmarshalEntry(pe); err != nil {
 					t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 				}
 			}
diff --git a/pkg/types/intoto/v0.0.1/entry_test.go b/pkg/types/intoto/v0.0.1/entry_test.go
index 5d4151e..f01b854 100644
--- a/pkg/types/intoto/v0.0.1/entry_test.go
+++ b/pkg/types/intoto/v0.0.1/entry_test.go
@@ -270,7 +270,7 @@ func TestV001Entry_Unmarshal(t *testing.T) {
 				if err != nil {
 					t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tt.name, err)
 				}
-				canonicalEntry, err := types.NewEntry(pe)
+				canonicalEntry, err := types.UnmarshalEntry(pe)
 				if err != nil {
 					t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tt.name, err)
 				}
diff --git a/pkg/types/jar/v0.0.1/entry_test.go b/pkg/types/jar/v0.0.1/entry_test.go
index e56c62e..42ff533 100644
--- a/pkg/types/jar/v0.0.1/entry_test.go
+++ b/pkg/types/jar/v0.0.1/entry_test.go
@@ -108,7 +108,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/rekord/v0.0.1/entry_test.go b/pkg/types/rekord/v0.0.1/entry_test.go
index d0d7cee..0e79731 100644
--- a/pkg/types/rekord/v0.0.1/entry_test.go
+++ b/pkg/types/rekord/v0.0.1/entry_test.go
@@ -233,7 +233,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/rfc3161/v0.0.1/entry_test.go b/pkg/types/rfc3161/v0.0.1/entry_test.go
index 72f90d8..9f39f5b 100644
--- a/pkg/types/rfc3161/v0.0.1/entry_test.go
+++ b/pkg/types/rfc3161/v0.0.1/entry_test.go
@@ -182,7 +182,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/rpm/v0.0.1/entry_test.go b/pkg/types/rpm/v0.0.1/entry_test.go
index 6819ee4..3e50873 100644
--- a/pkg/types/rpm/v0.0.1/entry_test.go
+++ b/pkg/types/rpm/v0.0.1/entry_test.go
@@ -173,7 +173,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/tuf/v0.0.1/entry_test.go b/pkg/types/tuf/v0.0.1/entry_test.go
index 4dbc16c..f8d090f 100644
--- a/pkg/types/tuf/v0.0.1/entry_test.go
+++ b/pkg/types/tuf/v0.0.1/entry_test.go
@@ -208,7 +208,7 @@ func TestCrossFieldValidation(t *testing.T) {
 			if err != nil {
 				t.Errorf("unexpected err from Unmarshalling canonicalized entry for '%v': %v", tc.caseDesc, err)
 			}
-			if _, err := types.NewEntry(pe); err != nil {
+			if _, err := types.UnmarshalEntry(pe); err != nil {
 				t.Errorf("unexpected err from type-specific unmarshalling for '%v': %v", tc.caseDesc, err)
 			}
 		}
diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go
index 9be029f..ea7c530 100644
--- a/pkg/types/types_test.go
+++ b/pkg/types/types_test.go
@@ -65,7 +65,7 @@ func (e UnmarshalErrorValidEntry) ContextValidate(context context.Context, forma
 	return nil
 }
 
-func TestNewEntry(t *testing.T) {
+func TestUnmarshalEntry(t *testing.T) {
 	type TestCase struct {
 		entry         models.ProposedEntry
 		expectSuccess bool
@@ -83,7 +83,7 @@ func TestNewEntry(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		if _, err := NewEntry(tc.entry); (err == nil) != tc.expectSuccess {
+		if _, err := UnmarshalEntry(tc.entry); (err == nil) != tc.expectSuccess {
 			t.Errorf("unexpected error creating entry of type '%v': %v", tc.entry.Kind(), err)
 		}
 	}