feat: add SUSE/openSUSE support#172
Conversation
Reviewer's GuideAdds SUSE/openSUSE support to the PostgreSQL system role by introducing SUSE-specific vars, handling absence of postgresql-setup via direct initdb calls, generalizing certificate paths/permissions, and updating tests and metadata accordingly. Flow diagram for database initialization task selectiongraph TD
Start[Start: PostgreSQL role runs] --> CheckConf[Check if postgresql.conf exists]
CheckConf -->|exists| SkipInit[Skip initdb tasks]
CheckConf -->|missing| CheckBooted[Check __postgresql_is_booted]
CheckBooted -->|booted true| CheckSetupBooted[Check __postgresql_has_setup_cmd]
CheckBooted -->|booted false| CheckSetupNonBooted[Check __postgresql_has_setup_cmd]
CheckSetupBooted -->|true| BootedWithSetup[Task: Init DB on booted systems with postgresql-setup]
CheckSetupBooted -->|false| BootedWithoutSetup[Task: Init DB on booted systems without postgresql-setup using initdb]
CheckSetupNonBooted -->|true| NonBootedWithSetup[Task: Init DB on non-booted systems with patched postgresql-setup]
CheckSetupNonBooted -->|false| NonBootedWithoutSetup[Task: Init DB on non-booted systems without postgresql-setup using initdb]
BootedWithSetup --> End[postgresql.conf created]
BootedWithoutSetup --> End
NonBootedWithSetup --> End
NonBootedWithoutSetup --> End
SkipInit --> End
Flow diagram for certificate installation and permissionsgraph TD
Start[Start: Certificate tasks] --> DefaultDir[Set __postgresql_cert_directory]
DefaultDir --> LinkCert[Create link for server certificate]
LinkCert --> EnsurePrivDir[Ensure private key directory is traversable]
EnsurePrivDir --> CheckNameType[Check if certificate name is absolute]
CheckNameType -->|not abs| EnsureKeyPerms[Ensure private key owned by postgres and mode 0600]
CheckNameType -->|abs| SkipKeyPerms[Skip ownership and mode adjustment]
EnsureKeyPerms --> LinkKey[Create link for server key from cert_directory private]
SkipKeyPerms --> LinkKey
LinkKey --> End[Certificate and key linked with correct permissions]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new initdb tasks for booted/non-booted systems with and without postgresql-setup are nearly identical; consider consolidating them (e.g. via a single task with conditional command or a small include) to reduce duplication and keep the init logic in one place.
- In tests_custom_certificate.yml, the certificate name path is defined as
"{{ __postgresql_cert_directory }}//certs/postgresql_test"(double slash); this is likely unintended and should be normalized to a single/for consistency with other usages. - For the
initdbcommand invocations, consider quoting{{ __postgresql_data_dir }}in thecmdargument to avoid potential issues if the data directory path ever contains spaces or shell-special characters.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new initdb tasks for booted/non-booted systems with and without postgresql-setup are nearly identical; consider consolidating them (e.g. via a single task with conditional command or a small include) to reduce duplication and keep the init logic in one place.
- In tests_custom_certificate.yml, the certificate name path is defined as `"{{ __postgresql_cert_directory }}//certs/postgresql_test"` (double slash); this is likely unintended and should be normalized to a single `/` for consistency with other usages.
- For the `initdb` command invocations, consider quoting `{{ __postgresql_data_dir }}` in the `cmd` argument to avoid potential issues if the data directory path ever contains spaces or shell-special characters.
## Individual Comments
### Comment 1
<location path="tasks/certificate.yml" line_range="24-27" />
<code_context>
owner: postgres
when: (postgresql_certificates | first)['name'] is not abs
+ - name: Ensure private key directory is traversable
+ file:
+ path: "{{ __postgresql_cert_directory }}/private"
+ mode: "o+x"
+
+ - name: Ensure private key is readable by postgres
</code_context>
<issue_to_address>
**🚨 issue (security):** Relaxing directory permissions with `mode: "o+x"` may be overly permissive for a private key directory.
This makes the key directory traversable by every local user, which is broader access than is appropriate for private key material. If you only need `postgres` to traverse it, consider restricting to owner/group (e.g. `u+rx,g+rx,o-x`) and ensuring `postgres` is in the right group, or use ACLs to grant access specifically to `postgres` instead of `others`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - name: ansible.posix | ||
| - name: fedora.linux_system_roles | ||
| - name: community.general | ||
| version: '>=6.6.0,<12.0.0' |
There was a problem hiding this comment.
| vars: | ||
| postgresql_password: redhat | ||
| block: | ||
| - name: Load postgresql role platform variables |
There was a problem hiding this comment.
The best way to get vars from the role into the test is to use include_role with tasks_from: set_vars.yml:
- name: Load postgresql role platform variables
include_role:
name: linux-system-roles.postgresql
tasks_from: set_vars.yml
public: trueThis will also get __postgresql_is_ostree and __postgresql_is_booted
Co-authored-by: Sergei Petrosian <30409084+spetrosi@users.noreply.github.com>
|
[citest] |
tasks/main.yml
Outdated
| state: present | ||
| use: "{{ (__postgresql_is_ostree | d(false)) | | ||
| ternary('ansible.posix.rhel_rpm_ostree', omit) }}" | ||
| ternary('ansible.posix.rhel_rpm_ostree', omit) }}" |
There was a problem hiding this comment.
please preserve the original formatting
tasks/main.yml
Outdated
| (__postgresql_is_ostree | d(false)) | | ||
| ternary(__postgresql_packages | reject('match', '^@'), | ||
| __postgresql_packages) | list }}" | ||
| __postgresql_packages) | list }}" |
There was a problem hiding this comment.
please preserve the original formatting
|
@HVSharma12 the leap test is failing? |
|
yes it requires the certificate role fixes |
Done. The certificate role with your fixes is now published to Galaxy. |
|
@HVSharma12 lgtm - ready to merge? |
Enhancement: add support for SUSE/openSUSE
Reason: The role fails with "command not found" when trying to initialize the database on SUSE.
Result: Role now works for SLES 15.6+/16 and leap 15.6+/16
Issue Tracker Tickets (Jira or BZ if any): na
Summary by Sourcery
Add SUSE and openSUSE compatibility to the PostgreSQL system role, including database initialization and SSL certificate handling adjustments.
New Features:
Enhancements:
Tests: