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 a section on Response rewriting to Application Gateway details #15750

Merged

Conversation

majakurcius
Copy link
Contributor

Description

Add a section on Response rewriting to Application Gateway details.

Changes proposed in this pull request:

  • Describe how Application Gateway handles redirects within the same App CR and service

Related issue(s)
#14717

@majakurcius majakurcius added area/documentation Issues or PRs related to documentation area/application-connector Issues or PRs related to application connectivity labels Oct 7, 2022
@majakurcius majakurcius added this to the 2.8 milestone Oct 7, 2022
@majakurcius majakurcius self-assigned this Oct 7, 2022
@kyma-bot kyma-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 7, 2022
@netlify
Copy link

netlify bot commented Oct 7, 2022

🥰 Documentation preview ready! 🥰

Name Link
🔨 Latest commit ef2f731
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/63403519ee33780009bb9923
😎 Deploy Preview https://deploy-preview-15750--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor

@janmedrek janmedrek left a comment

Choose a reason for hiding this comment

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

I would also suggest adding some basic information to the user-facing Application Gateway doc. Something along these lines:

Application Gateway supports redirects for the request flows in which the URL host remains unchanged.

@kyma-bot kyma-bot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Oct 7, 2022
@majakurcius majakurcius force-pushed the issue-14717-response-rewriting branch from 15f9892 to f597be2 Compare October 7, 2022 13:21
@kyma-bot kyma-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 7, 2022

## Response rewriting

If during a call to the external system, the target responds with a redirect (`3xx` status code) that points to the URL with the same host and a different path, the `Location` header is modified so that the original target path is replaced with the Application Gateway URL and port, with the sub-path pointing to the called service attached at the end, in this format: `{APP_GATEWAY_URL}:{APP_GATEWAY_PORT}/{APP_NAME}/{SERVICE_NAME}/{SUB-PATH}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If during a call to the external system, the target responds with a redirect (`3xx` status code) that points to the URL with the same host and a different path, the `Location` header is modified so that the original target path is replaced with the Application Gateway URL and port, with the sub-path pointing to the called service attached at the end, in this format: `{APP_GATEWAY_URL}:{APP_GATEWAY_PORT}/{APP_NAME}/{SERVICE_NAME}/{SUB-PATH}`.
If during a call to the external system, the target responds with a redirect (`3xx` status code) that points to the URL with the same host and a different path, the `Location` header is modified so that the original target path is replaced with the Application Gateway URL and port. The sub-path points to the called service attached at the end, in this format: `{APP_GATEWAY_URL}:{APP_GATEWAY_PORT}/{APP_NAME}/{SERVICE_NAME}/{SUB-PATH}`.

I'd split this long sentence :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can't be split this way, because that's not what it means, but you drew my attention to its ambiguity!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the sub-path that's attached and it's important to have it all in once sentence, even though I also hate that it's long :(


If during a call to the external system, the target responds with a redirect (`3xx` status code) that points to the URL with the same host and a different path, the `Location` header is modified so that the original target path is replaced with the Application Gateway URL and port, with the sub-path pointing to the called service attached at the end, in this format: `{APP_GATEWAY_URL}:{APP_GATEWAY_PORT}/{APP_NAME}/{SERVICE_NAME}/{SUB-PATH}`.

This functionality makes the HTTP clients that originally called Application Gateway follow redirects through the Gateway, and not to the service directly. This allows for passing authorization, custom headers, URL parameters, and the body without an issue.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This functionality makes the HTTP clients that originally called Application Gateway follow redirects through the Gateway, and not to the service directly. This allows for passing authorization, custom headers, URL parameters, and the body without an issue.
This functionality makes the HTTP clients that originally called Application Gateway follow redirects through the Gateway, and not to the service directly. This allows for passing authorization, custom headers, URL parameters, and the body.

I'd drop the without an issue part. It sounds like sth could go wrong. We don't want that :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could go wrong if there is no response rewriting.

@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 10, 2022
@kyma-bot kyma-bot merged commit 9b040e7 into kyma-project:main Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/application-connector Issues or PRs related to application connectivity area/documentation Issues or PRs related to documentation hacktoberfest-accepted lgtm Looks good to me! size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants