Module not work

[BaronMsk]

Describe the Bug

After commit: e7addfb not work modules
log join node
Notice: /Stage[main]/Main/Exec[kubeadm join]/returns: unknown flag: --config '/etc/confi.yaml' --ignore-preflight-errors 'Service-Docker'
Notice: /Stage[main]/Main/Exec[kubeadm join]/returns: To see the stack trace of this error execute with --v=5 or higher
Error: '["kubeadm", "join", "--config '/etc/confi.yaml' --ignore-preflight-errors 'Service-Docker'"]' returned 1 instead of one of [0]
Error: /Stage[main]/Main/Exec[kubeadm join]/returns: change from 'notrun' to ['0'] failed: '["kubeadm", "join", "--config '/etc/confi.yaml' --ignore-preflight-errors 'Service-Docker'"]' returned 1 instead of one of [0]

Expected Behavior

Should be like this
Debug: Execkubeadm join: Executing 'kubeadm join --config '/etc/kubernetes/config.yaml' --ignore-preflight-errors 'Service-Docker''
Debug: Executing: 'kubeadm join --config '/etc/kubernetes/config.yaml' --ignore-preflight-errors 'Service-Docker''

Steps to Reproduce

Steps to reproduce the behavior:
install or join new nodes

Environment

puppet-agent 6.28.0
puppet-server 6.20.0
Ubuntu focal

Additional Context

Someone tested this commit e7addfb ?
Why tests have been broken for more than two months ?
Broken build docker image tooling.
I'm trying to fix it all in my PR #583

[BaronMsk]

I found that this commit broke a lot more than it seems, I suggest to git revert

[ArsenyBelorukov]

in addition, the wrong regexp was added in this commit e7addfb#r86807409

[BaronMsk]

You can rollback this commit ?

[ArsenyBelorukov]

You can rollback this commit ?

No

[BaronMsk]

Why, this makes the module not working ?

[ArsenyBelorukov]

Why, this makes the module not working ?

I am not a member of puppet/modules

[ArsenyBelorukov]

Maybe @david22swan could help

[BaronMsk]

@GSPatton help please

[pmcmaw]

Hey @BaronMsk
Apologies, I can see something has went wrong. We have taken note and this is something we are working on to resolve.
In order to avoid issues like this arising in the future, I advise you to pull released versions of this module from the Forge. Our main branch may not always be as stable as we would like it to be. When a module is released it is our way of saying, this module has been thoroughly tested and it stable for end users to consume.

[BaronMsk]

@pmcmaw tnx

[LukasAud]

Hi @BaronMsk, we have been investigating this issue. Upon inspection of our original codebase hardening PR, we have been able to spot a few issue with our previous implementation and implement appropriate fixes recently.

However, we are currently having issues investigating as we cannot replicate on our end.
The fix that was mentioned previously may be a potential fix for your issue but we are unable to confirm. Could you provide us with a more detailed 'Steps to reproduce' section in your issue report? Alternatively, would it be possible to test out the change?

[BaronMsk]

@LukasAud I can test your branch on my servers

[LukasAud]

Hi @BaronMsk, thanks for offering to test this. The current latest build we have in here in GitHub already has the patch I mentioned earlier. If you could update to our latest push and let me know if there is any visible impact in comparison to the failing version, that would be great.

[BaronMsk]

not work
Error: /Stage[main]/Kubernetes::Cluster_roles/Kubernetes::Kubeadm_join[k8s-test-worker-dtln-3]/Exec[kubeadm join]: Could not evaluate: Could not find command 'kubectl get nodes | grep k8s-test-worker-dtln-3'

[BaronMsk]

swapoff - not work Debug: /Stage[main]/Kubernetes::Packages/Exec[disable swap]: '["swapoff", "-a"]' won't be executed because of failed check 'unless' Debug: /Stage[main]/Kubernetes::Packages/File_line[remove swap in /etc/fstab]: Nothing to manage: no ensure and the resource doesn't exist

[LukasAud]

Thats interesting. I think I know what might the failure point for both of these errors. Both seem to be caused by my modification of the 'unless' commands. It seems wrapping unbreakable commands in arrays (and later, "fixed" by nesting those arrays) might have been counterproductive.

It seems I will have to revert some of my updates. I still cannot be sure if this will help with the original issue but I will keep this thread updated as I work on the module.

[BaronMsk]

@LukasAud Yes, commit revert will help 100%. I already checked it!

[LukasAud]

Unfortunately, this commit is important to our modules health and we cannot fully revert it. Our ideal scenario is to revert only whatever code is breaking the normal module behaviour and leave the rest. I will discuss this issue with a senior engineer in our team and see whats the best approach to fixing this.

[BaronMsk]

@LukasAud What problem did you want to solve with this commit?

[LukasAud]

@BaronMsk This commit was part of a larger project that was set to address concerns about code vulnerabilities in some of our modules. Unfortunately, some modules are giving us a harder time than others during this work. However, we consider this to be essential for the health of our modules and for establishing appropriate coding standards in the future.

[BaronMsk]

@LukasAud Where can I find these vulnerabilities ?

[LukasAud]

@BaronMsk This article we wrote should be able to explain the security issue better than me. More precisely, during our security update project, we have been targeting executable commands that contained interpolated user input. That involved examining 'command', 'onlyif' and 'unless' parameters and breaking them down where possible (this is not possible in certain scenarios, such as when a pipe | is present).

[LukasAud]

Hey @BaronMsk, we have merged this bugfix which should address some of the latest issues. However, I cannot assure that this will fix the original problem. Right now, it looks like our team will have to take some time to dig deeper into the module and, most likely, do some maintenance before we can properly investigate the failure.

Our forge build should still be a stable version of the module, so I would recommend sticking to that one for now. Sorry for the inconvenience.

[BaronMsk]

@LukasAud it doesn't seem to work there can only be a string, otherwise, the expression is treated as different commands

[deric]

@BaronMsk I think #598 and #599 should fix most (if not all) of the problems mentioned above.

[chelnak]

+1 to the above - clusters seem to be spinning up from the HEAD of main.

[jordanbreen28]

@BaronMsk Thanks for raising this one, seems this issue has been resolved.
Are you able to test this out?

[BaronMsk]

@jordanbreen28 hi, i can check it in a few days.

[jordanbreen28]

Nice one @BaronMsk - if you could then update the thread here that'd be great!

[LukasAud]

Hey @BaronMsk, can we assume that this issue was resolved? If so, we would like to close it.

[BaronMsk]

@LukasAud Yes, you can close it