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.

Sign up for GitHub

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

Jump to bottom

Remove endpoint_suffix dependency on account key #2485

Merged
merged 1 commit into from
Apr 5, 2022
Merged

Remove endpoint_suffix dependency on account key #2485

merged 1 commit into from
Apr 5, 2022

Conversation

raphlopez
Copy link
Contributor

Signed-off-by: Raphael Lopez [email protected]

Description

Previously, the repository-azure plugin failed to load when a SAS token was provided for authentication rather than an Azure Storage Account key. This was due to a dependency in the ENDPOINT_SUFFIX_SETTING - more details in the issue description.

I opted to remove the dependency altogether since the plugin has a check in buildConnectString() to ensure that either a SAS token or a Storage Account key is provided.

Issues Resolved

Relates to #2462

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5e87223
Log 3443

Reports 3443

@raphlopez raphlopez marked this pull request as ready for review March 17, 2022 17:35
@raphlopez raphlopez requested a review from a team as a code owner March 17, 2022 17:35
@nknize
Copy link
Collaborator

nknize commented Mar 21, 2022

Another flake documented in #1703; refiring

REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=5F38717E6653A507 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-BO -Dtests.timezone=America/Chicago -Druntime.java=17

org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([5F38717E6653A507:584EA4B55DD5D35A]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)

@nknize
Copy link
Collaborator

nknize commented Mar 21, 2022

start gradle check

nknize
nknize approved these changes Mar 21, 2022
View reviewed changes
Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

LGTM. Thx!

@nknize nknize added the v2.0.0 Version 2.0.0 label Mar 21, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5e87223
Log 3595

Reports 3595

@dblock
Copy link
Member

dblock commented Mar 21, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5e87223
Log 3622

Reports 3622

@tlfeng
Copy link
Collaborator

tlfeng commented Apr 2, 2022

3622 is issue #1703
start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 5e87223
Log 4065

Reports 4065

@tlfeng tlfeng added Plugins enhancement Enhancement or improvement to existing feature or request v2.1.0 Issues and PRs related to version 2.1.0 v3.0.0 Issues and PRs related to version 3.0.0 backport 2.x Backport to 2.x branch and removed v2.0.0 Version 2.0.0 labels Apr 2, 2022
@tlfeng
Copy link
Collaborator

tlfeng commented Apr 2, 2022

Thank you for providing the solution! Just double check, will this code change resolves issue #2462?

@raphlopez
Copy link
Contributor Author

My pleasure! And yes, #2462 will be resolved once this is merged.

@dblock dblock merged commit 365e07c into opensearch-project:main Apr 5, 2022
@dblock dblock added the backport 2.0 Backport to 2.0 branch label Apr 5, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 5, 2022
@raphlopez @github-actions
Remove endpoint_suffix dependency on account key (#2485)
3afcb28 
Signed-off-by: Raphael Lopez <[email protected]>
(cherry picked from commit 365e07c)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Apr 5, 2022
Closed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 5, 2022
@raphlopez @github-actions
Remove endpoint_suffix dependency on account key (#2485)
8d923c4 
Signed-off-by: Raphael Lopez <[email protected]>
(cherry picked from commit 365e07c)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Apr 5, 2022
Closed
raphlopez added a commit to raphlopez/OpenSearch that referenced this pull request Apr 7, 2022
@raphlopez
Remove endpoint_suffix dependency on account key (opensearch-project#…
8d131be 
…2485)

Signed-off-by: Raphael Lopez <[email protected]>
(cherry picked from commit 365e07c)
@raphlopez raphlopez mentioned this pull request Apr 7, 2022
Merged
raphlopez added a commit to raphlopez/OpenSearch that referenced this pull request Apr 7, 2022
@raphlopez
Remove endpoint_suffix dependency on account key (opensearch-project#…
f129c78 
…2485)

Signed-off-by: Raphael Lopez <[email protected]>
(cherry picked from commit 365e07c)
@raphlopez raphlopez mentioned this pull request Apr 7, 2022
Merged
dblock pushed a commit that referenced this pull request Apr 11, 2022
@raphlopez
Remove endpoint_suffix dependency on account key (#2485) (#2807)
fa43294 
Signed-off-by: Raphael Lopez <[email protected]>
(cherry picked from commit 365e07c)
dblock pushed a commit that referenced this pull request Apr 11, 2022
@raphlopez
Remove endpoint_suffix dependency on account key (#2485) (#2808)
dfbf801 
Signed-off-by: Raphael Lopez <[email protected]>
(cherry picked from commit 365e07c)
@raphlopez raphlopez mentioned this pull request Apr 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nknize nknize nknize approved these changes

Assignees
No one assigned
Labels
backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch enhancement Enhancement or improvement to existing feature or request Plugins v2.1.0 Issues and PRs related to version 2.1.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@raphlopez @opensearch-ci-bot @nknize @dblock @tlfeng