Replacing the exclusionary words whitelist and blacklist that won't impact the compatibility. #1970

aponb · 2022-01-24T20:51:54Z

Description

Replacing the exclusionary words whitelist and blacklist that won't impact the compatibility. Changes just in code comments, internal variables, methods and class names.

Issues Resolved

This Pull request replaces Pull request #744

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Andreas Pre [email protected]

opensearch-ci-bot · 2022-01-24T20:52:04Z

Can one of the admins verify this patch?

opensearch-ci-bot · 2022-01-24T21:08:35Z

❌ Gradle Check failure 31f4eac
Log 2036

Reports 2036

nknize · 2022-01-26T21:01:18Z

Thx @aponb.. looks like a legit comparison failure. I could repro locally:

REPRODUCE WITH: ./gradlew ':modules:reindex:test' --tests "org.opensearch.index.reindex.ReindexFromRemoteWhitelistTests.testUnwhitelistedRemote" -Dtests.seed=95E696791CA7DA1F -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sv-SE -Dtests.timezone=Australia/Melbourne -Druntime.java=17

org.opensearch.index.reindex.ReindexFromRemoteWhitelistTests > testUnwhitelistedRemote FAILED
    org.junit.ComparisonFailure: expected:<...ist:1344494910] not [white]listed in reindex.re...> but was:<...ist:1344494910] not [allow]listed in reindex.re...>
        at __randomizedtesting.SeedInfo.seed([95E696791CA7DA1F:C249AF47E210FE99]:0)
        at org.junit.Assert.assertEquals(Assert.java:117)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at org.opensearch.index.reindex.ReindexFromRemoteWhitelistTests.testUnwhitelistedRemote(ReindexFromRemoteWhitelistTests.java:134)

tlfeng · 2022-01-27T08:08:37Z

modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexFromRemoteWhitelistTests.java

        Exception e = expectThrows(
            IllegalArgumentException.class,
-            () -> checkRemoteWhitelist(buildRemoteWhitelist(whitelist), newRemoteInfo("not in list", port))
+            () -> checkRemoteAllowlist(buildRemoteAllowlist(allowlist), newRemoteInfo("not in list", port))
        );
        assertEquals("[not in list:" + port + "] not whitelisted in reindex.remote.whitelist", e.getMessage());


Thank @nknize pointing out the reason to the test failure, seems the whitelisted is missed.

tlfeng · 2022-01-27T08:11:31Z

I really appreciate you for taking the time in this PR! 🏆 Especially in picking out the non-inclusive terms that doesn't break any compatibility at all.

tlfeng · 2022-01-27T08:17:40Z

Ah, please use git commit --amend -s to append the DCO sign-off to your last commit. Github Website didn't add the sign-off.

aponb · 2022-01-27T16:05:29Z

Thx @aponb.. looks like a legit comparison failure. I could repro locally:

REPRODUCE WITH: ./gradlew ':modules:reindex:test' --tests "org.opensearch.index.reindex.ReindexFromRemoteWhitelistTests.testUnwhitelistedRemote" -Dtests.seed=95E696791CA7DA1F -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sv-SE -Dtests.timezone=Australia/Melbourne -Druntime.java=17

Sorry my bad. Thanks for your help.

aponb · 2022-01-27T17:03:35Z

Ah, please use git commit --amend -s to append the DCO sign-off to your last commit. Github Website didn't add the sign-off.

Have done it. Hope it worked!

opensearch-ci-bot · 2022-01-27T17:22:40Z

❌ Gradle Check failure 35bae52
Log 2097

Reports 2097

tlfeng · 2022-01-27T18:03:31Z

About test log 2097, the format check failure is a known issue in current main branch.
#1994

Update:
The existing test failure has been fixed in commit 054595b.
I think there will be no issue after:

git commit --amend -s to add the DCO sign-off to your last commit of "merge"
Merge the current main branch into your branch, with DCO sign-off

Thank you! 😁

aponb · 2022-01-27T19:58:23Z

About test log 2097, the format check failure is a known issue in current main branch. #1994

Update: The existing test failure has been fixed in commit 054595b. I think there will be no issue after:
1. `git commit --amend -s` to add the DCO sign-off to your last commit of "merge"

2. Merge the current `main` branch into your branch, with DCO sign-off
Thank you! grin

Done. Hope that was ok!

opensearch-ci-bot · 2022-01-27T20:00:17Z

❌ Gradle Check failure 7eb2d11
Log 2102

Reports 2102

opensearch-ci-bot · 2022-01-27T20:14:03Z

❌ Gradle Check failure 35bb463
Log 2103

Reports 2103

tlfeng · 2022-01-27T21:24:33Z

Thanks @aponb ! The DCO check is fine now. Only the unit test failure left.

tlfeng · 2022-02-14T22:42:00Z

Hi @aponb, when you get a time, could you resolve the remaining small issues in the Pull Request? So that it can be merged.
My above comment was wrong, there is still problem with DCO check. 😐

There are 2 commits doesn’t have DCO sign-off. It’s a bit troublesome, you need either squash your commits in the PR into one, or modify the 2 commits in the middle.
A unit test failure caused by L134 in ReindexFromRemoteWhitelistTests.java

Feel free to tell us if you need any help. Thank you! 👍

opensearch-ci-bot · 2022-02-15T17:55:23Z

❌ Gradle Check failure 7e9ecf6
Log 2403

Reports 2403

opensearch-ci-bot · 2022-02-15T19:10:44Z

❌ Gradle Check failure fb79b60
Log 2408

Reports 2408

tlfeng · 2022-02-15T19:14:24Z

Hi @aponb ,

To fix the DCO sign-off check, I'd like to provide a solution:

Backup your current branch with all the changes in the Pull Request, into a new branch, such as named "backup_whitelist_blacklist": git checkout -b backup_whitelist_blacklist. In case your current code change is lost by mistake during the following commands
Follow the solution here to squash all commits on your branch to overwrite the commits that doesn’t have sign-off message: https://stackoverflow.com/a/50880042/11408564. Since your changes are in main branch, you need a slightly different command:

# checkout the branch in this Pull Request
git checkout main
# Remove all your new commits, while remain your code change.
git reset --soft origin/main
# Add files for the commit.
git add .
git commit -s -m "Replacing the exclusionary word whitelist that won't impact the compatibility."
# force push the commit to your remote repo
git push —force

In addition, I suggest use a new branch other than the default main branch to create Pull Requests.

because any new changes you add to the master branch will automatically show up in the pull request. That means changes you did not intend to be there, would now be there.

(source: https://blog.jasonmeridth.com/posts/do-not-issue-pull-requests-from-your-master-branch/)

tlfeng · 2022-02-15T19:22:48Z

Another small test failure:
Looks like the whitelisted needs to be changed in https://github.com/opensearch-project/OpenSearch/blob/1.2.4/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml#L311

> Task :modules:reindex:yamlRestTest

REPRODUCE WITH: ./gradlew ':modules:reindex:yamlRestTest' --tests "org.opensearch.index.reindex.ReindexClientYamlTestSuiteIT.test {yaml=reindex/20_validation/unwhitelisted remote host fails}" -Dtests.seed=35B7ED975A16E179 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=nb-NO -Dtests.timezone=America/Anchorage -Druntime.java=17

org.opensearch.index.reindex.ReindexClientYamlTestSuiteIT > test {yaml=reindex/20_validation/unwhitelisted remote host fails} FAILED
    java.lang.AssertionError: Failure at [reindex/20_validation:310]: the error message was expected to match the provided regex but didn't
    Expected: \[badremote:9200\] not whitelisted in reindex.remote.whitelist
         but: was "{root_cause=[{type=illegal_argument_exception, reason=[badremote:9200] not allowlisted in reindex.remote.whitelist, stack_trace=[[badremote:9200] not allowlisted in reindex.remote.whitelist]; nested: IllegalArgumentException[[badremote:9200] not allowlisted in reindex.remote.whitelist];\n\tat

@aponb Thanks a lot for your rapid response! 👍

opensearch-ci-bot · 2022-02-15T20:43:07Z

✅ Gradle Check success 32c6db0d37d4f7ffbabc21355dc641660b52e3e5
Log 2412

Reports 2412

aponb · 2022-02-15T21:15:43Z

Another small test failure: Looks like the whitelisted needs to be changed in https://github.com/opensearch-project/OpenSearch/blob/1.2.4/modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml#L311

> Task :modules:reindex:yamlRestTest

REPRODUCE WITH: ./gradlew ':modules:reindex:yamlRestTest' --tests "org.opensearch.index.reindex.ReindexClientYamlTestSuiteIT.test {yaml=reindex/20_validation/unwhitelisted remote host fails}" -Dtests.seed=35B7ED975A16E179 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=nb-NO -Dtests.timezone=America/Anchorage -Druntime.java=17

org.opensearch.index.reindex.ReindexClientYamlTestSuiteIT > test {yaml=reindex/20_validation/unwhitelisted remote host fails} FAILED
    java.lang.AssertionError: Failure at [reindex/20_validation:310]: the error message was expected to match the provided regex but didn't
    Expected: \[badremote:9200\] not whitelisted in reindex.remote.whitelist
         but: was "{root_cause=[{type=illegal_argument_exception, reason=[badremote:9200] not allowlisted in reindex.remote.whitelist, stack_trace=[[badremote:9200] not allowlisted in reindex.remote.whitelist]; nested: IllegalArgumentException[[badremote:9200] not allowlisted in reindex.remote.whitelist];\n\tat

@aponb Thanks a lot for your rapid response! +1

Thanks! This is fixed now.

opensearch-ci-bot · 2022-02-15T21:17:11Z

✅ Gradle Check success 842bb8be598fdcc17a3ac43c88d28916be085802
Log 2416

Reports 2416

aponb · 2022-02-15T21:18:08Z

Hi @aponb ,

To fix the DCO sign-off check, I'd like to provide a solution:
1. Backup your current branch with all the changes in the Pull Request, into a new branch, such as named "backup_whitelist_blacklist":  `git checkout -b backup_whitelist_blacklist`. In case your current code change is lost by mistake during the following commands

2. Follow the solution here to squash all commits on your branch to overwrite the commits that doesn’t have sign-off message: https://stackoverflow.com/a/50880042/11408564. Since your changes are in `main` branch, you need a slightly different command:
# checkout the branch in this Pull Request
git checkout main
# Remove all your new commits, while remain your code change.
git reset --soft origin/main
# Add files for the commit.
git add .
git commit -s -m "Replacing the exclusionary word whitelist that won't impact the compatibility."
# force push the commit to your remote repo
git push —force
In addition, I suggest use a new branch other than the default main branch to create Pull Requests.

because any new changes you add to the master branch will automatically show up in the pull request. That means changes you did not intend to be there, would now be there.

(source: https://blog.jasonmeridth.com/posts/do-not-issue-pull-requests-from-your-master-branch/)

I followed your suggestion. It seems it worked now. Thanks for your help and patience!

opensearch-ci-bot · 2022-02-15T21:38:15Z

❌ Gradle Check failure 3282447349b617c58cb8d49b9deab81f3cd32445
Log 2419

Reports 2419

tlfeng · 2022-02-15T22:08:09Z

Hi @aponb, you are very welcome! Thanks to your patience as well.
Hmmm, seems I made a small mistake on the command git reset --soft origin/main in my suggestion. Sorry that my solution was not comprehensive.
DCO sign-off and the previous test failures are totally fine, but there are many unrelated commits checked in the PR, which must caused by the git reset command...
Only the 4 commits should be shown in the PR:
f03dcab 0021475 86157af 703309a

Please keep your backup branch (https://github.com/aponb/OpenSearch/tree/backup_whitelist_blacklist3), let me find a way to deal with it.

tlfeng · 2022-02-18T01:05:04Z

Hi @aponb, finally I got a solution to our current situation 😁.
In this way, you will need to raise a new Pull Request based on your feature branch instead the current main branch, and we will merge your code changes in the new PR.

Backup your current main branch by creating a new branch based on main.

# ensure current branch is main
git checkout main

# cut a new branch based on main branch as the backup, whatever the new branch name is
git checkout -b backup-whitelist-blacklist

Sync up the current main branch of your fork repository with the main branch of upstream repository (https://github.com/opensearch-project/OpenSearch)
Reference: https://stackoverflow.com/a/42332860/11408564

# Add a remote repository of the upstream repository, if there is not one locally
git remote add upstream [email protected]:opensearch-project/OpenSearch.git

# ensure current branch is main
git checkout main

# fetch all new commits made to upstream/main
git fetch upstream main

# Reset your local main branch, and this will delete all your local changes to main
git reset --hard upstream/main

# take care, this will delete all your changes on your forked main branch
git push origin main --force

Use a new feature branch to hold your code changes

# ensures current branch is main
git checkout main 

# Cut a new branch from main branch, whatever the new branch name is.
git checkout -b replace-whitelist-blacklist

# Cherry pick the 4 commits of your actual code changes
# (or running `git cherry-pick <hash of the commit>` separately for each commit)
git cherry-pick f03dcaba52f4ed33a97d1031ba95cf259a6ff3f4 002147537d64a90d5a15e66e24f479619b0820bf 86157af85251152be290dbe3b85c50166ce98245 703309af2b011921f10d29499f84fc3c47f8f276

# Push the feature branch to your remote repository
git push --set-upstream origin replace-whitelist-blacklist

Create a new Pull Request based on the feature branch

Thanks for your time!

aponb · 2022-02-20T12:29:08Z

Hi @aponb, finally I got a solution to our current situation grin. In this way, you will need to raise a new Pull Request based on your feature branch instead the current main branch, and we will merge your code changes in the new PR.

1. Backup your current `main` branch by creating a new branch based on `main`.

# ensure current branch is main
git checkout main

# cut a new branch based on main branch as the backup, whatever the new branch name is
git checkout -b backup-whitelist-blacklist

2. Sync up the current `main` branch of your fork repository with the `main` branch of upstream repository (https://github.com/opensearch-project/OpenSearch)
   Reference: https://stackoverflow.com/a/42332860/11408564

# Add a remote repository of the upstream repository, if there is not one locally
git remote add upstream [email protected]:opensearch-project/OpenSearch.git

# ensure current branch is main
git checkout main

# fetch all new commits made to upstream/main
git fetch upstream main

# Reset your local main branch, and this will delete all your local changes to main
git reset --hard upstream/main

# take care, this will delete all your changes on your forked main branch
git push origin main --force

3. Use a new feature branch to hold your code changes

# ensures current branch is main
git checkout main 

# Cut a new branch from main branch, whatever the new branch name is.
git checkout -b replace-whitelist-blacklist

# Cherry pick the 4 commits of your actual code changes
# (or running `git cherry-pick <hash of the commit>` separately for each commit)
git cherry-pick f03dcaba52f4ed33a97d1031ba95cf259a6ff3f4 002147537d64a90d5a15e66e24f479619b0820bf 86157af85251152be290dbe3b85c50166ce98245 703309af2b011921f10d29499f84fc3c47f8f276

# Push the feature branch to your remote repository
git push --set-upstream origin replace-whitelist-blacklist

4. Create a new Pull Request based on the feature branch

Thanks for your time!

Thanks for your effort. Please see #2178

tlfeng · 2022-02-20T23:57:20Z

Hi @aponb, I'm so glad that we finally made it! 🎉 🤝

aponb · 2022-02-21T08:43:29Z

Hi @aponb, I'm so glad that we finally made it! tada handshake

Thanks for your help and your cooperation. I learned a lot, which will be hopefully helpful in future!

aponb requested a review from a team as a code owner January 24, 2022 20:51

aponb mentioned this pull request Jan 24, 2022

replacing blacklist by blocklist and whitelist by allowlist #744

Closed

nknize added the enhancement Enhancement or improvement to existing feature or request label Jan 26, 2022

tlfeng added v1.3.0 v2.0.0 Version 2.0.0 labels Jan 27, 2022

tlfeng reviewed Jan 27, 2022

View reviewed changes

tlfeng added the backport 1.x label Jan 27, 2022

aponb force-pushed the main branch from 842bb8b to 3282447 Compare February 15, 2022 21:11

aponb closed this Feb 20, 2022

aponb force-pushed the main branch from 3282447 to 5b3b984 Compare February 20, 2022 12:12

aponb mentioned this pull request Feb 20, 2022

Replace exclusionary words whitelist and blacklist in the places that won't impact backwards compatibility #2178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replacing the exclusionary words whitelist and blacklist that won't impact the compatibility. #1970

Replacing the exclusionary words whitelist and blacklist that won't impact the compatibility. #1970

aponb commented Jan 24, 2022

opensearch-ci-bot commented Jan 24, 2022

opensearch-ci-bot commented Jan 24, 2022

nknize commented Jan 26, 2022

tlfeng Jan 27, 2022

tlfeng commented Jan 27, 2022

tlfeng commented Jan 27, 2022

aponb commented Jan 27, 2022

aponb commented Jan 27, 2022

opensearch-ci-bot commented Jan 27, 2022

tlfeng commented Jan 27, 2022 •

edited

Loading

aponb commented Jan 27, 2022

opensearch-ci-bot commented Jan 27, 2022

opensearch-ci-bot commented Jan 27, 2022

tlfeng commented Jan 27, 2022

tlfeng commented Feb 14, 2022 •

edited

Loading

opensearch-ci-bot commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

tlfeng commented Feb 15, 2022 •

edited

Loading

tlfeng commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

aponb commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

aponb commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

tlfeng commented Feb 15, 2022 •

edited

Loading

tlfeng commented Feb 18, 2022 •

edited

Loading

aponb commented Feb 20, 2022

tlfeng commented Feb 20, 2022

aponb commented Feb 21, 2022 •

edited

Loading

Replacing the exclusionary words whitelist and blacklist that won't impact the compatibility. #1970

Replacing the exclusionary words whitelist and blacklist that won't impact the compatibility. #1970

Conversation

aponb commented Jan 24, 2022

Description

Issues Resolved

opensearch-ci-bot commented Jan 24, 2022

opensearch-ci-bot commented Jan 24, 2022

nknize commented Jan 26, 2022

tlfeng Jan 27, 2022

Choose a reason for hiding this comment

tlfeng commented Jan 27, 2022

tlfeng commented Jan 27, 2022

aponb commented Jan 27, 2022

aponb commented Jan 27, 2022

opensearch-ci-bot commented Jan 27, 2022

tlfeng commented Jan 27, 2022 • edited Loading

aponb commented Jan 27, 2022

opensearch-ci-bot commented Jan 27, 2022

opensearch-ci-bot commented Jan 27, 2022

tlfeng commented Jan 27, 2022

tlfeng commented Feb 14, 2022 • edited Loading

opensearch-ci-bot commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

tlfeng commented Feb 15, 2022 • edited Loading

tlfeng commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

aponb commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

aponb commented Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

tlfeng commented Feb 15, 2022 • edited Loading

tlfeng commented Feb 18, 2022 • edited Loading

aponb commented Feb 20, 2022

tlfeng commented Feb 20, 2022

aponb commented Feb 21, 2022 • edited Loading

tlfeng commented Jan 27, 2022 •

edited

Loading

tlfeng commented Feb 14, 2022 •

edited

Loading

tlfeng commented Feb 15, 2022 •

edited

Loading

tlfeng commented Feb 15, 2022 •

edited

Loading

tlfeng commented Feb 18, 2022 •

edited

Loading

aponb commented Feb 21, 2022 •

edited

Loading