Fix minor bugs in foreman bind

* Remove invalid provider argument
* Handle additional error message format
* Write tests for interpret_response
* Make kwargs usable

broker/binds/foreman.py

@@ -16,26 +16,26 @@ class ForemanBind:
     }
 
     def __init__(self, **kwargs):
-        self.foreman_username = settings.foreman.foreman_username
-        self.foreman_password = settings.foreman.foreman_password
-        self.url = settings.foreman.foreman_url
-        self.prefix = settings.foreman.name_prefix
-        self.verify = settings.foreman.verify
+        self.foreman_username = kwargs.get("foreman_username", settings.foreman.foreman_username)
+        self.foreman_password = kwargs.get("foreman_password", settings.foreman.foreman_password)
+        self.url = kwargs.get("url", settings.foreman.foreman_url)
+        self.prefix = kwargs.get("prefix", settings.foreman.name_prefix)
+        self.verify = kwargs.get("verify", settings.foreman.verify)
+
         self.session = requests.session()
 
     def _interpret_response(self, response):
         """Handle responses from Foreman, in particular catch errors."""
         if "error" in response:
-            if "Unable to authenticate user" in response["error"]["message"]:
-                raise exceptions.AuthenticationError(response["error"]["message"])
+            error = response["error"]
+            message = error.get("message")
+            if message is not None and "Unable to authenticate user" in message:
+                raise exceptions.AuthenticationError(message)
             raise exceptions.ForemanBindError(
-                provider=self.__class__.__name__,
-                message=" ".join(response["error"]["full_messages"]),
+                " ".join(error["full_messages"]),
             )
         if "errors" in response:
-            raise exceptions.ForemanBindError(
-                provider=self.__class__.__name__, message=" ".join(response["errors"]["base"])
-            )
+            raise exceptions.ForemanBindError(" ".join(response["errors"]["base"]))
         return response
 
     def _get(self, endpoint):
@@ -95,8 +95,7 @@ def obtain_id_from_name(self, resource_type, resource_name):
             raise
         except StopIteration:
             raise exceptions.ForemanBindError(
-                provider=self.__class__.__name__,
-                message=f"Could not find {resource_name} in {resource_type}",
+                f"Could not find {resource_name} in {resource_type}",
             )
         return id_
 

tests/providers/test_foreman.py

@@ -1,11 +1,13 @@
+from contextlib import nullcontext
 import json
+
 import pytest
 
 from broker import Broker
+from broker.binds.foreman import ForemanBind
+from broker.exceptions import AuthenticationError, ForemanBindError, ProviderError
 from broker.helpers import MockStub
 from broker.providers.foreman import Foreman
-from broker.binds.foreman import ForemanBind
-from broker.exceptions import ProviderError
 
 HOSTGROUP_VALID = "hg1"
 HOSTGROUP_INVALID = "hg7"
@@ -139,3 +141,34 @@ def test_negative_remote_execution(foreman_stub):
 
     assert simple_result.status == 1
     assert complex_result.status == 100
+
+
+@pytest.mark.parametrize(
+    "response,expected,context",
+    [
+        ({1: 2}, {1: 2}, nullcontext()),
+        (
+            {"error": {"message": "Unable to authenticate user"}},
+            None,
+            pytest.raises(AuthenticationError),
+        ),
+        (
+            {"error": {"full_messages": "foo", "message": "bar"}},
+            None,
+            pytest.raises(ForemanBindError),
+        ),
+        ({"errors": {"base": "foo"}}, None, pytest.raises(ForemanBindError)),
+        ({"error": {"full_messages": ["bar"]}}, None, pytest.raises(ForemanBindError)),
+    ],
+)
+def test_interpret_response(response, expected, context):
+    bind = ForemanBind(
+        foreman_username="",
+        foreman_password="",
+        url="",
+        prefix="",
+        verify=False,
+    )
+
+    with context:
+        assert bind._interpret_response(response) == expected