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.

Sign up for GitHub

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

Jump to bottom

Modified agent registration adding groups and arch #2677

Merged
merged 9 commits into from
Dec 29, 2020
Merged

Modified agent registration adding groups and arch #2677

merged 9 commits into from
Dec 29, 2020

Conversation

pablomarga
Copy link
Contributor

Hi team! This PR change the form of an agent registration adding the options of selecting the architecture of your OS and enrolling the agent to a group.
This PR close #2652 and close #2666

@pablomarga pablomarga marked this pull request as draft December 4, 2020 16:21
@pablomarga pablomarga marked this pull request as ready for review December 9, 2020 11:24
frankeros
frankeros approved these changes Dec 9, 2020
View reviewed changes
Copy link
Contributor

@frankeros frankeros left a comment

Choose a reason for hiding this comment

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

LGTM!

@snaow snaow added this to the Sprint 121 - WUI milestone Dec 9, 2020
gabiwassan
gabiwassan approved these changes Dec 9, 2020
View reviewed changes
Copy link
Contributor

@gabiwassan gabiwassan left a comment

Choose a reason for hiding this comment

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

LGTM

@gabiwassan gabiwassan self-requested a review December 9, 2020 18:51
gabiwassan
gabiwassan suggested changes Dec 9, 2020
View reviewed changes
Copy link
Contributor

@gabiwassan gabiwassan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi Pablo,
I did some tests and detected an error, here are the steps to reproduce it:

  1. Select Redhat / CentOS and CentOS6 or Higher.
  2. Select aarch64.
  3. We switch to CentOS5:

In the Copy Command of "Install and enroll the agent" it is left with the word "undefined" at the end, this is because it does not have a default architecture selection when changing the options.

image

Just some details but Great job!! 👍🏼

public/controllers/agent/components/register-agent.js Outdated Show resolved Hide resolved
Desvelao
Desvelao reviewed Dec 10, 2020
View reviewed changes
public/controllers/agent/components/register-agent.js Outdated
@@ -34,6 +37,46 @@ import {
} from '@elastic/eui';
import { WzRequest } from '../../../react-services/wz-request';


const architectureBottons = [
Copy link
Member

Choose a reason for hiding this comment

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

Typo: I guess you mean Buttons here

public/controllers/agent/components/register-agent.js Outdated
}
];

const versionBottonsCentos = [
Copy link
Member

Choose a reason for hiding this comment

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

Typo: I guess you mean Buttons here

@Desvelao
Copy link
Member

Desvelao commented Dec 10, 2020 •
edited
Loading

I was testing the PR, and found a pair of issues:

  1. The step which allows you to introduce the password is not showing:
    image
    How to reproduce this:
    Enable the use_password setting in the auth block of manager's ossec.conf. You can use the app to do this in Management > Configuration > Edit configuration or edit the file using the terminal with an editor. After that, you should restart the manager.
    image
    The input with the password should appear in the next cases:
  • Use password enabled but not defined: use_password to yes (manager's ossec.conf). File in manager path /var/ossec/etc/authd.pass didn't create.
  • Use password enabled and defined: use_password to yes (manager's ossec.conf). File in manager path /var/ossec/etc/authd.pass created with a password.

You can find more info here:
2. There is no separation between the definition of the variable WAZUH_PASSWORD and the rest of the command
image

@Desvelao
Copy link
Member

Desvelao commented Dec 10, 2020 •
edited
Loading

Hi Pablo,
I did some tests and detected an error, here are the steps to reproduce it:

  1. Select Redhat / CentOS and CentOS6 or Higher.
  2. Select aarch64.
  3. We switch to CentOS5:

In the Copy Command of "Install and enroll the agent" it is left with the word "undefined" at the end, this is because it does not have a default architecture selection when changing the options.

image

Just some details but Great job!! 👍🏼

Maybe we should sure all the options (OS, OS version, architecture) are selected to show the command? We are showing an url package when architecture is not selected for example.
image

Desvelao
Desvelao reviewed Dec 10, 2020
View reviewed changes
public/controllers/agent/components/register-agent.js Outdated
Comment on lines 191 to 193
setVersion(selectedVersion) {
this.setState({ selectedVersion });
}
Copy link
Member

@Desvelao Desvelao Dec 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Should we restart the selectedArchitecture variable (set to empty string '') when an OS version is selected (selectedVersion variable)?

@Desvelao
Copy link
Member

I tested this PR and if you don't select architecture for Debian or a version or architecture for Centos, the step to install the agent is missing and the start agent step is showing. Maybe we should show in this step some message saying some information is needed or something like that. Same for the step to start the agent.

Screenshot:
image

@Desvelao
feat(): Add message to show information about OS options are missing:
8483058 
  - Replaced OS buttons by button group
  - Show a message with OS, OS version or OS architecture are missing.
    Hide the command to install the agent when some of these options are
    missing.
  - The start agent step is showing when OS options are all selected
@Desvelao
fix: fix text in input to select group on the deploy agent section
fd0d124
@Desvelao
Copy link
Member

Desvelao commented Dec 18, 2020 •
edited
Loading

Changes:

  • Replaced OS options buttons by a buttons group:
    image

  • Added a message when some OS options is missing:
    image
    image
    image

  • Hid the start agent step when there are OS options missing

  • Modified some texts

frankeros
frankeros reviewed Dec 18, 2020
View reviewed changes
public/controllers/agent/components/register-agent.js
@@ -328,7 +336,7 @@ export class RegisterAgent extends Component {
const groupInput = (
<EuiText>
<p>
You can predefine the group where you want to enroll the agent.
Select one or more existing groups
Copy link
Contributor

Choose a reason for hiding this comment

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

current instad of existing (?)

@Desvelao Desvelao linked an issue Dec 18, 2020 that may be closed by this pull request
Agent deployment URLs don't work for some versions or architectures #2652
Closed
@frankeros frankeros merged commit f0e4594 into 4.0-7.9 Dec 29, 2020
@frankeros frankeros deleted the feature/add-option-register-agent branch December 29, 2020 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frankeros frankeros frankeros approved these changes

@Desvelao Desvelao Desvelao approved these changes

@gabiwassan gabiwassan gabiwassan approved these changes

Assignees

@pablomarga pablomarga

Labels
None yet
Projects
None yet
Milestone
Sprint 121 - WUI
Development

Successfully merging this pull request may close these issues.

Agent deployment URLs don't work for some versions or architectures
5 participants
@pablomarga @Desvelao @gabiwassan @frankeros @snaow