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

LDAP: match on tuples instead of the #eldap_search_result record for OTP 24.3 compat #4285

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

michaelklishin
Copy link
Member

@michaelklishin michaelklishin commented Mar 17, 2022

In erlang/otp#5538, the eldap_search_result
record structure has changed:
https://github.com/erlang/otp/pull/5538/files#diff-30e064e89b115da7e974f229ed5c92f28e489da679ef42f17e70b9e7cf874179R24

It does have a default but for code
compiled on, say, Erlang 23.0, which is the case for current RabbitMQ
releases, it would still be a breaking change resulting in
case expression matching failures (a case_clause).

Note that CI would not hit this issue. In order to reproduce, you have to build on Erlang pre-24.3 but run on 24.3.

Closes #4284.

In erlang/otp#5538, the eldap_search_result
record structure has changed:
https://github.com/erlang/otp/pull/5538/files#diff-30e064e89b115da7e974f229ed5c92f28e489da679ef42f17e70b9e7cf874179R24

It does have a default but for code
compiled on, say, Erlang 23.0, which is the case for current RabbitMQ
releases, it would still be a breaking change resulting in
case expression matching failures (a case_clause).

Closes #4284.
@michaelklishin michaelklishin changed the title LDAP: match on eldap_search_result record for OTP 24.3 compat LDAP: match on tuples instead of the #eldap_search_result record for OTP 24.3 compat Mar 17, 2022
@lukebakken
Copy link
Collaborator

@michaelklishin should this API change have been released in a minor OTP version? 🤔 https://github.com/erlang/otp/pull/5538/files#diff-30e064e89b115da7e974f229ed5c92f28e489da679ef42f17e70b9e7cf874179R21-R24

Seems like it should have waited until OTP 25.

@michaelklishin
Copy link
Member Author

@lukebakken if you compile on 24.3 and run on 24.3, you wouldn't run into this. So I think it was OK to backport for the general Erlang/Elixir user base, just not for projects such as RabbitMQ. We've been there before.

Copy link
Collaborator

@lukebakken lukebakken left a comment

Choose a reason for hiding this comment

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

In my local environment eldap_search_result is always a tuple with two elements, not three. So, I can't reproduce the original issue. I applied the patch and my LDAP environment continued to work correctly (as expected).

This is an easy-to-understand fix for a well-reported issue, so 👍

@michaelklishin michaelklishin merged commit e2d0569 into master Mar 17, 2022
@michaelklishin michaelklishin deleted the rabbitmq-server-4284 branch March 17, 2022 16:17
michaelklishin added a commit that referenced this pull request Mar 17, 2022
LDAP: match on tuples instead of the #eldap_search_result record for OTP 24.3 compat (backport #4285)
michaelklishin added a commit that referenced this pull request Mar 17, 2022
LDAP: match on tuples instead of the #eldap_search_result record for OTP 24.3 compat (backport #4285) (backport #4286)
@michaelklishin michaelklishin added this to the 3.8.28 milestone Mar 17, 2022
michaelklishin added a commit that referenced this pull request Mar 17, 2022
LDAP: match on tuples instead of the #eldap_search_result record for OTP 24.3 compat (backport #4285) (backport #4286) (backport #4287)
lukebakken added a commit that referenced this pull request Apr 5, 2022
mergify bot pushed a commit that referenced this pull request Apr 5, 2022
Reported here
#4281 (reply in thread)

Fixes #4444

Follow-up to #4285

(cherry picked from commit 8d8847e)
mergify bot pushed a commit that referenced this pull request Apr 5, 2022
Reported here
#4281 (reply in thread)

Fixes #4444

Follow-up to #4285

(cherry picked from commit 8d8847e)
(cherry picked from commit 8ce7679)
mergify bot pushed a commit that referenced this pull request Apr 5, 2022
Reported here
#4281 (reply in thread)

Fixes #4444

Follow-up to #4285

(cherry picked from commit 8d8847e)
(cherry picked from commit 8ce7679)
(cherry picked from commit ec231d8)

# Conflicts:
#	deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap.erl
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.

eldap search result record has changed in Erlang 24.3
2 participants