Initial API for Github provenance generation.#3
Conversation
laurentsimon
left a comment
There was a problem hiding this comment.
Thanks!
I like the breakdown of functionalities.
In case it's useful, here's one design pattern we use in scorecard to share the same arguments for API and CLI https://github.com/ossf/scorecard/blob/main/cmd/root.go#L47 and https://github.com/ossf/scorecard/blob/main/options/options.go
Maybe not be needed, but pointing it out in case you find it useful.
Yeah, cosign and friends do this too. I may get there eventually. Thanks for pointing it out :) |
|
BTW, @laurentsimon are we using "The GOSST team" for the copyright headers in these projects? |
|
Made a couple of new (hopefully) minor API decisions
|
asraa
left a comment
There was a problem hiding this comment.
This looks so good! Thank you Ian!
"override" means there will be defaults, what will they be? |
Override was maybe the wrong word. "Defaults" are set by NewWorkflowRun which creates a generic WorkflowRun with a proveance-only buildType ( It would be nice to have better extension points for other builders to set their own buildType and override specific bits like At least, that was my thinking. The simplest way to use it would be something like: r := slsa.NewWorkflowRun(subjects, githubContext)
r.BuildType = golangSpecificBuildType
r.BuildConfig = /* custom Go builder config (steps etc.) */
p, _ := slsa.HostedActionsProvenance(r)
/* sign and upload */ |
Let's start with this, it's simple and is flexible to test in various scenarios/use cases. |
Just a last-minute thought. We could also have r := slsa.NewWorkflowRun(subjects, githubContext)
.WithBuildType("some_string")
.WithBuildConfig(some_struct)func (self * WorkflowRun) WithBuildConfig(arg interface{}) *WorkflowRun{
self.BuildConfig = arg
return self
}Can do that in a follow-up PR if we want |
|
@ianlewis let us know when you've taken notes of the latest comments, and we'll merge. Thanks! |
BuildConfig is empty so this works but I think the use cases for others will often be to alter individual fields rather than set the full value ( |
Maybe |
|
I think we're good to merge from my end. |
Filed #13 |
|
Merging, thanks @ianlewis ! |
See #2