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

Make repo compliant #126

Merged
merged 3 commits into from
Feb 14, 2024
Merged

Make repo compliant #126

merged 3 commits into from
Feb 14, 2024

Conversation

RaphaelVogel
Copy link
Member

@RaphaelVogel RaphaelVogel commented Feb 8, 2024

What this PR does / why we need it:
What this PR does / why we need it:

  • Created/Updated LICENSE file
  • Deleted SECURITY.md file. It is centrally managed
  • Created/Updated .reuse/dep5 file
  • Created/Updated LICENSES folder
  • Added badge for reuse compliance in README.md file

@RaphaelVogel RaphaelVogel added the area/ipcei IPCEI (Important Project of Common European Interest) label Feb 8, 2024
@RaphaelVogel RaphaelVogel requested a review from a team as a code owner February 8, 2024 13:30
@gardener-robot gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Feb 8, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 8, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 8, 2024
@RaphaelVogel RaphaelVogel added the status/on-hold Issue on hold (e.g. because work was suspended) label Feb 9, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 9, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 13, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 13, 2024
@dimityrmirchev
Copy link
Member

Thank you for opening this PR! I will review it tomorrow

/assign

@dimityrmirchev
Copy link
Member

So I ran

docker run --rm --volume $(pwd):/data fsfe/reuse lint

against the main branch.

The following files come up as missing copyright and licensing info

# MISSING COPYRIGHT AND LICENSING INFORMATION

The following files have no copyright and licensing information:
* .ci/hack/component_descriptor
* .github/ISSUE_TEMPLATE/bug_report.md
* .github/ISSUE_TEMPLATE/enhancement_request.md
* .github/dependabot.yaml
* .github/pull_request_template.md
* .github/workflows/upload-diki-binaries.yaml
* .gitignore
* .golangci.yaml
* CODEOWNERS
* CONTRIBUTING.md
* README.md
* SECURITY.md
* VERSION
* docs/development/getting-started.md
* docs/providers/gardener.md
* docs/providers/managedk8s.md
* docs/providers/virtualgarden.md
* docs/rulesets/disa-k8s-stig.md
* example/config/gardener.yaml
* example/config/managedk8s.yaml
* example/config/virtualgarden.yaml
* go.mod
* go.sum
* pkg/report/templates/html/_styles.tpl
* pkg/report/templates/html/input.css
* pkg/report/templates/html/merged_report.html
* pkg/report/templates/html/output.css
* pkg/report/templates/html/report.html
* tailwind.config.js

This PR introduces a DEP5 file which uses * to select all files in this repository. This causes warnings such as

/opt/venv/lib/python3.11/site-packages/reuse/project.py:286: PendingDeprecationWarning: Copyright and licensing information for 'pkg/kubernetes/utils/utils_suite_test.go' has been found in both 'pkg/kubernetes/utils/utils_suite_test.go' and in the DEP5 file located at '.reuse/dep5'. The information for these two sources has been aggregated. In the future this behaviour will change, and you will need to explicitly enable aggregation. See <https://github.com/fsfe/reuse-tool/issues/779>. You need do nothing yet. Run with `--suppress-deprecation` to hide this warning.

In addition to that I am not sure that we should license trivial files like github PR templates, .gitignore, go.mod, go.sum, tailwind.config.js, .golangci.yaml.

Some of the files in pkg/report/templates/html represent a special case since they are already licensed. See output.css as an example. Also the reports that Diki generates embed this licensed file https://github.com/gardener/diki/blob/81266b4b2c8630d6bf4eadc52a538287db5be47d/pkg/report/templates/html/report.html#L8C1-L8C30

So my two questions are:

  1. How can we respect the license info of css code w.r.t generated reports and other files like https://github.com/gardener/diki/blob/main/pkg/report/templates/html/_styles.tpl?
  2. What can we do about trivial files so that the tool is happy but also we do not put our licensing info on them?

@RaphaelVogel
Copy link
Member Author

RaphaelVogel commented Feb 14, 2024

Thanks for your feedback/comments

This PR introduces a DEP5 file which uses * to select all files in this repository. This causes warnings such as.......

This is a known issue of the reuse tool discussed here and a bug here. For now we don't need to worry about it.

Answer to question 2)

In addition to that I am not sure that we should license trivial files like github PR templates, .gitignore, go.mod, go.sum, tailwind.config.js, .golangci.yaml.

The reason for having * in dep5 file is ease of maintenance. The problem I see with explicitly defining single/specific files like it is done for example here is that it will most likely be outdated soon, but you are free to manage single files yourself if you like. It's your repo/your responsibility and this is just a suggestion how we do it for the gardener org.

Answer to question 1)

For the multiple license issue, REUSE suggests that you add both licenses. I would suggest to add this in the dep5 file like this:

# --------------------------------------------------
# third-party

Files: pkg/report/templates/html/*
Copyright: 
  SAP SE or an SAP affiliate company and Gardener contributors
  Tailwind authors (https://github.com/tailwindlabs/tailwindcss)
License: Apache-2.0 and MIT
Comment: Source files in this directory are multi licensed.

dep5 file will be processed from top to bottom and the last entry wins (override semantics). So this config would work.

WDYT?

@dimityrmirchev
Copy link
Member

The reason for having * in dep5 file is ease of maintenance. The problem I see with explicitly defining single/specific files like it is done for example here is that it will most likely be outdated soon

This might not be the case if an automatic check is also introduced and ran when a PR is submitted. Similar to how we run tests, linters, formatters.

you are free to manage single files yourself if you like. It's your repo/your responsibility and this is just a suggestion how we do it for the gardener org.

Yes, I get that the decision multiple files vs * is in the hands of the maintainers of the said repository, however I am missing a general guidance or document that defines all of the requirements that need to be followed in the Gardener org. From my point of view this repository already has copyright information on all (or almost all) relevant files, regardless if the tool recognises them or not.

I am not convinced that the following is desirable since there are files in that folder that contain code that is written by the Gardener authors and I am not sure that the Tailwind authors want to impose any copyright claims on it. And what about having files that include just some parts that come from a different source?

# --------------------------------------------------
# third-party

Files: pkg/report/templates/html/*
Copyright: 
  SAP SE or an SAP affiliate company and Gardener contributors
  Tailwind authors (https://github.com/tailwindlabs/tailwindcss)
License: Apache-2.0 and MIT
Comment: Source files in this directory are multi licensed.

Since some of the suggestions refer to Reuse recommendations, I want to point out that Reuse also has a section describing insignificant files (which I mentioned above) and suggests to add a CC0 license for them. However, I have not seen this in any Gardener repository.

As I am not a lawyer or a person that is familiar with all the legal aspects of these licenses It is really hard for me to evaluate/review these changes without some general guidance document that I can refer to.

@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 14, 2024
@dimityrmirchev
Copy link
Member

I did some minor changes to the already suggested modifications. Please check if everything seems OK and we can proceed.

@RaphaelVogel
Copy link
Member Author

/lgtm

@gardener-robot gardener-robot added the reviewed/lgtm Has approval for merging label Feb 14, 2024
@gardener-robot gardener-robot removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Feb 14, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 14, 2024
Copy link
Member

@dimityrmirchev dimityrmirchev left a comment

Choose a reason for hiding this comment

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

/lgtm

@dimityrmirchev dimityrmirchev merged commit 244af22 into gardener:main Feb 14, 2024
8 checks passed
@gardener-robot gardener-robot added status/closed Issue is closed (either delivered or triaged) and removed status/on-hold Issue on hold (e.g. because work was suspended) labels Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipcei IPCEI (Important Project of Common European Interest) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants