Proxy features: REX

[evgeni]

No description provided.

[evgeni]

This gets executed too early on fresh installs. Damn.

[arvind4501]

Now i wonder why is refreshing feature needed at the first place?

[evgeni]

Foreman stores the list of features a Proxy has in its database, when the configuration of a proxy changes, you need to "refresh" that data.
And as we're deploying without REX first in the tests, we need to refresh once we enabled REX.

[Gauravtalreja1]

These two tasks can be consolidated into a loop to create the SSH key/pub secrets, which will also ensure the handler runs only once, OR just run the handler when SSH secrets are mounted to the container

[evgeni]

Handlers run only once, no matter how often they are notified.

[Gauravtalreja1]

Suggested change

	name: foreman-proxy-remote_execution_ssh-ssh-key
	name: foreman-proxy-remote_execution_ssh-key

[adamruzicka]

theforeman/smart_proxy_remote_execution_ssh#128 dropped ssh-async mode , would you include this patch to keep the config file in sync 0001-Adjust-sp-rex-ssh-config-file-to-async-ssh-removal.patch ?

[adamlazik1]

theforeman/smart_proxy_remote_execution_ssh#126 added new settings for SSH certs, here is a patch that adds the new settings:

diff --git a/src/roles/foreman_proxy/templates/settings.d/remote_execution_ssh.yml.j2 b/src/roles/foreman_proxy/templates/settings.d/remote_execution_ssh.yml.j2
index b7d6402..76ad344 100644
--- a/src/roles/foreman_proxy/templates/settings.d/remote_execution_ssh.yml.j2
+++ b/src/roles/foreman_proxy/templates/settings.d/remote_execution_ssh.yml.j2
@@ -11,6 +11,15 @@
 # Mode of operation, one of ssh, pull, pull-mqtt
 :mode: ssh
 
+# Enables the use of SSH certificate for smart proxy authentication
+# The file should contain an SSH CA public key that the SSH public key of smart proxy is signed by
+# :ssh_user_ca_public_key_file:
+
+# Enables the use of SSH host certificates for host authentication
+# The file should contain a list of trusted SSH CA authorities that the host certs can be signed by
+# Example file content: @cert-authority * <SSH CA public key>
+# :ssh_ca_known_hosts_file:
+
 # Defines how often (in seconds) should the runner check
 # for new data leave empty to use the runner's default
 # :runner_refresh_interval: 1

[evgeni]

@adamlazik1 thanks, applied

[adamruzicka]

What remains to be done here until this can be undrafted?

[evgeni]

What remains to be done here until this can be undrafted?

This PR was my PoC to figure out what we need to put into our feature metadata (see #372).

This PR also has two dependencies that can be reviewed/merged independently:

• add smart_proxy_refresh module foreman-ansible-modules#1930
• add REX to foreman-proxy container foreman-oci-images#13

Once those are settled (and the "temp" commits dropped), we can continue here.

[arvind4501]

So, while adding feature remote_execution it adds dynflow as dependency but when i remove the same feature it disabled remote_execution config enabled: false in remote_execution_ssh.yml but dynflow.yml still has :enabled: true . are we planning for disabling the dependencies once main feature is removed?

[evgeni]

This is actually an oversight of the implementation in this PR.
https://github.com/theforeman/foremanctl/pull/309/changes#diff-06bd482f2e704a345c2e14e3791f97693a3c34717be4af8dd6f46b736d1a8982 defines "known" features, and "dynflow" is not on that list -- so it doesn't know to disable it.
If we feed that list based off the metadata (and we should!) then it will solve itself

[arvind4501]

Are we limiting this feature_enabled to true and false, as i see it can have http, https etc as well?

[evgeni]

It can, but given that we only configure a https port right now, it's kinda pointless to differentiate.

[arvind4501]

If we feed that list based off the metadata (and we should!) then it will solve itself

Yes, that should work, but if i understand correctly the design is to keep known features list limited to what user asked for(like a user said i want feature A and B then A, B should be in the known feature list). but the extra dependencies features are something user never touch. so then should we technically put dependencies in that known feature list, i am bit confused here?

Also from a broder view removing a feature or disabling a feature is completely distinct thing, as removing should remove configs,(maybe binaries too), and disabling should just update the config. but here removing feature just disables 😕

[evgeni]

Yes, that should work, but if i understand correctly the design is to keep known features list limited to what user asked for(like a user said i want feature A and B then A, B should be in the known feature list). but the extra dependencies features are something user never touch. so then should we technically put dependencies in that known feature list, i am bit confused here?

No, I think we still need to map out those dependency features as features, just sometimes ones that we don't show the user (in #372 I introduce an internal: true flag for that)

Our metadata lists all known features (and thus all known foreman proxy plugins). Which right now means ['remote_execution_ssh', 'dynflow']. For the fun of the example, let's add ansible to that list.

Now the user says "give me remote_exucution", which our code translates to enabled_proxy_plugins = ['remote_execution_ssh', 'dynflow'] (as dynflow is a dependency).
Because disabled_proxy_plugins = known_proxy_plugins - enabled_proxy_plugins, that list is ['ansible'].

If the user now decides to disable remote_execution, the enabled_proxy_plugins becomes [] and disabled_proxy_plugins becomes ['remote_execution_ssh', 'dynflow', 'ansible'], disabling everything.

Also from a broder view removing a feature or disabling a feature is completely distinct thing, as removing should remove configs,(maybe binaries too), and disabling should just update the config. but here removing feature just disables 😕

We can't remove binaries, they are baked into the container.

[evgeni]

@ekohl can you remind me why we're force-enabling logs?
This is there since #279 but no explanation why we enable it.

[evgeni]

I've split out a few smaller changes in own PRs to make them easier to review:

• correctly configure ssl headers #376
• introduce client_hostname and client_fqdn fixtures #377
• parse foreman proxy features as json, makes the assert cleaner #378