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

Added the scorecard github action and its badge #10421

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

harshitasao
Copy link
Contributor

Type of change

  • Enhancement / new feature

Description

PR to add the Scorecard GitHub Action and its badge in the README file.

Fixes #10342

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

@scholzj scholzj added this to the 0.43.0 milestone Aug 5, 2024
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I left some questions, but I guess it looks good. But I assume it will really run only after we merge it, right?

on:
# For Branch-Protection check. Only the default branch is supported. See
# https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection
branch_protection_rule:
Copy link
Member

Choose a reason for hiding this comment

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

Is this expected to have no value?

# To guarantee Maintained check is occasionally updated. See
# https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained
schedule:
- cron: '39 7 * * 6'
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for choosing this particular time (it seems to be 7:39)? I assume having a different run time on different repo helps to not overload the servers. But I wonder if some comment explaining it would help to avoid someone changing it in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The time is generated by the scorecard github action; it is chosen randomly.

@scholzj scholzj requested review from ppatierno and a team August 5, 2024 23:58
@Frawless
Copy link
Member

Frawless commented Aug 6, 2024

Thanks for the PR. I left some questions, but I guess it looks good. But I assume it will really run only after we merge it, right?

I think if it will be executed even as part of the this PR when the cron expression is removed, but I am not sure. At least when I was adding some GA in different repos it worked like that when I didnt have there cron trigger.

id-token: write
# Uncomment the permissions below if installing in a private repository.
# contents: read
# actions: read
Copy link
Member

Choose a reason for hiding this comment

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

this is not a private repo I guess we can remove this line. It seems they are coming from a template you are adapting here but we can remove what we don't need I guess, or?

# - you want to enable the Branch-Protection check on a *public* repository, or
# - you are installing Scorecard on a *private* repository
# To create the PAT, follow the steps in https://github.com/ossf/scorecard-action?tab=readme-ov-file#authentication-with-fine-grained-pat-optional.
# repo_token: ${{ secrets.SCORECARD_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

The comment says "you want to enable the Branch-Protection check on a public repository" so should we uncomment this repo_token. AFAIU we are enabling the branch protection due to the first branch_protection_rule field (even if it's empty, see Jakub's question).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Branch-Protection doesn’t work when running the Scorecard Action. Or, rather, we’d need to feed it a GitHub Personal Access Token (PAT), and the risks should the PAT ever leak are greater than the benefit we get from having the Action run the check, so Scorecard itself recommends against giving a PAT.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I didn't get you. So is branch protection enabled or not via your contribution? What about the branch_protection_rule: field?

# - See https://github.com/ossf/scorecard-action#publishing-results.
# For private repositories:
# - `publish_results` will always be set to `false`, regardless
# of the value entered here.
Copy link
Member

Choose a reason for hiding this comment

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

again I would just delete this comment about private repositories.

@scholzj scholzj modified the milestones: 0.43.0, 0.44.0 Aug 19, 2024
Signed-off-by: harshitasao <[email protected]>
@scholzj
Copy link
Member

scholzj commented Aug 27, 2024

@ppatierno is this fine with you now?

Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@scholzj
Copy link
Member

scholzj commented Aug 28, 2024

Thanks for the PR @harshitasao

@scholzj scholzj merged commit 6d25471 into strimzi:main Aug 28, 2024
13 checks passed
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.

5 participants