Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved multiple domain support #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nieuwtje
Copy link

Whenever multiple domain controllers are defined, but they don't have a port assigned, only the last domain controller was given a port -> Added support for multiple servers without assigned ports.

Also, when multiple domain controllers are defined, only the ones in the domain you're looking at need to be checked -> Added check which checks whether hostname ends with the given domain, if not, it's not in the domain you're looking at.

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@jtnord
Copy link
Member

jtnord commented Feb 5, 2016

can you split this into to separate PRs?

The not adding the port is perfectly valid and not contentiuos. Filtering the preferred server is as this may result in not adding a preferred server that would be network local and serving the global catalogue (so login times could increase due to network RTT and/or quering multiple servers in the case of a bad password).

@nieuwtje
Copy link
Author

nieuwtje commented Feb 5, 2016

Moved port assignment to a new PR

Whenever multiple domain controllers are defined, but they don't have a port assigned, only the last domain controller was given a port -> Added support for multiple servers without assigned ports.

… a port assigned, only the last domain controller was given a port. Added support for multiple servers without assigned ports.

Also, when multiple domain controllers are defined, only the ones in the domain you're looking at need to be checked. Add check if hostname ends with the given domain, if not, it's not in the domain you're looking at.
@nieuwtje nieuwtje force-pushed the improved-multiple-domain-controller-support branch from 40160f8 to 6124e27 Compare February 5, 2016 20:36
@jtnord
Copy link
Member

jtnord commented Mar 3, 2016

So I'm not convinced that this is correct. An AD server for sub.bob.com could be in foo.com and not sub.bob.com (e.g. a global catalog in the forest). If global.foo.com is site local and server.sub.bob.com is the other side of the world this would have a massive performance impact.

yaroslavafenkin pushed a commit to yaroslavafenkin/active-directory-plugin that referenced this pull request Nov 1, 2024
yaroslavafenkin pushed a commit to yaroslavafenkin/active-directory-plugin that referenced this pull request Nov 1, 2024
Pop up "Loading..." screen when re-evaluating cascade parameters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants