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

feat(router-proxy): added support for proxy endpoint #855

Closed

Conversation

carloseustaquio
Copy link

Description

Implements proxy method in the Router interface that uses the proxy option of hapi.js server. Allow clients to redirect app requests to a custom backend server.

Issues Resolved

  1. Add proxy option on Router #854

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

@opensearch-ci-bot
Copy link
Collaborator

❌   DCO Check Failed 45b6937
Run ./dev-tools/signoff-check.sh remotes/origin/main 45b6937b557deec19fa568e0698655dc83d16dc6 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

@rios0rios0
Copy link

Many thanks!

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 349d346

@kavilla kavilla linked an issue Oct 11, 2021 that may be closed by this pull request
@kavilla
Copy link
Member

kavilla commented Oct 13, 2021

Thanks for opening this will check it out as soon as possible.

@tmarkley tmarkley requested a review from boktorbb October 14, 2021 18:26
@boktorbb
Copy link
Contributor

Thanks for the contribution! I will check this out sometime today

@boktorbb
Copy link
Contributor

@carloseustaquio would it be possible to get more information about the security implications here? More information can be added in your issue or an RFC issue can be opened for a more detailed breakdown

@boktorbb
Copy link
Contributor

@carloseustaquio just checking in on the security information

@rios0rios0
Copy link

@carloseustaquio just checking in on the security information

@boktorbb-amzn we will answer you and @kavilla. At the moment we are running to deliver another feature. When we have time, we will attend to your questions. Give some time, please.

@tmarkley
Copy link
Contributor

@carloseustaquio @rios0rios0 any updates? Just want to make sure this PR doesn't stay stale for too long.

@rios0rios0
Copy link

@carloseustaquio @rios0rios0 any updates? Just want to make sure this PR doesn't stay stale for too long.

We don't know the kind of security information / RFC that @boktorbb-amzn needs.
We're exposing the HAPI feature in a different way. Because in our case, we need a plugin that forward requests to another backend. We are using OSD authentication only. So, we created an interface that some backend plugins can implement, registering the routes.

With this PR, we can do:

router.proxy({
  path: '<SOME_PATH_HERE>',
  proxyPath: '<DESTINATION_PATH_HERE>',
  validate: false,
  additionalHeaders: async request => {
    <SOME AUTH STUFF>
  },
  onResponse: (_err, res, req) => {
    <SOME LOGGIN STUFF>
    return res;
  },
});

What do we need to test to give make sure that we aren't violating security?

@boktorbb
Copy link
Contributor

@rios0rios0 your last comment actually addressed my concern. The PR looks good to me

@tmarkley
Copy link
Contributor

So, we created an interface that some backend plugins can implement, registering the routes.

Can we add documentation for this? We should address the [ ] New functionality has been documented. step.

@kavilla
Copy link
Member

kavilla commented Dec 12, 2021

Hello @carloseustaquio, this pull request has not had activity recently. We will close this in 7 days if no further updates. If it closes by the time you make the requested updates then please re-open the pull request. Thank you!

@kavilla kavilla closed this Dec 20, 2021
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.

Add proxy option on Router
6 participants