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

adding proxy configuration #20231107 #20230503 in 4314 issue #6952

Merged
merged 12 commits into from
May 16, 2024

Conversation

AntonEliatra
Copy link
Contributor

Description

Adding details on proxy configuration for cross cluster search and basic details for proxy configuration itself using Nginx

Issues Resolved

#20231107 - /docs/latest/security/authentication-backends/proxy/ | Even though everything is set, I am still facing 401 error
#20230503 - /docs/latest/security/access-control/cross-cluster-search/ |What about other details? What if my port is proxied?

Checklist

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

@hdhalter hdhalter added security 4 - Doc review PR: Doc review in progress backport 2.13 PR: Backport label for 2.13 3 - Tech review PR: Tech review in progress and removed 4 - Doc review PR: Doc review in progress labels Apr 15, 2024
Copy link
Contributor

@stephen-crawford stephen-crawford left a comment

Choose a reason for hiding this comment

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

This looks reasonable, but I don't know anything about cross cluster search of the proxy configuration being discussed so let's just make sure we are extra confident in what is shared.

@AntonEliatra
Copy link
Contributor Author

Thanks @scrawfor99 I tested this fully, using 2 separate clusters prior to raising the PR, therefore can confirm this does indeed works as described

@hdhalter
Copy link
Contributor

Thanks, @scrawfor99. Let's have someone from the cluster team take a look. I'll ping @rohiwali.

@hdhalter
Copy link
Contributor

hdhalter commented Apr 23, 2024

Hi @natebower , when we come across a proper noun that is getting flagged by Vale, do we always add it to our approved list or handle some other way? This is the name getting flagged: https://www.nginx.com/. Thanks!

Update: I just checked our style guide and this is already included as an accepted term, just capitalized.

Copy link
Contributor

@hdhalter hdhalter left a comment

Choose a reason for hiding this comment

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

Thanks, @AntonEliatra . Here are some wording suggestions and fixes to the Vale errors.

_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>
Copy link
Member

@ankitkala ankitkala left a comment

Choose a reason for hiding this comment

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

Few minor comments. Rest LGTM!

_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
@AntonEliatra
Copy link
Contributor Author

Thank you @ankitkala, that's been updated now

_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
@hdhalter hdhalter added 4 - Doc review PR: Doc review in progress and removed 3 - Tech review PR: Tech review in progress labels Apr 25, 2024
@hdhalter
Copy link
Contributor

Thanks for your thorough review, @ankitkala ! I'll push this through to doc review.

_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>
Copy link
Collaborator

@Naarcha-AWS Naarcha-AWS left a comment

Choose a reason for hiding this comment

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

One small comment but otherwise LGTM

_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
@Naarcha-AWS Naarcha-AWS added 5 - Editorial review PR: Editorial review in progress and removed 4 - Doc review PR: Doc review in progress labels May 1, 2024
Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>
Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@AntonEliatra @Naarcha-AWS Please see my comments and changes and let me know if you have any questions. Thanks!

_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
_search-plugins/cross-cluster-search.md Outdated Show resolved Hide resolved
AntonEliatra and others added 2 commits May 10, 2024 08:54
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>
@hdhalter hdhalter added backport 2.14 PR: Backport label for 2.14 and removed backport 2.13 PR: Backport label for 2.13 labels May 15, 2024
@hdhalter hdhalter added 3 - Done Issue is done/complete and removed 5 - Editorial review PR: Editorial review in progress labels May 16, 2024
@hdhalter hdhalter merged commit 00fa73e into opensearch-project:main May 16, 2024
6 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 16, 2024
* adding proxy configuration #20231107 #20230503 in 4314 issue

Signed-off-by: AntonEliatra <[email protected]>

* adding proxy configuration #20231107 #20230503 in 4314 issue

Signed-off-by: AntonEliatra <[email protected]>

* Update cross-cluster-search.md

Signed-off-by: AntonEliatra <[email protected]>

* Apply suggestions from code review

Co-authored-by: Heather Halter <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

* adding prerequisite to cross-cluster page

Signed-off-by: AntonEliatra <[email protected]>

* typo on cross-cluster page

Signed-off-by: AntonEliatra <[email protected]>

* Apply suggestions from code review

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

* Update _search-plugins/cross-cluster-search.md

Co-authored-by: Naarcha-AWS <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

* Apply suggestions from code review

Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: AntonEliatra <[email protected]>

* updating proxy details as per comments

Signed-off-by: AntonEliatra <[email protected]>

* Update cross-cluster-search.md

Signed-off-by: Naarcha-AWS <[email protected]>

---------

Signed-off-by: AntonEliatra <[email protected]>
Signed-off-by: Naarcha-AWS <[email protected]>
Co-authored-by: Heather Halter <[email protected]>
Co-authored-by: Stephen Crawford <[email protected]>
Co-authored-by: Naarcha-AWS <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
(cherry picked from commit 00fa73e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done Issue is done/complete backport 2.14 PR: Backport label for 2.14 security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants