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.

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

reaction: init at 1.3.0 #272264

Merged
merged 1 commit into from
Feb 12, 2024
Merged

reaction: init at 1.3.0 #272264

merged 1 commit into from
Feb 12, 2024

Conversation

ppom0
Copy link
Contributor

@ppom0 ppom0 commented Dec 5, 2023

Description of changes

reaction is a daemon that scans program outputs for repeated patterns, and takes action.
A common usage is to scan ssh and webserver logs, and to ban hosts that cause multiple authentication errors.

It is an alternative to fail2ban with a cleaner and lighter configuration, along with overall better performance.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ppom0
Copy link
Contributor Author

ppom0 commented Jan 4, 2024

Just updated from 1.0.2 to 1.0.3

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3207

@ppom0 ppom0 changed the title reaction: init at 1.0.2 reaction: init at 1.0.3 Jan 4, 2024
@onemoresuza
Copy link
Contributor

onemoresuza commented Jan 4, 2024

Reviewed points

Result of nixpkgs-review pr 272264 run on x86_64-linux 1

1 package built:
  • reaction
  • package path fits guidelines
  • package name fits guidelines
  • package version fits guidelines
  • package builds on x86_64-linux
  • executables tested on x86_64-linux
  • meta.description is set and fits guidelines
  • meta.license fits upstream license
  • meta.platforms is set
  • meta.maintainers is set
  • build time only dependencies are declared in nativeBuildInputs
  • source is fetched using the appropriate function
  • the list of phases is not overridden
  • when a phase (like installPhase) is overridden it starts with runHook preInstall and ends with runHook postInstall.
    • N/A: No phase is overriden
  • patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed
    • N/A: No patch is applied
  • patches that are remotely available are fetched rather than vendored
    • N/A: No patch is applied

Possible improvements

agpl3 is deprecated: It should be changed

It should be changed to either agpl3Only or agpl3Plus. As mentioned above, the project seems to use the latter.

ip46tables PATH availability: Possible usage for wrapProgram (OPTIONAL)

Usually when packages depend on other packages being available on PATH a wrapper function is used. For instance, the postInstall hook could be turned into the following, after adding the makeWrapper pattern to nativeBuildInputs:

postInstall = ''
  wrapProgram $out/bin/reaction \
    --prefix PATH : "${lib.makeBinPath [ip46tables]}"
'';

@ppom0
Copy link
Contributor Author

ppom0 commented Jan 7, 2024

@onemoresuza Thanks for your review. I just updated the license.

Concerning adding ip46tables to reaction's PATH, I carefully read your proposal, thanks. I prefer managing this on the reaction module (I'll make the PR when I'll have this merged), even if the overhead is minimal, I prefer not wrapping the reaction binary.

@ppom0 ppom0 changed the title reaction: init at 1.0.3 reaction: init at 1.1.2 Jan 12, 2024
@ppom0
Copy link
Contributor Author

ppom0 commented Jan 17, 2024

Just added ldflags so that reaction is aware of its version.

Copy link
Member

@pbsds pbsds left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 272264 run on x86_64-linux 1

1 package built:
  • reaction

pkgs/by-name/re/reaction/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/re/reaction/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/re/reaction/package.nix Outdated Show resolved Hide resolved
@pbsds
Copy link
Member

pbsds commented Feb 10, 2024

You may also have to mark meta.broken = stdenv.isDarwin; or set meta.platforms = lib.platforms.linux;

@ppom0
Copy link
Contributor Author

ppom0 commented Feb 11, 2024

@pbsds I did not test reaction on darwin but it does build, so I put platforms = unix.

@ppom0 ppom0 changed the title reaction: init at 1.1.2 reaction: init at 1.3.0 Feb 11, 2024
@RaitoBezarius RaitoBezarius merged commit dd864dc into NixOS:master Feb 12, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants