-
Notifications
You must be signed in to change notification settings - Fork 3
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
[CLOYSTER-57] Implement fail2ban #63
Conversation
Signed-off-by: Lucas Gracioso <[email protected]>
Signed-off-by: Lucas Gracioso <[email protected]>
Signed-off-by: Lucas Gracioso <[email protected]>
Signed-off-by: Lucas Gracioso <[email protected]>
[[maybe_unused]] virtual void configure() = 0; | ||
|
||
public: | ||
[[maybe_unused]] virtual void install() = 0; |
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.
If it's a virtual void function does it make sense to be [[maybe_unused]]
? I don't get it.
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.
As an example, the last PR approved that added ITool used install. This one doesn't..
@@ -23,7 +23,8 @@ class NFS : public IService { | |||
const boost::asio::ip::address& address, | |||
const std::string& permissions); | |||
|
|||
void configure(); | |||
void install() final {}; |
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.
It does nothing?
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.
But it should do.
@@ -123,6 +127,7 @@ class AnswerFile { | |||
|
|||
void loadFile(const std::filesystem::path& path); | |||
std::vector<std::shared_ptr<ITool>> getTools(); | |||
std::vector<std::shared_ptr<IService>> getServices(); |
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.
Same question regarding the std::shared_ptr
.
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.
I had the feeling we would use it later in different ways, at least when I imagined it and created. That's why I put shared.
For now it's a very simple implementation only to install and configure the tools in a easier way.
I think we can hold this PR a little bit, because to it work correctly we need to have a Firewall infrastructure on the code, and we don't have it. The Firewall will be probably |
The name says everything.