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

Let wcom execute check-manager-config query. #8357

Merged
merged 3 commits into from
May 12, 2021

Conversation

antoniomanuelfr
Copy link
Contributor

@antoniomanuelfr antoniomanuelfr commented Apr 26, 2021

Related issue
#8356

Description

This PR aims to avoid execd to execute check-manager-config and restart-manager API requests. Now, these requests that were executed by execd will be executed by the wcom thread. This will avoid the problems described in #7968 once this development is completed.

If the check-manager-config request is sent to execd after the change, the following error will appear:

2021/04/26 06:57:29 wazuh-execd: ERROR: (1311): Invalid command name 'check-manager-configuration' provided.

Closes #8356

Test active response disabled

root@ubuntumanager:/vagrant/Scripts# python3 test_execq.py e
Traceback (most recent call last):
  File "test_execq.py", line 135, in <module>
    response = test_exec()
  File "test_execq.py", line 89, in test_exec
    raise e
  File "test_execq.py", line 86, in test_exec
    datagram = api_socket.recv(4096)
socket.timeout: timed out
root@ubuntumanager:/vagrant/Scripts# python3 test_execq.py w
/var/ossec/queue/sockets/com
b'{"error":0,"message":"ok"}'
root@ubuntumanager:/vagrant/Scripts# 

Test active response enabled

root@ubuntumanager:/vagrant/Scripts# python3 test_execq.py e
Traceback (most recent call last):
  File "test_execq.py", line 135, in <module>
    response = test_exec()
  File "test_execq.py", line 89, in test_exec
    raise e
  File "test_execq.py", line 86, in test_exec
    datagram = api_socket.recv(4096)
socket.timeout: timed out
root@ubuntumanager:/vagrant/Scripts# python3 test_execq.py w
/var/ossec/queue/sockets/com
b'{"error":0,"message":"ok"}'
root@ubuntumanager:/vagrant/Scripts# 

The exception that is raised is because the API was expecting to connect to the alerts/execa sockets, which is no longer used and it won't be created.

Valgrind report

==29915== LEAK SUMMARY:
==29915==    definitely lost: 0 bytes in 0 blocks
==29915==    indirectly lost: 0 bytes in 0 blocks
==29915==      possibly lost: 272 bytes in 1 blocks
==29915==    still reachable: 144 bytes in 1 blocks
==29915==         suppressed: 0 bytes in 0 blocks
  • Compilation without warnings in every supported platform
    • Linux
  • Source installation
  • Source upgrade
  • Memory tests for Linux
    • Scan-build report
    • Coverity
    • Valgrind (memcheck and descriptor leaks check)

@antoniomanuelfr antoniomanuelfr self-assigned this Apr 26, 2021
@antoniomanuelfr antoniomanuelfr marked this pull request as ready for review April 26, 2021 08:24
@Molter73 Molter73 self-requested a review April 26, 2021 16:29
src/os_execd/wcom.c Outdated Show resolved Hide resolved
src/os_execd/wcom.c Outdated Show resolved Hide resolved
@antoniomanuelfr antoniomanuelfr force-pushed the 8356-avoid-execd-processing-api-request branch from 77823c6 to 3a83c19 Compare April 28, 2021 11:28
Molter73
Molter73 previously approved these changes Apr 28, 2021
Copy link
Contributor

@Molter73 Molter73 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Molter73 Molter73 force-pushed the 8356-avoid-execd-processing-api-request branch from 3a83c19 to 1c8c30e Compare April 29, 2021 18:35
Molter73
Molter73 previously approved these changes Apr 30, 2021
@antoniomanuelfr antoniomanuelfr force-pushed the 8356-avoid-execd-processing-api-request branch 2 times, most recently from 3e44045 to d47a8ff Compare May 4, 2021 08:04
@antoniomanuelfr antoniomanuelfr force-pushed the 8356-avoid-execd-processing-api-request branch from d47a8ff to d3e1044 Compare May 4, 2021 13:48
Antonio Fresneda added 2 commits May 10, 2021 10:01
- This commit also enhances the code of wcom_check_manager_config.
@antoniomanuelfr antoniomanuelfr force-pushed the 8356-avoid-execd-processing-api-request branch from d3e1044 to 127f8a7 Compare May 10, 2021 08:14
@antoniomanuelfr antoniomanuelfr changed the base branch from master to 4.2 May 10, 2021 08:14
…8535)

* Rework manager validation to use wcom instead of execq and execa

* Update core manager unit tests and improve coverage

* Update SDK manager unit tests
@vikman90 vikman90 merged commit ac6515c into 4.2 May 12, 2021
@vikman90 vikman90 deleted the 8356-avoid-execd-processing-api-request branch May 12, 2021 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid execd to execute restart_manager and check_manager_config queries
4 participants