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

Proposal for long-term changes #76

Open
avitacco opened this issue Jan 24, 2025 · 1 comment
Open

Proposal for long-term changes #76

avitacco opened this issue Jan 24, 2025 · 1 comment

Comments

@avitacco
Copy link
Contributor

Proposal for long-term changes

There are a number of changes I would like to make that would, in my opinion,
greatly improve the quality, maintainability, and usability of this module.
Some of them are small things, but others are large and would break
compatibility with older versions of the module. I have outlined them below and
am open to input from the maintainers of this module and the greater community.

Refactoring init.pp

The fact that this class does nothing but tell you to use another class does not
make a lot of sense, currently. Historically, this module managed the frontend
and backend components of Graylog. This made a whole lot of sense when those
were actually separate components. When they were merged into one monolithic
service that paradigm made a lot less sense. This change to Graylog made a lot
of sense, but the result for this module was that nearly all of the actual
functionality is accessed through graylog::server.

Proposal

Consolidate the module's main functionality into init.pp. This will enable
users to rely on the base class rather than needing the server-specific class
and reflects the current Graylog architecture.

Class chaining

This whole module requires class chaining to be used in a specific order. This
made a lot of sense with earlier versions of puppet. However, it is pretty
awkward and not a recommended way of developing modules currently.

Proposal

Include or require sub-classes through init.pp. This change will allow users
not to have to worry about the dependency chain of classes. It also completely
eliminates the need for anchor resources, the use of which is recommended
against.

Convert graylog.conf.erb to .epp

Currently, this module uses both .erb and .epp template formats, which adds
unnecessary complexity to development.

Proposal

To maintain consistency and simply template management, I propose converting
graylog.conf.erb to an .epp template.

Apt repository simplification

The graylog::repository::apt class is overly complex due to past limitations
in the puppetlabs/apt module. However, over the last years updates to the
apt module now allow for a more streamlined approach.

Proposal

Leveraging the improved functionality of the apt module to manage the PGP key,
the repository, and the proxy configuration. This will greatly simplify the
code, improve maintainability, and reduce technical debt.

Changes to the java environment variables

Currently, this module relies on outdated OS-specific methods to pass
environment variables to the Graylog service which can be inconsistent across
different distributions.

Proposal

Use systemd overrides to manage environment variables fro the Graylog service.
This approach is simpler, more modern, and compatible with all supported Linux
distributions.

Remove allinone class

The graylog::allinone class does not offer any great functionality, it simply
takes configuration for other modules and passes it to the other modules. The
dependencies for this class are not included in the metadata.json file, and
this class does not currently work on RedhHat-based distributions with newer
puppet versions due to the use of legacy facts in the dependent opensearch
module.

Proposal

Remove this class and replace it with an example. This reduces the dependencies
for this module and simplifies the code base while still providing documentation
for creating an all-in-one system.

Adding examples in examples directory

This module lacks an examples directory with examples for using this module.

Proposal

Create some simple examples for using this module to manage a Graylog server.
Doing so will bring this module closer to being able to be submitted for
"Approved" status with Puppet.

@bernd
Copy link
Member

bernd commented Jan 28, 2025

@avitacco Thank you for the detailed description! Your proposed improvements all sound great to me. Please feel free to submit PRs for them.

Since we have to bump the major version of the module due to #73, it's perfectly fine to do breaking changes and document them.

avitacco added a commit to avitacco/puppet-graylog that referenced this issue Jan 29, 2025
As discussed in issue Graylog2#76, I am removing the allinone class and the
associated unit test.
@avitacco avitacco mentioned this issue Jan 29, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants