Fix dynamic base path resolving #1770

klopfdreh · 2021-09-30T08:34:26Z

Addresses issue: #747

The context path is now preserved from the initial call so that if server.servlet.context-path is set the ui automatically adopts it.

I did some initial tests, but would you be so kind and also test the functionality of the ui, so that there are no other issues I didn't figured out, yet.

As you can see in the following screenshots the "demoContext" base href is preserved while navigating.

This is a call against the dashboard uri:

This is a call against the tasks uri:

klopfdreh · 2021-10-04T16:19:20Z

There was an issue during the build with compodoc. The build was mentioning that I should file an issue. I noticed that the version was a bit outdated. I updated it from 1.0.4 to 1.1.15.

Locally I was able to fix this issue and tested it with npm run doc:

klopfdreh · 2021-10-13T04:51:11Z

Any updates to this or anything I can help with? 😀

klopfdreh · 2021-10-25T05:11:43Z

Hey @oodamien - are there any information regarding this PR? We heavily relying on this issue, because of routing in cloud services.

jvalkeal · 2021-10-27T07:57:50Z

I appreciate your efforts if you find a way to do this. Unless I did something wrong by doing a real server build with UI having this PR, it's failing to load. Angular dev instance is a bit different way to run UI.

We have had this issue on table a bit too long and it'd be nice to get it to work but path to make it work has been a little unclear. Angular do have build option to change context path but obviously it's no use if you want it to be dynamic.

jvalkeal · 2021-10-27T08:31:39Z

If I remove <base href="/">from ui/src/index.html so that angular build wont replace it I get things to load and then it fails trying to access http://localhost:9393/security/info.

I'm not sure if removing base href is correct way to do it, if it is then I'd expect we have some of those hardcoded paths in app code itself.

klopfdreh · 2021-10-27T09:20:37Z

Now I see - when the ui is build the base href is injected in a static way during the build time - I will try to find a solution for that. The dev-Runtime uses node.js to serve the content which adjusts the base href regarding the base path the app is accessed.

jvalkeal · 2021-10-27T09:28:52Z

Yeah, if you do ./mvn clean install which would then allow you to build server with a snapshot from local m2, you see something like:

[INFO] Running 'npm run mavenbuild-without-tests' in /home/jvalkealahti/repos/spring/spring-cloud-dataflow-ui/ui
[INFO] 
[INFO] > spring-cloud-dataflow-ui@3.0.0 mavenbuild-without-tests /home/jvalkealahti/repos/spring/spring-cloud-dataflow-ui/ui
[INFO] > node --max_old_space_size=4096 ./node_modules/@angular/cli/bin/ng build --configuration production --output-path ./../target/classes/public/dashboard --base-href /dashboard/

That's where base ref is coming from and getting replaced if it's defined in index.html.

klopfdreh · 2021-10-27T14:25:03Z

Hey @jvalkeal - I provided a slightly bigger change to what I already did 😄- but it is working for me now.

Steps I did to check if everything is working like expected:

Clone spring-cloud-dataflow-server and add the following to the application.yml:

server:
  servlet:
    context-path: /scdf

Run mvn clean install in the spring-cloud-dataflow-ui project
Because it is installed locally through maven I added the dependency of spring-cloud-dataflow-ui to spring-cloud-dataflow-server in the pom.xml

<dependency>
	<groupId>org.springframework.cloud</groupId>
	<artifactId>spring-cloud-dataflow-ui</artifactId>
	<version>3.2.0-SNAPSHOT</version>
</dependency>

Then I started spring-cloud-dataflow-server and opened http://localhost:9393/scdf/dashboard in my browser.

I only noticed some 404 during creation of a task but I guess this is correct because the input validates if a task is already present - right?

I also checked ng build and ng serve --open.

Can you check if this is also working for you?

klopfdreh · 2021-10-27T14:36:40Z

Sorry for the noise - I moved /assets/ into the service and forgot to remove it in the component markup.

jvalkeal · 2021-10-27T14:51:14Z

Thx, I'll check it out.

sabbyanandan · 2021-10-27T15:41:27Z

@klopfdreh: Thanks for picking up this ambiguous portion of the Dashboard! A few team members attempted to solve this, but it warranted breaking changes, so it is incredible to see the progress without dramatic refactoring - kudos to you!

klopfdreh · 2021-10-27T18:13:41Z

@sabbyanandan: I try my best and we also need to have the option to configure the context path. I hope I don't miss any other scenarios.

klopfdreh · 2021-10-28T08:47:08Z

I patched the Spring Cloud Data Flow Server (Version 2.9.0) and applied the Spring Cloud Data Flow UI (3.2.0-SNAPSHOT) of this PR to it. As of the Release Notes those Versions should be compatible.

This patched version I deployed in a cloud environment and did some tests.

I installed Applications and Tasks with DSL which worked perfectly
I started those Tasks via DSL and via UI which also was working without any issues - all properties were listed within the Freetext and within the Builder as expected
The Data Pipeline is also shown as expected without any issues
Because we are using Kubernetes integration I had a look into the Logs via UI and they were also shown as expected

During the navigation I noticed no issues within the dev console of Chrome.

But two little issues are:

If you press Ctrl + R for Browser reload the UI is not shown, but the API, because the dashboard for tasks for example is present at <context-path>/dashboard/#/task-jobs/tasks and not at <context-path>/#/task-jobs/tasks
If you access the dashboard the first time the application entry is not shown initially

If you navigate through the UI normally however there are no other issues I noticed, yet.

klopfdreh · 2021-10-28T09:00:24Z

I think we can fix this if we add a calculateDashboardUrl which applies a /dashboard/ to the base url in url-utilities.service.ts and use this in app-routing.module.ts for Routes > path and in app.module.ts for APP_BASE_HREF

jvalkeal · 2021-10-28T09:53:08Z

ui/src/app/app-routing.module.ts


 const routes: Routes = [
  {
-    path: '',
+    path: UrlUtilities.calculateBaseApiUrl(),


We don't have a "landing page" in UI and I think this was used to redirect to apps page which doesn't happen anymore.

Can you take care of this? Just take the commits from my branch and apply the fixes to it.

jvalkeal · 2021-10-29T11:45:45Z

Right, we've moved repo main branches to next minor dev version(dataflow 2.10.x) which allows to merge something like this which may need further tweaks and other fixes on a server side for other tooling which may still use hardcoded paths, etc. I'm going to polish some things(like linting errors which I didn't want to complain as we moved to eslint and that layer is still a bit broken).

Anyway, thx a lot as this work allows some nice setups. I don't yet know if it's possible backport this to dataflow 2.9.x as we first need to see how well we get i.e. security working as redirects with oauth providers are also usually hardcoded, but that's beyond this base work.

klopfdreh · 2021-10-29T12:25:50Z

Hey, I just forgot to mention that we have OAuth Security enabled within our infrastructure. We use Authorization Code Flow and it redirects also to the right path based on the contextPath. So this is working, too.

I keep an eye on your polish commits so that I see what I broke regarding the linting. During the build I didn’t saw any warnings - maybe it would be good to fail the build if linting reports issues.

jvalkeal · 2021-10-29T14:14:26Z

Ah good to know if you didn't things immediately break up with oauth.

For linting I think we don't currently fail for it but moved to eslint so that we get better support to use prettier and format code but currently at main branch I get:

$ npm run lint

> spring-cloud-dataflow-ui@3.0.0 lint /home/jvalkealahti/repos/jvalkeal/spring-cloud-dataflow-ui/ui
> ng lint


Linting "dataflow"...

All files pass linting.

Schema validation failed with the following errors:
  Data path "" must have required property 'lintFilePatterns'.
npm ERR! code ELIFECYCLE

And that result some real linting errors with your PR. That's what I meant that eslint migration is a still in progress as we should fail PR checks if linter fails. We're trying to get into point were you can run npm run pretty before issuing PR. Don't worry about this now as it's our mess right now.

klopfdreh · 2021-10-29T17:46:50Z

Thanks a lot for information - if I provide any PR in future I am going to run the linting first. 👍

jvalkeal · 2021-11-02T09:56:30Z

I polished this PR a little when merged:

Removed change to compodoc dep as dep updates should not get done with generic PR's
Don't update lock file
Removed use of APP_BASE_HREF which then made LeadingBaseHrefLocationStrategy defunct
Restored path redirect from '' to 'apps'
Fixed most of linting errors and left indentation errors in place as there's
a know default conflict with prettier/eslint which needs to be fixed later
We now get links like http://localhost:9393/scdf/dashboard/index.html#/apps
Hard reload now works
Generic polish

klopfdreh · 2021-11-02T12:33:57Z

Hey @jvalkeal - I just tested the ui from master branch ( commit 5caf4a7 ) with scdf server 2.9.0 and I can confirm that everything is working like a charm, now.

Refresh is working and also the initial redirection to Applications (apps)

Thanks for you help! 😃

Manu10744 · 2022-02-02T10:31:01Z

@klopfdreh
I downloaded the spring-cloud-dataflow-server-2.9.2.jar from https://mavenrepository.com and modified the application.yaml as follows:

server:
    port: 8337
    servlet:
        context-path: /scdf

Though, the dashboard still breaks at my end?

Console logs:

The JSON from http://localhost:8337/scdf

http://localhost:8337/scdf/dashboard only shows a frozen loading indicator and some errors in console:

The jar has spring-cloud-dataflow-ui-3.2.2.jar as dependency, so the fix is included, right? Have i done anything wrong??

klopfdreh · 2022-02-02T16:34:53Z

I just had a look into the 3.2.2 tag and didn't noticed any of my changes - I guess they are not released, yet. @oodamien - I hope I didn't miss anything.

oodamien · 2022-02-07T15:33:08Z

Yes, it's not release yet, it will be include in 2.10 release

klopfdreh added 2 commits September 30, 2021 10:19

Fix dynamic base path resolving

cc1a5a7

fix: formatting and move compodoc to 1.1.15

36b2790

fix: dynamic paths for artifact build

e71d110

klopfdreh added 3 commits October 27, 2021 16:28

fix: minor issue with node asset path

d3abdac

fix: another issue with node asset path

f53daf7

fix: assets at shared shapes

b235b8b

jvalkeal mentioned this pull request Oct 27, 2021

Setting server context-path breaks dashboard #747

Closed

fix: double slash in asset url

889c370

jvalkeal reviewed Oct 28, 2021

View reviewed changes

jvalkeal closed this Nov 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dynamic base path resolving #1770

Fix dynamic base path resolving #1770

klopfdreh commented Sep 30, 2021

klopfdreh commented Oct 4, 2021 •

edited

klopfdreh commented Oct 13, 2021

klopfdreh commented Oct 25, 2021

jvalkeal commented Oct 27, 2021

jvalkeal commented Oct 27, 2021

klopfdreh commented Oct 27, 2021

jvalkeal commented Oct 27, 2021

klopfdreh commented Oct 27, 2021 •

edited

klopfdreh commented Oct 27, 2021

jvalkeal commented Oct 27, 2021

sabbyanandan commented Oct 27, 2021

klopfdreh commented Oct 27, 2021 •

edited

klopfdreh commented Oct 28, 2021 •

edited

klopfdreh commented Oct 28, 2021

jvalkeal Oct 28, 2021

klopfdreh Oct 28, 2021

jvalkeal Oct 29, 2021

jvalkeal commented Oct 29, 2021

klopfdreh commented Oct 29, 2021

jvalkeal commented Oct 29, 2021

klopfdreh commented Oct 29, 2021

jvalkeal commented Nov 2, 2021

klopfdreh commented Nov 2, 2021

Manu10744 commented Feb 2, 2022

klopfdreh commented Feb 2, 2022

oodamien commented Feb 7, 2022

Fix dynamic base path resolving #1770

Fix dynamic base path resolving #1770

Conversation

klopfdreh commented Sep 30, 2021

klopfdreh commented Oct 4, 2021 • edited

klopfdreh commented Oct 13, 2021

klopfdreh commented Oct 25, 2021

jvalkeal commented Oct 27, 2021

jvalkeal commented Oct 27, 2021

klopfdreh commented Oct 27, 2021

jvalkeal commented Oct 27, 2021

klopfdreh commented Oct 27, 2021 • edited

klopfdreh commented Oct 27, 2021

jvalkeal commented Oct 27, 2021

sabbyanandan commented Oct 27, 2021

klopfdreh commented Oct 27, 2021 • edited

klopfdreh commented Oct 28, 2021 • edited

klopfdreh commented Oct 28, 2021

jvalkeal Oct 28, 2021

Choose a reason for hiding this comment

klopfdreh Oct 28, 2021

Choose a reason for hiding this comment

jvalkeal Oct 29, 2021

Choose a reason for hiding this comment

jvalkeal commented Oct 29, 2021

klopfdreh commented Oct 29, 2021

jvalkeal commented Oct 29, 2021

klopfdreh commented Oct 29, 2021

jvalkeal commented Nov 2, 2021

klopfdreh commented Nov 2, 2021

Manu10744 commented Feb 2, 2022

klopfdreh commented Feb 2, 2022

oodamien commented Feb 7, 2022

klopfdreh commented Oct 4, 2021 •

edited

klopfdreh commented Oct 27, 2021 •

edited

klopfdreh commented Oct 27, 2021 •

edited

klopfdreh commented Oct 28, 2021 •

edited