Conversation
7100b9a to
c81b6c2
Compare
4b3451c to
64f8540
Compare
| name: foreman-proxy-remote_execution_ssh-ssh-key | ||
| path: /root/foreman-proxy-ssh | ||
| notify: | ||
| - Restart Foreman Proxy | ||
| - Refresh Foreman Proxy | ||
|
|
||
| - name: Create SSH Pub podman secret | ||
| containers.podman.podman_secret: | ||
| state: present | ||
| name: foreman-proxy-remote_execution_ssh-ssh-pub | ||
| path: /root/foreman-proxy-ssh.pub |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Handlers run only once, no matter how often they are notified.
src/roles/foreman_proxy/tasks/feature/remote_execution_ssh.yaml
Outdated
Show resolved
Hide resolved
922ca79 to
d103cc6
Compare
6d74c47 to
3ee17bd
Compare
|
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 ? |
3ee17bd to
194332a
Compare
|
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 |
194332a to
caf8416
Compare
|
@adamlazik1 thanks, applied |
|
What remains to be done here until this can be undrafted? |
eaf7e75 to
53daace
Compare
c6bc7ff to
f309355
Compare
f309355 to
4e4c695
Compare
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:
Once those are settled (and the "temp" commits dropped), we can continue here. |
|
So, while adding feature |
04ac733 to
f388e94
Compare
|
There is still one "temp" commit here: But otherwise I think this can be finally undrafted. |
ae4c055 to
7533b1d
Compare
|
Okay, and cleaned up the last temp commit. LET'S GO! |
|
From the user's perspective this feature is |
|
Ansible is a standalone feature, given it's an own gem and comes with a lot more data. For ssh vs mqtt I'd do a dedicated option, like today? At some point Ewoud (I think it was Ewoud) suggested "rex/ssh" and "rex/mqtt" (so "feat/subfeat"), but I don't really like it |
|
Do we expect users to see it that way? I was hoping we could help connect our deployment tooling and options with how the user is presented these features through the application itself and our documentation. I won't block progress on this with that question, but I think we should answer it eventually. |
I'm the wrong one to tell the user view. But it gets even more complicated than this. |
Otherwise the "refresh" handler runs while the Proxy is not yet registered, which doesn't work. To avoid double-restarts by the "restart" handler, only restart proxy if it's not freshly started
| Users want to enable abstract features, which means the deployment needs to know how to translate a feature name to a set of changes (configuration files, services, etc). | ||
|
|
||
| The metadata is a Hash with the feature name as the key and the feature definition as the value. | ||
| The metadata is a Hash with the feature (using dashes, not underscores if needed) name as the key and the feature definition as the value. |
There was a problem hiding this comment.
A general thought: maybe we should define the schema using a standard tool like openAPI or JSONSchema.
There was a problem hiding this comment.
Yepp, that's a good idea long term
src/filter_plugins/foremanctl.py
Outdated
|
|
||
|
|
||
| def foreman_proxy_plugins(value): | ||
| dependencies = [FEATURE_MAP.get(feature, {}).get('dependencies', []) for feature in filter_content(value) if feature not in BASE_FEATURES] |
There was a problem hiding this comment.
Is this function recursive? As in does it pull dependencies of dependencies?
| name: foreman-proxy-{{ feature_name }}-yml | ||
| data: "{{ lookup('ansible.builtin.template', 'settings.d/' + feature_name + '.yml.j2') }}" | ||
| notify: | ||
| - Restart Foreman Proxy |
There was a problem hiding this comment.
While the notification will be executed only once, but just creating the secret should not trigger restart. Only the mounting of a new secret should cause a restart.
Unless this also overwrites an existing secret, even if it is already mounted.
| containers.podman.podman_secret: | ||
| state: present | ||
| name: foreman-proxy-{{ feature_name }}-yml | ||
| data: "{{ lookup('ansible.builtin.template', 'settings.d/' + feature_name + '.yml.j2') }}" |
There was a problem hiding this comment.
This means the settings.d template file should always exist. What about features that do not have a proxy plugin, like IoP?
We still want the feature.yaml file to execute the Include additional tasks part, but we don't want it to fail on missing settings.d template.
There was a problem hiding this comment.
No, in that case we have no proxy plugin, just a standalone role
| foreman_proxy_base_features: | ||
| - logs | ||
| foreman_proxy_plugins: [] | ||
| foreman_proxy_features: "{{ foreman_proxy_base_features + foreman_proxy_plugins }}" |
There was a problem hiding this comment.
If foreman_proxy_plugins contains only the list of ruby smart-proxy implementation of plugins, we will miss features that do not have a plugin associated with them, like IoP.
There was a problem hiding this comment.
So? This role is only responsible for smart proxy plugins.
| validate_certs: false | ||
|
|
||
| - name: Flush handlers to restart services | ||
| ansible.builtin.meta: flush_handlers |
There was a problem hiding this comment.
Why do you need an explicit run of handlers at this point? The handlers should be executed after all the roles have been executed: https://docs.ansible.com/projects/ansible/latest/playbook_guide/playbooks_handlers.html#controlling-when-handlers-run
| @@ -1,2 +1,2 @@ | |||
| --- | |||
| :enabled: https | |||
| :enabled: {{ feature_enabled }} | |||
There was a problem hiding this comment.
It will be true/false, shouldn't it be https/false?
There was a problem hiding this comment.
True is an alias for "all enabled protocols"
No description provided.