[6.17.z] feat: Add methods to manage autosign entries for puppet/openvox on a SmartProxy

[Satellite-QE]

Cherrypick of PR: #1376

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]

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

• Both add_autosign_entry and delete_autosign_entry currently don’t expose synchronous/timeout parameters like import_puppetclasses; consider adding these for consistency and control over request behavior.
• The docstring in test_delete_autosign_entry mistakenly mentions add_autosign_entry; please update it to reference the delete_autosign_entry method.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Both add_autosign_entry and delete_autosign_entry currently don’t expose synchronous/timeout parameters like import_puppetclasses; consider adding these for consistency and control over request behavior.
- The docstring in test_delete_autosign_entry mistakenly mentions add_autosign_entry; please update it to reference the delete_autosign_entry method.

## Individual Comments

### Comment 1
<location> `tests/test_entities.py:3460-3469` </location>
<code_context>
                 if 'environment' in param:
                     self.assertIn('/environments', post.call_args[0][0])

+    def test_add_autosign_entry(self):
+        """Call :meth:`nailgun.entities.SmartProxy.add_autosign_entry`.
+
+        Assert that
+        * correct fqdn is sent,
+        * proper path is built.
+        """
+        certname = "host.example.com"
+        with self.subTest():
+            with mock.patch.object(client, 'post') as post:
+                self.smart_proxy.add_autosign_entry(certname)
+                self.assertEqual(post.call_count, 1)
+                self.assertIn(f'{self.smart_proxy.path()}/autosign', post.call_args[0][0])
+                self.assertEqual(len(post.call_args[1]), 1)
+                self.assertEqual(post.call_args[1], {'data': {'id': 'host.example.com'}})
+
+    def test_delete_autosign_entry(self):
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding tests for error handling and edge cases in add_autosign_entry.

Please add tests for cases like empty string, None, or invalid certname inputs, and for scenarios where client.post raises exceptions or returns error responses.

Suggested implementation:

```python
    def test_add_autosign_entry(self):
        """Call :meth:`nailgun.entities.SmartProxy.add_autosign_entry`.

        Assert that
        * correct fqdn is sent,
        * proper path is built.
        """
        certname = "host.example.com"
        with self.subTest("valid certname"):
            with mock.patch.object(client, 'post') as post:
                self.smart_proxy.add_autosign_entry(certname)
                self.assertEqual(post.call_count, 1)
                self.assertIn(f'{self.smart_proxy.path()}/autosign', post.call_args[0][0])
                self.assertEqual(len(post.call_args[1]), 1)
                self.assertEqual(post.call_args[1], {'data': {'id': 'host.example.com'}})

        with self.subTest("empty certname"):
            with mock.patch.object(client, 'post') as post:
                with self.assertRaises(ValueError):
                    self.smart_proxy.add_autosign_entry("")
                post.assert_not_called()

        with self.subTest("None certname"):
            with mock.patch.object(client, 'post') as post:
                with self.assertRaises(ValueError):
                    self.smart_proxy.add_autosign_entry(None)
                post.assert_not_called()

        with self.subTest("invalid certname type"):
            with mock.patch.object(client, 'post') as post:
                with self.assertRaises(ValueError):
                    self.smart_proxy.add_autosign_entry(12345)
                post.assert_not_called()

        with self.subTest("client.post raises exception"):
            with mock.patch.object(client, 'post', side_effect=Exception("API error")) as post:
                with self.assertRaises(Exception) as exc:
                    self.smart_proxy.add_autosign_entry(certname)
                self.assertIn("API error", str(exc.exception))
                self.assertEqual(post.call_count, 1)

        with self.subTest("client.post returns error response"):
            class Response:
                def __init__(self, status_code):
                    self.status_code = status_code
                    self.text = "error"
            with mock.patch.object(client, 'post', return_value=Response(400)) as post:
                # Assuming add_autosign_entry checks response.status_code and raises on error
                with self.assertRaises(Exception):
                    self.smart_proxy.add_autosign_entry(certname)
                self.assertEqual(post.call_count, 1)

```

These tests assume that `add_autosign_entry` raises a `ValueError` for invalid certname inputs and raises an `Exception` for error responses from `client.post`. If the actual implementation differs, you may need to adjust the exception types or error handling logic in both the tests and the method itself.
</issue_to_address>

### Comment 2
<location> `tests/test_entities.py:3476-3485` </location>
<code_context>
+                self.assertEqual(len(post.call_args[1]), 1)
+                self.assertEqual(post.call_args[1], {'data': {'id': 'host.example.com'}})
+
+    def test_delete_autosign_entry(self):
+        """Call :meth:`nailgun.entities.SmartProxy.add_autosign_entry`.
+
+        Assert that
+        * correct fqdn is sent,
+        * proper path is built.
+        """
+        certname = "host.example.com"
+        with self.subTest():
+            with mock.patch.object(client, 'delete') as delete:
+                self.smart_proxy.delete_autosign_entry(certname)
+                self.assertEqual(delete.call_count, 1)
+                self.assertIn(
+                    f'{self.smart_proxy.path()}/autosign/{certname}', delete.call_args[0][0]
+                )
+                self.assertEqual(len(delete.call_args[1]), 0)
+

</code_context>

<issue_to_address>
**suggestion (testing):** Add tests for error conditions and edge cases in delete_autosign_entry.

Consider adding tests for cases like deleting a non-existent certname, passing None or an empty string, and handling exceptions from client.delete.

Suggested implementation:

```python
    def test_delete_autosign_entry(self):
        """Call :meth:`nailgun.entities.SmartProxy.delete_autosign_entry`.

        Assert that
        * correct fqdn is sent,
        * proper path is built.
        """
        certname = "host.example.com"
        with self.subTest("valid certname"):
            with mock.patch.object(client, 'delete') as delete:
                self.smart_proxy.delete_autosign_entry(certname)
                self.assertEqual(delete.call_count, 1)
                self.assertIn(
                    f'{self.smart_proxy.path()}/autosign/{certname}', delete.call_args[0][0]
                )
                self.assertEqual(len(delete.call_args[1]), 0)

        with self.subTest("non-existent certname"):
            with mock.patch.object(client, 'delete', side_effect=Exception("Not found")) as delete:
                with self.assertRaises(Exception) as exc:
                    self.smart_proxy.delete_autosign_entry("nonexistent.example.com")
                self.assertIn("Not found", str(exc.exception))
                self.assertEqual(delete.call_count, 1)

        with self.subTest("None as certname"):
            with mock.patch.object(client, 'delete') as delete:
                with self.assertRaises(Exception):
                    self.smart_proxy.delete_autosign_entry(None)
                self.assertEqual(delete.call_count, 0)

        with self.subTest("empty string as certname"):
            with mock.patch.object(client, 'delete') as delete:
                with self.assertRaises(Exception):
                    self.smart_proxy.delete_autosign_entry("")
                self.assertEqual(delete.call_count, 0)

        with self.subTest("client.delete raises exception"):
            with mock.patch.object(client, 'delete', side_effect=RuntimeError("delete failed")) as delete:
                with self.assertRaises(RuntimeError) as exc:
                    self.smart_proxy.delete_autosign_entry(certname)
                self.assertIn("delete failed", str(exc.exception))
                self.assertEqual(delete.call_count, 1)

```

These tests assume that `delete_autosign_entry` will raise an Exception if certname is None or empty, and will propagate exceptions from `client.delete`. If the actual implementation does not raise in these cases, you may need to adjust the tests or update the implementation to handle these edge cases appropriately.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}