Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 82 additions & 59 deletions README.md

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/badfish/helpers/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
15 changes: 13 additions & 2 deletions src/badfish/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
2 changes: 2 additions & 0 deletions tests/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":[]}'
35 changes: 25 additions & 10 deletions tests/test_base.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sys
import os
from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch

import pytest
Expand Down Expand Up @@ -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

Expand Down
18 changes: 18 additions & 0 deletions tests/test_execution.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down
6 changes: 0 additions & 6 deletions tests/test_hosts_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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
21 changes: 21 additions & 0 deletions tests/test_main_coverage.py
Original file line number Diff line number Diff line change
@@ -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."
)
Loading