Fixes #38478 - Introduce SSH CA certificate support #977

adamlazik1 · 2025-06-09T12:42:55Z

No description provided.

adamlazik1 · 2025-06-09T12:43:06Z

Only drafting stage.

adamlazik1 · 2025-06-23T09:31:11Z

I will keep this in draft because there will be four PRs in total that should get merged at roughly the same time, but I do believe that this is now ready for review.

adamruzicka · 2025-06-26T08:14:32Z

app/lib/proxy_api/remote_execution_ssh.rb

+    rescue
+      Rails.logger.info("Unable to fetch CA public key. Using public key authentication instead.")


So if the proxy doesn't have the ca_pubkey endpoint, the get will raise and we'll get this warning. If the proxy has that endpoint, but doesn't have a ca_pubkey configured, the get will silently return an empty string, right?

Right. Looking at it now I don't know why I made it like this, the message makes little sense. I updated it to reflect the behavior of the pubkey api call.

app/models/concerns/foreman_remote_execution/smart_proxy_extensions.rb

app/models/concerns/foreman_remote_execution/host_extensions.rb

adamlazik1 · 2025-07-21T09:05:02Z

Switching back to draft since the feature got postponed to 3.16

lhellebr · 2025-12-22T14:39:50Z

/packit build

packit-as-a-service · 2025-12-22T14:39:58Z

Account lhellebr has no write access nor is author of PR!

adamlazik1 · 2025-12-22T14:40:45Z

/packit build

adamlazik1 · 2026-01-05T11:18:41Z

/packit build

adamruzicka

The N-2 support needs to be handled, otherwise lgtm

adamruzicka · 2026-01-05T13:04:32Z

app/lib/proxy_api/remote_execution_ssh.rb

+    def ca_pubkey
+      get('ca_pubkey')&.strip
+    rescue => e
+      raise ProxyException.new(url, e, N_('Unable to fetch CA public key'))


This could be problematic. We should support running with N-2 proxies, which may not have the ca_pubkey endpoint at all. Either we could treat 404s as not an error or have different behaviour based on the version of the proxy.

Would returning nil instead of throwing exception suffice? Rest of the application logic treats nil as unset CA pubkey and proceeds with the standard pubkey.

I would prefer not using a catch-all like this to prevent masking real issues.

CC @lhellebr change of behavior: if capsule does not have the ca_pubkey endpoint due to older version, nil ca_pubkey is returned instead of erroring out.

adamruzicka · 2026-01-05T14:09:15Z

app/lib/proxy_api/remote_execution_ssh.rb

+    def ca_pubkey
+      get('ca_pubkey')&.strip
+    rescue RestClient::ResourceNotFound => e
+      Foreman::Logging.exception("Unable to fetch CA public key", e)


nitpick:
On my dev setup, this generates roughly 110 lines in the logs which is a bit excessive for a non-error case. I'd be fine with this being silent or just logging a single line

Copilot

Pull request overview

This pull request introduces SSH Certificate Authority (CA) support for Foreman Remote Execution, allowing SSH proxies to use CA-signed certificates for authentication instead of individual public keys.

Adds database column and API support for storing CA public keys on SmartProxy models
Implements logic to use CA keys when available and exclude regular SSH keys from proxies with CA configured
Extends host parameters to include SSH CA keys alongside regular SSH keys with proper merging from both proxies and host-level parameters

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
db/migrate/20250606125543_add_ca_pub_key_to_smart_proxy.rb	Adds ca_pubkey column to smart_proxies table
app/views/api/v2/smart_proxies/ca_pubkey.json.rabl	Exposes CA public key via API as remote_execution_ca_pubkey
lib/foreman_remote_execution/plugin.rb	Registers the new CA pubkey RABL template extension
app/lib/proxy_api/remote_execution_ssh.rb	Implements ca_pubkey API method to fetch CA public key from smart proxy
app/models/concerns/foreman_remote_execution/smart_proxy_extensions.rb	Adds ca_pubkey getter and update_ca_pubkey method to SmartProxy model
app/models/concerns/foreman_remote_execution/host_extensions.rb	Adds remote_execution_ssh_ca_keys method and modifies key handling to exclude regular keys when CA is configured
test/unit/concerns/host_extensions_test.rb	Adds comprehensive test coverage for SSH CA key functionality including parameter merging and proxy behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

app/lib/proxy_api/remote_execution_ssh.rb

app/models/concerns/foreman_remote_execution/host_extensions.rb

app/models/concerns/foreman_remote_execution/smart_proxy_extensions.rb

adamlazik1 · 2026-01-06T09:05:50Z

Updated based on copilot feedback.

adamruzicka · 2026-01-06T09:24:50Z

Thank you @adamlazik1 !

pr-processor bot added the Not yet reviewed label Jun 9, 2025

adamlazik1 force-pushed the ssh-cert-support branch 7 times, most recently from 2727578 to f691cbc Compare June 16, 2025 08:25

adamlazik1 mentioned this pull request Jun 16, 2025

Fixes #38499 - Introduce SSH cert support theforeman/foreman#10571

Merged

adamlazik1 force-pushed the ssh-cert-support branch from f691cbc to 2785acd Compare June 20, 2025 11:23

adamruzicka reviewed Jun 26, 2025

View reviewed changes

pr-processor bot removed the Not yet reviewed label Jun 26, 2025

adamlazik1 force-pushed the ssh-cert-support branch from 2785acd to 3299a3e Compare June 26, 2025 09:37

pr-processor bot added the Waiting on contributor label Jun 26, 2025

adamlazik1 force-pushed the ssh-cert-support branch from 9093a40 to 7707987 Compare June 26, 2025 09:40

pr-processor bot added Needs re-review and removed Waiting on contributor labels Jun 26, 2025

adamruzicka reviewed Jun 26, 2025

View reviewed changes

app/models/concerns/foreman_remote_execution/host_extensions.rb Outdated Show resolved Hide resolved

pr-processor bot removed the Needs re-review label Jun 26, 2025

adamlazik1 force-pushed the ssh-cert-support branch from 7707987 to a4914f8 Compare June 26, 2025 10:37

adamlazik1 force-pushed the ssh-cert-support branch from a4914f8 to 3a29a7b Compare July 11, 2025 06:28

adamlazik1 marked this pull request as ready for review July 11, 2025 06:55

adamlazik1 force-pushed the ssh-cert-support branch from 3a29a7b to 96ae1a6 Compare July 17, 2025 10:27

adamlazik1 marked this pull request as draft July 21, 2025 09:05

adamlazik1 force-pushed the ssh-cert-support branch from 96ae1a6 to 7d39bd4 Compare July 21, 2025 09:14

adamlazik1 force-pushed the ssh-cert-support branch from 7d39bd4 to 25a85f5 Compare October 22, 2025 11:49

adamlazik1 force-pushed the ssh-cert-support branch from 25a85f5 to 1e30bac Compare November 11, 2025 09:54

adamlazik1 force-pushed the ssh-cert-support branch 2 times, most recently from b757ba9 to 620dc78 Compare December 18, 2025 14:35

adamruzicka requested changes Jan 5, 2026

View reviewed changes

pr-processor bot added the Waiting on contributor label Jan 5, 2026

adamlazik1 force-pushed the ssh-cert-support branch from 620dc78 to b528a23 Compare January 5, 2026 13:19

pr-processor bot added Needs re-review and removed Waiting on contributor Needs re-review labels Jan 5, 2026

adamlazik1 force-pushed the ssh-cert-support branch from b528a23 to 38a2724 Compare January 5, 2026 13:54

adamruzicka reviewed Jan 5, 2026

View reviewed changes

adamlazik1 force-pushed the ssh-cert-support branch from 38a2724 to d0139f6 Compare January 5, 2026 14:20

adamruzicka requested a review from Copilot January 5, 2026 15:09

Copilot started reviewing on behalf of adamruzicka January 5, 2026 15:10 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

app/lib/proxy_api/remote_execution_ssh.rb Outdated Show resolved Hide resolved

app/models/concerns/foreman_remote_execution/host_extensions.rb Show resolved Hide resolved

app/models/concerns/foreman_remote_execution/smart_proxy_extensions.rb Show resolved Hide resolved

pr-processor bot added the Waiting on contributor label Jan 6, 2026

adamlazik1 force-pushed the ssh-cert-support branch from af3052e to faefe6e Compare January 6, 2026 09:04

pr-processor bot added Needs re-review and removed Waiting on contributor labels Jan 6, 2026

adamruzicka approved these changes Jan 6, 2026

View reviewed changes

pr-processor bot removed the Needs re-review label Jan 6, 2026

Fixes #38478 - Introduce SSH CA certificate support

961df7e

adamlazik1 force-pushed the ssh-cert-support branch from faefe6e to 961df7e Compare January 6, 2026 09:11

adamruzicka merged commit f01bdb3 into theforeman:master Jan 6, 2026
17 checks passed

adamlazik1 deleted the ssh-cert-support branch January 6, 2026 09:29

		rescue
		Rails.logger.info("Unable to fetch CA public key. Using public key authentication instead.")

Fixes #38478 - Introduce SSH CA certificate support #977

Fixes #38478 - Introduce SSH CA certificate support #977

Conversation

adamlazik1 commented Jun 9, 2025

Uh oh!

adamlazik1 commented Jun 9, 2025

Uh oh!

adamlazik1 commented Jun 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adamlazik1 commented Jul 21, 2025

Uh oh!

lhellebr commented Dec 22, 2025

Uh oh!

packit-as-a-service bot commented Dec 22, 2025

Uh oh!

adamlazik1 commented Dec 22, 2025

Uh oh!

adamlazik1 commented Jan 5, 2026

Uh oh!

adamruzicka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamlazik1 commented Jan 6, 2026

Uh oh!

Uh oh!

adamruzicka commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants