knative / hack

Knative common scripts.
Apache License 2.0
18 stars 64 forks source link

update_licenses should still run when repos don't have vendor folder #315

Closed dprotaso closed 1 year ago

dprotaso commented 1 year ago

see discussion here: https://github.com/knative/pkg/pull/2810#discussion_r1319110291

Caused by #311

dprotaso commented 1 year ago

/assign @cardil

cardil commented 1 year ago

I don't agree this is required. There is no such requirement anywhere in CNCF docs.

See either https://github.com/cncf/foundation/blob/main/charter.md#11-ip-policy and https://github.com/cncf/foundation/blob/main/allowed-third-party-license-policy.md

Both docs focus on license use and checking. None of them refers to a need to embedding the deps license in a specific folder, nor in a container.

dprotaso commented 1 year ago

This has been the practice of the project since it's inception. The requirements were coming from Google's OSPO office. I opened a CNCF request months back to see if we should continue it.

Also the update_licenses checks also ensures we don't pull in GPL code etc.

Whether we change this practice should be a separate discussion from vendoring dependencies.

Screenshot 2023-09-19 at 8 18 24 AM
cardil commented 1 year ago

@dprotaso: Also the update_licenses checks also ensures we don't pull in GPL code etc.

We still run that check, within the presubmit build test: https://github.com/knative/hack/blob/0bb79ff2d16224e9e3d689d4651066142831eb21/presubmit-tests.sh#L157

upodroid commented 1 year ago

We should keep the checks for forbidden licenses but drop the licenses being published to third_party folder

cardil commented 1 year ago

@upodroid We should keep the checks for forbidden licenses but drop the licenses being published to third_party folder

Exaclly what https://github.com/knative/hack/pull/311 did.

dprotaso commented 1 year ago

There is no such requirement anywhere in CNCF docs.

See either https://github.com/cncf/foundation/blob/main/charter.md#11-ip-policy and https://github.com/cncf/foundation/blob/main/allowed-third-party-license-policy.md

We have MPL in some repos - given that the CNCF Allowlist License Policy states

It is stored unmodified in a designated third-party folder; AND

Again - I'm not saying we shouldn't change what we are doing. I'm stating let's separate this from the 'vendorless' work

upodroid commented 1 year ago

Hmm, that is an important detail we missed

cardil commented 1 year ago

@dprotaso We have MPL in some repos - given that the CNCF Allowlist License Policy states

It is stored unmodified in a designated third-party folder; AND

Doesn't that section says that the whole third-party component should be kept in the third-party folder, not just the license?

cardil commented 1 year ago

However, looking at some of the graduated CNCF projects, it's hard to find the third-party folder like ours (from https://landscape.cncf.io/card-mode):

I did find only the https://github.com/kubernetes/kubernetes actually holds some third-party components in a designated folder (not only the LICENSE files)

dprotaso commented 1 year ago

We keep going back and forth. My point here is let's not deviate from what we are currently doing without a discussion at the TOC level - since this impacts all subgroups. This is a separate discussion from vendorless work

So let's continue our license disclosure practice until we have consensus at that level.

Thus let's

  1. Fix the hack script to continue to call update_licenses (this issue)
  2. Create a new community issue regarding the license disclosure practice
  3. Bring it up with the TOC
cardil commented 1 year ago

Moving this issue to knative/hack

evankanderson commented 1 year ago

To provide additional context, this was original implemented to meet the second clause of the BSD 2-clause license:

  1. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.

By embedding the license in the container image, people who received the OCI image (for example, by pulling from a repo which the image had been cloned to) would also receive a copy of the license, which would trivially satisfy "reproduce the above copyright notice". Since I'm not a lawyer, I'm not going to venture whether this was an overly-restrictive reading of this clause. (This also similarly trivially satisfies the MIT requirement of including a liability disclaimer notice.)

dprotaso commented 1 year ago

I'm going to move my question to a public cncf foundation issue - it probably shouldn't be a service desk issue.

dprotaso commented 1 year ago

https://github.com/cncf/foundation/issues/642

cardil commented 1 year ago

@dprotaso I've opened the issue you mention: https://github.com/knative/community/issues/1441