-
Notifications
You must be signed in to change notification settings - Fork 7
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
Command Manager client prototype #274
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add Command Manager related settings Add ClusterInfoHelper utility class
Add CommandHandler to handle new endpoint
Add basic Command model Improve the specific HttpClients functions avoiding unnecesary parameters
AlexRuiz7
requested changes
Feb 14, 2025
...ins/content-manager/src/main/java/com/wazuh/contentmanager/model/commandmanager/Command.java
Outdated
Show resolved
Hide resolved
...nager/src/main/java/com/wazuh/contentmanager/client/commandmanager/CommandManagerClient.java
Outdated
Show resolved
Hide resolved
plugins/content-manager/src/main/java/com/wazuh/contentmanager/client/cti/CTIClient.java
Outdated
Show resolved
Hide resolved
Make HttpClient non-abstract Remove dependencies licenses validation
Add docstrings to new classes Remove unused functions
Add more docstrings Remove legacy HttpClient
Add required headers to CommandManager command generation endpoint
mcasas993
approved these changes
Feb 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
AlexRuiz7
approved these changes
Feb 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Code review: ✔️
- Test: ✔️
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements a refactor of the
HttpClient
making it a generic abstract class to handle HTTP requests which is inherited in the specificCommandManagerClient
, andCTIClient
classes to handle the connections to its corresponding APIs.The endpoints were proved to keep working as they were and how it should
Unit tests were added only for the base HttpClient class, this is because I've found difficult to mock the
ClusterService
object required by thePluginSetting
used in the rest of clients.Working evidence
Get catalog endpoint
Get changes endpoint
Command Manager request is correctly sent and captured by imposter
Request sent captured by ContentManager logs
Request received by imposter
Note
To receive the request in imposter we have to modify the setting that defines the Command Manager URL
settings/PluginSettings.java: Line 108
to return the the hardcoded valuehttp://127.0.0.1:8080
, this is because the API URL is obtained using the Cluster service.Issues Resolved
Closes #239