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

Fixing exec usage, handling and optimise code #516

Merged
merged 7 commits into from
Jul 27, 2022

Conversation

cruelsmith
Copy link
Contributor

@cruelsmith cruelsmith commented Jul 4, 2022

Pull Request (PR) description

https://puppet.com/docs/puppet/7/types/exec.html#exec-description

exec resource

Any command in an exec resource must be able to run multiple times without causing harm --- that is, it must be idempotent.

That is why this PR fixes the issues with unnecessary exec and missing exec conditions. It also fixes some other issues with the code see the following list of changes:

  • Fix usage of execs without refresh_only, created, onlyif or unless
  • Replaced the usage of exec resources with curl to download files. Uses now file resource with http / https resource instead.
  • Replaced the usage of exec resources to copy the certfiles. Uses file resource with replace => false
    • Only copy the content from the source when the file does not exist. The file resource still manages the permissions.
  • Replace the exec resource with echo with file_line resource.
  • Remove the unneeded exec { 'Waiting for Wazuh indexer...': } in wazuh::dashboard since there is no use case for it anymore
    • Relevance has been removed with commit 9d176ef
  • Fix missing name parameter for service resources since indexer_service and dashboard_service are class parameter but has been never used.
    • Only for wazuh::indexer, wazuh::dashboard and wazuh::filebeat_oss
  • Fix wrong user name for the /etc/security/limits.conf configuration
    • The process run under the wazuh-indexer user and which also already owns the directories for this.
  • Replace the usage of legacy facts vars $::osfamily instead of $facts['os']['family']
    • Only for wazuh::indexer, wazuh::dashboard and wazuh::filebeat_oss
  • Replace the usage of find_file() in wazuh::certificates
  • Fix resource references that usages other things than the title or alias
  • Fix missing configuration of server.host and server.port for wazuh-dashboard in /etc/wazuh-dashboard/opensearch_dashboards.yml
    • The variables does already exist in the manifest but the has not been used. The config from the package used port 443 instead of 5601 as put down in wazuh::dashboard.
  • Add resource ordering references to ensure resources will be done in the right order

Note: wazuh::certificates will run everytime when /tmp has been cleaned like by a reboot of the system. But because of the new file resource handling (replace => false) the certs will not be replaced for wazuh::indexer and wazuh::dashboard but created when they do not exist.

This Pull Request (PR) fixes the following issues

Closes #512
Closes #489

There is no usecase anymore for it since commit
9d176ef
Ensure that we recognize content changes of the local and remote file.
The variables has been already there but not used at any point.
Needed after updateing the exec for cleanup wazuh-template.json.
@cruelsmith
Copy link
Contributor Author

Hi Wazuh team (@vcerenu, @fcaffieri, @alberpilot, @okynos)

we (@pixelpark) currently did not see any feedback for our created issue #512 and the three created Pull-Requests to improve this puppet module for wazuh.
Please let us know if we missed something before to contribute to this module and to get the process rolling.

Cheers

@vcerenu vcerenu self-assigned this Jul 27, 2022
Copy link
Member

@vcerenu vcerenu left a comment

Choose a reason for hiding this comment

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

Hello @cruelsmith

Thanks for your patience and contribution. We are working hard to review all the requests as soon as possible. Currently, we are reviewing this PR.

The development of this PR is greatly appreciated, we will be merging it and it will be available in the next Wazuh release.

Thank you very much and if you have any questions or ideas, do not hesitate to contact us.

@okynos okynos merged commit c064588 into wazuh:4.3 Jul 27, 2022
@cruelsmith
Copy link
Contributor Author

Thanks @vcerenu,

let me know when you need help to merge this change to the other needed branches like master, stable or 4.4.

@vcerenu
Copy link
Member

vcerenu commented Jul 27, 2022

Hello @cruelsmith

Thanks for the help, we update the master, stable and 4.4 branches after each release, so it would not be necessary.

Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants