Module not work about puppetlabs-kubernetes HOT 31 CLOSED

@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).

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

You can rollback this commit ?

from puppetlabs-kubernetes.

You can rollback this commit ?

No

from puppetlabs-kubernetes.

Why, this makes the module not working ?

from puppetlabs-kubernetes.

Why, this makes the module not working ?

I am not a member of puppet/modules

from puppetlabs-kubernetes.

Maybe @david22swan could help

from puppetlabs-kubernetes.

@GSPatton help please

from puppetlabs-kubernetes.

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.

from puppetlabs-kubernetes.

@pmcmaw tnx

from puppetlabs-kubernetes.

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?

from puppetlabs-kubernetes.

@LukasAud I can test your branch on my servers

from puppetlabs-kubernetes.

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.

from puppetlabs-kubernetes.

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'

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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.

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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.

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

@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.

from puppetlabs-kubernetes.

@LukasAud Where can I find these vulnerabilities ?

from puppetlabs-kubernetes.

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.

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

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

from puppetlabs-kubernetes.

@LukasAud Yes, you can close it

from puppetlabs-kubernetes.