Skip to content

feat: Add methods to manage autosign entries for puppet/openvox on a SmartProxy - #1376

Merged
ogajduse merged 1 commit into
SatelliteQE:masterfrom
ATIX-AG:manage-autosign-entries
Nov 14, 2025
Merged

feat: Add methods to manage autosign entries for puppet/openvox on a SmartProxy#1376
ogajduse merged 1 commit into
SatelliteQE:masterfrom
ATIX-AG:manage-autosign-entries

Conversation

@knoppi

@knoppi knoppi commented Nov 10, 2025

Copy link
Copy Markdown
Contributor

Added two methods to SmartProxy to manage autosign entries on the puppetserver.
The methods allow to add and delete autosign entries.

This is an implementation of https://apidocs.theforeman.org/foreman/3.16/apidoc/v2/autosign.html for POST and DELETE requests.

Example:

host = target_sat.api.Host().search(query={'name': some_host_name})[0]
host.puppet_ca_proxy.add_autosign_entry(host.name)
host.puppet_ca_proxy.delete_autosign_entry(host.name)

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The docstring in test_delete_autosign_entry still references add_autosign_entry—please update it to mention delete_autosign_entry instead.
  • add_autosign_entry and delete_autosign_entry share similar setup for kwargs and path construction; consider extracting that into a common helper to reduce duplication.
  • It might be helpful to add a test that patches _handle_response (or asserts on its return) to verify these methods correctly propagate the server response.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The docstring in test_delete_autosign_entry still references add_autosign_entry—please update it to mention delete_autosign_entry instead.
- add_autosign_entry and delete_autosign_entry share similar setup for kwargs and path construction; consider extracting that into a common helper to reduce duplication.
- It might be helpful to add a test that patches _handle_response (or asserts on its return) to verify these methods correctly propagate the server response.

## Individual Comments

### Comment 1
<location> `nailgun/entities.py:8087-8082` </location>
<code_context>
+            self._server_config,
+        )
+
+    def delete_autosign_entry(self, certname, **kwargs):
+        """Delete an entry from the puppetserver's autosign file.
+
+        :param certname: Name of the host to be deleted from the autosign file
+        """
+        kwargs = kwargs.copy()
+        kwargs.update(self._server_config.get_client_kwargs())
+        path = f'{self.path()}/autosign/{certname}'
+        return _handle_response(
+            client.delete(path, **kwargs),
+            self._server_config,
</code_context>

<issue_to_address>
**suggestion:** Check for edge cases with certname in URL path construction.

URL-encode certname or validate its format to prevent issues with special characters or slashes in the path.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Satellite-QE

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@dosas
dosas requested review from ogajduse and vsedmik November 11, 2025 15:47
@dosas dosas added CherryPick PR needs CherryPick to previous branches 6.16.z 6.17.z 6.18.z labels Nov 11, 2025
@ogajduse
ogajduse requested a review from a team November 13, 2025 14:48
@ogajduse
ogajduse merged commit 32df4df into SatelliteQE:master Nov 14, 2025
21 of 22 checks passed
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…xy (#1376)

Co-authored-by: Jan Bundesmann <bundesmann@atix.de>
(cherry picked from commit 32df4df)
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…xy (#1376)

Co-authored-by: Jan Bundesmann <bundesmann@atix.de>
(cherry picked from commit 32df4df)
github-actions Bot pushed a commit that referenced this pull request Nov 14, 2025
…xy (#1376)

Co-authored-by: Jan Bundesmann <bundesmann@atix.de>
(cherry picked from commit 32df4df)
ogajduse pushed a commit that referenced this pull request Nov 14, 2025
…xy (#1376) (#1380)

(cherry picked from commit 32df4df)

Co-authored-by: Jan Bundesmann <github@knofafo.de>
Co-authored-by: Jan Bundesmann <bundesmann@atix.de>
ogajduse pushed a commit that referenced this pull request Nov 14, 2025
…xy (#1376) (#1381)

(cherry picked from commit 32df4df)

Co-authored-by: Jan Bundesmann <github@knofafo.de>
Co-authored-by: Jan Bundesmann <bundesmann@atix.de>
ogajduse pushed a commit that referenced this pull request Nov 14, 2025
…xy (#1376) (#1379)

(cherry picked from commit 32df4df)

Co-authored-by: Jan Bundesmann <github@knofafo.de>
Co-authored-by: Jan Bundesmann <bundesmann@atix.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.16.z 6.17.z 6.18.z CherryPick PR needs CherryPick to previous branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants