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

ui: all xhr paths from db console are now relative #109694

Merged

Conversation

dhartunian
Copy link
Collaborator

@dhartunian dhartunian commented Aug 29, 2023

This commit removes all prefix / characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path.

Resolves: cockroachdb/helm-charts#228
Resolves: #91429

Epic CRDB-21265

Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.

@dhartunian dhartunian requested review from kpatron-cockroachlabs and a team August 29, 2023 19:23
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@dhartunian dhartunian requested review from a team and Santamaura and removed request for a team August 29, 2023 19:24
@dhartunian dhartunian force-pushed the fix-db-console-relative-routes branch from 54be61f to d57b907 Compare August 29, 2023 20:30
Copy link
Contributor

@Santamaura Santamaura left a comment

Choose a reason for hiding this comment

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

Reviewed 17 of 21 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @kpatron-cockroachlabs)

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Can you confirm all pages are working with these changes? With a loom or something like that

Reviewed 17 of 21 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @kpatron-cockroachlabs)

@dhartunian
Copy link
Collaborator Author

This commit removes all prefix `/` characters from request paths in
the DB Console and Cluster UI codebases. This ensures that if the
DB Console is proxied at a subpath the requests continue to work as
expected and are relative to the correct base path.

Resolves: cockroachdb/helm-charts#228
Resolves: cockroachdb#91429

Epic: None

Release note (ops change, ui change): The DB Console now constructs
client-side requests using relative URLs instead of absolute ones.
This enables proxying of the DB Console at arbitrary subpaths.
@dhartunian dhartunian force-pushed the fix-db-console-relative-routes branch from d57b907 to 727d847 Compare September 6, 2023 19:04
@dhartunian dhartunian added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Sep 6, 2023
Copy link
Contributor

@zachlite zachlite left a comment

Choose a reason for hiding this comment

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

Looks great. Appreciate the loom.
:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @kpatron-cockroachlabs, @maryliag, and @Santamaura)

Copy link
Contributor

@j82w j82w left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @kpatron-cockroachlabs, @maryliag, and @Santamaura)

@abarganier
Copy link
Contributor

@dhartunian There's been a request for a v22.x backport here: https://github.com/cockroachlabs/support/issues/2504

Just letting you know for awareness. I'll leave the backport strategy up to you, of course 🙂

@dhartunian
Copy link
Collaborator Author

TFTRs

bors r=santamaura,zachlite,j82w

@craig
Copy link
Contributor

craig bot commented Sep 8, 2023

Build succeeded:

@craig craig bot merged commit 9db41b3 into cockroachdb:master Sep 8, 2023
@blathers-crl
Copy link

blathers-crl bot commented Sep 8, 2023

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 727d847 to blathers/backport-release-22.2-109694: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2.x failed. See errors above.


error creating merge commit from 727d847 to blathers/backport-release-23.1-109694: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
7 participants