Merge pull request #516 from sadsfae/development

feat: support auth via env vars

Author: grafuls

README.md

@@ -10,8 +10,8 @@ def create_parser():
         allow_abbrev=False,
     )
     parser.add_argument("-H", "--host", help="iDRAC host address")
-    parser.add_argument("-u", help="iDRAC username", required=True)
-    parser.add_argument("-p", help="iDRAC password", required=True)
+    parser.add_argument("-u", help="iDRAC username")
+    parser.add_argument("-p", help="iDRAC password")
     parser.add_argument("-i", help="Path to iDRAC interfaces yaml", default=None)
     parser.add_argument("-t", help="Type of host as defined on iDRAC interfaces yaml")
     parser.add_argument("-l", "--log", help="Optional argument for logging results to a file")

src/badfish/helpers/parser.py

@@ -2450,8 +2450,19 @@ async def set_nic_attribute(self, fqdd, attribute, value):
 
 
 async def execute_badfish(_host, _args, logger, format_handler=None):
-    _username = _args["u"]
-    _password = _args["p"]
+    _username = _args.get("u") or os.environ.get("BADFISH_USERNAME")
+    _password = _args.get("p") or os.environ.get("BADFISH_PASSWORD")
+
+    if _args.get("p"):
+        logger.warning(
+            "Passing secrets via command line arguments can be unsafe. "
+            "Consider using environment variables (BADFISH_USERNAME, BADFISH_PASSWORD)."
+        )
+
+    if not _username or not _password:
+        logger.error("Missing credentials. Please provide credentials via CLI arguments or environment variables.")
+        return _host, False
+
     host_type = _args["t"]
     interfaces_path = _args["i"]
     force = _args["force"]

src/badfish/main.py

@@ -2347,3 +2347,5 @@ def render_device_dict(index, device):
 {RESPONSE_UNSOPPORTED_IDRAC_VERSION}
 {RESPONSE_NIC_ATTR_GET_ERROR}
 """
+MANAGER_INSTANCE_RESP = '{"Jobs":{"@odata.id":"/redfish/v1/Managers/iDRAC.Embedded.1/Jobs"}}'
+JOBS_RESP = '{"Members":[]}'

tests/config.py

@@ -1,4 +1,5 @@
 import sys
+import os
 from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch
 
 import pytest
@@ -53,21 +54,35 @@ def badfish_call(
         mock_host=config.MOCK_HOST,
         mock_user=config.MOCK_USER,
         mock_pass=config.MOCK_PASS,
+        use_cli_secrets=False
     ):
         argv = []
+        env_vars = os.environ.copy()
 
         if mock_host is not None:
             argv.extend(("-H", mock_host))
-        if mock_user is not None:
-            argv.extend(("-u", mock_user))
-        if mock_pass is not None:
-            argv.extend(("-p", mock_pass))
-
-        argv.extend(self.args)
-        try:
-            main(argv)
-        except BadfishException:
-            pass
+
+        if use_cli_secrets:
+            # Legacy behavior: Pass secrets via CLI args to test warning logic
+            if mock_user is not None:
+                argv.extend(("-u", mock_user))
+            if mock_pass is not None:
+                argv.extend(("-p", mock_pass))
+            argv.extend(self.args)
+        else:
+            # Default behavior for tests: Use Env Vars to suppress warnings
+            if mock_user is not None:
+                env_vars["BADFISH_USERNAME"] = mock_user
+            if mock_pass is not None:
+                env_vars["BADFISH_PASSWORD"] = mock_pass
+            argv.extend(self.args)
+
+        with patch.dict(os.environ, env_vars):
+            try:
+                main(argv)
+            except BadfishException:
+                pass
+
         out, err = self._capsys.readouterr()
         return out, err
 

tests/test_base.py

@@ -13,8 +13,11 @@
     RESPONSE_INIT_SYSTEMS_RESOURCE_NOT_FOUND,
     ROOT_RESP,
     SUCCESSFUL_HOST_LIST,
+    SYS_RESP,
     WRONG_BADFISH_EXECUTION,
     WRONG_BADFISH_EXECUTION_HOST_LIST,
+    MANAGER_INSTANCE_RESP,
+    JOBS_RESP,
 )
 from tests.test_base import TestBase
 
@@ -82,6 +85,21 @@ def test_host_list_extras(self, mock_get, mock_post, mock_delete):
 class TestInitialization(TestBase):
     args = ["--ls-jobs"]
 
+    @patch("aiohttp.ClientSession.delete")
+    @patch("aiohttp.ClientSession.post")
+    @patch("aiohttp.ClientSession.get")
+    def test_cli_secrets_warning(self, mock_get, mock_post, mock_delete):
+        """Test that passing credentials via CLI triggers a warning."""
+        responses = [ROOT_RESP] * 4 + [SYS_RESP, MAN_RESP, MANAGER_INSTANCE_RESP, JOBS_RESP]
+        self.set_mock_response(mock_get, 200, responses)
+        self.set_mock_response(mock_post, 200, "OK")
+        self.set_mock_response(mock_delete, 200, "OK")
+
+    # Explicitly use CLI secrets to trigger the warning
+        _, err = self.badfish_call(use_cli_secrets=True)
+
+        assert "Passing secrets via command line arguments can be unsafe" in err
+
     @patch("aiohttp.ClientSession.delete")
     @patch("aiohttp.ClientSession.post")
     @patch("aiohttp.ClientSession.get")

tests/test_execution.py

@@ -24,10 +24,7 @@ def test_hosts_good(self):
         for call in badfish_mock.await_args_list:
             _host, _args, _logger, _fh = call[0]
             assert _host == config.MOCK_HOST
-
             assert _args["host_list"] == self.mock_hosts_good_path
-            assert _args["u"] == config.MOCK_USER
-            assert _args["p"] == config.MOCK_PASS
 
     def test_hosts_non_existent(self):
         self.args = [self.option_arg, "non/existent/file"]
@@ -61,7 +58,4 @@ def test_hosts_bad(self):
         for call in badfish_mock.await_args_list:
             _host, _args, _logger, _fh = call[0]
             assert _host == config.MOCK_HOST
-
             assert _args["host_list"] == self.mock_hosts_garbled_path
-            assert _args["u"] == config.MOCK_USER
-            assert _args["p"] == config.MOCK_PASS

tests/test_hosts_file.py

@@ -0,0 +1,21 @@
+import pytest
+from unittest.mock import patch, MagicMock
+import asyncio
+import logging
+
+from src.badfish.main import execute_badfish
+
+@pytest.mark.asyncio
+async def test_missing_credentials():
+    host = "test_host"
+    args = {}  # No 'u' or 'p' in args
+    logger = MagicMock(spec=logging.Logger)
+    format_handler = None
+
+    with patch('os.environ.get', side_effect=lambda k: None):  # Mock env vars as missing
+        result = await execute_badfish(host, args, logger, format_handler)
+
+    assert result == (host, False)
+    logger.error.assert_called_once_with(
+        "Missing credentials. Please provide credentials via CLI arguments or environment variables."
+    )