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

rabbitmq: fix unit file generation #1251

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

dpausp
Copy link
Member

@dpausp dpausp commented Jan 24, 2025

Fixes changes made in #1242

optionalString generates an empty string if the condition is not met. This resulted in an overides.conf for rabbitmq.service which just contains some auto-generated env vars in [service].

For the "frozen" rabbitmq 3.6 service/role, the new PATH replaced the PATH set by the original unit, leading to a "command not found" for rabbitmqctl in postStart.

Setting feature flags isn't possible for rabbitmq 3.6, the rabbitmqctl command must only run for current rabbitmq versions.

For other machines without rabbitmq, this also generated an empty unit file because there was no condition to check if rabbitmq is actually enabled on this machine.

We need to limit this to the current Rabbitmq role and use mkIf here to prevent setting the option at all when conditions are not met.

PL-133374

@flyingcircusio/release-managers

Release process

  • Created changelog entry using ./changelog.sh
    • not necessary

PR release workflow (internal)

  • PR has internal ticket
  • internal issue ID (PL-…) part of branch name
  • internal issue ID mentioned in PR description text
  • ticket is on Platform agile board
  • ticket state set to Pull request ready
  • if ticket is more urgent than within the next few days, directly contact a member of the Platform team

Design notes

  • Provide a feature toggle if the change might need to be adjusted/reverted quickly depending on context. Consider whether the default should be on or off. Example: rate limiting.
  • All customer-facing features and (NixOS) options need to be discoverable from documentation. Add or update relevant documentation such that hosted and guided customers can understand it as well.

Security implications

  • Security requirements defined? (WHERE)
  • Security requirements tested? (EVIDENCE)
    • checked on a test VM with the rabbitmq role that rabbitmq starts and no overrides.conf is present
    • checked on a test VM without any rabbitmq roles that no rabbitmq.service is present

optionalString generates an empty string if the condition
is not met. This resulted in an overides.conf for rabbitmq.service which just contains some auto-generated env vars in [service].

For the "frozen" rabbitmq 3.6 service/role, the new PATH replaced the PATH set by the original unit, leading to a "command not found" for rabbitmqctl in `postStart`.

Setting feature flags isn't possible for rabbitmq 3.6, the rabbitmqctl command must only
run for current rabbitmq versions.

For other machines *without* rabbitmq, this also generated an empty unit file because there was no condition to check if rabbitmq is actually enabled on this machine.

We need to limit this to the current Rabbitmq role and use mkIf here to prevent setting the option at all when
conditions are not met.

PL-133374
@osnyx osnyx merged commit 70fc001 into fc-24.05-dev Jan 24, 2025
1 of 2 checks passed
@osnyx osnyx deleted the PL-133374-rabbitmq-service-fix branch January 24, 2025 12:50
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

Successfully merging this pull request may close these issues.

2 participants