-
Notifications
You must be signed in to change notification settings - Fork 9
Conversation
internal/model/provenance.go
Outdated
@@ -12,36 +12,23 @@ | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. | |||
|
|||
// Package common provides utility functions for building and verifying released binaries. | |||
package common | |||
// Package provenance provides the internal representation of a provenance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong package name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Fixed.
internal/verification/reference.go
Outdated
"github.com/pelletier/go-toml" | ||
) | ||
|
||
// ReferenceValues given by the product team to verify provenances against. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't particularly like the "product team" terminology. How about "verifier" or "client"? I don't really like those either, but I think we should keep thinking about a better alternative (after this PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't care so much where these values come from, at least in this documentation. Updated the comments.
// ReferenceValues given by the product team to verify provenances against. | ||
type ReferenceValues struct { | ||
// The digests of the binaries whose provenance the product team wants to verify. | ||
BinarySHA256Digests []string `toml:"binary_sha256_digests"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why these are only sha256? Could they not be arbitrary digests, as long as they have the appropriate prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general they could, but we use SHA256. We can relax this restriction in the future if needed.
// If true the product team wants the provenance to have a non-empty build command. | ||
WantBuildCmds bool `toml:"want_build_cmds"` | ||
// The digests of the builder images the product team trusts to build the binary. | ||
BuilderImageSHA256Digests []string `toml:"builder_image_sha256_digests"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For slice fields, I assume that means that any of those values is sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Updated the comment to better reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review.
internal/model/provenance.go
Outdated
@@ -12,36 +12,23 @@ | |||
// See the License for the specific language governing permissions and | |||
// limitations under the License. | |||
|
|||
// Package common provides utility functions for building and verifying released binaries. | |||
package common | |||
// Package provenance provides the internal representation of a provenance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Fixed.
internal/verification/reference.go
Outdated
"github.com/pelletier/go-toml" | ||
) | ||
|
||
// ReferenceValues given by the product team to verify provenances against. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't care so much where these values come from, at least in this documentation. Updated the comments.
// ReferenceValues given by the product team to verify provenances against. | ||
type ReferenceValues struct { | ||
// The digests of the binaries whose provenance the product team wants to verify. | ||
BinarySHA256Digests []string `toml:"binary_sha256_digests"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general they could, but we use SHA256. We can relax this restriction in the future if needed.
// If true the product team wants the provenance to have a non-empty build command. | ||
WantBuildCmds bool `toml:"want_build_cmds"` | ||
// The digests of the builder images the product team trusts to build the binary. | ||
BuilderImageSHA256Digests []string `toml:"builder_image_sha256_digests"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Updated the comment to better reflect this.
Fixes #220
Splits the package common moving some parts of it to a new package
verification
and renaming the rest tomodel
.