Skip to content

Add BMC feature to foreman-proxy #484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
arvind4501 wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add BMC feature to foreman-proxy #484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e9c751c
Add BMC feature to foreman-proxy
May 4, 2026
602b4fd
Add Redfish configurations
May 4, 2026
3f2dfe9
tests
May 5, 2026
a199b4c
update parameters doc
May 6, 2026
7c4db9f
refactor tests
May 6, 2026
3a93f2c
use server ca
May 6, 2026
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
9 changes: 7 additions & 2 deletions docs/user/parameters.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ There are multiple use cases from the users perspective that dictate what parame

## Smart Proxy

### Mapped

| Parameter | Description | foreman-installer Parameters |
| --------- | ----------- | ---------------------------- |
| `--bmc-default-provider` | Default BMC provider | `--foreman-proxy-bmc-default-provider` |
| `--bmc-redfish-verify-ssl` | Verify SSL certificates for Redfish BMC connections | `--foreman-proxy-bmc-redfish-verify-ssl` |

### Undetermined

| Installer Parameter | Description | Module | Puppet Parameter |
Expand Down Expand Up @@ -152,8 +159,6 @@ There are multiple use cases from the users perspective that dictate what parame
| `--foreman-proxy-plugin-remote-execution-script-mode` | | foreman_proxy::plugin::remote_execution_script | mode |
| `--foreman-proxy-plugin-openscap-ansible-module` | | foreman_proxy::plugin::openscap | ansible_module |
| `--foreman-proxy-plugin-openscap-puppet-module` | | foreman_proxy::plugin::openscap | puppet_module |
| `--foreman-proxy-bmc` | | | |
| `--foreman-proxy-bmc-default-provider` | | | |
| `--foreman-proxy-content-enable-ostree` | | | |
| `--foreman-proxy-content-pulpcore-additional-import-paths` | | | |
| `--foreman-proxy-http` | | | |
Expand Down
4 changes: 4 additions & 0 deletions src/features.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,7 @@ iop:
description: iop services
dependencies:
- rh-cloud
bmc:
description: Power management for bare metal hosts (IPMI, Redfish, SSH)
foreman_proxy:
plugin_name: bmc
11 changes: 11 additions & 0 deletions src/playbooks/deploy/metadata.obsah.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ variables:
type: AbsolutePath
parameter: --certificate-server-ca-certificate
persist: false
foreman_proxy_bmc_default_provider:
parameter: --bmc-default-provider
help: Default BMC provider.
choices:
Copy link
Copy Markdown
Contributor

@stejskalleos stejskalleos May 6, 2026

Choose a reason for hiding this comment

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

In the bmc.yaml.example are more providers:

# Available providers:
# - freeipmi / ipmitool - requires the appropriate package installed, and the rubyipmi gem
# - redfish - requires the redfish_client gem
# - shell - for local reboot control (requires sudo access to /sbin/shutdown for the proxy user)
# - ssh - limited remote control (status, reboot, turn off)
:bmc_default_provider: freeipmi

- freeipmi
- ipmitool
- redfish
Comment on lines +47 to +53
Copy link
Copy Markdown
Member

@evgeni evgeni May 5, 2026

Choose a reason for hiding this comment

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

While the parameter is called "default provider", the overall behavior is a bit different.

The smart proxy code takes the provider from the request, and falls back to the default if unset (honoring the "default" definition):
https://github.com/theforeman/smart-proxy/blob/a5780bc6e0132d7e91fa3074845b2a3d1cc30f01/modules/bmc/bmc_api.rb#L392-L393

But then Foreman comes (who is the only consumer of this API) and always sets the provider unless the selected provider in Foreman is "IPMI":
https://github.com/theforeman/foreman/blob/f2d892f7585c61d71f671298c8158712daace8e8/app/models/nic/bmc.rb#L27

Which basically makes the setting "used IPMI implementation" (as one can't pick one in Foreman), not "fallback provider".

To add insult to injury, everyone disagrees what a provider is. Foreman defines IPMI, Redfish and SSH, but the Proxy knows about freeipmi, ipmitool, redfish, shell and ssh 🤯

I don't think we can (or even should) fix all of that mess here and now, but maybe calling this ipmi_implementation instead of default_provider and dropping redfish would slightly reduce the user confusion?

👀 @stejskalleos

PS: I couldn't find any users of the "Shell" provider, which is basically giving power control over the machine running the smart proxy. First I thought that'd be used in Discovery, but there we have an own implementation. It was added in https://projects.theforeman.org/issues/2387 but the explanation is rather… slim

Copy link
Copy Markdown
Contributor Author

@arvind4501 arvind4501 May 5, 2026

Choose a reason for hiding this comment

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

But then Foreman comes (who is the only consumer of this API) and always sets the provider unless the selected provider in Foreman is "IPMI":
https://github.com/theforeman/foreman/blob/f2d892f7585c61d71f671298c8158712daace8e8/app/models/nic/bmc.rb#L27

In this case of user selected IPMI in foreman UI, the smart proxy does not get any bmc_provider in request and falls back to Proxy::BMC::Plugin.settings.bmc_default_provider , which is ipmitool if its installed via foreman-installer.

To add insult to injury, everyone disagrees what a provider is. Foreman defines IPMI, Redfish and SSH, but the Proxy knows about freeipmi, ipmitool, redfish, shell and ssh 🤯

Yes and also with ssh provider, i am not sure if its supported as provider by foreman, our doc does not say that and even i can't see any ssh key deployed at https://github.com/theforeman/puppet-foreman_proxy/blob/master/manifests/init.pp#L401 path

foreman_proxy_bmc_redfish_verify_ssl:
Copy link
Copy Markdown
Member

@evgeni evgeni May 5, 2026

Choose a reason for hiding this comment

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

Should we expose the "foreman_proxy" part to the user? Or should this be --bmc-… given the feature is "bmc"?

Copy link
Copy Markdown
Contributor Author

@arvind4501 arvind4501 May 5, 2026

Choose a reason for hiding this comment

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

i think it should be --bmc-*, as we have done with foreman related params such as --initial-organization and --initial-location, will update it

parameter: --bmc-redfish-verify-ssl
help: Verify SSL certificates for Redfish BMC connections.
type: Boolean

constraints:
required_together:
Expand Down
4 changes: 4 additions & 0 deletions src/roles/foreman_proxy/defaults/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@ foreman_proxy_available_features: "{{ [] | available_foreman_proxy_plugins }}"
foreman_proxy_disabled_features: "{{ foreman_proxy_available_features | difference(foreman_proxy_features) }}"

foreman_proxy_foreman_server_url: "https://{{ ansible_facts['fqdn'] }}"

# BMC settings
foreman_proxy_bmc_default_provider: ipmitool
foreman_proxy_bmc_redfish_verify_ssl: true
7 changes: 7 additions & 0 deletions src/roles/foreman_proxy/templates/settings.d/bmc.yml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
:enabled: {{ feature_enabled }}
:bmc_default_provider: {{ foreman_proxy_bmc_default_provider }}

{% if foreman_proxy_bmc_default_provider == 'redfish' %}
:redfish_verify_ssl: {{ foreman_proxy_bmc_redfish_verify_ssl }}
{% endif %}
14 changes: 6 additions & 8 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,22 +191,20 @@ def wait_for_tasks(foremanapi, search=None):
def wait_for_metadata_generate(foremanapi):
wait_for_tasks(foremanapi, 'label = Actions::Katello::Repository::MetadataGenerate')


def is_iop_enabled():
def enabled_features():
test_dir = os.path.dirname(os.path.abspath(__file__))
foremanctl_dir = os.path.dirname(test_dir)
params_file = os.path.join(foremanctl_dir, '.var', 'lib', 'foremanctl', 'parameters.yaml')

if os.path.exists(params_file):
with open(params_file, 'r') as f:
params = yaml.safe_load(f)
features = params.get('features', [])
features = yaml.safe_load(f).get('features', [])
if isinstance(features, str):
features = features.split()
return 'iop' in features

return False
return features
return []

def is_iop_enabled():
return 'iop' in enabled_features()

def pytest_configure(config):
config.addinivalue_line("markers", "iop: tests requiring IOP to be enabled")
Expand Down
24 changes: 24 additions & 0 deletions tests/foreman_proxy_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,24 @@
import datetime
import json
import pytest
import yaml
from conftest import enabled_features

FOREMAN_PROXY_PORT = 8443
BMC_CONFIG = "/etc/foreman-proxy/settings.d/bmc.yml"

def get_proxy_feature_setting(server, feature, default=None):
cmd = server.run(f"podman exec foreman-proxy cat /etc/foreman-proxy/settings.d/{feature}.yml")
if cmd.succeeded:
return yaml.safe_load(cmd.stdout)
return default

def is_bmc_enabled():
return 'bmc' in enabled_features()

def get_default_bmc_provider(server):
bmc_setting = get_proxy_feature_setting(server, 'bmc')
return bmc_setting.get(':bmc_default_provider', 'ipmitool')

def test_foreman_proxy_features(server, certificates, server_fqdn):
cmd = server.run(f"curl --cacert {certificates['server_ca_certificate']} --silent https://{server_fqdn}:{FOREMAN_PROXY_PORT}/features")
Expand All @@ -11,6 +27,8 @@ def test_foreman_proxy_features(server, certificates, server_fqdn):
assert "logs" in features
assert "script" in features
assert "dynflow" in features
if is_bmc_enabled():
assert "bmc" in features

def test_foreman_proxy_service(server):
foreman_proxy = server.service("foreman-proxy")
Expand All @@ -33,3 +51,9 @@ def test_foreman_proxy_client_auth_to_foreman(server, certificates, server_fqdn)
)
assert cmd.succeeded
assert cmd.stdout == '201'

@pytest.mark.skipif("not is_bmc_enabled()")
def test_bmc_default_provider(server):
cmd = server.run(f"podman exec foreman-proxy grep ':bmc_default_provider:' {BMC_CONFIG}")
assert cmd.succeeded
assert get_default_bmc_provider(server) in cmd.stdout