charts,salt,build,tests: Bump Dex chart to v0.8.2 #3765

JBWatenbergScality · 2022-05-11T09:29:31Z

rm -rf charts/dex
helm repo add dex https://charts.dexidp.io
helm repo update
helm fetch -d charts --untar dex/dex

Render chart to salt state using

./charts/render.py dex charts/dex.yaml charts/dex \
  --namespace metalk8s-auth \
  --service-config dex metalk8s-dex-config \
  metalk8s/addons/dex/config/dex.yaml.j2 metalk8s-auth \
  > salt/metalk8s/addons/dex/deployed/chart.sls

Component:

Context:

Summary:

Acceptance criteria:

Closes: #ISSUE_NUMBER

gdemonet

Just a few things to add in the CHANGELOG, otherwise LGTM

CHANGELOG.md

charts/dex/values.yaml

TeddyAndrieux · 2022-05-18T09:10:58Z

charts/dex/templates/ingress.yaml

-{{- if .Values.https.enabled -}}
-  {{- $svcPort = .Values.service.ports.https.port -}}
-{{- end }}


It's sad but since this ticket is not done upstream dexidp/helm-charts#15
We need to restore this "manual patch"

Good catch! Btw, we should likely consider a more predictable way for applying these patches, maybe something similar to what they do in https://github.com/scality/Zenko

salt/metalk8s/addons/dex/deployed/chart.sls

gdemonet

Actually, looks like this change isn't working, I see the tests fail with 502 when attempting to reach Dex (yes, this info is lost in 30k+ lines of traceback....... 😮‍💨). I'll try to run it in debug and investigate. Also, I'll rebase your branch on the latest dev, because of #3781

TeddyAndrieux

Btw Dex 2.31.2 just get released (with the new zlib version, so CVE fix) maybe worth bumping the image

salt/metalk8s/addons/dex/deployed/chart.sls

salt/metalk8s/addons/dex/deployed/post-downgrade.sls

salt/metalk8s/addons/dex/deployed/post-upgrade.sls

CHANGELOG.md

buildchain/buildchain/versions.py

gdemonet · 2022-06-01T08:17:56Z

Now tests are failing with the latest version of Dex, we can see in its logs:

http: URL query contains semicolon, which is no longer a supported separator; parts of the query may be stripped when parsed; see golang.org/issue/25192

Dex is working fine with browser access though, so this really has to do with our queries from the test runner. Investigating...

Edit: I thought this came from our way of encoding query parameters, but it actually came from the login URL we extract from the HTML response sent by Dex when we "navigate" to the "login page". The fix in 6cca5bf should be sufficient.

``` rm -rf charts/dex helm repo add dex https://charts.dexidp.io helm repo update helm fetch -d charts --untar dex/dex ``` Render chart to salt state using ``` ./charts/render.py dex charts/dex.yaml charts/dex \ --namespace metalk8s-auth \ --service-config dex metalk8s-dex-config \ metalk8s/addons/dex/config/dex.yaml.j2 metalk8s-auth \ > salt/metalk8s/addons/dex/deployed/chart.sls ```

…clusterrolebinding, dex role and roleBinding on post downgrade

This bumps the base image from alpine:3.15.1 to 3.16.0, which gets rid of the latest zlib CVE (https://nvd.nist.gov/vuln/detail/CVE-2018-25032). See dexidp/dex@v2.31.1...v2.31.2

This URL was extracted from the generated HTML returned by Dex, however it contains a semicolon, which is not accepted by the latest Go http library, used by Dex. So our test was broken, since the POST request was rejected with a 400 status.

bert-e · 2022-06-01T12:45:55Z

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 · 2022-06-01T16:21:46Z

/approve

bert-e · 2022-06-01T16:21:52Z

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

bert-e · 2022-06-02T09:44:37Z

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/123.0

The following branches will NOT be impacted:

development/2.0
development/2.1
development/2.10
development/2.11
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

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 · 2022-06-02T18:31:18Z

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

✔️ development/123.0

The following branches have NOT changed:

development/2.0
development/2.1
development/2.10
development/2.11
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8
development/2.9

Please check the status of the associated issue None.

Goodbye jbwatenbergscality.

JBWatenbergScality requested a review from a team as a code owner May 11, 2022 09:29

This comment was marked as spam.

Sign in to view

This comment was marked as outdated.

Sign in to view

gdemonet suggested changes May 18, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

charts/dex/values.yaml Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

TeddyAndrieux reviewed May 18, 2022

View reviewed changes

salt/metalk8s/addons/dex/deployed/chart.sls Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

JBWatenbergScality force-pushed the improvement/bump-dex branch from 31fe870 to a972f68 Compare May 27, 2022 14:42

This comment was marked as outdated.

Sign in to view

JBWatenbergScality requested review from TeddyAndrieux and gdemonet May 27, 2022 14:43

gdemonet approved these changes May 31, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

gdemonet suggested changes May 31, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

gdemonet force-pushed the improvement/bump-dex branch from a972f68 to 3d89c6e Compare May 31, 2022 06:56

TeddyAndrieux reviewed May 31, 2022

View reviewed changes

salt/metalk8s/addons/dex/deployed/chart.sls Outdated Show resolved Hide resolved

gdemonet force-pushed the improvement/bump-dex branch from 3d89c6e to be97530 Compare May 31, 2022 07:30

TeddyAndrieux reviewed May 31, 2022

View reviewed changes

salt/metalk8s/addons/dex/deployed/post-downgrade.sls Show resolved Hide resolved

TeddyAndrieux reviewed May 31, 2022

View reviewed changes

salt/metalk8s/addons/dex/deployed/post-upgrade.sls Show resolved Hide resolved

gdemonet force-pushed the improvement/bump-dex branch from a6cef63 to 5415aa7 Compare May 31, 2022 09:07

TeddyAndrieux reviewed May 31, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

TeddyAndrieux reviewed May 31, 2022

View reviewed changes

buildchain/buildchain/versions.py Outdated Show resolved Hide resolved

gdemonet force-pushed the improvement/bump-dex branch from 5415aa7 to 3ba5837 Compare May 31, 2022 14:12

gdemonet previously approved these changes Jun 1, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

JBWatenbergScality and others added 5 commits June 1, 2022 14:45

dex: Restore https manual patch of the chart

77495df

dex: remove dex cluster role binding on post upgrade and dex-cluster …

f18b2e9

…clusterrolebinding, dex role and roleBinding on post downgrade

build,charts: Bump Dex image to v2.31.2

100fb51

This bumps the base image from alpine:3.15.1 to 3.16.0, which gets rid of the latest zlib CVE (https://nvd.nist.gov/vuln/detail/CVE-2018-25032). See dexidp/dex@v2.31.1...v2.31.2

tests/dex: Remove semicolon from login URL

8562abb

This URL was extracted from the generated HTML returned by Dex, however it contains a semicolon, which is not accepted by the latest Go http library, used by Dex. So our test was broken, since the POST request was rejected with a 400 status.

gdemonet dismissed their stale review via 8562abb June 1, 2022 12:45

gdemonet force-pushed the improvement/bump-dex branch from 6cca5bf to 8562abb Compare June 1, 2022 12:45

gdemonet approved these changes Jun 2, 2022

View reviewed changes

bert-e merged commit 1bbf69c into development/123.0 Jun 2, 2022

bert-e deleted the improvement/bump-dex branch June 2, 2022 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

charts,salt,build,tests: Bump Dex chart to v0.8.2 #3765

charts,salt,build,tests: Bump Dex chart to v0.8.2 #3765

JBWatenbergScality commented May 11, 2022

This comment was marked as spam.

This comment was marked as outdated.

gdemonet left a comment

This comment was marked as outdated.

TeddyAndrieux May 18, 2022

gdemonet May 19, 2022

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

gdemonet left a comment

This comment was marked as outdated.

TeddyAndrieux left a comment

gdemonet commented Jun 1, 2022 •

edited

Loading

This comment was marked as outdated.

This comment was marked as resolved.

bert-e commented Jun 1, 2022

JBWatenbergScality commented Jun 1, 2022

bert-e commented Jun 1, 2022

bert-e commented Jun 2, 2022

bert-e commented Jun 2, 2022

charts,salt,build,tests: Bump Dex chart to v0.8.2 #3765

charts,salt,build,tests: Bump Dex chart to v0.8.2 #3765

Conversation

JBWatenbergScality commented May 11, 2022

This comment was marked as spam.

This comment was marked as outdated.

gdemonet left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

TeddyAndrieux May 18, 2022

Choose a reason for hiding this comment

gdemonet May 19, 2022

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

gdemonet left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

TeddyAndrieux left a comment

Choose a reason for hiding this comment

gdemonet commented Jun 1, 2022 • edited Loading

This comment was marked as outdated.

This comment was marked as resolved.

bert-e commented Jun 1, 2022

Waiting for approval

JBWatenbergScality commented Jun 1, 2022

bert-e commented Jun 1, 2022

Waiting for approval

bert-e commented Jun 2, 2022

In the queue

bert-e commented Jun 2, 2022

gdemonet commented Jun 1, 2022 •

edited

Loading