Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add provenance-name to the outputs #1844

Merged
merged 5 commits into from
Mar 22, 2023
Merged

feat: add provenance-name to the outputs #1844

merged 5 commits into from
Mar 22, 2023

Conversation

developer-guy
Copy link
Contributor

Fixes #1842

@developer-guy developer-guy changed the title add provenance-name to the outputs feat: add provenance-name to the outputs Mar 19, 2023
Copy link
Member

@ianlewis ianlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

  • Can you add an note to the CHANGELOG.md for the new output?
  • Can you add documention for the output to the README for the Go workflow?

.github/workflows/builder_go_slsa3.yml Outdated Show resolved Hide resolved
@developer-guy
Copy link
Contributor Author

  • Can you add an note to the CHANGELOG.md for the new output?
  • Can you add documention for the output to the README for the Go workflow?

PTAL @ianlewis

@developer-guy developer-guy requested review from ianlewis and removed request for asraa, joshuagl and laurentsimon March 21, 2023 09:51
@asraa
Copy link
Collaborator

asraa commented Mar 21, 2023

PTAL @ianlewis

@developer-guy oops, I think a commit may be missing!

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
@developer-guy
Copy link
Contributor Author

You'll need to update code to populate the provenance filename, probably in

// Share the resolved name of the binary.
if err := github.SetOutput("go-binary-name", filename); err != nil {
return err
}
// Share the command used.
if err := github.SetOutput("go-command", command); err != nil {
return err
}
// Share the env variables used.
if err := github.SetOutput("go-env", menv); err != nil {
return err
}
// Share working directory necessary for issuing the vendoring command.
return github.SetOutput("go-working-dir", dir)

is this necessary ? 🤔

@developer-guy
Copy link
Contributor Author

because I updated the tests according to the changes to see whether it is working, and it worked.

@laurentsimon
Copy link
Collaborator

You'll need to update code to populate the provenance filename, probably in

// Share the resolved name of the binary.
if err := github.SetOutput("go-binary-name", filename); err != nil {
return err
}
// Share the command used.
if err := github.SetOutput("go-command", command); err != nil {
return err
}
// Share the env variables used.
if err := github.SetOutput("go-env", menv); err != nil {
return err
}
// Share working directory necessary for issuing the vendoring command.
return github.SetOutput("go-working-dir", dir)

is this necessary ? 🤔

nope, I delete my comment. I mis-read ^^

@laurentsimon
Copy link
Collaborator

laurentsimon commented Mar 21, 2023

You need to run the toc CLI as in the failing pre-submit in order to format the .md properly

@laurentsimon
Copy link
Collaborator

$ npm install markdown-toc
$ ./node_modules/markdown-toc/cli.js --bullets="-" -i README.md

should work. Let me know.

Signed-off-by: Ian Lewis <ianlewis@google.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
Copy link
Member

@ianlewis ianlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I went ahead and made the changes based on @laurentsimon's comments since he went ahead and committed changes to our e2e tests.

@ianlewis ianlewis enabled auto-merge (squash) March 22, 2023 02:59
@ianlewis ianlewis merged commit 66b541d into slsa-framework:main Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] SLSA 3 Go Builder add provenance name to the workflow outputs
4 participants