Skip to content
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.

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

Allow to set node_terminus variable #921

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

Conversation

d1nuc0m
Copy link

@d1nuc0m d1nuc0m commented Nov 22, 2024

Allow to override default value of [server]/node_terminus in puppet.conf.
Useful in case of standalone (non-Foreman) Puppet Server installs without an ENC.

@d1nuc0m d1nuc0m force-pushed the server_terminus_variable branch 2 times, most recently from 9422ede to d5ca407 Compare November 22, 2024 13:25
Allow to override default value of [server]/node_terminus in puppet.conf
Useful in case of standalone (non-Foreman) Puppet Server installs
without an ENC.
Tests are updated as now without Foreman, node terminus is explicitly set
to the default 'plain' value

Reference
https://www.puppet.com/docs/puppet/8/configuration.html#node-terminus
@d1nuc0m d1nuc0m force-pushed the server_terminus_variable branch from d5ca407 to 87a6f6d Compare November 22, 2024 13:26
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Test failures look related.

manifests/server/config.pp Outdated Show resolved Hide resolved
@@ -33,14 +33,21 @@
## General configuration
$ca_server = $puppet::ca_server
$ca_port = $puppet::ca_port
$server_node_terminus = $puppet::server::node_terminus
$server_external_nodes = $puppet::server::external_nodes
$server_environment_timeout = $puppet::server::environment_timeout
$trusted_external_command = $puppet::server::trusted_external_command
$primary_envs_dir = $puppet::server::envs_dir[0]

if $server_external_nodes and $server_external_nodes != '' {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should look at $server_node_terminus and only when it's set to exec this should we include the ENC. So perhaps:

if $server_node_terminus == 'exec' {
  class { 'puppet::server::enc':
    node_terminus => $server_node_terminus,
    enc_path      => $server_external_nodes,
  }
}

Ideally we would change $server_external_nodes to Optional[Stdlib::Absolutepath] then and drop empty string handling. That could be considered a breaking change, but I think it's for the better.

I haven't look at all at the consequences of this so there's a good chance I'm missing something.

Copy link
Author

@d1nuc0m d1nuc0m Nov 25, 2024

Choose a reason for hiding this comment

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

Thanks, as there also is classifier option I think in that case it must be written. Maybe there also are some changes to do in the without foreman tests, as it might be possible to not want the Foreman integration but use an ENC

d1nuc0m and others added 2 commits November 25, 2024 09:26
Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
node_terminus should default to exec and write nothing if set to plain
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.

2 participants