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

fix(ui) Fix navigation between customer-domain accounts #44625

Merged
merged 3 commits into from
Feb 15, 2023

Conversation

markstory
Copy link
Member

Improve navigation and subdomain support for yarn dev-ui. With these changes the following works:

Switching between organizations will also work, as domains will be rewritten based on the initial page configuration.

Users of dev-ui will still need to clone their session cookies and set host file entries if they want to use acme.localhost domains.

Refs HC-429

Improve navigation and subdomain support for `yarn dev-ui`. With these
changes the following works:

- https://acme.localhost:7999/issues/
- https://acme.dev.getsentry.net:7999/issues/
- https://acme.abc1243.sentry.dev/issues/

Switching between organizations will also work, as domains will be
rewritten based on the initial page configuration.

Users of dev-ui will still need to clone their session cookies and set
host file entries if they want to use `acme.localhost` domains.

Refs HC-429
@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
src/sentry/static/sentry/dist/entrypoints/app.js 20 KB (-0.02% 🔽)
src/sentry/static/sentry/dist/entrypoints/sentry.css 32.99 KB (0%)

}
const host = window.location.host;
const validHostPattern =
/((?:localhost|[^.]+\.sentry\.dev|dev\.getsentry\.net)(?:\:\d*)?)/;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be some kind of constant, we have a similar regex above

Copy link
Member Author

Choose a reason for hiding this comment

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

The capturing groups are different between the patterns though. The one in useResolveRoute() needs to extract the slug, while this function needs to match the trailing domain + port.

I can try replacing the regex patterns with split on . operations.

Copy link
Member

@priscilawebdev priscilawebdev left a comment

Choose a reason for hiding this comment

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

code looks good to me. Thanks for fixing it @markstory 🥔 🙏

@priscilawebdev
Copy link
Member

I can confirm that https://acme.localhost:7999/issues/ works...but it's no longer possible to switch organizations anymore because of the domain change... is there anything we can do to make this possible again?

@markstory
Copy link
Member Author

is there anything we can do to make this possible again?

Yes when you clone your cookies into localhost make sure to set the domain attribute to localhost and not acme.localhost. That will allow the cookie to be shared across all of the subdomains.

@markstory markstory merged commit 0fb31f9 into master Feb 15, 2023
@markstory markstory deleted the fix-devui-domains branch February 15, 2023 16:38
wmak pushed a commit that referenced this pull request Feb 16, 2023
Improve navigation and subdomain support for `yarn dev-ui`. With these
changes the following works:

- https://acme.localhost:7999/issues/
- https://acme.dev.getsentry.net:7999/issues/
- https://acme.abc1243.sentry.dev/issues/

Switching between organizations will also work, as domains will be
rewritten based on the initial page configuration.

Users of dev-ui will still need to clone their session cookies and set
host file entries if they want to use `acme.localhost` domains.

Refs HC-429
jjbayer added a commit that referenced this pull request Feb 17, 2023
The change to `devServer.allowedHost` made in
#44625 caused Relay to fail
authentication in our dev setup:

```
relay   2023-02-16T15:42:01Z [relay_server::actors::upstream] INFO: registering with upstream (http://host.docker.internal:8000/)
relay   2023-02-16T15:42:01Z [relay_server::actors::upstream] ERROR: authentication encountered error: could not send request
relay     caused by: failed to parse JSON response
relay     caused by: expected value at line 1 column 1
```

because the dev server responded with the string `Invalid Host header`.

I'm not sure why webpack is even in the loop for API requests, but
adding the `.docker.internal` suffix to `allowedHosts` seems to fix the
problem.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants