DEENG-491 Support multiple AD backends #467
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note: due to the config change, we need to update the documentation. However, the code itself is good for review. |
|
and documentation is now updated! |
Codecov Report
@@ Coverage Diff @@
## main #467 +/- ##
==========================================
+ Coverage 83.85% 83.98% +0.13%
==========================================
Files 71 73 +2
Lines 7483 7513 +30
==========================================
+ Hits 6275 6310 +35
+ Misses 910 907 -3
+ Partials 298 296 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
denisonbarbosa
requested changes
Oct 19, 2022
...ation_tests/testdata/ServiceStatus/golden/Invalid startup time leads to unknown refresh time
Show resolved
Hide resolved
GabrielNagy
approved these changes
Oct 20, 2022
We are going to separate backends from the service and ad object itself.
Change the parameters to accept backend and possible option for each backend (sssd only now).
Provide a mock backend for our tests. Change our internal tests and don’t export sssd objects.
Simplify most of the tests for ad package tests. This includes having default configuration, less parameters for each tests and create dynamically the krb5 tokens as needed. Fix some duplicated and mismatch tests. Don’t rely on GetStatus, which doesn’t exist anymore.
Ensure we control their result with golden files.
This code and tests all migrated in sssd.
We now load the backend here, adapt the existing tests for it and minimize the number of parameters. Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Add additional tests for backend selection. For this, we need to export sssd and systemd on the local bus. Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
The initialization only load the static configuration. Then, the Active Server is loaded (if no static configuration for server is provided) everytime we request for the server URL. This may this fails at that stage. Changing the backend interface signature and move it to the backend package. Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
As we thus can have another element failing, add more tests to it. Rename the tests to TestSSSD at the same time as we cover everything in a single test. Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Change the config to use sssd by default and don’t bypass the backend with the static configuration (which does not exists anymore). Simplify some of the test logic by minimizing the number of parameters. Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Add more test cases to cover the error expectations that moved from New to the direct call. Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Remove old config Only required part of new config, with a static server selected to avoid relying on dbus for those tests.
We were pointing to the config instead of the cache constant.
Those were due to leftover unused struct/fields, conforming names, additional spaces…
Co-authored-by: Jean-Baptiste Lallement <jean-baptiste@ubuntu.com>
Co-authored-by: Denison Barbosa <denisonbarbosa@users.noreply.github.com>
denisonbarbosa
approved these changes
Oct 20, 2022
2 tasks
GabrielNagy
added a commit
that referenced
this pull request
Feb 14, 2024
This restores the functionality prior to the refactor in PR #467, where the case of having a domain section without the ad_domain setting resorted to using the domain from the sssd.domains setting. This is valid behavior supported and suggested[1] by sssd. In addition to that, avoid being too lenient and still raise an error if the domain section is empty or does not exist. Fixes #910 / UDENG-2268 [1] https://sssd.io/docs/ad/ad-provider-manual.html#id4
GabrielNagy
added a commit
that referenced
this pull request
Feb 14, 2024
This restores the functionality prior to the refactor in PR #467, where the case of having a domain section without the ad_domain setting resorted to using the domain from the sssd.domains setting. This is valid behavior supported and suggested[1] by sssd. In addition to that, avoid being too lenient and still raise an error if the domain section is empty or does not exist. Fixes #910 / UDENG-2268 [1] https://sssd.io/docs/ad/ad-provider-manual.html#id4
GabrielNagy
added a commit
that referenced
this pull request
Feb 15, 2024
This restores the functionality prior to the refactor in PR #467, where the case of having a domain section without the `ad_domain` setting resorted to using the domain from the `sssd.domains` setting. This is valid behavior supported and [suggested](https://sssd.io/docs/ad/ad-provider-manual.html#id4) by sssd. In addition to that, avoid being too lenient and still raise an error if the domain section is empty or does not exist. Fixes #910 / UDENG-2268
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR are the changes needed for supporting multiple AD backends.
Default backend is SSSd.