feat: Support SQL Server 2025 on RHEL 9 and RHEL 10 - #428
Conversation
Feature: Support SQL Server 2025 on RHEL 9 and RHEL 10. Reason: Microsoft release SQL Server 2025 for RHEL 9 and 10 in February 2026, the role should support this. Result: You can install SQL Server 2025 on RHEL 9, and newly on RHEL 10.
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (42)
📝 WalkthroughWalkthroughThe role adds SQL Server 2025 and EL 10 support. It updates repository keys, platform mappings, SELinux handling, documentation, integration tests, HA tests, and cleanup tagging. ChangesPlatform and version support
SQL Server 2025 validation
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| - name: Determine the SQL Server versions supported on this platform | ||
| include_role: | ||
| name: linux-system-roles.mssql | ||
| tasks_from: set_vars |
There was a problem hiding this comment.
does this work without the .yml suffix?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/tests_include_vars_from_parent.yml (1)
121-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTag every cleanup task with
tests::cleanup.Line [131] uses
cleanupinstead oftests::cleanup. The localCleanuptask at Lines [121-125] has no cleanup tag. Tag both tasks so debug runs can skip the complete cleanup sequence.As per path instructions, cleanup tasks must be tagged with
tests::cleanupso cleanup can be skipped for debug purposes.Also applies to: 129-131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/tests_include_vars_from_parent.yml` around lines 121 - 125, Tag the local Cleanup task and the cleanup task around the referenced loop with tests::cleanup, replacing the existing cleanup tag where present. Ensure both cleanup tasks use the exact tests::cleanup tag so debug runs can skip the entire cleanup sequence.Source: Path instructions
🧹 Nitpick comments (1)
defaults/main.yml (1)
39-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a usage example for
mssql_rpm_key_2025.
README.mddocuments the variable and its default value, but it does not show how to override it. Add the variable to a relevant example playbook.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@defaults/main.yml` around lines 39 - 40, Add mssql_rpm_key_2025 to a relevant example playbook in README.md, showing how users can override its default URL. Keep the existing variable documentation and default value unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mssql-server-selinux-rhel10-bug-report.md`:
- Around line 79-92: Narrow the finding in the section describing mssql_server_t
access to only the demonstrated dir:write permission on mssql_var_t and
mssql_opt_t. Remove unsupported claims about add_name, create, file, or
mssql_db_t permissions, or add corresponding sesearch queries for any such
claims before stating them.
- Around line 137-152: Update the Reproduction section to establish a clean
first boot by removing or resetting any existing /var/opt/mssql state before
installation and setup. Add an explicit sqlservr service start command and
capture its failure, ensuring the documented sequence supports the first-start
failure claim; otherwise remove that claim.
- Around line 113-123: Revise the root-cause and package-fix sections to
distinguish fcontext lookup from creation-time labeling: do not treat
matchpathcon returning usr_t as proof that newly created children receive usr_t.
Document the required clean RHEL 10 reproduction data—creator domain, parent and
child labels, and AVCs—before assigning the creation result. Accurately describe
the role’s scope: it adds fcontexts and restore requests only for configured
mssql_datadir and mssql_logdir when both SELinux variables are enabled, and does
not cover .system.
- Around line 185-191: Update the Impact section to retain
linux-system-roles.mssql and document that mssql-conf setup runs before SELinux
contexts are applied, with null default mssql_datadir and mssql_logdir values
and no role context for /var/opt/mssql/.system. Add a RHEL 10 role test that
captures setup results, post-setup SELinux labels, and AVCs.
- Around line 154-170: Update the “Workaround” section to distinguish fcontext
configuration from audit2allow policy generation: state that audit2allow -M only
creates AVC-derived allow rules, while intended mappings for /var/opt/mssql/data
and .system must be defined separately, including creation-transition rules
where runtime-created paths need them. Clarify that semanage permissive -a
mssql_server_t is diagnostic-only and document its removal with semanage
permissive -d mssql_server_t; retain the requirement to review generated rules
before loading.
In `@README.md`:
- Around line 204-211: Update the confined SELinux documentation text to use “EL
9 and later” consistently in the heading and applicability/default statements,
while retaining the restriction to SQL Server versions 2022 and 2025.
- Around line 425-426: Align the README description of mssql_rpm_key_2025 with
the predicate used by tasks/main.yml by documenting it as applicable only when
mssql_version is 2025, unless the task condition is intentionally broadened to
>= 2025. Add a playbook usage example that sets mssql_rpm_key_2025, following
the README’s existing variable examples.
In `@tasks/main.yml`:
- Around line 243-244: Correct the spelling of “isntall” to “install” in the
comment near the mssql_rpm_key requirement, without changing the surrounding
wording or behavior.
In `@tests/tasks/upgrade_and_assert.yml`:
- Line 31: Replace the cleanup tag with tests::cleanup at
tests/tasks/upgrade_and_assert.yml:31-31, tests/tests_2019_upgrade.yml:75-75,
tests/tests_2022_upgrade.yml:75-75, tests/tests_accept_eula.yml:41-41,
tests/tests_configure_ha_cluster_external.yml:267-267,
tests/tests_tcp_firewall_2017.yml:20-20,
tests/tests_tcp_firewall_2019.yml:20-20,
tests/tests_tcp_firewall_2022.yml:25-25, tests/tests_tls_2017.yml:23-23,
tests/tests_tls_2019.yml:23-23, and tests/tests_tls_2022.yml:27-27, preserving
the surrounding task definitions.
In `@tests/tests_2025_upgrade.yml`:
- Around line 81-83: All affected cleanup task includes use the wrong tag. In
tests/tests_2025_upgrade.yml:81-83, tests/tests_idempotency_2025.yml:18-20,
tests/tests_input_sql_file_2025.yml:22-24, tests/tests_password_2025.yml:18-21,
both tests/tests_selinux_enforcing_2025.yml:40-42 and :84-86,
tests/tests_tcp_firewall_2025.yml:23-25, and tests/tests_tls_2025.yml:25-27,
replace tags: cleanup with tags: tests::cleanup, including both SELinux cleanup
includes.
In `@tests/tests_configure_ha_cluster_external_read_only.yml`:
- Line 163: The cleanup tags use an inconsistent contract. In
tests/tests_configure_ha_cluster_external_read_only.yml:163-163,
tests/tests_configure_ha_cluster_read_scale.yml:98-98,
tests/tests_default_2019.yml:25-25, tests/tests_ha_single_2017.yml:111-111,
tests/tests_ha_single_2019.yml:111-111, tests/tests_ha_single_2022.yml:111-111,
tests/tests_idempotency_2017.yml:19-19, tests/tests_idempotency_2019.yml:19-19,
and tests/tests_idempotency_2022.yml:20-20, replace the cleanup tag with
tests::cleanup; in tests/tests_include_vars_from_parent.yml:121-131, tag the
local cleanup task and update the included cleanup tag to tests::cleanup.
In `@tests/tests_ha_single_2025.yml`:
- Around line 109-111: Update the cleanup task entry in the test playbook to use
the required `tests::cleanup` tag instead of `cleanup`, preserving the existing
`include_tasks` reference and task behavior.
- Around line 104-107: Update the cleanup task tags in the test workflow,
including the block around “Run tests_ha_single with SQL Server {{ mssql_version
}}” and the corresponding cleanup task in tests_ha_single.yml, replacing the
generic cleanup tag with tests::cleanup while preserving all other tags and
behavior.
In `@tests/tests_input_sql_file_2017.yml`:
- Line 20: Update the cleanup task tags from cleanup to tests::cleanup in
tests/tests_input_sql_file_2017.yml:20, tests/tests_input_sql_file_2019.yml:20,
tests/tests_input_sql_file_2022.yml:24, tests/tests_password_2017.yml:19,
tests/tests_password_2019.yml:19, tests/tests_password_2022.yml:20, and both
cleanup tag sites in tests/tests_selinux_enforcing_2022.yml:42 and :86. No other
changes are needed.
In `@tests/tests_selinux_enforcing_2025.yml`:
- Around line 65-69: Remove the direct fedora.linux_system_roles.selinux
include_role block from the test playbook, move the SELinux permissive setup
into a dedicated test task file, and invoke that file via
ansible.builtin.include_tasks while preserving selinux_state: permissive.
In `@vars/Fedora.yml`:
- Line 1: Remove the Fedora platform mapping by deleting the vars/Fedora.yml
symbolic link, then remove Fedora from the supported-platform entries in
meta/main.yml and all Fedora support documentation. Do not replace it with
another platform mapping; preserve only platforms officially supported by the
project.
---
Outside diff comments:
In `@tests/tests_include_vars_from_parent.yml`:
- Around line 121-125: Tag the local Cleanup task and the cleanup task around
the referenced loop with tests::cleanup, replacing the existing cleanup tag
where present. Ensure both cleanup tasks use the exact tests::cleanup tag so
debug runs can skip the entire cleanup sequence.
---
Nitpick comments:
In `@defaults/main.yml`:
- Around line 39-40: Add mssql_rpm_key_2025 to a relevant example playbook in
README.md, showing how users can override its default URL. Keep the existing
variable documentation and default value unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 978125b2-13f9-4b3a-af82-d5e8294d9c3e
📒 Files selected for processing (55)
README.mddefaults/main.ymlmeta/main.ymlmssql-server-selinux-rhel10-bug-report.mdtasks/main.ymltests/playbooks/tests_ad_integration.ymltests/playbooks/tests_ad_integration_w_keytab.ymltests/tasks/assert_fail_on_unsupported_ver.ymltests/tasks/tests_ha_single.ymltests/tasks/upgrade_and_assert.ymltests/tests_2019_upgrade.ymltests/tests_2022_upgrade.ymltests/tests_2025_upgrade.ymltests/tests_accept_eula.ymltests/tests_configure_ha_cluster_external.ymltests/tests_configure_ha_cluster_external_read_only.ymltests/tests_configure_ha_cluster_read_scale.ymltests/tests_default_2019.ymltests/tests_ha_single_2017.ymltests/tests_ha_single_2019.ymltests/tests_ha_single_2022.ymltests/tests_ha_single_2025.ymltests/tests_idempotency_2017.ymltests/tests_idempotency_2019.ymltests/tests_idempotency_2022.ymltests/tests_idempotency_2025.ymltests/tests_include_vars_from_parent.ymltests/tests_input_sql_file_2017.ymltests/tests_input_sql_file_2019.ymltests/tests_input_sql_file_2022.ymltests/tests_input_sql_file_2025.ymltests/tests_password_2017.ymltests/tests_password_2019.ymltests/tests_password_2022.ymltests/tests_password_2025.ymltests/tests_selinux_enforcing_2022.ymltests/tests_selinux_enforcing_2025.ymltests/tests_tcp_firewall_2017.ymltests/tests_tcp_firewall_2019.ymltests/tests_tcp_firewall_2022.ymltests/tests_tcp_firewall_2025.ymltests/tests_tls_2017.ymltests/tests_tls_2019.ymltests/tests_tls_2022.ymltests/tests_tls_2025.ymlvars/AlmaLinux_10.ymlvars/AlmaLinux_9.ymlvars/CentOS_10.ymlvars/CentOS_9.ymlvars/Fedora.ymlvars/RedHat_10.ymlvars/RedHat_9.ymlvars/Rocky_10.ymlvars/Rocky_9.ymlvars/main.yml
| - name: Set SELinux to permissive to test without selinux_confined | ||
| include_role: | ||
| name: fedora.linux_system_roles.selinux | ||
| vars: | ||
| selinux_state: permissive |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the direct role invocation from this test playbook.
Line 66 uses ansible.builtin.include_role directly. Move the SELinux setup into a test task file, then call that file with ansible.builtin.include_tasks.
As per path instructions, files matching tests/tests_*.yml must never use ansible.builtin.include_role directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/tests_selinux_enforcing_2025.yml` around lines 65 - 69, Remove the
direct fedora.linux_system_roles.selinux include_role block from the test
playbook, move the SELinux permissive setup into a dedicated test task file, and
invoke that file via ansible.builtin.include_tasks while preserving
selinux_state: permissive.
Source: Path instructions
| company: Red Hat, Inc. | ||
| license: MIT | ||
| min_ansible_version: "2.9" | ||
| platforms: |
There was a problem hiding this comment.
This doesn't look right to remove the entire platforms section?
There was a problem hiding this comment.
Ansible-lint was arguing, I guess Ansible doesn't maintain platforms any more hence there is no need to maintain it from our side.
schema[meta]: roles/mssql/meta/main.yml#L1
$.galaxy_info.platforms[1].versions[0] '10' is not one of ['6.1', '7.1', '7.2', 'all']. See https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_reuse_roles.html#using-role-dependencies
schema[meta]: .ansible/collections/ansible_collections/fedora/linux_system_roles/roles/mssql/meta/main.yml#L1
$.galaxy_info.platforms[1].versions[0] '10' is not one of ['6.1', '7.1', '7.2', 'all']. See https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_reuse_roles.html#using-role-dependencies
There was a problem hiding this comment.
ansible-lint does not like having version "10" under
platforms:
- name: EL
versions:which is why we moved to using the tag el10 under galaxy_tags
There was a problem hiding this comment.
so the only change to meta/main.yml should be to add el10 under galaxy_tags
There was a problem hiding this comment.
Then platform won't list EL 10, and tags will. That's a conflict.
There was a problem hiding this comment.
Then platform won't list EL 10, and tags will. That's a conflict.
This is fine, not a conflict. This is what every other role does e.g. https://github.com/linux-system-roles/auditd/blob/main/meta/main.yml#L15
platforms:
- name: Fedora
versions:
- all
- name: EL
versions:
- "8"
- "9"
- name: Debian
versions:
- all
galaxy_tags:
- audit
- auditd
- security
- el8
- el9
- el10
3247cfe to
ca8f3b8
Compare
|
There is an issue with mssql-server on the Microsoft side: on RHEL 10 with SELinux enforcing, running SQL Server 2025 as a confined application fails during mssql-conf setup with Permission denied. The shipped mssql-server-selinux policy module appears incomplete — it defines no file contexts for the runtime directories under /var/opt/mssql (e.g. data and .system, which fall back to usr_t), and mssql_server_t has no rules to write its own directory types. As a result, confined setup is denied out of the box, and it can't be worked around from the role (a restorecon has no target context to apply). I've let Microsoft know and am waiting for them to resolve this issue. |
|
[citest] |
2 participants
Enhancement: Support SQL Server 2025 on RHEL 9 and RHEL 10.
Reason: Microsoft release SQL Server 2025 for RHEL 9 and 10 in February 2026, the role should support this.
Result: You can install SQL Server 2025 on RHEL 9, and newly on RHEL 10.
Summary by CodeRabbit
New Features
Documentation
Tests