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

Authd replacement configurations QA #2171

Merged
merged 57 commits into from
Nov 5, 2021

Conversation

palaciosjeremias
Copy link
Contributor

@palaciosjeremias palaciosjeremias commented Nov 4, 2021

Related issue
#2050

Description

As part of the epic #2050, we created all the necessary changes to cope with the new feature developed in wazuh/wazuh#9550.
In this PR we:

  • Fixed legacy Wazuh-DB test cases that didn´t expect the new disconnection_time in gobal.db.
  • Fixed legacy Authd tests that expected a raw replacement of the agent and now were rejected.
  • Adapted all the force_insert and force_time configurations to the new force block .
  • Reformulated key_hash test to check the correct parse of the K option in the enrollment message.
  • Created a new test to validate the correct behavior of Authd with all the valid force options configuration (test_authd_force_options.py).
  • Created a new test to validate the error handling of Authd with invalid force options (test_authd_force_options_invalid_config.py).

Tests

  • Proven that tests pass when they have to pass.
  • Proven that tests fail when they have to fail.
  • Python codebase satisfies PEP-8 style style guide. pycodestyle --max-line-length=120 --show-source --show-pep8 file.py.
  • Python codebase is documented following the DocGenerator schema v2.0.
  • DocGenerator sanity check is executed in the new code without errors.

@palaciosjeremias palaciosjeremias force-pushed the dev-9550-authd-replacement-configurations-improvement branch from 21c139f to 56d473b Compare November 4, 2021 17:27
@MiguelazoDS
Copy link
Member

DocGenerator sanity without errors.

image

Comment on lines +1 to +2
# force options tests
These tests aim to check the correct behavior of Authd under the different possible configurations of the force block.
Copy link
Member

Choose a reason for hiding this comment

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

This file shouldn't be here

@juliamagan
Copy link
Member

juliamagan commented Nov 5, 2021

05/11/2021

Package

Version Revision Link
4.3.0 40301 https://packages-dev.wazuh.com/warehouse/pullrequests/4.3/rpm/var/wazuh-manager-4.3.0-0.commitf39f878.x86_64.rpm

Testing

tests/integration/test_authd

OS Local Jenkins Notes
PS1 🟢 🟢
PS2 🟢 🟢
PS3 🟢 🟢

  • 🟢: All pass
  • 🟡: Some warnings
  • 🔴: Some errors/fails
  • 🔵: In progress

Copy link
Member

@Rebits Rebits left a comment

Choose a reason for hiding this comment

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

GJ, some changes should be implemented in order to improve readability or fit with the repository standards. Due to the urgency of this development, these changes should be implemented in a separate Issue/PR in which we could solve other test_authd module errors.

Args:
input (dict): Dictionary with the content of the request command.
"""
command = ""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
command = ""
command = ''


if 'password' in input:
password = input['password']
command = command + f'OSSEC PASS: {password} '
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
command = command + f'OSSEC PASS: {password} '
command = command + f"OSSEC PASS: {password} "

return command


# Functions
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Functions

response (str): The Authd response to be validated.
expected (dict): Dictionary with the items to validate.
"""
response = response.split(sep=" ", maxsplit=1)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
response = response.split(sep=" ", maxsplit=1)
response = response.split(sep=' ', maxsplit=1)

Comment on lines +95 to +98
id = agent_key[0]
name = agent_key[1]
ip = agent_key[2]
key = agent_key[3]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id = agent_key[0]
name = agent_key[1]
ip = agent_key[2]
key = agent_key[3]
id, name, ip, key = agent_key[0:4]

'''
import os
import time
import pytest
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import pytest
import pytest

tests = []
test_case_ids = []
for file in os.listdir(tests_path):
group_name = file.split(".")[0]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
group_name = file.split(".")[0]
group_name = file.split('.')[0]

# Variables
log_monitor_paths = []

receiver_sockets_params = [(("localhost", 1515), 'AF_INET', 'SSL_TLSv1_2')]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
receiver_sockets_params = [(("localhost", 1515), 'AF_INET', 'SSL_TLSv1_2')]
receiver_sockets_params = [(('localhost', 1515), 'AF_INET', 'SSL_TLSv1_2')]

tests = []
test_case_ids = []
for file in os.listdir(tests_path):
group_name = file.split(".")[0]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
group_name = file.split(".")[0]
group_name = file.split('.')[0]


truncate_file(LOG_FILE_PATH)
try:
control_service("restart", daemon=DAEMON_NAME)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
control_service("restart", daemon=DAEMON_NAME)
control_service('restart', daemon=DAEMON_NAME)

@DProvinciani
Copy link
Contributor

All the suggestions made by the reviewers, as they are not critical, will be implemented as part of the issue #2184 after integrating this PR.

@snaow
Copy link
Contributor

snaow commented Nov 5, 2021

LGTM. GJ Everyone.

@snaow snaow merged commit f069346 into master Nov 5, 2021
@snaow snaow deleted the dev-9550-authd-replacement-configurations-improvement branch November 5, 2021 23:45
@snaow snaow mentioned this pull request Jan 27, 2022
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.

Implement Authd tests with the new force options
8 participants