[FLINK-29363][runtime-web] allow fully redirection in web dashboard

[HuangZhenQiu]

What is the purpose of the change

Remove operator related configs from flink runtime config, so that users will not see any operator related config in web ui.

Brief change log

• remove operator configs before deployments in Flink services.

Verifying this change

This change is a trivial rework / code cleanup with unit test coverage.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (no)
• The public API, i.e., is any changes to the CustomResourceDescriptors: (no)
• Core observer or reconciler logic that is regularly executed: (no)

Documentation

• Does this pull request introduce a new feature? (no)
• If yes, how is the feature documented? (not applicable)

[flinkbot]

CI report:

• 13d3480 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[gaborgsomogyi]

I've taken a look at the RFC and it lists multiple 3xx codes. Just wondering how did you picked the listed codes?

[HuangZhenQiu]

The code path is mainly to fetching job metadata. Multiple Choices, Use Proxy, Unused are not fit for the scenarios or data type. But I am open to add more status code to make it more robust.

[gaborgsomogyi]

Starting an image build and then coming back here w/ an in-depth consideration from my side...

[gaborgsomogyi]

I was thinking about to add 302 Found but that's optional which is denoted w/ the MAY keyword in the documentation:

It's good as-is from my perspective.

[gaborgsomogyi]

I've started to build an image to test the feature end-t-end. Let's see how it goes...
Additionally plz change the heading which is Flink PR compliant: [FLINK-29363][runtime-web] ...

[gaborgsomogyi]

@flinkbot run azure

[gaborgsomogyi]

I presume a rebase to the latest master is needed since second time the following error arrived:

Sep 22 08:19:47 	Suppressed: java.lang.AssertionError: Test failed Error while running command to get file permissions : java.io.IOException: Cannot run program "ls": error=1, Operation not permitted

[rmetzger]

Yes, this issue with ls is a known build failure.

@gaborgsomogyi thanks a lot for the review. Lmk once you've approved the PR, I'll then take a look and merge it.

[gaborgsomogyi]

@rmetzger thanks for the help in advance!
I think the current change set is not going to work with same-origin policy.
Let's hear what @HuangZhenQiu thinks about this.

[HuangZhenQiu]

@gaborgsomogyi @rmetzger

Let's say proxy server is served in domain A. If the token/cookie times out, requests need to be redirected to domain B. In this case, Users need to configure CSP and CORS as below for security considerations.
For the security considerations, users need to set

Content-Security-Policy: sandbox, allow-form, allow-scripts, allow-same-origin
Access-Control-Allow-Origin: A, B

CSP guarantee the app runs in an isolated environment and also make sure cookies are attached to request to the allowed domain. Putting B in the Access-Control-Allow-Origin will make the redirection work, otherwise the redirection will be blocked by browser due to the cross origin access.

[gaborgsomogyi]

If I understand it correctly then the proxy would add CORS, right? If we can test it in a real environment then I'm fine w/ the actual code.

[gaborgsomogyi]

I've tested it manually on live cluster and it works.
@HuangZhenQiu could you do a rebase to the latest master where tests are more stable?

[gaborgsomogyi]

LGTM. Only green unit tests needed.

[rmetzger]

Thanks for manually testing it.

[rmetzger]

A rebase would be nice just to make sure there haven't been major changes on the UI side since September.

[gaborgsomogyi]

@rmetzger now it's all green and good to go :)

[wanglijie95]

Hi all, what's the current status of this PR? Looks like it's ready to be merged, but hasn't been merged

[gaborgsomogyi]

You see it well...

[rmetzger]

Merging change ...