Skip to content

Fix recreating cobbler entries on import#91

Open
aaannz wants to merge 3 commits intouyuni-project:mainfrom
aaannz:cobbler-resync-fix
Open

Fix recreating cobbler entries on import#91
aaannz wants to merge 3 commits intouyuni-project:mainfrom
aaannz:cobbler-resync-fix

Conversation

@aaannz
Copy link
Contributor

@aaannz aaannz commented Mar 6, 2026

This PR add missing removal and translation of the invalid characters to properly sanitize cobbler entries naming.
Import now skips recreating distro profiles if there is no image present at all.

Issues:

@aaannz aaannz requested a review from Copilot March 6, 2026 15:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Cobber entry naming sanitization to avoid unnecessary recreation on import, and skips distro profile refresh when there are no images to process.

Changes:

  • Add name/org sanitization for Cobbler entry naming helpers.
  • Skip updateDistroProfiles work when the image list is empty.
  • Add unit tests for Cobbler name construction.

Reviewed changes

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

File Description
inter-server-sync.changes.oholecek.fix_cobbler_imports Changelog entry describing the import/sanitization fixes
cobbler/cobbler_test.go Adds unit tests covering Cobbler naming helpers and sanitization
cobbler/cobbler.go Implements sanitization helpers and early-return behavior for empty image input

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

Comment on lines +282 to +285
if len(images) == 0 {
log.Debug().Msg("No image to process, skipping")
return nil
}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior was added to updateDistroProfiles (early return when images is empty), but there’s no corresponding unit test validating it. Add a test that calls updateDistroProfiles([]Image{}) and asserts it returns nil and does not attempt any external calls (e.g., via the test hook used for command execution in this package).

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,5 @@
- Add missing name sanitizations for cobbler entries
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider changing 'sanitizations' to 'sanitization' for more natural wording in the changelog entry.

Suggested change
- Add missing name sanitizations for cobbler entries
- Add missing name sanitization for cobbler entries

Copilot uses AI. Check for mistakes.
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

Successfully merging this pull request may close these issues.

2 participants