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

[TEST] Add logging when LDAP server starts/stops #75322

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Jul 14, 2021

This commit adds logging to the in memory LDAP server in LdapTestCase
that indicates when the server starts and stops and the addresses on
which it is listening.

Relates: #75176

This commit adds logging to the in memory LDAP server in LdapTestCase
that indicates when the server starts and stops and the addresses on
which it is listening.

Relates: elastic#75176
@tvernum tvernum added >test Issues or PRs that are addressing/adding tests :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 auto-backport Automatically create backport pull requests when merged v7.15.0 labels Jul 14, 2021
@tvernum tvernum requested a review from ywangd July 14, 2021 07:12
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Jul 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +113 to +123
String listenerConfig = listeners.stream()
.map(
l -> String.format(
Locale.ROOT,
"(%s @ %s:%d)",
l.getListenerName(),
NetworkAddress.format(resolveListenAddress(l.getListenAddress())),
ldapServer.getListenPort(l.getListenerName())
)
)
.collect(Collectors.joining(","));
Copy link
Member

Choose a reason for hiding this comment

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

Why not use InMemoryListenerConfig#toString? Is it because of the listenAddress?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More due to the port - typically the config has port 0 and we want to know what port number was actually assigned.

@tvernum tvernum added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jul 14, 2021
@elasticsearchmachine elasticsearchmachine merged commit f1aa1cf into elastic:master Jul 14, 2021
elasticsearchmachine pushed a commit to elasticsearchmachine/elasticsearch that referenced this pull request Jul 14, 2021
This commit adds logging to the in memory LDAP server in LdapTestCase
that indicates when the server starts and stops and the addresses on
which it is listening.

Relates: elastic#75176
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
7.x

masseyke pushed a commit to masseyke/elasticsearch that referenced this pull request Jul 16, 2021
This commit adds logging to the in memory LDAP server in LdapTestCase
that indicates when the server starts and stops and the addresses on
which it is listening.

Relates: elastic#75176
tvernum pushed a commit that referenced this pull request Jul 26, 2021
This commit adds logging to the in memory LDAP server in LdapTestCase
that indicates when the server starts and stops and the addresses on
which it is listening.

Relates: #75176
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v7.15.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants