Cobbler: add stale-host diff UI with confirmed prune flow by PrathamGhaywat · Pull Request #380 · openSUSE/orthos2

PrathamGhaywat · 2026-04-10T09:29:55Z

What

Implements a safe stale-host cleanup workflow for Cobbler:

Adds backend stale-prune helpers (remove_by_name, prune_stale).
Integrates optional prune in RegenerateCobbler.execute() via feature flags.
Adds frontend diff modal for Cobbler server action:

show Orthos vs Cobbler machines
preselect stale machines for deletion
allow checkbox edits
confirm before delete
show delete count

Why

Regeneration previously deployed active Orthos machines but did not remove stale Cobbler entries, causing drift. This adds an explicit, safe reconciliation path.

Key behavior

Domain safety guard is enforced: only FQDNs ending with .<domain> are considered for deletion.
UI deletion is never immediate:

fetch diff first
user reviews and adjusts selection
confirmation prompt required

Automatic prune remains opt-in:

cobbler.prune.enabled = bool:false (default)
cobbler.prune.dryrun = bool:true (default)

Files changed

cobbler.py
cobbler.py
serverconfigs.json
regenerate.py
overview.html
test_cobbler.py
tests.py
test_regenerate_cobbler_prune.py

Validation

python -m flake8 orthos2 passed.
New/updated feature tests passed:

prune utility tests
task prune flag tests
frontend diff/prune view tests

Should close #256

codacy-production · 2026-04-10T09:30:52Z

Not up to standards ⛔

🔴 Issues 1 critical · 2 high

Alerts:
⚠ 3 issues (≤ 0 issues of at least minor severity)

Results:
3 new issues

Category Results

Security 1 critical
2 high

View in Codacy

🟢 Metrics 37 complexity · 14 duplication

Metric Results

Complexity 37

Duplication 14

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

Copilot

Pull request overview

Adds an explicit stale-host reconciliation workflow for Cobbler by introducing backend prune helpers, wiring optional pruning into the regenerate task via feature flags, and adding a UI diff/prune modal so users can review and confirm deletions.

Changes:

Add remove_by_name() and prune_stale() utilities to support stale-entry pruning with a domain-suffix guard.
Add feature-flagged prune support to RegenerateCobbler.execute() and seed default flags in serverconfigs.json.
Add a frontend diff/prune flow (diff modal, selection, confirm, delete count) plus new tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
orthos2/utils/cobbler.py	Adds removal-by-FQDN and stale pruning helper logic.
orthos2/utils/tests/test_cobbler.py	Adds unit tests for `remove_by_name` and `prune_stale`.
orthos2/taskmanager/tasks/cobbler.py	Optionally invokes stale pruning after deploy based on ServerConfig flags.
orthos2/taskmanager/tests.py	Adds task-level tests covering prune flag behavior.
orthos2/frontend/views/regenerate.py	Adds diff/prune modes for Cobbler server regeneration endpoint.
orthos2/frontend/templates/frontend/machines/detail/overview.html	Adds diff/prune modal UI and JS wiring for the Cobbler server action.
orthos2/frontend/tests/user/test_regenerate_cobbler_prune.py	Adds view tests for diff/prune behavior and foreign-domain protection.
orthos2/data/fixtures/serverconfigs.json	Seeds `cobbler.prune.enabled` and `cobbler.prune.dryrun` defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T09:35:51Z

orthos2/utils/cobbler.py

+        try:
+            self._xmlrpc_server.remove_system(fqdn, self._token, False)
+        except xmlrpc.client.Fault as xmlrpc_fault:
+            logging.error('Removing %s failed with "%s"', fqdn, xmlrpc_fault.faultString)
+


remove_by_name() swallows xmlrpc.client.Fault exceptions (logs and continues). Callers (notably the new prune UI) will report machines as deleted even when the XML-RPC remove failed. Consider raising a CobblerException (or returning a success boolean) on Fault so the caller can surface an error and avoid false success reporting.

Copilot · 2026-04-10T09:35:52Z

orthos2/utils/cobbler.py

+    def prune_stale(self, orthos_fqdns: set[str], dry_run: bool = True) -> list[str]:
+        """
+        Prune stale machines from Cobbler for the server domain.
+
+        :param orthos_fqdns: Set of active Orthos FQDNs for this domain.
+        :param dry_run: Whether to only log stale entries instead of removing them.
+        :returns: List of stale FQDNs found for this domain.
+        """
+        cobbler_fqdns = self.get_machines()
+        domain_suffix = "." + self._domain.name
+        stale = [
+            fqdn
+            for fqdn in cobbler_fqdns
+            if fqdn.endswith(domain_suffix) and fqdn not in orthos_fqdns
+        ]
+
+        for fqdn in stale:
+            if dry_run:
+                logger.info("[DRY-RUN] prune stale machine from Cobbler: %s", fqdn)
+            else:
+                logger.info("Prune stale machine from Cobbler: %s", fqdn)
+                self.remove_by_name(fqdn)
+
+        return stale


prune_stale() can call remove_by_name() and then returns the full stale list regardless of whether deletions succeeded. If remove_by_name() is updated to raise/return failures, prune_stale() should also reflect per-host delete success (e.g., return only successfully removed FQDNs or a structured result), otherwise automation may assume removals happened when they did not.

Copilot · 2026-04-10T09:35:52Z

orthos2/frontend/views/regenerate.py

+    if mode == "prune":
+        selected_fqdns = set(request.GET.getlist("fqdn"))
+        diff = _collect_cobbler_diff(machine)
+        allowed_fqdns = set(diff["stale"])
+        deletable_fqdns = sorted(selected_fqdns & allowed_fqdns)
+
+        target_domains = machine.cobbler_server_for.all()
+        for domain in target_domains:
+            domain_suffix = "." + domain.name
+            server = CobblerServer(domain)
+            for fqdn in deletable_fqdns:
+                if fqdn.endswith(domain_suffix):
+                    server.remove_by_name(fqdn)
+


The new mode=prune branch performs deletions via a GET request (request.GET.getlist(...)). This makes the endpoint CSRF-prone and violates HTTP semantics for destructive actions (GET should be safe/idempotent). Switch prune to POST (read selected FQDNs from the body) and enforce CSRF protection; update the JS to send a POST accordingly.

Copilot · 2026-04-10T09:35:52Z

orthos2/frontend/views/regenerate.py

+    for domain in target_domains:
+        domain_orthos_fqdns = set(
+            domain.machine_set.exclude(active=False).values_list("fqdn", flat=True)
+        )
+        orthos_fqdns.update(domain_orthos_fqdns)
+
+        server = CobblerServer(domain)
+        domain_cobbler_fqdns = server.get_machines()
+        domain_suffix = "." + domain.name
+        domain_scoped_fqdns = {
+            fqdn for fqdn in domain_cobbler_fqdns if fqdn.endswith(domain_suffix)
+        }
+
+        cobbler_fqdns.update(domain_scoped_fqdns)
+        stale_fqdns.update(domain_scoped_fqdns - domain_orthos_fqdns)
+


_collect_cobbler_diff() and the mode=diff/prune branches don't handle CobblerException / XML-RPC failures (e.g., Cobbler down). A single failing domain will currently bubble up as a 500. Consider catching expected Cobbler errors and returning a JSON status with an appropriate HTTP code (e.g., 503) and message so the UI can display a controlled failure.

Copilot · 2026-04-10T09:35:52Z

orthos2/frontend/views/regenerate.py

+        target_domains = machine.cobbler_server_for.all()
+        for domain in target_domains:
+            domain_suffix = "." + domain.name
+            server = CobblerServer(domain)
+            for fqdn in deletable_fqdns:
+                if fqdn.endswith(domain_suffix):
+                    server.remove_by_name(fqdn)
+


In the prune loop, each selected FQDN is tested against every domain suffix via endswith(). If domains can overlap (e.g., example.com and sub.example.com), the same FQDN can match multiple suffixes and trigger multiple remove_by_name() calls, leading to spurious errors or redundant work. Consider assigning each FQDN to a single best-matching domain (e.g., longest suffix match) and deleting it once.

Copilot · 2026-04-10T09:35:53Z

orthos2/frontend/templates/frontend/machines/detail/overview.html

 function regenerate_domain_cobbler() {
  $.ajax({
-    url: '{% url 'frontend:regenerate_domain_cobbler' machine.id %}',
+    url: '{% url 'frontend:regenerate_domain_cobbler' machine.id %}?mode=diff',
    beforeSend: function() {


regenerate_domain_cobbler() now calls ...?mode=diff and only opens the diff/prune modal; it no longer triggers the actual regeneration endpoint (which runs when mode is absent). Given the sidebar button text is still “Regenerate Cobbler Server”, this looks like a behavior regression. Consider either keeping regeneration as the default action (and offering diff/prune separately) or triggering regeneration after the diff/prune flow completes.

Copilot · 2026-04-10T09:35:53Z

orthos2/frontend/templates/frontend/machines/detail/overview.html

+      orthosMachines.forEach(function (fqdn) {
+        $('#orthos-machine-list').append('<li class="list-group-item py-1">' + fqdn + '</li>');
+      });
+
+      cobblerMachines.forEach(function (fqdn) {
+        $('#cobbler-machine-list').append('<li class="list-group-item py-1">' + fqdn + '</li>');
+      });
+
+      staleMachines.forEach(function (fqdn, idx) {
+        var checkboxId = 'stale-machine-' + idx;
+        $('#stale-machine-checkbox-list').append(
+          '<li class="list-group-item py-1">' +
+          '<div class="form-check">' +
+          '<input class="form-check-input stale-machine-checkbox" type="checkbox" checked value="' + fqdn + '" id="' + checkboxId + '">' +
+          '<label class="form-check-label" for="' + checkboxId + '">' + fqdn + '</label>' +
+          '</div>' +


FQDN values from the AJAX response are concatenated directly into HTML strings via .append(...) / value="' + fqdn + '" / label text. If a machine name ever contains unexpected characters, this becomes an XSS injection vector. Prefer creating DOM elements and setting text via .text() (and attributes via .attr()), or otherwise HTML-escaping the inserted values.

Copilot · 2026-04-10T09:35:53Z

orthos2/frontend/templates/frontend/machines/detail/overview.html

+  $.ajax({
+    url: '{% url 'frontend:regenerate_domain_cobbler' machine.id %}',
+    data: {
+      mode: 'prune',
+      fqdn: selected,
+    },
+    traditional: true,
+    beforeSend: function() {
+      $('#confirm-cobbler-prune-btn').addClass('disabled')
+    },
+    success: function(data) {
+      showMachineStatusBarMessage(data);
+      $('#cobbler-prune-modal').modal('hide');
+      $('#confirm-cobbler-prune-btn').removeClass('disabled')
+    },


The “Delete Selected” button is only toggled via .addClass('disabled')/.removeClass('disabled'). For <button> elements, this typically does not prevent clicks (it mainly changes styling), so users can still trigger multiple concurrent prune requests. Set the disabled attribute/property (e.g., via .prop('disabled', true/false)) in addition to any CSS class.

PrathamGhaywat · 2026-04-10T10:35:55Z

codacy alerts are only about test secrets

SchoolGuy · 2026-04-10T13:09:08Z

This PR cannot be tested locally because of #383. As such, this has to be solved beforehand. After merging the related PR, this PR has to be rebased.

Copilot AI review requested due to automatic review settings April 10, 2026 09:29

Copilot started reviewing on behalf of PrathamGhaywat April 10, 2026 09:30 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

PrathamGhaywat added 3 commits April 10, 2026 14:45

Add Cobbler stale-host diff and confirmed prune workflow

602bc44

Add dedicated Cobbler cleanup page with diff and prune flow

08cd40f

format

968aa89

PrathamGhaywat force-pushed the feature/cobbler-prune-stale-hosts branch from eba2198 to 968aa89 Compare April 10, 2026 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cobbler: add stale-host diff UI with confirmed prune flow#380

Cobbler: add stale-host diff UI with confirmed prune flow#380
PrathamGhaywat wants to merge 3 commits intoopenSUSE:masterfrom
PrathamGhaywat:feature/cobbler-prune-stale-hosts

PrathamGhaywat commented Apr 10, 2026

Uh oh!

codacy-production bot commented Apr 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

PrathamGhaywat commented Apr 10, 2026

Uh oh!

SchoolGuy commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

PrathamGhaywat commented Apr 10, 2026

What

Why

Key behavior

Files changed

Validation

Uh oh!

codacy-production bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Not up to standards ⛔

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

PrathamGhaywat commented Apr 10, 2026

Uh oh!

SchoolGuy commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codacy-production bot commented Apr 10, 2026 •

edited

Loading