ui: Fix configuration when changing the basePath and fix permissions management in the navbar #3254

JBWatenbergScality · 2021-03-31T18:28:09Z

Component:

salt, ui

Context:

When deploying the UI on another path than / the shell-ui config couldn't be fetched.

Also this PR brings a fix for the shell-ui which was checking if the user have all the groups instead of a single one to enable access to an entry.

bert-e · 2021-03-31T18:28:10Z

Hello jbwatenbergscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2021-03-31T18:28:14Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

gdemonet

We also need to update the redirect URI in the Dex client configuration and in Shell UI configuration.

salt/metalk8s/addons/ui/deployed/ingress.sls

bert-e · 2021-04-01T13:49:59Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

gdemonet

Only one thing blocking about leading/trailing slashes in Jinja templates, otherwise LGTM

salt/metalk8s/addons/ui/deployed/ui.sls.in

salt/metalk8s/addons/dex/config/dex.yaml.j2

salt/metalk8s/addons/ui/config/metalk8s-shell-ui-config.yaml.j2

shell-ui/babel.config.js

shell-ui/src/auth/permissionUtils.js

CHANGELOG.md

salt/metalk8s/addons/ui/config/metalk8s-shell-ui-config.yaml.j2

salt/metalk8s/addons/ui/deployed/ingress.sls

bert-e · 2021-04-08T13:42:44Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality · 2021-04-08T13:45:39Z

/approve

bert-e · 2021-04-08T15:18:11Z

Build failed

The build for commit did not succeed in branch bugfix/path-config.

The following options are set: approve

bert-e · 2021-04-09T07:46:06Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

gdemonet · 2021-04-09T08:23:35Z

salt/metalk8s/addons/ui/deployed/ui.sls.in

@@ -27,7 +27,7 @@ include:
 %}

 {%- set stripped_base_path = metalk8s_ui_config.spec.basePath.strip('/') %}
-{%- set normalized_base_path = '/' ~ stripped_base_path if stripped_base_path else '' %}
+{%- set normalized_base_path = '/' ~ stripped_base_path if stripped_base_path else '/' %}


Maybe a stupid comment, but isn't this equivalent to

{%- set normalized_base_path = '/' ~ stripped_base_path %}

?

done, I squashed and took care of it at the same time

…{{ metalk8s_ui_config.spec.basePath }}/shell/config.json As we have a nginx location rule on /shell/config.json in UI nginx, the path seen from there should be /shell/config.json instead of /{{ metalk8s_ui_config.spec.basePath }}/shell/config.json

Closes: #3263

gdemonet

bert-e · 2021-04-14T06:22:30Z

Build failed

The build for commit did not succeed in branch bugfix/path-config.

The following options are set: approve

bert-e · 2021-04-14T09:54:59Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2021-04-14T12:12:27Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.9

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8

Please check the status of the associated issue None.

Goodbye jbwatenbergscality.

Adding a `rewrite-target` annotation is needed when the base path is not /, because NGINX does not know where to find the documentation static files. We now always expose MetalK8s docs behind /docs, optionally appending the configured base path for MetalK8s UI (e.g. /docs/platform). This will avoid strange conflicts with the behaviour of the current Ingress when running MetalK8s at a non-root path, because mixing regular expressions and exact matches does not play well with ingress-nginx. See: #3254 See: https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/#use-regex

JBWatenbergScality requested a review from a team as a code owner March 31, 2021 18:28

JBWatenbergScality added topic:ui UI-related issues topic:salt Everything related to SaltStack in our product labels Mar 31, 2021

ChengYanJin changed the title ~~salt: Fix UI path when deploying on another paht than /~~ salt: Fix UI path when deploying on another path than / Mar 31, 2021

ChengYanJin changed the title ~~salt: Fix UI path when deploying on another path than /~~ salt: Fix UI path when deploying on another path than / Mar 31, 2021

ChengYanJin changed the title ~~salt: Fix UI path when deploying on another path than /~~ salt: Fix UI path when deploying on another path than / Mar 31, 2021

gdemonet suggested changes Apr 1, 2021

View reviewed changes

salt/metalk8s/addons/ui/deployed/ingress.sls Outdated Show resolved Hide resolved

salt/metalk8s/addons/ui/deployed/ingress.sls Outdated Show resolved Hide resolved

JBWatenbergScality force-pushed the bugfix/path-config branch 2 times, most recently from b080214 to a36d3af Compare April 2, 2021 07:36

JBWatenbergScality changed the title ~~salt: Fix UI path when deploying on another path than /~~ ui: Fix configuration when changing the basePath and fix permissions management in the navbar Apr 2, 2021

JBWatenbergScality force-pushed the bugfix/path-config branch from 542e722 to cde2685 Compare April 2, 2021 11:48

gdemonet suggested changes Apr 6, 2021

View reviewed changes

JBWatenbergScality force-pushed the bugfix/path-config branch 4 times, most recently from 4e7166f to c3fc82c Compare April 6, 2021 11:55

gdemonet suggested changes Apr 6, 2021

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

salt/metalk8s/addons/ui/config/metalk8s-shell-ui-config.yaml.j2 Outdated Show resolved Hide resolved

salt/metalk8s/addons/ui/deployed/ingress.sls Outdated Show resolved Hide resolved

JBWatenbergScality force-pushed the bugfix/path-config branch from 6a0e822 to 84c9576 Compare April 8, 2021 13:29

gdemonet previously approved these changes Apr 8, 2021

View reviewed changes

JBWatenbergScality dismissed gdemonet’s stale review via ddfc72e April 9, 2021 07:46

gdemonet reviewed Apr 9, 2021

View reviewed changes

JBWatenbergScality added 2 commits April 14, 2021 00:15

salt: Fix config path when basePath is not /

2ae74c0

JBWatenbergScality added 4 commits April 14, 2021 00:15

salt: Use UI base path to compute OIDC redirect URI

eb94388

shell-ui: Add styled-component namespacing

ebde300

Closes: #3263

shell-ui: Fix permission check to display navbar menu entry

b9f0b7c

changelog: Mention ui/navbar changes in the changelog

73c0adb

JBWatenbergScality force-pushed the bugfix/path-config branch from ddfc72e to 73c0adb Compare April 13, 2021 22:15

gdemonet approved these changes Apr 14, 2021

View reviewed changes

bert-e merged commit 852af4a into development/2.9 Apr 14, 2021

bert-e deleted the bugfix/path-config branch April 14, 2021 12:12

gdemonet mentioned this pull request Apr 19, 2021

salt: Add dedicated Ingress for MetalK8s docs #3299

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: Fix configuration when changing the basePath and fix permissions management in the navbar #3254

ui: Fix configuration when changing the basePath and fix permissions management in the navbar #3254

JBWatenbergScality commented Mar 31, 2021 •

edited

Loading

bert-e commented Mar 31, 2021

bert-e commented Mar 31, 2021

gdemonet left a comment

bert-e commented Apr 1, 2021

gdemonet left a comment

bert-e commented Apr 8, 2021

JBWatenbergScality commented Apr 8, 2021

bert-e commented Apr 8, 2021

bert-e commented Apr 9, 2021

gdemonet Apr 9, 2021

JBWatenbergScality Apr 13, 2021

gdemonet left a comment

bert-e commented Apr 14, 2021

bert-e commented Apr 14, 2021

bert-e commented Apr 14, 2021

ui: Fix configuration when changing the basePath and fix permissions management in the navbar #3254

ui: Fix configuration when changing the basePath and fix permissions management in the navbar #3254

Conversation

JBWatenbergScality commented Mar 31, 2021 • edited Loading

bert-e commented Mar 31, 2021

Hello jbwatenbergscality,

bert-e commented Mar 31, 2021

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Apr 1, 2021

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Apr 8, 2021

Waiting for approval

JBWatenbergScality commented Apr 8, 2021

bert-e commented Apr 8, 2021

Build failed

bert-e commented Apr 9, 2021

Waiting for approval

gdemonet Apr 9, 2021

Choose a reason for hiding this comment

JBWatenbergScality Apr 13, 2021

Choose a reason for hiding this comment

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Apr 14, 2021

Build failed

bert-e commented Apr 14, 2021

In the queue

bert-e commented Apr 14, 2021

JBWatenbergScality commented Mar 31, 2021 •

edited

Loading