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

Add search option exact_match for data source rancher2_principal #1331

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

Conversation

manhtukhang
Copy link

@manhtukhang manhtukhang commented Mar 28, 2024

Issue: #1330

Problem

When using the data source rancher2_principal to search for an LDAP user, sometimes it returns the wrong result if the inputted name has multiple matched results.

Solution

Because the provider picks first element in the list returned from Rancher API. But that list
is unsorted or just in random order, therefore, picking the first element is not the best way. We should loop through it to find the exact match, but changing the default behavior can break compatibility.
So I decided to add an option exact_match to control this behavior.

Testing

Set option exact_match=true must only return an exactly matched user as the inputted name

Engineering Testing

Manual Testing

Automated Testing

QA Testing Considerations

Regressions Considerations

@manhtukhang manhtukhang force-pushed the update/data_source_principal branch 2 times, most recently from 0c920db to f2c283a Compare March 29, 2024 07:56
@manhtukhang manhtukhang force-pushed the update/data_source_principal branch from f2c283a to 1dd9a29 Compare March 29, 2024 10:01
@wgjak47
Copy link

wgjak47 commented Apr 4, 2024

Do we really need a exact_match option? The data source rancher2_principal will return a random result when we have multiple principals like k8s-admin, k8s-admin-dev, k8s-admin-staging. It is meaningless when you set name "k8s-admin" but return the "k8s-admin-dev".

@manhtukhang
Copy link
Author

manhtukhang commented Apr 17, 2024

Do we really need a exact_match option? The data source rancher2_principal will return a random result when we have multiple principals like k8s-admin, k8s-admin-dev, k8s-admin-staging. It is meaningless when you set name "k8s-admin" but return the "k8s-admin-dev".

@wgjak47 I want to keep compatibility with current behavior, that won't break any use case, in the future release (2 or more versions) we can make this true by default.

@jakefhyde @jiaqiluo Could you take a look at this PR? Thanks!

Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

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

Hi @manhtukhang, thank you for reporting the issue and making the fix! The PR LGTM.
I am going to add the issue to the team's working queue, but would like to let you know that when the fix is merged and a new version is released will depend on the team's priority and capacity. Thank you for understanding.

@jiaqiluo jiaqiluo requested review from a team and matttrach April 17, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants