Skip to content

Fixes #39321 - Remove SSH providers from BMC#10984

Merged
stejskalleos merged 1 commit into
theforeman:developfrom
arvind4501:remove-bmc-ssh
May 15, 2026
Merged

Fixes #39321 - Remove SSH providers from BMC#10984
stejskalleos merged 1 commit into
theforeman:developfrom
arvind4501:remove-bmc-ssh

Conversation

@arvind4501
Copy link
Copy Markdown

In RFC , there is proposal to remove SSH and Shell as BMC providers, “SSH” is exposed in the Foreman UI (even if not functional in the default config).

@arvind4501 arvind4501 changed the title Fixes #XXXXXX - Remove SSH providers from BMC Fixes #39321 - Remove SSH providers from BMC May 14, 2026
Copy link
Copy Markdown
Contributor

@stejskalleos stejskalleos left a comment

Choose a reason for hiding this comment

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

Changes LGTM, I have not found any other places where the SSH provider is used.
Once the CI is green, we can merge

@arvind4501 once the Smart Proxy PR is ready, please link it here.

@stejskalleos
Copy link
Copy Markdown
Contributor

The Katello failures are not related to the changes in this PR, let's get this in!

@stejskalleos stejskalleos merged commit c9c3ec5 into theforeman:develop May 15, 2026
33 of 39 checks passed
ipmi = bmc_nic
return false if ipmi.nil?
(ipmi.credentials_present? && %w(IPMI Redfish).include?(ipmi.provider)) || ipmi.provider == 'SSH'
(ipmi.credentials_present? && %w(IPMI Redfish).include?(ipmi.provider))
Copy link
Copy Markdown
Member

@evgeni evgeni May 15, 2026

Choose a reason for hiding this comment

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

I know this is already merged, but given IPMI and Redfish are the only providers left, this could be:

Suggested change
(ipmi.credentials_present? && %w(IPMI Redfish).include?(ipmi.provider))
ipmi.credentials_present?

the "credentials present and IPMI/Redfish" was only needed to shortcut to "or SSH"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Interestingly, this was done like this since 40df7df (and only a8d5cb3 added a second BMC implementation)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, looking at #7616 , it added credentials must be present if provider is Redfish or IPMI.

I have created a updated the logic #10987 , should it have its own redmine or there is a way to link 2 PRs to one redmine?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMHO you can just do Refs #39321 as that's not in a released version of Foreman yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants