Skip to content

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

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
dhartunian:fix-db-console-relative-routes
Sep 8, 2023
Merged

ui: all xhr paths from db console are now relative#109694
craig[bot] merged 1 commit intocockroachdb:masterfrom
dhartunian:fix-db-console-relative-routes

Conversation

@dhartunian
Copy link
Copy Markdown
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 a team and kpatron-cockroachlabs August 29, 2023 19:23
@cockroach-teamcity
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only label Sep 6, 2023
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator Author

TFTRs

bors r=santamaura,zachlite,j82w

@craig
Copy link
Copy Markdown
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
Copy Markdown

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 PAST MAINTENANCE SUPPORT: 23.1 patch releases via ER request only

Projects

None yet

7 participants