refactor: Allow superset to be deployed under a prefixed URL #30134

martyngigg · 2024-09-03T11:22:57Z

SUMMARY

Introduces changes in the front and backend to allow Superset to be deployed under a prefixed-URL path rather than the root of the domain.

The backend superset.app:create_app has been augemented with a new app_root argument that accepts a root path for the application. This is passed through the FLASK_APP environment variable or the flask --app cli argument. When the path is not / a middleware is created that intercepts paths and updates them to strip the app_root and forward them to the rest of the application. This was inspired by Application Dispatching.

The backend has also been refactored to avoid using hardcoded paths in redirects and instead uses Flask.url_for to construct all references.

The frontend has been updated to include the prefix in any resource links along with two new helper utilities, assetUrl & pathUtils, to help construct the paths. The SupersetClient has been updated to remove the unused baseUrl field and rename it as basePath such that the object can be initialized with the base path and calling any endpoints through this class can use the same relative link as before. QUESTION: Have I broken an API here?

Documentation has been added to the "Configuring Superset" pages that demonstrate how to use the new arugment.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

rusackas · 2024-09-05T15:45:04Z

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

martyngigg · 2024-09-09T09:52:22Z

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts

    );
-    this.baseUrl = url.href.replace(/\/+$/, ''); // always strip trailing slash
+    this.basePath = basePath.replace(/\/+$/, ''); // always strip trailing slash


mistercrunch · 2024-09-19T05:39:27Z

I think this may be quite an undertaking and might have to be tackled in phases. Would love to see this done :)

I remember giving it a shot early on in the project at least once, and never wrapped up the work. My main advice would be to avoid taking in side mission or bigger refactor while tackling this.

mistercrunch · 2024-09-19T05:43:02Z

docker/docker-frontend.sh

@@ -17,19 +17,23 @@
 #
 set -e

+# zstd exe is required by simple-zstd package
+apt-get update
+apt-get install -y zstd


there's a recent fix in master for this

ravikantkml · 2024-09-19T15:49:42Z

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes.
Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

kasper-rutten · 2024-10-03T12:34:14Z

For what it's worth, we've been running this fork to serve superset on a path prefix in a Kubernetes(K3S) + traefik context without any problems so far. Though testing has been limited.

Thanks for the work so far

martyngigg · 2024-10-15T14:56:50Z

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes. Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

Sorry for the delay in replying to this. I've had zero time to spend on this lately.

The latest commits rebase on the current master and I have also added some documentation to the configuring superset pages that describe the new variables with an example nginx.conf. Does that help?

martyngigg · 2024-10-15T14:56:54Z

@rusackas I think this is ready for review but in terms of it being related to SIP-112 is that the right thing to do in terms of the process?

rusackas · 2024-10-28T17:02:12Z

If you think it's ready for review, I'd say you can go ahead and move it from Draft to "Ready for Review" - though it looks like CI might need a little more love still.

As for the SIP, since @sfirke reopened the GitHub issue, I set the SIP itself back to "pre-discussion" state for its official consensus. Did you want to re-kindle that pre-existing SIP, or would it be easier to open a new one? I'm happy to help you carry that SIP through to fruition if you'd like. Feel free to DM me on slack if it's easier.

ascendlin · 2024-10-30T06:40:30Z

When clicking on the user list or role list, and then clicking on datasets, a 404 error appears.

ravikantkml · 2024-11-04T13:02:00Z

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes. Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

Sorry for the delay in replying to this. I've had zero time to spend on this lately.

The latest commits rebase on the current master and I have also added some documentation to the configuring superset pages that describe the new variables with an example nginx.conf. Does that help?

Thanks a lot for the response.
We tried the changes as recommended and observed that most of the things are working when we specify prefix. However there are few things where prefix is not applied correctly as shown in the image below (chart menu has prefix applied correctly)

Also dashboard menu items export to PDF/image not working (Dashboard -> Download -> Export to PDF/Download as Image)

ravikantkml · 2024-11-11T15:15:40Z

Thanks for tackling this... it's a popular request, no doubt about it. If you think it's relevant, we might also want to add an entry in the documentation somewhere.

Yes agreed I definitely need to add some documentation for how to use this. I wanted to wait until I was more confident that the general approach was acceptable before working on describing how to use it. In terms of a review as part of #26319 is there anything I need to do at the moment?

Thanks for these changes. Can you please mention values for BASE_PATH and ASSET_BASE_URL. Just wanted to check these changes with superset and nginx.

Sorry for the delay in replying to this. I've had zero time to spend on this lately.
The latest commits rebase on the current master and I have also added some documentation to the configuring superset pages that describe the new variables with an example nginx.conf. Does that help?

Thanks a lot for the response. We tried the changes as recommended and observed that most of the things are working when we specify prefix. However there are few things where prefix is not applied correctly as shown in the image below (chart menu has prefix applied correctly)

Also dashboard menu items export to PDF/image not working (Dashboard -> Download -> Export to PDF/Download as Image)

I tried to fix above issue of missing prefixes in top-level menu. Please check this PR 2

Dinesh-4320 · 2024-11-22T08:00:13Z

The solution really works. Solved a long term issue in our deployment behind proxies. Hoping to get merged soon with official code base

harri2012 · 2024-11-29T09:55:50Z

Very good, looking forward to merging the code soon

watercraft · 2024-12-11T16:51:53Z

For whom it may concern, I'm testing with a port of this work to 3.1.1rc1 available from watercraft:superset branch prefix

jeanpommier · 2024-12-19T09:31:28Z

Hi,
Good job there, thank you very much for tackling this. I have just discovered that Superset did not support that and was feeling disappointed.

I can see this PR is still flagged as DRAFT. What's left to do to get it "Ready for Review" ?

@martyngigg do you need help in completing this PR ? I'm quite a superset newbie, but I'm willing to provide some help if necessary to get this merged.

Docker version of superset_config sets ENABLE_PROXY_FIX=True if BASE_PATH not empty.

…ntext

Also simplifies Nginx server block so it no longer needs modifying when a prefix is set or not - simply set the BASE_PATH/ASSET_BASE_URL environment variables when running docker compose and it will take case of the rest.

If not an empty string, a new middleware class is attached to the wsgi_app callable that intercepts each request and transforms the SCRIPT_NAME & PATH_INFO variables in the request environment so that the rest of the code still sees non-prefixed paths yet the application itself appears to sit on a common subpath. A new environment variable SUPERSET_APP_ROOT has been introduced into the docker config to control the value of the application root.

Adjusted Flask app templates to include the static_assets_prefix and webpack build dir while rendering. This removes the need to rebuild the frontend in order to Superset it at a different application prefix.

korbit-ai · 2025-01-20T17:38:10Z

Korbit doesn't automatically review large (500+ lines changed) pull requests such as this one. If you want me to review anyway, use /korbit-review.

superset-frontend/src/dashboard/components/SaveModal.tsx

+          window.location.href = ensureBasePath(
+            `/superset/dashboard/${resp.json.result.id}/`,
+          );


superset-frontend/src/features/home/ChartTable.tsx

@@ -193,7 +194,7 @@
            ),
            buttonStyle: 'tertiary',
            onClick: () => {
-              window.location.assign('/chart/add');
+              window.location.assign(ensureBasePath('/chart/add'));


superset-frontend/src/features/home/DashboardTable.tsx

@@ -196,19 +197,20 @@
            ),
            buttonStyle: 'tertiary',
            onClick: () => {
-              window.location.assign('/dashboard/new');
+              window.location.assign(ensureBasePath('/dashboard/new'));


superset-frontend/src/features/home/EmptyState.tsx

@@ -171,7 +173,7 @@
        <Button
          buttonStyle="primary"
          onClick={() => {
-            window.location.href = favRedirects[tableName];
+            window.location.href = ensureBasePath(favRedirects[tableName]);


superset-frontend/src/features/home/Menu.tsx

@@ -313,11 +314,11 @@
          >
            {isFrontendRoute(window.location.pathname) ? (
              <GenericLink className="navbar-brand" to={brand.path}>
-                <img src={brand.icon} alt={brand.alt} />
+                <img src={assetUrl(brand.icon)} alt={brand.alt} />


superset-frontend/src/features/home/RightMenu.tsx

@@ -460,7 +461,7 @@
                        {menu.label}
                      </Link>
                    ) : (
-                      <a href={menu.url}>
+                      <a href={ensureBasePath(menu.url || '')}>


superset-frontend/src/features/home/RightMenu.tsx

@@ -495,7 +496,9 @@
                      {isFrontendRoute(child.url) ? (
                        <Link to={child.url || ''}>{menuItemDisplay}</Link>
                      ) : (
-                        <a href={child.url}>{menuItemDisplay}</a>
+                        <a href={ensureBasePath(child.url || '')}>


superset-frontend/src/features/home/SavedQueries.tsx

@@ -264,7 +266,7 @@
            name: t('View All »'),
            buttonStyle: 'link',
            onClick: () => {
-              window.location.href = '/savedqueryview/list';
+              window.location.href = ensureBasePath('/savedqueryview/list');


superset-frontend/src/pages/AnnotationLayerList/index.tsx

@@ -277,7 +278,7 @@
  };

  const onLayerAdd = (id?: number) => {
-    window.location.href = `/annotationlayer/${id}/annotation`;
+    window.location.href = ensureBasePath(`/annotationlayer/${id}/annotation`);


superset-frontend/src/pages/DashboardList/index.tsx

@@ -682,7 +683,7 @@
      ),
      buttonStyle: 'primary',
      onClick: () => {
-        window.location.assign('/dashboard/new');
+        window.location.assign(ensureBasePath('/dashboard/new'));


pull-request-size bot added the size/L label Sep 3, 2024

github-actions bot added api Related to the REST API packages labels Sep 3, 2024

martyngigg mentioned this pull request Sep 3, 2024

[SIP-112] Allow to deploy Superset under the prefix of an URL #26319

Open

michael-s-molina added the review:draft label Sep 3, 2024

michael-s-molina requested a review from dpgaspar September 3, 2024 17:44

github-advanced-security bot found potential problems Sep 9, 2024

View reviewed changes

mistercrunch reviewed Sep 19, 2024

View reviewed changes

martyngigg force-pushed the allow_prefixed_paths branch from 93d66c7 to 09a05b3 Compare October 15, 2024 14:23

pull-request-size bot added size/XL and removed size/L labels Oct 15, 2024

github-actions bot added the doc Namespace | Anything related to documentation label Oct 15, 2024

martyngigg force-pushed the allow_prefixed_paths branch from 09a05b3 to fe3b01a Compare October 15, 2024 14:51

pull-request-size bot added size/L and removed size/XL labels Oct 15, 2024

martyngigg added 20 commits January 20, 2025 15:56

Add ASSET_BASE_URL and BASE_PATH arguments to Docker build.

6270846

Docker version of superset_config sets ENABLE_PROXY_FIX=True if BASE_PATH not empty.

Add documentation on running Superset on a prefix path

10d6e51

Fix SliceAdder create new chart link

d2d729b

Fix resource export api link

7a10e26

Remove creating a test request context that hides the real request co…

109fe29

…ntext

Configure WEBDRIVER_BASEURL to go through the reverse proxy

b5f52d4

Set thumbnail cache to redis cache for docker setup

453fcee

Fix non-frontend route links in RightMenu when base prefix present.

b7a91bd

Use environment variables for base path in docker compose

fa104bf

Also simplifies Nginx server block so it no longer needs modifying when a prefix is set or not - simply set the BASE_PATH/ASSET_BASE_URL environment variables when running docker compose and it will take case of the rest.

Pass application_root and static_assets_prefix through to frontend.

422d12b

Adjusted Flask app templates to include the static_assets_prefix and webpack build dir while rendering. This removes the need to rebuild the frontend in order to Superset it at a different application prefix.

Set a default SUPERSET_APP_ROOT if not defined.

4dfc11b

Remove BASE_PATH from Dockerfile and docker-compose

d7adf61

Set the default app_root to "/" to match Flask APPLICATION_ROOT

3617317

Remove BASE_PATH from frontend.

c676994

Fix Dockerfile health check.

1ed1efe

Rewrite documentation on deploying Superset on a non-root prefix.

8848824

Fix mypy errors in app.py

00edad2

Fix backend formatting.

392e0fd

Hit frontend code with prettier

e0eb2ed

martyngigg force-pushed the allow_prefixed_paths branch from 9b77478 to e0eb2ed Compare January 20, 2025 17:37

pull-request-size bot added size/XL and removed size/L labels Jan 20, 2025

github-actions bot removed the dependencies:npm label Jan 20, 2025

martyngigg marked this pull request as ready for review January 20, 2025 17:38

dosubot bot added change:backend Requires changing the backend change:frontend Requires changing the frontend labels Jan 20, 2025

Add Apache license to Nginx template file.

0774c7a

github-advanced-security bot found potential problems Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Allow superset to be deployed under a prefixed URL #30134

refactor: Allow superset to be deployed under a prefixed URL #30134

martyngigg commented Sep 3, 2024 •

edited

Loading

rusackas commented Sep 5, 2024

martyngigg commented Sep 9, 2024

mistercrunch commented Sep 19, 2024 •

edited

Loading

mistercrunch Sep 19, 2024

ravikantkml commented Sep 19, 2024

kasper-rutten commented Oct 3, 2024

martyngigg commented Oct 15, 2024

martyngigg commented Oct 15, 2024

rusackas commented Oct 28, 2024

ascendlin commented Oct 30, 2024

ravikantkml commented Nov 4, 2024

ravikantkml commented Nov 11, 2024

Dinesh-4320 commented Nov 22, 2024

harri2012 commented Nov 29, 2024

watercraft commented Dec 11, 2024

jeanpommier commented Dec 19, 2024

korbit-ai bot commented Jan 20, 2025

refactor: Allow superset to be deployed under a prefixed URL #30134

Are you sure you want to change the base?

refactor: Allow superset to be deployed under a prefixed URL #30134

Conversation

martyngigg commented Sep 3, 2024 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

rusackas commented Sep 5, 2024

martyngigg commented Sep 9, 2024

mistercrunch commented Sep 19, 2024 • edited Loading

mistercrunch Sep 19, 2024

Choose a reason for hiding this comment

ravikantkml commented Sep 19, 2024

kasper-rutten commented Oct 3, 2024

martyngigg commented Oct 15, 2024

martyngigg commented Oct 15, 2024

rusackas commented Oct 28, 2024

ascendlin commented Oct 30, 2024

ravikantkml commented Nov 4, 2024

ravikantkml commented Nov 11, 2024

Dinesh-4320 commented Nov 22, 2024

harri2012 commented Nov 29, 2024

watercraft commented Dec 11, 2024

jeanpommier commented Dec 19, 2024

korbit-ai bot commented Jan 20, 2025

martyngigg commented Sep 3, 2024 •

edited

Loading

mistercrunch commented Sep 19, 2024 •

edited

Loading