Undefined default parameter for ssh::server::match_block

[smoeding]

I'm getting the following error after updating from release 8.0.0 to 9.0.0:

Could not retrieve catalog from remote server: Error 500 on SERVER: Server
Error: Evaluation Error: Error while evaluating a Resource Statement,
Ssh::Server::Match_block[sftponly]: parameter 'target' expects
a Stdlib::Absolutepath = Variant[Stdlib::Windowspath =
Pattern[/\A(([a-zA-Z]:[\\\/])|([\\\/][\\\/][^\\\/]+[\\\/][^\\\/]+)|([\\\/][\\\/]\?[\\\/][^\\\/]+)).*\z/],
Stdlib::Unixpath = Pattern[/\A\/([^\n\/\0]+\/*)*\z/]] value, got Undef (file:
/etc/puppetlabs/code/environments/production/modules/ssh/manifests/server.pp,
line: 79)

It happens on all machines where ssh::server::match_block is used. The defined type now has a new parameter target using a default of $ssh::sshd_config. But that variable is never defined.

[saz]

Looks like as it should be set to $ssh::params::sshd_config.

[saz]

@smoeding Can you try the current master, if it's fixing the issue for you?

[smoeding]

Unfortunately I now have a "duplicate resource error". I declare ssh::server in a local profile class and ssh::server::match_block includes ssh which also declares that class. So I will have to make bigger changes before I can test anything...

[saz]

There's an open PR (#325) which will set ssh:: server and ssh::client as private.

Why are you using ssh::server directly? I'm curious, because this is also a remark on the PR I've mentioned above.
I'm thinking about adding a parameter to enable/disable management of the client or the server.

[kajinamit]

We are hitting the exact same issue, because of direct usage of ssh::server to set up server component only.
We have two sshd services with different match configuration so we use ssh::server::match_block directly to implement additional configuration for one of these services.

Though we can update our implementation to use the base ssh class, I'm wondering why the ssh::server::match_block needs to include the base ssh class. I've submitted #326 to loose that dependency, which would ease the current problem until people updates their implementation to use the base ssh class.

[smoeding]

There's an open PR (#325) which will set ssh:: server and ssh::client as private.

Why are you using ssh::server directly? I'm curious, because this is also a remark on the PR I've mentioned above. I'm thinking about adding a parameter to enable/disable management of the client or the server.

The ssh_hardening module (https://forge.puppet.com/modules/hardening/ssh_hardening) was in use when I joined the company. That uses ssh::server and ssh::client from your module.
Now ssh_hardening seems to be abandoned for some years and when we needed to support more operating systemes I just copied and modified the code to go on. Seems I need to do some refactoring now...

[saz]

@smoeding I hope to find time over the weekend to get it working again as it has been before. Not sure, if it's really worth it, but I've added this as a feature back then.

[saz]

Please have a look at #330 if it's solving your issue

[smoeding]

I just looked at the new branch and it seems the location for the default parameters moves in the file:

Notice: /Stage[main]/Ssh::Client::Config/File[/etc/ssh/ssh_config]/content:
--- /etc/ssh/ssh_config 2021-07-08 14:23:48.808072975 +0200
+++ /tmp/puppet-file20220420-24537-ihdo71 2022-04-20 12:02:21.383885197 +0200
@@ -1,5 +1,8 @@
 # File managed by Puppet

+Host *
+    HashKnownHosts yes
+    SendEnv LANG LC_*
 AddressFamily inet
 Port 22
 Protocol 2
@@ -20,6 +23,3 @@
 Tunnel no
 PermitLocalCommand no
 Compression yes
-Host *
-    HashKnownHosts yes
-    SendEnv LANG LC_*

This changes the semantics as soon as an additional Host entry is added, because all the global params (like AddressFamily and Port) then only apply to the second Host entry.

[saz]

Are you adding

Host *
    HashKnownHosts yes
    SendEnv LANG LC_*

in your manifests/hiera?

If possible, send me an example how you're using this module.

[smoeding]

No, I'm not adding it myself. It's part of your module: see data/common.yaml line 35.

[saz]

Looks like I've got something wrong in merging options. I'll compare the generated ssh_config of v8.0.0 with the one generated by this branch.

Is there any other issue you're seeing with PR #330 right now? I'm thankful for any feedback to get the PR merged

[saz]

I've found the issue and it's fixed in #331 (accidentally closed #330 and it's not possible to reopen it).

Pull the newest changes from the branch and give it a try.

[smoeding]

With commit 69aa770 I don't see any errors or unexpected changes to a config file any more.
Looks good to me!

[saz]

@kajinamit If you've got some time, feedback on #331 and if it's resolving your issue would be awesome!

[saz]

@smoeding Just to be sure: #331 is fixing this issue, right?

[smoeding]

@smoeding Just to be sure: #331 is fixing this issue, right?

Yes, I don't see any more problems when using 69aa770

[kvedder-amplex]

Did this change make it into 9.0.0? If not, could you push out a new release?