Skip to content

snapm: fix Manager.split_snapshot_sets() with empty sources#180

Merged
bmr-cymru merged 1 commit intomainfrom
bmr-fix-empty-splits
Apr 1, 2025
Merged

snapm: fix Manager.split_snapshot_sets() with empty sources#180
bmr-cymru merged 1 commit intomainfrom
bmr-fix-empty-splits

Conversation

@bmr-cymru
Copy link
Copy Markdown
Contributor

The snapm.manager.Manger.split_snapshot_sets() method checks whether the original snapshot set would be empty after the operation and raises an error if this is the case. There is no corresponding check to ensure that the destination snapshot set is not empty (i.e. the sources passed to the split_snapshot_set() method were empty).

The snapm.command wrappers already require the sources argument:

  root@localhost:~/src/git/snapm# snapm snapset split upgrade noupgrade
  usage: snapm snapset split [-h] NAME NEW_NAME SOURCE [SOURCE ...]
  snapm snapset split: error: the following arguments are required: SOURCE

But the API does not enforce this, leading to a snapshot set with no sources and NrSnapshots == 0:

  >>> p = m.split_snapshot_set("upgrade", "noupgrade", [])
  DEBUG - Blocking termination signals {<Signals.SIGINT: 2>, <Signals.SIGTERM: 15>}
  INFO - Attempting to split snapshot set 'upgrade'
  DEBUG - Splitting snapshot  from snapshot set 'upgrade'
  INFO - Attempting rename for snapshot set 'upgrade' to 'noupgrade'
  DEBUG - Unblocking termination signals {<Signals.SIGINT: 2>, <Signals.SIGTERM: 15>}
  >>> p
  <snapm._snapm.SnapshotSet object at 0x7eff5a90d220>
  >>> print(p)
  SnapsetName:      noupgrade
  Sources:
  NrSnapshots:      0
  Time:             2025-04-01 15:02:37
  UUID:             fab385b4-5912-54f6-8458-9b487eaf7b50
  Status:           Active
  Autoactivate:     yes
  Bootable:         no

In practice this makes the call a no-op but it's pointless and confusing to allow this combination of arguments: raise a SnapmArgumentError if the source_specs list is empty or None.

Fixes: a32e85f
Related: #98, #174
Resolves: #179

The snapm.manager.Manger.split_snapshot_sets() method checks whether the
original snapshot set would be empty after the operation and raises an
error if this is the case. There is no corresponding check to ensure
that the destination snapshot set is not empty (i.e. the sources passed
to the split_snapshot_set() method were empty).

The snapm.command wrappers already require the sources argument:

  root@localhost:~/src/git/snapm# snapm snapset split upgrade noupgrade
  usage: snapm snapset split [-h] NAME NEW_NAME SOURCE [SOURCE ...]
  snapm snapset split: error: the following arguments are required: SOURCE

But the API does not enforce this, leading to a snapshot set with no
sources and NrSnapshots == 0:

  >>> p = m.split_snapshot_set("upgrade", "noupgrade", [])
  DEBUG - Blocking termination signals {<Signals.SIGINT: 2>, <Signals.SIGTERM: 15>}
  INFO - Attempting to split snapshot set 'upgrade'
  DEBUG - Splitting snapshot  from snapshot set 'upgrade'
  INFO - Attempting rename for snapshot set 'upgrade' to 'noupgrade'
  DEBUG - Unblocking termination signals {<Signals.SIGINT: 2>, <Signals.SIGTERM: 15>}
  >>> p
  <snapm._snapm.SnapshotSet object at 0x7eff5a90d220>
  >>> print(p)
  SnapsetName:      noupgrade
  Sources:
  NrSnapshots:      0
  Time:             2025-04-01 15:02:37
  UUID:             fab385b4-5912-54f6-8458-9b487eaf7b50
  Status:           Active
  Autoactivate:     yes
  Bootable:         no

In practice this makes the call a no-op but it's pointless and confusing
to allow this combination of arguments: raise a SnapmArgumentError if
the source_specs list is empty or None.

Fixes: a32e85f
Related: #98, #174
Resolves: #179
@bmr-cymru bmr-cymru added the Bug Something isn't working label Apr 1, 2025
@bmr-cymru bmr-cymru self-assigned this Apr 1, 2025
@packit-as-a-service
Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/snapshotmanager-snapm-180
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@bmr-cymru bmr-cymru merged commit 8032906 into main Apr 1, 2025
15 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Bug Fixes Apr 1, 2025
@bmr-cymru bmr-cymru deleted the bmr-fix-empty-splits branch May 30, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

snapm: Manager.split_snapshot_sets() should not accept empty source_specs

1 participant