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

Add Labels to Docker Images #73

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jabdoa2
Copy link

@jabdoa2 jabdoa2 commented Aug 7, 2024

As a user I want to know where stuff in my image came from. Those standardized tags allow automated tools to find the source code. For instance, this will allow renovate bot to show a changelog (based on the github releases) and provide a nice link to the repo.

As a user I want to know where stuff in my image came from. Those standardized tags allow automated tools to find the source code. For instance, this will allow renovate bot to show a changelog (based on the github releases) and provide a nice link to the repo.
@cla-bot cla-bot bot added the cla/signed label Aug 7, 2024
@Al2Klimov Al2Klimov requested a review from martialblog August 8, 2024 09:39
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution and for using Icinga DB. Please consider the feedback, thanks.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Outdated
@@ -35,6 +35,10 @@ RUN ["chmod", "-R", "u=rwX,go=rX", "/rootfs"]

FROM scratch

LABEL org.opencontainers.image.documentation='https://icinga.com/docs/icinga-db/latest/doc/01-About/' \
org.opencontainers.image.source='https://github.com/Icinga/icingadb' \
org.opencontainers.image.licenses='GPL-2.0+'
Copy link
Member

Choose a reason for hiding this comment

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

According to the docs, .licenses should be an SPDX License Expression.

Suggested change
org.opencontainers.image.licenses='GPL-2.0+'
org.opencontainers.image.licenses='GPL-2.0-or-later'

Copy link
Member

Choose a reason for hiding this comment

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

Nope.

Suggested change
org.opencontainers.image.licenses='GPL-2.0+'
org.opencontainers.image.licenses='GPL-2.0-only'

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure? The Dockerfile has GPLv2+ in its header.

# Icinga DB Docker image | (c) 2020 Icinga GmbH | GPLv2+

Copy link
Author

@jabdoa2 jabdoa2 Aug 8, 2024

Choose a reason for hiding this comment

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

The LICENSE file states either version 2 of the License, or (at your option) any later version. In my opinion that would be the final authorative legal source and indicates GPL-2.0+/GPL-2.0-or-later

Copy link
Member

Choose a reason for hiding this comment

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

Let's skip the question how it is now directly to how should it be. GPLv2 or v2+? @lippserd

Copy link
Author

Choose a reason for hiding this comment

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

We can also drop the label for now. It is not super important to tag that to the image (at least for us). If you want I can remove it.

Dockerfile Show resolved Hide resolved
@jabdoa2 jabdoa2 requested review from oxzi and Al2Klimov August 8, 2024 15:50
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Thanks again for your contribution!

There is still the licensing question, where @Al2Klimov pointed out in his comment #73 (comment), that it should be GPL-2.0-only.

IANAL, but the mere presence of @jabdoa2's quoted chapter in the GPL 2 license document does not affect the project, #73 (comment). This very document states how the license should be applied, but unfortunately almost nobody does this exactly this way, https://github.com/Icinga/icingadb/blob/main/LICENSE#L282.

Back in Icinga/icingadb#9 @Al2Klimov attached licensing headers to the Icinga DB source, implying GPLv2+. In the meantime, those headers have disappeared almost everywhere, except in the two schema files, https://github.com/Icinga/icingadb/blob/f9ce7a57bcb6a5b6b2ebfafafd8f91367e56bbf5/schema/mysql/schema.sql#L1. I am not quite certain, but it seems they were removed during the project restructuring in Icinga/icingadb#277.

Because you were there, @Al2Klimov, I would like to hear your insights or final words.

@jabdoa2
Copy link
Author

jabdoa2 commented Aug 9, 2024

The change looks good to me. Thanks again for your contribution!

There is still the licensing question, where @Al2Klimov pointed out in his comment #73 (comment), that it should be GPL-2.0-only.

IANAL, but the mere presence of @jabdoa2's quoted chapter in the GPL 2 license document does not affect the project, #73 (comment). This very document states how the license should be applied, but unfortunately almost nobody does this exactly this way, https://github.com/Icinga/icingadb/blob/main/LICENSE#L282.

Back in Icinga/icingadb#9 @Al2Klimov attached licensing headers to the Icinga DB source, implying GPLv2+. In the meantime, those headers have disappeared almost everywhere, except in the two schema files, https://github.com/Icinga/icingadb/blob/f9ce7a57bcb6a5b6b2ebfafafd8f91367e56bbf5/schema/mysql/schema.sql#L1. I am not quite certain, but it seems they were removed during the project restructuring in Icinga/icingadb#277.

Because you were there, @Al2Klimov, I would like to hear your insights or final words.

That would be indeed good to be very clear about that as the legal department in every large cooperation will request this information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants