nextcloud / docker

⛴ Docker image of Nextcloud
https://hub.docker.com/_/nextcloud/
GNU Affero General Public License v3.0
6.08k stars 1.83k forks source link

Add static, or immutable, image tags that always point to the same image build #2197

Closed sbe-arg closed 1 month ago

sbe-arg commented 7 months ago

When nextcloud version 28.0.3 version is release and a immutable tag is created, images are built from that tag. When a vulnerability or feature is found/fixed and bumped to version 28.0.4 a new version is release but what I see is that docker images with the latest semantic version in dockerhub are continuously updated which defeats the purpose of semantic versioning for docker images.

The only continuously updated docker tags should be latest and non-semantic versioned tags.

We should be able to trust version pinning otherwise the compose and charts will have to be pinned to shas of the builds. This is a security risk IMO.

This is seen specifically in truenas scale and docker compose if we do docker compose pull without changing the docker version.

see: image image

https://github.com/truenas/charts/issues/2286 https://hub.docker.com/_/nextcloud/tags?page=&page_size=&ordering=&name=28.0.4

last edit: the title might sound like I'm angry or something, but in reality I have no visibility of how the images are created, where are the docker tag and docker push instructions?

intersectRaven commented 7 months ago

I think it's because the Nextcloud software remains the same. It's the other layers that may have changed. For example, the apache image uses php:8.2-apache-bookworm as it's base layer. If you look at it, that image was updated a few hours ago (at time of writing this). So the nextcloud image may have some trigger so it will update when the base changes.

sbe-arg commented 7 months ago

This should not happen for stability and security purposes.

One should be able to trust a tag. And not having a semantic tag being update continuously irrelevant of layers.

Only latest should be updated IMO

intersectRaven commented 7 months ago

I agree with your point with regards to stability BUT disagree from a security standpoint. An example would be a scenario where there is a vulnerability in PHP below 8.2.9. The Nextcloud image should be updated to fix it if it uses a version that is susceptible. As to whether it is automatically done so or done manually is the thing to be submitted to debate.

sbe-arg commented 7 months ago

For people in that context they should drive on latest and not on a semantic tag

sbe-arg commented 7 months ago

Or add and addition of a extension to the tag

Such as 28.0.4-patch1, 28.0.4-patch2 etc

But updating existing tags is not good practice in general only adding new semantic tags.

sbe-arg commented 7 months ago

@intersectRaven Also to explain the problem in truenas for example in more detail when you see this sha mismatch between the image you have already pulled when the chart got updated last time and the one in the registry. You get this weird update available warning, when you hit update the app as normal user would. nothing really happens. the reason is the bad combination with the chart pull strategy of ifnotpresent here https://github.com/truenas/charts/blob/master/charts/nextcloud/1.6.59/ix_values.yaml#L2 and the image being present with an existing tag. nothing is pulled so you get a false release. The only way to update the containers is manually updating the image in truenas and then killing the pods. All manual work.

More detail here: https://github.com/truenas/charts/issues/2286

intersectRaven commented 7 months ago

Personally, I don't see that as a problem with the tagging strategy currently adopted by this image. Maybe there is merit to considering the way the official Nextcloud all-in-one image does it where instead of a version there is a date in the tag (e.g.20230912_084059-latest) or some variant of it but to me it's already starting to get too long. That's for the maintainers of this image to consider.

I've already forked this repo though and made my own changes while cherry-picking updates every once and a while though so I'm not going to be too affected by whatever they adopt going forward since, in security terms, I want some level of control and further oversight with images I deploy on my network. Maybe, you should adopt that strategy too for now until you get a response from the maintainers. This is a community project though so you could also just create a push request for the changes you want to implement and maybe they'll consider it faster.

sbe-arg commented 7 months ago

@intersectRaven I feel you don't use true nas and don't fully understand the problem of charts semver pins impacting releases and deployments for this particular use case.

intersectRaven commented 7 months ago

@sbe-arg Same here, I feel you also don't understand that this isn't a docker or a problem for this image.

For the latter case, I already mentioned that you are free to propose changes as this is a community run project if it really is a problem for the image.

Mentioning security as an issue is invalid due to the reason I stated as well which is in a truly secure setup, you won't even be using images directly as pulled and, as much as possible, will build your own and use that.

TrueNAS isn't the only use case to consider for this image and it alone would not merit changes to be prioritized unless you yourself create the change, and have a PR done.

sbe-arg commented 7 months ago

Both repos are under the nextcloud org and the truenas app is offered as officially supported by nextcloud in their site. If the image of choice is a community only supported image, then thats a problem.

I don't know why are you getting so defensive when it is not common practice to reuse semantic versions once they are built, tagged and pushed. Its misleading.

Let other contributors express their opinion as well. Its not a matter of me finding my own solution but finding alignment-due the above listed reasons such as official contribution by nextcloud.

joshtrichards commented 7 months ago

I can see how it might be convenient or preferred to have an additional set of tags that are only ever associated with one artifact (or at least a single release cycle).

But Docker already supports specifying an individual image's digest instead of the tag:

https://docs.docker.com/compose/compose-file/05-services/#image

And this gets you pretty close (maybe closer, depending on your concerns).

Either way, I think the Nextcloud Server version coupled tags (e.g. nextcloud:28.0.4-apache) have to stay rolling. Mostly because... there are reasons to update previously published images that aren't coupled to the release cycle of Nextcloud Server itself.

D-side commented 6 months ago

there are reasons to update previously published images that aren't coupled to the release cycle of Nextcloud Server itself

There is a concept of a package revision (actual names vary), added to the version of the packaged contents to indicate changes in packaging for the same upstream release, resulting in a version number such as 1.9-1 (1.9 being the software version and 1 being packaging revision). See debian_revision in Debian manual and pkgrel in Arch.

One example use of this can be seen in Arch's response to the recent xz backdoor — when the news broke, 5.6.1-1 was already released, so the fixed release was published as 5.6.1-2.

Technically using SHAs directly accomplishes this too, but they're semantically opaque and have to be accompanied by a comment specifying the actual version to make infrastructure definitions more informative and easier to maintain.

And while I'm not sure about the security implications of this, I have concerns about compatibility — I already had to track down the cause for my Nextcloud to be stuck in a crash loop, which ended up being caused by the change in a dependency: Redis client library. Being able to switch back to an earlier base image with the same Nextcloud version would've been a minor additional convenience there:

intersectRaven commented 6 months ago

I think that works in packages since the package managers can automatically use the newest release since it has logic to take into account package revisions. In docker though, if you pin it to a tag, it won't go and use another tag which will be a problem if you're updating the base and not, in this case, nextcloud itself due to a security concern.

D-side commented 6 months ago

Since I'm not sure my idea came across in that flurry of details: my suggestion is to introduce another set of Docker tags that do take into account base image updates for the same Nextcloud version as an additional version suffix (e. g. 28.0.4-1-apache) and are not reused.

Meaning people can use an existing rolling tag if they prefer, but semantic hard-pinned tags would also be available for environments that benefit from them, such as the aforementioned TrueNAS, with additional utility for troubleshooting.

sbe-arg commented 6 months ago

Also in truenas you cannot stay in a previous nextcloud version it seems.

If I wanted to stay on NC28 or 27 for longer. If you patch you have to upgrade?

joshtrichards commented 3 months ago

Since I'm not sure my idea came across in that flurry of details: my suggestion is to introduce another set of Docker tags that do take into account base image updates for the same Nextcloud version as an additional version suffix (e. g. 28.0.4-1-apache) and are not reused.

I understand what you're asking for, but the way that Docker Official Images work is that they're rebuilt automatically by Docker (not us) when the underlying base images are updated. I'm not sure we could even do that if we wanted to.

The official Docker position (from my understanding) is if you want immutable image references, you use the digest, which already exists.

From a security standpoint, another set of tags isn't helpful anyhow since tags aren't immutable (even in the form suggested above) other than on a "best effort" and "cross your fingers" basis (i.e. they can always be re-pointed mistakenly... or maliciously).

joshtrichards commented 1 month ago

Closing since I think this has been discussed out.

For posterity (nothing that hasn't already been discussed in this Issue here though): docker-library/official-images#557