Fixes #16915: Firewall technique #1596
Conversation
|
PR updated with a new commit |
| "conf_pre" string => "flush ruleset${const.n}"; | ||
| } | ||
|
|
||
| bundle agent rudder_condition_from_string_compare(condition_prefix, string1, string2) { |
There was a problem hiding this comment.
you need the &RudderUniqueID& don't you?
There was a problem hiding this comment.
Do we also use it in mono-instance techniques?
There was a problem hiding this comment.
if it's monoinstance, its not necessary, so you can remove it from everywhere in the technique
There was a problem hiding this comment.
could you move this in a common part of code ? idealy ncf?
There was a problem hiding this comment.
These two bundle should be generic methods but I had enough of CFEngine
There was a problem hiding this comment.
you should use a different naming there, more specific, like with the technique name, else we'll risk duplicate bundle names
There was a problem hiding this comment.
I added rudder_ before the name of what the generic method would be, what do you suggest to add?
There was a problem hiding this comment.
the name of the technique, so that we have a "perfect" scoping
we have many bundles starting with rudder_ everywhere, so we might accidentaly create another with this name
| "${condition_prefix}_false" not => "${condition_prefix}_true", scope => "namespace"; | ||
| } | ||
|
|
||
| bundle agent rudder_variable_string_canonify(prefix, name, string) { |
There was a problem hiding this comment.
could you move this in a common part of code ? idealy ncf?
There was a problem hiding this comment.
opened https://issues.rudder.io/issues/17513 and https://issues.rudder.io/issues/17512, the technique will be able to use them once added without compatibility problem.
There was a problem hiding this comment.
ok, so you should use a different naming there, more specific, like with the technique name, else we'll risk duplicate bundle names
| } | ||
|
|
||
| # Uses the systemd service, compatible with recent debian, rhel and derivatives | ||
| bundle common rudder_nftables { |
There was a problem hiding this comment.
this could go in another file that would be a resource, so unique
There was a problem hiding this comment.
could you also have a more specific name, as long as it's in the technique?
|
PR updated with a new commit |
| @@ -0,0 +1,2 @@ | |||
| -- Alexis Mousset <alexis.mousset@rudder.io> Mon, 26 Sep 2020 17:19:00 +0100 | |||
There was a problem hiding this comment.
i didn't realized it was so old
There was a problem hiding this comment.
I think I just changed the year because I was not sure of the correct date format... I can this.
There was a problem hiding this comment.
Now this is in the future. I'm amazed that we can access code from september.
|
PR updated with a new commit |
|
PR updated with a new commit |
| "conf_pre" string => "flush ruleset${const.n}"; | ||
| } | ||
|
|
||
| bundle agent rudder_condition_from_string_compare(condition_prefix, string1, string2) { |
There was a problem hiding this comment.
you should use a different naming there, more specific, like with the technique name, else we'll risk duplicate bundle names
| } | ||
|
|
||
| # Uses the systemd service, compatible with recent debian, rhel and derivatives | ||
| bundle common rudder_nftables { |
There was a problem hiding this comment.
could you also have a more specific name, as long as it's in the technique?
|
PR updated with a new commit |
|
This PR is not mergeable to upper versions. |
|
OK, squash merging this PR |
amousset
force-pushed
the
bug_16915/firewall_technique
branch
from
e1c3f51
to
cb6fc6b
Compare
https://issues.rudder.io/issues/16915