Skip to content

ipa trust bugfix and improvement of handling unknown trust type error#8258

Merged
alexey-tikhonov merged 2 commits intoSSSD:masterfrom
justin-stephenson:get_by_trust_type
Dec 5, 2025
Merged

ipa trust bugfix and improvement of handling unknown trust type error#8258
alexey-tikhonov merged 2 commits intoSSSD:masterfrom
justin-stephenson:get_by_trust_type

Conversation

@justin-stephenson
Copy link
Contributor

No description provided.

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 introduces two important bug fixes for IPA trust handling. The first change in src/providers/ipa/ipa_subdomains_id.c corrects the error handling for unknown trust types, ensuring it returns a more accurate EINVAL error code instead of ENOMEM. The second change in src/providers/ipa/ipa_subdomains_server.c fixes a logical bug by correcting a condition to check for IPA_TRUST_IPA, which was previously checking for IPA_TRUST_AD twice. Both changes are correct and improve the stability and correctness of the provider. My review did not find any further issues.

@justin-stephenson
Copy link
Contributor Author

Note to self @justin-stephenson to cherry-pick d865ac3 into sssd 2-11 after this automatic backport for 2-11 is opened.

@sumit-bose
Copy link
Contributor

Note to self @justin-stephenson to cherry-pick d865ac3 into sssd 2-11 after this automatic backport for 2-11 is opened.

Hi,

maybe it would be easier if you pick my one-liner and include it into this PR and I close mine?

bye,
Sumit

@sumit-bose
Copy link
Contributor

Note to self @justin-stephenson to cherry-pick d865ac3 into sssd 2-11 after this automatic backport for 2-11 is opened.

Hi,

maybe it would be easier if you pick my one-liner and include it into this PR and I close mine?

bye, Sumit

Oh, I'm late to the game, Alexey made me aware that my PR is already merged and backported.

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,

thank you for the fixes, both changes make sense, ACK.

bye,
Sumit

@alexey-tikhonov
Copy link
Member

@alexey-tikhonov alexey-tikhonov added the coverity Trigger a coverity scan label Dec 5, 2025
@alexey-tikhonov
Copy link
Member

Note: Covscan is green.

@alexey-tikhonov alexey-tikhonov added Accepted and removed coverity Trigger a coverity scan labels Dec 5, 2025
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
If trust type is unknown for an IPA subdomain request, return
EINVAL instead of ENOMEM, and improve the logged error message.

Trust type should always be IPA_TRUST_IPA, or IPA_TRUST_AD, if
trust discovery and initialization fails early on we can reach this
codepath however.

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
@sssd-bot
Copy link
Contributor

sssd-bot commented Dec 5, 2025

The pull request was accepted by @alexey-tikhonov with the following PR CI status:


🟢 CodeFactor (success)
🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-41-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-41) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-41) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
➖ Coverity scan / coverity (skipped)
🟢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted backport-to-sssd-2-9 backport-to-sssd-2-11 Bugzilla Trivial A single reviewer is sufficient to review the Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants