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

go modules: Allowlist specific hashicorp modules #1595

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

oshoval
Copy link
Collaborator

@oshoval oshoval commented Aug 14, 2023

What this PR does / why we need it:
Due to [1] we need to make sure not to use BSL modules.

Luckily the current we use have not changed.
The ones that are not changed are SDK/API and general Go libraries.
"HashiCorp APIs, SDKs, and almost all other libraries will remain MPL 2.0." [1] [2]

This commit creates a github action which whitelists them.
Any other module of hashicorp will be rejected,
and will need to be manually examined if it uses MPL (or other non restrictive license) or BSL.

[1] https://www.hashicorp.com/blog/hashicorp-adopts-business-source-license
[2] cncf/foundation#617 (comment)

Special notes for your reviewer:

Release note:

None

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Aug 14, 2023
@oshoval
Copy link
Collaborator Author

oshoval commented Aug 14, 2023

/cc @maya-r

@maya-r
Copy link

maya-r commented Aug 14, 2023

I wonder what happened to our fossa lanes, which seem to be deactivated for kubevirt/kubevirt, which served this purpose but more widely - they tested the licenses of all our modules. (However, I am not opposed to a list of allowed modules)

@oshoval
Copy link
Collaborator Author

oshoval commented Aug 15, 2023

Print all errors at once
Remove IFS because lines dont start / end with whitespace, and even if so, it won't affect

@oshoval oshoval changed the title go mod: Whitelist specific hashicorp modules go modules: Whitelist specific hashicorp modules Aug 15, 2023
module=$(echo "$line" | cut -d ' ' -f 1)
if [[ $module == github.com/hashicorp/* ]]; then
if ! [[ " ${allowed_hashicorp_modules[*]} " == *" $module "* ]]; then
echo "found non whitelisted hashicorp module: $module"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "found non whitelisted hashicorp module: $module"
echo "found non allowlisted hashicorp module: $module"

done < go.sum

if [[ $error_found == true ]]; then
echo "non whitelisted hashicorp modules found. exiting with error"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "non whitelisted hashicorp modules found. exiting with error"
echo "non 03589504 hashicorp modules found. exiting with error"

Please include reasoning why we cannot allow any unlisted modules. And also what should be done to resolve the issue (review the licensing and either add it to the list or use a different library)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is in the commit message, self explanatory isn't it ?
Will add if you prefer of course.

This commit creates a github action which allowlists them.
Any other module of hashicorp will be rejected, and will need
to be manually examined if it uses MPL (or other non restrictive license)
or BSL.

Copy link
Member

Choose a reason for hiding this comment

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

When the check fails on my PR, i will not be looking for the PR that added the check. I would like the message on my PR to tell me what I should do to get it merged

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done :)

echo "non whitelisted hashicorp modules found. exiting with error"
exit 1
fi
echo "All hashicorp modules are whitelisted"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "All hashicorp modules are whitelisted"
echo "All included hashicorp modules are allowlisted"

@oshoval oshoval changed the title go modules: Whitelist specific hashicorp modules go modules: Allowlist specific hashicorp modules Aug 23, 2023
Due to [1] We need to make sure not to use BSL modules.

Luckily the current we use have not changed.
The ones that are not changed are SDK/API and general Go libraries.
"HashiCorp APIs, SDKs, and almost all other libraries will remain MPL 2.0." [1] [2]

This commit creates a github action which allowlists them.
Any other module of hashicorp will be rejected, and will need
to be manually examined if it uses MPL (or other non restrictive license)
or BSL.

[1] https://www.hashicorp.com/blog/hashicorp-adopts-business-source-license
[2] cncf/foundation#617 (comment)

Signed-off-by: Or Shoval <oshoval@redhat.com>
@oshoval
Copy link
Collaborator Author

oshoval commented Aug 23, 2023

Thanks
Addressed comments

@sonarcloud
Copy link

sonarcloud bot commented Aug 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@kubevirt-bot
Copy link
Collaborator

@oshoval: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-e2e-cnao-multus-dynamic-networks-functests 6dd00f5 link false /test pull-e2e-cnao-multus-dynamic-networks-functests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Copy link
Member

@phoracek phoracek left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 23, 2023
@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: phoracek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 23, 2023
@kubevirt-bot kubevirt-bot merged commit de53805 into kubevirt:main Aug 23, 2023
1 check passed
@oshoval
Copy link
Collaborator Author

oshoval commented Aug 24, 2023

Thanks,
Since we want to use it on all of our repos, should we just copy the file all over?
or

  1. Have the other repos fetch the list from CNAO (the list will resides on a dedicated file)
  2. Have CNAO run those checks when it clones the components during e2e, it will detect problems on component bumping, not on component pushes, but maybe it is good enough?

Not sure it worth it, but it will allow central changes, which is nice

@phoracek
Copy link
Member

I would vote to just copy it. It would be simpler and it would keep those repos independent

@oshoval
Copy link
Collaborator Author

oshoval commented Sep 7, 2023

It seems MPL2 need to be in exception list [1], [2]

but we need to do so only for the direct dependencies (BenTheElder raised it, and also on [1])

still not sure about those, monitoring the decisions of kubevirt maintainers

[1] cncf/foundation#617 (comment)
[2] https://github.com/cncf/foundation/blob/main/license-exceptions/cncf-exceptions-2023-06-27.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants