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

🌱 Update Scorecard API usage #336

Merged
merged 2 commits into from
Jun 8, 2022
Merged

🌱 Update Scorecard API usage #336

merged 2 commits into from
Jun 8, 2022

Conversation

azeemshaikh38
Copy link
Contributor

@azeemshaikh38 azeemshaikh38 commented Jun 6, 2022

Recent changes to scorecard-webapp API updates how Scorecard Action should integrate with the API. This PR make the necessary updates:

  • Use the updated POST URL - /projects/{host}/{org}/{repo}
  • Use the updated POST JSON
  • Pass the GITHUB_TOKEN to the API for scalability

Along with these changes, updates how Golang Scorecard Action will be built and tested:

  • Adds a new Dockerfile.golang and cloudbuild.yaml to update how Golang Action is built.
  • Updates documentation about e2e tests.

Part of #133. golang-staging branch will be deleted and need not be maintained after necessary changes to e2e tests are deployed.

Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Left some style comments and questions.

@ossf/scorecard-maintainers -- As an overarching goal, but also using this PR as context, we need to work on pull request hygiene/telling a story for future passerbys (which may include us).

Here's a quick list of questions/comments that I have:

Part of #133. Will update golang-staging branch to use the public image scorecard-action

Dockerfile.golang Show resolved Hide resolved
cloudbuild.yaml Show resolved Hide resolved
signing/signing.go Show resolved Hide resolved
signing/signing.go Outdated Show resolved Hide resolved
@azeemshaikh38 azeemshaikh38 force-pushed the azeems/golang-bug branch 2 times, most recently from e810343 to c2f8f16 Compare June 6, 2022 20:36
signing/signing.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 6, 2022

Codecov Report

Merging #336 (b8a85e5) into main (8e9099b) will decrease coverage by 0.39%.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #336      +/-   ##
==========================================
- Coverage   64.28%   63.88%   -0.40%     
==========================================
  Files           4        4              
  Lines         210      216       +6     
==========================================
+ Hits          135      138       +3     
- Misses         67       69       +2     
- Partials        8        9       +1     
Impacted Files Coverage Δ
signing/signing.go 32.14% <73.33%> (+1.37%) ⬆️

@azeemshaikh38
Copy link
Contributor Author

  • How is the golang-staging branch used?
  • Is this well-documented?

There is some documentation here about how golang-staging branch is being used in e2e tests - https://github.com/ossf/scorecard-action/tree/main/e2e. TLDR; is that golang-staging branch is used to deploy Scorecard action Golang code as a GitHub action without affecting the current bash action. We use it to test any changes to the Golang code.

I made some updates to the scorecard-webapp repo recently which changed the API (needed for the badges work). The current PR basically makes the necessary updates here to keep the integration working.

I will make another attempt at the PR description once we have reached an agreement on the overall changes.

  • The additional infra changes, like the Dockerfile and cloudbuild.yaml, should be a separate commit, again explaining why they need to be added

Below answers should probably cover why these were added.

  • Is there a reason we prefer gcr.io instead of GH container registry?

No strong reason except for personal familiarity with Google Cloud infra and that Scorecard image itself resides in GCP. Ok to use anything else.

The extra Dockerfile is only temporary (until we release the Golang code and delete any bash related code). Even today, we maintain 2 separate Dockerfiles (the second one lives in golang-staging).

azeemshaikh38 added a commit that referenced this pull request Jun 7, 2022
* 🌱 Bump github.com/ossf/scorecard/v4 from 4.2.0 to 4.3.0 (#313)

* 🌱 Bump github.com/ossf/scorecard/v4 from 4.2.0 to 4.3.0

Bumps [github.com/ossf/scorecard/v4](https://github.com/ossf/scorecard) from 4.2.0 to 4.3.0.
- [Release notes](https://github.com/ossf/scorecard/releases)
- [Changelog](https://github.com/ossf/scorecard/blob/main/.goreleaser.yml)
- [Commits](ossf/scorecard@v4.2.0...v4.3.0)

---
updated-dependencies:
- dependency-name: github.com/ossf/scorecard/v4
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* options: Restore logic for publishing results

Signed-off-by: Stephen Augustus <foo@auggie.dev>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Stephen Augustus <foo@auggie.dev>

* 🌱 Bump github/codeql-action from 2.1.10 to 2.1.11 (#311)

* 🌱 Bump github/codeql-action from 2.1.10 to 2.1.11

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2.1.10 to 2.1.11.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@2f58583...a3a6c12)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Fix version comments

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Stephen Augustus (he/him) <justaugustus@users.noreply.github.com>

* 📖 docs/e2e: Add information about golang-staging branch tests (#170)

Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>

Co-authored-by: Stephen Augustus (he/him) <justaugustus@users.noreply.github.com>

* 🌱 .github: Add dependency review action (#165)

Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>

* Update README.md (#319)

* 🌱 Bump github.com/caarlos0/env/v6 from 6.9.2 to 6.9.3

Bumps [github.com/caarlos0/env/v6](https://github.com/caarlos0/env) from 6.9.2 to 6.9.3.
- [Release notes](https://github.com/caarlos0/env/releases)
- [Changelog](https://github.com/caarlos0/env/blob/main/.goreleaser.yml)
- [Commits](caarlos0/env@v6.9.2...v6.9.3)

---
updated-dependencies:
- dependency-name: github.com/caarlos0/env/v6
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* 🌱 Bump debian from `fbaacd5` to `06a93cb`

Bumps debian from `fbaacd5` to `06a93cb`.

---
updated-dependencies:
- dependency-name: debian
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

* 🌱 Bump actions/setup-go from 3.1.0 to 3.2.0

Bumps [actions/setup-go](https://github.com/actions/setup-go) from 3.1.0 to 3.2.0.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@fcdc436...b22fbbc)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* ✨ Bump container hash to use scorecard v4.3.1 (#324)

* Update Dockerfile

* Update Dockerfile

* Update README.md (#325)

* Update Scorecard API usage

* Add documentation for e2e tests

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Stephen Augustus <foo@auggie.dev>
Co-authored-by: Stephen Augustus (he/him) <justaugustus@users.noreply.github.com>
Co-authored-by: Naveen <172697+naveensrinivasan@users.noreply.github.com>
Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
Co-authored-by: Azeem Shaikh <azeems@google.com>
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Thanks @azeemshaikh38!
I may have more follow-up questions, but let's get the CI checks unblocked first and foremost.

@justaugustus justaugustus merged commit dcb9126 into main Jun 8, 2022
@justaugustus justaugustus deleted the azeems/golang-bug branch June 8, 2022 16:18
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.

4 participants