certtools / intelmq

IntelMQ is a solution for IT security teams for collecting and processing security feeds using a message queuing protocol.
https://docs.intelmq.org/latest/
GNU Affero General Public License v3.0
972 stars 295 forks source link

Upgrades error #2471

Closed gethvi closed 5 months ago

gethvi commented 7 months ago

This upgrade function can fail:

Traceback (most recent call last):
  File "/opt/venv/lib/python3.9/site-packages/intelmq/bin/intelmqctl.py", line 1172, in upgrade_conf
    retval, runtime, harmonization = function(runtime, harmonization, dry_run,
  File "/opt/venv/lib/python3.9/site-packages/intelmq/lib/upgrades.py", line 133, in v110_shadowserver_feednames
    if bot["parameters"]["feedname"] in mapping:
KeyError: 'feedname'
sebix commented 7 months ago

I wonder how you came to this situation. In version 1.1.0 (the version this upgrade function is for), the shadowserver parser required the parameter feedname. So it looks more like your state.json got reset or something similar.

gethvi commented 7 months ago

Yes you are correct. This is because we run IntelMQ in Docker which means fresh install every time. No state file is preserved.

This means that for every new installation (without state file) out there using newer shadowserver parser without feedname this exception can happen (unless they upgrade config before adding the parser). That is a bug.

Running old upgrades on newer versions should not cause an exception.

sebix commented 7 months ago

The docker image misses the prepared state file.

This is the file used for the packages: https://build.opensuse.org/package/view_file/home:sebix:intelmq/intelmq/state.json?expand=1

gethvi commented 7 months ago

Nice, thanks! This is what I was about to suggest: to skip old versions upgrades (but without a state file - by checking current version and the version for which each upgrade function is intended for).

We use custom Docker image anyway.

gethvi commented 7 months ago

Anyway I still think this is a bug. It's nice to have that state file for packages, but other installation methods don't have this.

sebix commented 7 months ago

Anyway I still think this is a bug. It's nice to have that state file for packages, but other installation methods don't have this.

Yeah, it's an additional bug.

However, I don't expect that all upgrade functions can ever be future-proof.

gethvi commented 7 months ago

I agree, they can not. What do you think about this suggestion:

Instead of running all upgrade functions (since version 1) we should only run upgrades for the current major version. So if I install version 3.x.x, we run all upgrades for major version 3, but not older.

(Perhaps with an option to run older upgrades as well, but with a warning it can fail.)

We only need to future-proof upgrade functions inside one major version. I think it's reasonable for user to expect that upgrade functions inside one major version are future proof. And it is realistic for us to keep them future proof.

gethvi commented 7 months ago

On second thought the easiest way might be to keep the version in the runtime.yaml meaning "this configuration is ment for IntelMQ version x.y.z". Upgrade-config can be run against a particular config file. No state file needed.

gethvi commented 7 months ago

Same problem in v310_shadowserver_feednames:

https://github.com/certtools/intelmq/blob/b257f04246d0f2287afbbed161c4599bd50ede09/intelmq/lib/upgrades.py#L762-L764