Skip to content

[autobackport: sssd-2-9] SSSD on IPA should fail with short names#8302

Merged
sumit-bose merged 3 commits intoSSSD:sssd-2-9from
sssd-bot:SSSD-sssd-backport-pr8261-to-sssd-2-9
Dec 19, 2025
Merged

[autobackport: sssd-2-9] SSSD on IPA should fail with short names#8302
sumit-bose merged 3 commits intoSSSD:sssd-2-9from
sssd-bot:SSSD-sssd-backport-pr8261-to-sssd-2-9

Conversation

@sssd-bot
Copy link
Contributor

@sssd-bot sssd-bot commented Dec 15, 2025

This is an automatic backport of PR#8261 SSSD on IPA should fail with short names to branch sssd-2-9, created by @thalman.

Caution

@thalman The patches did not apply cleanly. It is necessary to resolve conflicts before merging this pull request. Commits that introduced conflict are marked with CONFLICT!.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8261-to-sssd-2-9
git checkout SSSD-sssd-backport-pr8261-to-sssd-2-9
git push sssd-bot SSSD-sssd-backport-pr8261-to-sssd-2-9 --force

Original commits
1b71104 - IPA: Fail with short names
4816096 - IPA: remove re-declaration of ipa_trusted_subdom_init
09f574f - IPA: remove CONFDB_DEFAULT_FULL_NAME_FORMAT_INTERNAL
8360424 - tests: SSSD must refuse to start on IPA with short names

Backported commits

  • 59d103f - IPA: Fail with short names
  • 2f9e568 - IPA: remove re-declaration of ipa_trusted_subdom_init
  • 180c4bd - IPA: remove CONFDB_DEFAULT_FULL_NAME_FORMAT_INTERNAL
  • f9092c9 - CONFLICT! tests: SSSD must refuse to start on IPA with short names

Conflicting Files Information (check for deleted and re-added files)

  • CONFLICT! tests: SSSD must refuse to start on IPA with short names
On branch SSSD-sssd-backport-pr8261-to-sssd-2-9
You are currently cherry-picking commit 836042459.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   src/tests/system/tests/test_ipa.py

no changes added to commit (use "git add" and/or "git commit -a")

Original Pull Request Body

SSSD should refuse to start when SSSD is in server mode and full_name_format is set to %1s$.

gemini-code-assist[bot]
gemini-code-assist bot reviewed Dec 15, 2025
View reviewed changes
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request backports a change to make SSSD fail to start on an IPA server if full_name_format is configured for short names. The changes in the C source files are correct and implement the intended behavior effectively. A new test is also added to verify this behavior.

My main concern is with the new test file src/tests/system/tests/test_ipa.py. It appears to have been added as a result of a merge conflict resolution and contains a large number of tests unrelated to this change. This introduces a risk of test failures on this branch if those tests rely on features not present here. I've added a comment with a suggestion to only include the relevant test.

@thalman thalman force-pushed the SSSD-sssd-backport-pr8261-to-sssd-2-9 branch from f9092c9 to 028497c Compare December 15, 2025 13:57
@thalman thalman marked this pull request as ready for review December 15, 2025 14:30
@thalman
Copy link
Contributor

thalman commented Dec 15, 2025

@sumit-bose , @madhuriupadhye, the test_ipa.py was not present in 2-9 branch. Please check the update. Other commits fit well.

@madhuriupadhye
Copy link
Contributor

madhuriupadhye commented Dec 15, 2025

@sumit-bose , @madhuriupadhye, the test_ipa.py was not present in 2-9 branch. Please check the update. Other commits fit well.

Only branches where the file exists, so not in sssd-2-9.

@thalman
Copy link
Contributor

thalman commented Dec 15, 2025

@sumit-bose , @madhuriupadhye, the test_ipa.py was not present in 2-9 branch. Please check the update. Other commits fit well.

Only branches where the file exists, so not in sssd-2-9.

I understand that, but are we fine to create it like this or do you prefer something else?

@madhuriupadhye
Copy link
Contributor

madhuriupadhye commented Dec 15, 2025

@sumit-bose , @madhuriupadhye, the test_ipa.py was not present in 2-9 branch. Please check the update. Other commits fit well.

Only branches where the file exists, so not in sssd-2-9.

I understand that, but are we fine to create it like this or do you prefer something else?

As I understand it, we won't backport the test code.

@sumit-bose
Copy link
Contributor

sumit-bose commented Dec 16, 2025

@sumit-bose , @madhuriupadhye, the test_ipa.py was not present in 2-9 branch. Please check the update. Other commits fit well.

Only branches where the file exists, so not in sssd-2-9.

I understand that, but are we fine to create it like this or do you prefer something else?

As I understand it, we won't backport the test code.

Hi,

I'm confused, most of the backports are test code so far?

bye,
Sumit

@madhuriupadhye
Copy link
Contributor

madhuriupadhye commented Dec 16, 2025

@sumit-bose , @madhuriupadhye, the test_ipa.py was not present in 2-9 branch. Please check the update. Other commits fit well.

Only branches where the file exists, so not in sssd-2-9.

I understand that, but are we fine to create it like this or do you prefer something else?

As I understand it, we won't backport the test code.

Hi,

I'm confused, most of the backports are test code so far?

bye,

Sumit

Yes, but only backport to branches where the test files exist.

@sumit-bose
Copy link
Contributor

sumit-bose commented Dec 16, 2025

Yes, but only backport to branches where the test files exist.

Hi,

ok, so how to move forward? Just backport the feature and do not test it? Or break the rule and create the file in the backport or move the test to a different file which exists in the 2-9 branch?

bye,
Sumit

@madhuriupadhye
Copy link
Contributor

madhuriupadhye commented Dec 17, 2025

Yes, but only backport to branches where the test files exist.

Hi,

ok, so how to move forward? Just backport the feature and do not test it? Or break the rule and create the file in the backport or move the test to a different file which exists in the 2-9 branch?

bye, Sumit

Skip the test - backport only the functional code changes.
The feature is straightforward (preventing SSSD startup with invalid config) and can be validated through manual testing and downstream QE, especially considering the low importance/trivial nature of this change.

@thalman
Copy link
Contributor

thalman commented Dec 17, 2025

OK, removing test now...

@thalman thalman force-pushed the SSSD-sssd-backport-pr8261-to-sssd-2-9 branch from 028497c to 180c4bd Compare December 17, 2025 11:02
sumit-bose
sumit-bose approved these changes Dec 17, 2025
View reviewed changes
Copy link
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

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

Hi,

pity we lost the test, nevertheless, ACK.

bye,
Sumit

@sumit-bose sumit-bose added coverity Trigger a coverity scan Accepted and removed coverity Trigger a coverity scan labels Dec 17, 2025
@thalman @sssd-bot
IPA: Fail with short names
eb71ff8 
SSSD in server mode on IPA server is not allowed to use
short names.

Reviewed-by: Madhuri Upadhye <mupadhye@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit 1b71104)
@thalman @sssd-bot
IPA: remove re-declaration of ipa_trusted_subdom_init
3b3250b 
Reviewed-by: Madhuri Upadhye <mupadhye@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit 4816096)
@thalman @sssd-bot
IPA: remove CONFDB_DEFAULT_FULL_NAME_FORMAT_INTERNAL
6f4782a 
This format is not used anywhere except in unnecessary check.
This update removes it.

Reviewed-by: Madhuri Upadhye <mupadhye@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit 09f574f)
@sssd-bot
Copy link
Contributor Author

sssd-bot commented Dec 17, 2025

The pull request was accepted by @sumit-bose with the following PR CI status:

🟢 CodeQL (success)
🟢 rpm-build:centos-stream-9-x86_64:upstream (success)
🟢 Build / make-distcheck (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-9) (success)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)

There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot sssd-bot force-pushed the SSSD-sssd-backport-pr8261-to-sssd-2-9 branch from 180c4bd to 6f4782a Compare December 17, 2025 16:42
@sumit-bose sumit-bose merged commit 33b6082 into SSSD:sssd-2-9 Dec 19, 2025
10 checks passed
@sumit-bose sumit-bose mentioned this pull request Jan 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sumit-bose sumit-bose sumit-bose approved these changes

@madhuriupadhye madhuriupadhye madhuriupadhye approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sumit-bose sumit-bose

@madhuriupadhye madhuriupadhye

Labels

Accepted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sssd-bot @thalman @madhuriupadhye @sumit-bose