ungleich-public/cdist
ungleich-public/cdist
7
4
Fork 10
Code Issues 58 Pull requests 15 Projects Releases Wiki Activity

WIP: __fail2ban new type #379

Draft
smwltr wants to merge 1 commit from smwltr:fail2ban into master
pull from: smwltr:fail2ban
merge into: ungleich-public:master
Conversation 2 Commits 1 Files changed 3 +76
smwltr commented 2024-05-09 12:17:40 +00:00
First-time contributor
Copy link

Does this need anything other than a man page?

Does this need anything other than a man page?
smwltr added 1 commit 2024-05-09 12:17:41 +00:00
smwltr changed title from WIP: __fail2ban to WIP: __fail2ban new type 2024-05-09 12:37:42 +00:00
fnux requested changes 2024-05-13 05:44:47 +00:00
cdist/conf/type/__fail2ban/gencode-remote
@ -0,0 +30,4 @@
do
echo "[$(tput setaf 6)info$(tput sgr 0)] Enabling fail2ban for $service..." >&2
cat << EOF
perl -i -pe 'BEGIN{undef $/;} s/\[$service\].*[\n]*enabled.*=.*\n/\[$service\]\n\nenabled = true\n/g' $config_file
fnux commented 2024-05-13 05:38:06 +00:00
Collaborator
Copy link

I see two problems here:

  • The configuration should be generated and installed in the manifest, not in gencode-remote.
  • You should not use perl in core cdist types - we only assume POSIX sh on the remote hosts.
I see two problems here: * The configuration should be generated and installed in the manifest, not in gencode-remote. * You should not use `perl` in core cdist types - we only assume POSIX sh on the remote hosts.
cdist/conf/type/__fail2ban/manifest
@ -0,0 +31,4 @@
require=__package/epel-release __package fail2ban --state present
;;
*)
echo "Your operating system ($os) is currently untested for ${__type##*/}." >&2
fnux commented 2024-05-13 05:40:51 +00:00
Collaborator
Copy link

Just say that the type does not support $os and exit.

Just say that the type does not support `$os` and exit.
smwltr commented 2024-05-19 10:21:25 +00:00
Author
First-time contributor
Copy link

I'll need to make a testing set up for this. It's most likely that other OSes can use the same code, but I just haven't tested it.

I'll need to make a testing set up for this. It's most likely that other OSes can use the same code, but I just haven't tested it.
fnux commented 2024-05-21 12:44:04 +00:00
Collaborator
Copy link

Well, we only ship code that is known to be working :-)

Well, we only ship code that is known to be working :-)
cdist/conf/type/__fail2ban/manifest
@ -0,0 +36,4 @@
__package fail2ban --state present
;;
esac
fnux commented 2024-05-13 05:42:27 +00:00
Collaborator
Copy link

You also have to make sure that the fail2ban service starts on boot.

You also have to make sure that the fail2ban service starts on boot.
smwltr commented 2024-05-19 10:28:18 +00:00
Author
First-time contributor
Copy link

Is that typically what we do?

Or do we expect the user to use __start_on_boot?

Maybe I should rather fail when there is no fail2ban package installed. Then this type would be only for configuring the services that you want fail2ban to monitor (jails).

I could make an opiton and default to start it. Or I could add __start_on_boot and __package as an example in the man page?

What fits more with the cdist design?

Is that typically what we do? Or do we expect the user to use `__start_on_boot`? Maybe I should rather fail when there is no fail2ban package installed. Then this type would be only for configuring the services that you want fail2ban to monitor (jails). I could make an opiton and default to start it. Or I could add `__start_on_boot` and `__package` as an example in the man page? What fits more with the cdist design?
fnux commented 2024-05-21 12:45:26 +00:00
Collaborator
Copy link

We usually install and enable (i.e. start_on_boot) the service within the type. Please run both __package and __start_on_boot in your type.

We usually install and enable (i.e. start_on_boot) the service within the type. Please run both `__package` and `__start_on_boot` in your type.
👍 1
smwltr commented 2024-05-29 13:41:16 +00:00
Author
First-time contributor
Copy link

IMO, forcing something other than the default is not my style, but if that is standard cdist style, so be it.

IMO, forcing something other than the default is not my style, but if that is standard cdist style, so be it.
cdist/conf/type/__fail2ban/parameter/optional
@ -0,0 +1 @@
enable-services
fnux commented 2024-05-13 05:44:43 +00:00
Collaborator
Copy link

I find this parameter confusing - could you rename it? e.g. jail-service ?

I find this parameter confusing - could you rename it? e.g. `jail-service` ?
smwltr commented 2024-05-19 10:30:22 +00:00
Author
First-time contributor
Copy link

Would it be enable-jail? I suppose we need disable-jail too. Or?

Would it be `enable-jail`? I suppose we need `disable-jail` too. Or?
fnux commented 2024-05-21 12:49:36 +00:00
Collaborator
Copy link

The type could generate configuration for the services specified in --jail-service and remove anything that isn't provided. You could make --jail-service an optional_multiple parameter.

Also mind that if we need to add per-service/jail configuration, a second __fail2ban_jail service will be needed (so that every jailed service can have its own configuration).

The type could generate configuration for the services specified in `--jail-service` and remove anything that isn't provided. You could make `--jail-service` an optional_multiple parameter. Also mind that if we need to add per-service/jail configuration, a second __fail2ban_jail service will be needed (so that every jailed service can have its own configuration).
smwltr commented 2024-05-29 13:40:53 +00:00
Author
First-time contributor
Copy link

IIUC, that would mean that cdist team be responsible for shipping working tested config for fail2ban.

Do you think that is out of scope? Have you seen how many jails are in the default config?

That's the reason why all this type does is allow you to enable a jail that is already in the config.

Before I remove the perl code, I'd like to understand this more. If we are just dis/enabling jails from the config, I can use sed. But generating a config for fail2ban requires much more code and maintenance. It doesn't make sense to me.

WDYT?

IIUC, that would mean that cdist team be responsible for shipping working tested config for fail2ban. Do you think that is out of scope? Have you seen how many jails are in the default config? That's the reason why all this type does is allow you to enable a jail that is already in the config. Before I remove the perl code, I'd like to understand this more. If we are just dis/enabling jails from the config, I can use sed. But generating a config for fail2ban requires much more code and maintenance. It doesn't make sense to me. WDYT?
fnux commented 2024-05-29 14:09:25 +00:00
Collaborator
Copy link

IT would be a generic type to deploy a file in /etc/fail2ban/jail.d/myjail.conf (on debian at least). It doesn't have to support every configuration parameter, but should allow for custom configuration.

Its simplest form could looks like:

 __fail2ban_jail myjail --config <<- EOF
[apache-overflows]
enabled  = true
port     = http,https
filter   = apache-overflows
logpath  = /var/log/apache*/*error.log
maxretry = 2
EOF

I would also run fail2ban-client --test or similar in gencode-remote before reloading the configuration.

IT would be a generic type to deploy a file in /etc/fail2ban/jail.d/myjail.conf (on debian at least). It doesn't have to support every configuration parameter, but should allow for custom configuration. Its simplest form could looks like: ``` __fail2ban_jail myjail --config <<- EOF [apache-overflows] enabled = true port = http,https filter = apache-overflows logpath = /var/log/apache*/*error.log maxretry = 2 EOF ``` I would also run ` fail2ban-client --test` or similar in gencode-remote before reloading the configuration.
smwltr commented 2024-05-29 13:54:31 +00:00
Author
First-time contributor
Copy link

@fnux, sorry for the n00b question: how do I update a PR from my fork? I've pushed some changes, but I don't see them here.

@fnux, sorry for the n00b question: how do I update a PR from my fork? I've pushed some changes, but I don't see them here.
fnux commented 2024-05-29 14:03:34 +00:00
Collaborator
Copy link

@fnux, sorry for the n00b question: how do I update a PR from my fork? I've pushed some changes, but I don't see them here.

There's currently an issue with this gitea instance. I asked ungleich to investigate it.

> @fnux, sorry for the n00b question: how do I update a PR from my fork? I've pushed some changes, but I don't see them here. There's currently an issue with this gitea instance. I asked ungleich to investigate it.
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u fail2ban:smwltr-fail2ban
git checkout smwltr-fail2ban

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout master
git merge --no-ff smwltr-fail2ban
git checkout smwltr-fail2ban
git rebase master
git checkout master
git merge --ff-only smwltr-fail2ban
git checkout smwltr-fail2ban
git rebase master
git checkout master
git merge --no-ff smwltr-fail2ban
git checkout master
git merge --squash smwltr-fail2ban
git checkout master
git merge --ff-only smwltr-fail2ban
git checkout master
git merge smwltr-fail2ban
git push origin master
Sign in to join this conversation.
Reviewers
No reviewers
fnux
Labels
Clear labels   
bugfix

   
cleanup

   
discussion

   
documentation

   
doing

   
done

   
feature

   
improvement

   
packaging

   
Stale

   
testing

   
TODO
No labels
bugfix
cleanup
discussion
documentation
doing
done
feature
improvement
packaging
Stale
testing
TODO
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: ungleich-public/cdist#379
No description provided.
Delete branch "smwltr:fail2ban"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?