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

Rebuilding the image only when there is a new version #138

Open
mhajder opened this issue Mar 21, 2022 · 15 comments
Open

Rebuilding the image only when there is a new version #138

mhajder opened this issue Mar 21, 2022 · 15 comments

Comments

@mhajder
Copy link

mhajder commented Mar 21, 2022

Recently a cron has been added which rebuilds the image once a week:

As a result, many people who updated the container automatically with the watchtower had to disable it or their instance will be restarted once a week..

Maybe a better solution would be to rebuild the :latest image and build the image with the new version only when the new version is added to thelounge/thelounge repository?

An example of how this can be done is described HERE or HERE.

@brunnre8
Copy link
Member

brunnre8 commented Mar 21, 2022

That's not enough.
At a minimum you need to rebuild when the base image changes for whatever reason and when a TL release happens.

@mhajder
Copy link
Author

mhajder commented Mar 21, 2022

But is it really necessary to rebuild old version images that aren't really used anymore and stay only as archival versions? This is not some typical government software that has a high risk of attack, if there is an exploit on a base image, you can rebuild everything manually.

@brunnre8
Copy link
Member

Yes it is necessary, no it is not ok to leave things in the repo which are vulnerable to known CVEs.

Why do you care if an older version gets bumped?
You are supposed to pull in a specific version / label of a container so that should not impact you at all if some lower version does anything.

@brunnre8
Copy link
Member

Also, what do you consider old archived images? As far as I can tell the rebuild applied to the "latest" label plus the latest major release which is a sensible thing to rebuild

@mhajder
Copy link
Author

mhajder commented Mar 21, 2022

Why do you care if an older version gets bumped?

Because in that case the watchtower pull the new version of the same container and restarts my thelounge instance.
I don't mean the old versions, but mostly the tag :latest.

You are supposed to pull in a specific version / label of a container so that should not impact you at all if some lower version does anything.

But in that case the automatic update will not work.

Also, what do you consider old archived images? As far as I can tell the rebuild applied to the "latest" label plus the latest major release which is a sensible thing to rebuild

You're right, I didn't notice it, I looked at the tag :4 and thought previous versions are built automatically too.

Also, shouldn't the :latest tag be the last production version as it is done in other popular containers?

@brunnre8
Copy link
Member

You absolutely should pull in the new container when the base image updates, else you're running an insecure version.

That's the whole point of automatic updates.

Also, shouldn't the :latest tag be the last production version

Both ways have merits, not my choice and not really related to the issue at hand

@williamboman
Copy link
Member

Hello!

As a result, many people who updated the container automatically with the watchtower had to disable it or their instance will be restarted once a week..

Ah yeah I was a bit worried this would end up happening. Just out of curiosity, when you say many people, how many are we talking about and where did you gather this data?

Maybe a better solution would be to rebuild the :latest image and build the image with the new version only when the new version is added to thelounge/thelounge repository?

The reason for rebuilding and republishing the latest image versions is to harden them. There are primarily two vectors to keep in mind:

  • base image layers & the node runtime
  • thelounge's dependencies

I'd be fine with not doing this and just building fresh images whenever there's a new release upstream, but it's important to consider the lifecycle of TL's releases. A release may remain "latest" for a very long time (roughly counting 4.2.0 -> 4.3.0 took 15 months, for example).

Also, shouldn't the :latest tag be the last production version as it is done in other popular containers?

The :latest tag points to the latest stable release (not release candidates or pre-releases).

I'd be OK with not rebuilding on a scheduled basis but only when there's a reason that warrants a rebuild (for example flagged CVEs). I won't have the time to look into figuring out the best way to do this, but PRs are always welcomed.

@mhajder
Copy link
Author

mhajder commented Mar 22, 2022

Ah yeah I was a bit worried this would end up happening. Just out of curiosity, when you say many people, how many are we talking about and where did you gather this data?

From thelounge IRC channel.

These are not hundreds of people, but rather several. But probably many people do not know about this possibility, so one day we could add to the documentation information about the auto pull of the image through the watchtower.

A release may remain "latest" for a very long time (roughly counting 4.2.0 -> 4.3.0 took 15 months, for example).

You are right about that. I didn't think it through.

I'd be OK with not rebuilding on a scheduled basis but only when there's a reason that warrants a rebuild (for example flagged CVEs). I won't have the time to look into figuring out the best way to do this, but PRs are always welcomed.

In that case, I will look for what options we have available so that the container will be built when CVE is detected in the base image.

@brunnre8
Copy link
Member

Keep it simple and stupid, just update whenever the base image updates or TL releases.

No need to look for CVEs

@williamboman
Copy link
Member

Keep it simple and stupid, just update whenever the base image updates or TL releases.

Yeah I think this is a good start.

No need to look for CVEs

I think it'd be nice to capture CVEs flagged in any of the installed node dependencies, don't you think?

@brunnre8
Copy link
Member

brunnre8 commented Mar 22, 2022

Considering the version pins that wouldn't do anything for this repo and on the main thelounge one it is already tracked (dependabot) assuming you mean the npm deps of TL

@brunnre8
Copy link
Member

brunnre8 commented Mar 22, 2022

and the whole node installation is supposed to be managed by the base image layer, including deps node itself uses

@mhajder
Copy link
Author

mhajder commented Mar 22, 2022

You are right. As it is done now, it is correct. It makes no sense to look for any other option.

@williamboman
Copy link
Member

Considering the version pins that wouldn't do anything for this repo and on the main thelounge one it is already tracked (dependabot)

I just saw that thelounge pins the dependency versions in package.json, so yeah that'd be pointless. I was thinking it'd at least allow for unbounded patch releases, which could be used to distribute CVE fixes. Base image upgrades only it is!

@brunnre8
Copy link
Member

yeah well, looking at https://snyk.io/blog/peacenotwar-malicious-npm-node-ipc-package-vulnerability/ and similar happy events, allowing patch releases without vetting is bound to end in tears

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

No branches or pull requests

3 participants