salt: Fix shell-ui configuration upgrade #3499
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
JBWatenbergScality
changed the base branch from
development/2.11
to
development/2.10
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
JBWatenbergScality
force-pushed
the
bugfix/upgrade-shell-ui-version
branch
from
86ee888 to
2dd9ee9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
gdemonet
suggested changes
Aug 19, 2021
Comment on lines
+70
to
+99
| {%- if metalk8s_shell_ui_config is not none %} | ||
| {%- set config_data = metalk8s_shell_ui_config.data['config.yaml'] | load_yaml %} | ||
|
|
||
| {%- if config_data.apiVersion == 'addons.metalk8s.scality.com/v1alpha1' and 'oidc' in config_data.spec %} | ||
|
|
||
| Create metalk8s-ui-config ConfigMap: | ||
| metalk8s_kubernetes.object_present: | ||
| - manifest: | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: metalk8s-ui-config | ||
| namespace: metalk8s-ui | ||
| data: | ||
| config.yaml: |- | ||
| apiVersion: addons.metalk8s.scality.com/v1alpha2 | ||
| kind: UIConfig | ||
| spec: | ||
| kind: "OIDC" | ||
| {{ config_data.spec.oidc | yaml(False) | indent(14) }} | ||
|
|
||
| {%- else %} | ||
|
|
||
| metalk8s-ui-config ConfigMap already exist: | ||
| test.succeed_without_changes: [] | ||
|
|
||
| {%- endif %} | ||
|
|
||
| {%- else %} | ||
|
|
There was a problem hiding this comment.
Not sure to understand this part:
- we have both
metalk8s-ui-configandmetalk8s-shell-ui-configalready existing - we want to copy over the OIDC settings from the latter to the former, in case the
shell-uiconfig is in the old format
But if that's true, we're overwriting the previous metalk8s-ui-config, without checking its own apiVersion or previous contents (what if we changed the basePath?)
Comment on lines
+287
to
+310
| {%- if 'oidc' in config_data.spec %} | ||
| oidc: | ||
| {{ config_data.spec.oidc | yaml(False) | indent(16) }} | ||
| {%- endif %} | ||
| {%- if 'userGroupsMapping' in config_data.spec %} | ||
| userGroupsMapping: | ||
| {{ config_data.spec.userGroupsMapping | yaml(False) | indent(16) }} | ||
| {%- endif %} | ||
| {%- if 'logo' in config_data.spec %} | ||
| logo: | ||
| {{ config_data.spec.logo | yaml(False) | indent(16) }} | ||
| {%- endif %} | ||
| {%- if 'favicon' in config_data.spec %} | ||
| favicon: | ||
| {{ config_data.spec.favicon | yaml(False) | indent(16) }} | ||
| {%- endif %} | ||
| {%- if 'canChangeLanguage' in config_data.spec %} | ||
| canChangeLanguage: | ||
| {{ config_data.spec.canChangeLanguage | yaml(False) | indent(16) }} | ||
| {%- endif %} | ||
| {%- if 'canChangeTheme' in config_data.spec %} | ||
| canChangeTheme: | ||
| {{ config_data.spec.canChangeTheme | yaml(False) | indent(16) }} | ||
| {%- endif %} |
There was a problem hiding this comment.
That's really too verbose, don't you think? Would be good to reimplement the service_config_updated state module we built in another project (:wink:)...
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
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: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Component:
salt, shell-ui
Context:
When upgrading from 2.9.x to 2.10.x we lost patches to OIDC and users groups mapping configuration.
Summary:
This solves it by copying the old configuration to the new CMs.
Acceptance criteria:
Tested an upgrade from 2.9 with patched configuration to 2.10.3-dev and the configuration was correctly kept.
Closes: #3496