ncm-ssh: make config checks configurable. #645
Conversation
| @@ -117,6 +117,7 @@ type ssh_daemon_type = { | |||
| "options" ? ssh_daemon_options_type | |||
| "comment_options" ? ssh_daemon_options_type | |||
| "sshd_path" ? string | |||
| "validate_config" ? string with match (SELF, '^(yes|no|optional)$') | |||
There was a problem hiding this comment.
please use a boolean to validate or not.
and use another boolean to allow to skip the validation if sshd is missing (but if sshd is missing ,you cannot validate anyway, so this feels like the boolean means something else)
but why would one ever choose not to validate? and why would we allow that?
i think one boolean to indicate that the test can be skipped if sshd is missing is enough.
btw, what sort of system is this that there is no sshd available? (maybe not the one that you will run, but can't you use a system sshd to validate the config if the one from the network FS is missing? imho it's better than just skipping the config test)
There was a problem hiding this comment.
OK, I've changed it to a boolean called validate_optional. Disabling the checks completely has been removed.
Our sshd is a component which happens to be managed in the network filesystem, installing another through a package isn't something we would want to do. The config validation is very nice to have after build, but we've done without it previously and it would be easiest to just allow the check to skip at build time.
| @@ -117,6 +117,8 @@ type ssh_daemon_type = { | |||
| "options" ? ssh_daemon_options_type | |||
| "comment_options" ? ssh_daemon_options_type | |||
| "sshd_path" ? string | |||
| # if true and sshd doesn't exist, will skip config validation | |||
There was a problem hiding this comment.
Can you add this to the pod too please? I think "validation_optional" would be a more correct name too? Or we could reverse the sense and say "always_validate" : boolean = true ?
There was a problem hiding this comment.
or put the comment in @{}, so it will at least be picked up by the annotations
@ned21 but what is the inverse/opposite of always? never or optional
There was a problem hiding this comment.
The opposite of always is never but "never != (not always)". Or to put it another way: always = false does not imply never.
add always_validate parameter, set to false and if sshd doesn't exist then the config test will be skipped. useful if you do not have the sshd binary available during build time (eg. on a network FS). previous behaviour (always validate) has been kept as default.
|
Update with the latest feedback addressed hopefully |
|
LGTM |
1 similar comment
|
LGTM |
ncm-ssh: make config checks configurable.
small oversight on my part in the previous PR regarding this component during build time for us. sshd is on a network FS which isn't available at build time, so I've added a new param where you can make this check skip if sshd isn't existing.
existing behaviour, to always check, is still the default.