Add digest for dex config #2573

TeddyAndrieux · 2020-05-26T09:10:46Z

Component:

'salt', 'authentication'

Context:

#2569

Summary:

Add simple "salt slots" handling in Kubernetes manifest when using metalk8s_kubernetes module
Add digest of secret to checksum/config annotation for Dex pods
Remove kubectl rollout restart command from doc
Add test for Dex static user password change

Fixes: #2569

bert-e · 2020-05-26T09:10:48Z

Hello teddyandrieux,

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 · 2020-05-26T09:10:53Z

Conflict

A conflict has been raised during the creation of
integration branch w/2.6/bugfix/GH-2569-add-digest-for-dex-config with contents from bugfix/GH-2569-add-digest-for-dex-config
and development/2.6.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.6/bugfix/GH-2569-add-digest-for-dex-config origin/development/2.6
 $ git merge origin/bugfix/GH-2569-add-digest-for-dex-config
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.6/bugfix/GH-2569-add-digest-for-dex-config

bert-e · 2020-05-26T09:14:28Z

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

Those slots look cool 💯!

I'm a bit concerned of the flexibility however:

Hashing string(s) in a slot is a good idea, could be reused for other sources (like local files), but not in the current approach which is tied to K8s
Getting more than a single K8s object will very likely become a need, as workloads often consume data from both ConfigMap(s) and Secret(s)

salt/_modules/metalk8s.py

salt/_modules/metalk8s_kubernetes.py

salt/_modules/metalk8s.py

salt/_modules/metalk8s_kubernetes.py

TeddyAndrieux · 2020-05-26T12:23:49Z

I'm a bit concerned of the flexibility however:

I don't see why

Hashing string(s) in a slot is a good idea, could be reused for other sources (like local files), but not in the current approach which is tied to K8s

Why ? For local file you already have hashutil.digest_file from salt you do not need this function to handle it

Getting more than a single K8s object will very likely become a need, as workloads often consume data from both ConfigMap(s) and Secret(s)

Then we will just need a new function that do several call on metalk8s_kubernetes.get_object_digest but right now it's not needed

bert-e · 2020-05-26T12:47:40Z

History mismatch

Merge commit #ca1c1585a65bec1e0d94c77e78cb9023de1c1002 on the integration branch
w/2.6/bugfix/GH-2569-add-digest-for-dex-config is merging a branch which is neither the current
branch bugfix/GH-2569-add-digest-for-dex-config nor the development branch
development/2.6.

It is likely due to a rebase of the branch bugfix/GH-2569-add-digest-for-dex-config and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

TeddyAndrieux · 2020-05-26T12:47:45Z

/reset

bert-e · 2020-05-26T12:47:53Z

Reset complete

I have successfully deleted this pull request's integration branches.

bert-e · 2020-05-26T12:47:58Z

Conflict

A conflict has been raised during the creation of
integration branch w/2.6/bugfix/GH-2569-add-digest-for-dex-config with contents from bugfix/GH-2569-add-digest-for-dex-config
and development/2.6.

I have not created the integration branch.

Here are the steps to resolve this conflict:

 $ git fetch
 $ git checkout -B w/2.6/bugfix/GH-2569-add-digest-for-dex-config origin/development/2.6
 $ git merge origin/bugfix/GH-2569-add-digest-for-dex-config
 $ # <intense conflict resolution>
 $ git commit
 $ git push -u origin w/2.6/bugfix/GH-2569-add-digest-for-dex-config

bert-e · 2020-05-26T12:50:20Z

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

bert-e · 2020-05-27T06:45:21Z

History mismatch

Merge commit #981403740235cdba7971cc8f7d62d2090b78672c on the integration branch
w/2.6/bugfix/GH-2569-add-digest-for-dex-config is merging a branch which is neither the current
branch bugfix/GH-2569-add-digest-for-dex-config nor the development branch
development/2.6.

It is likely due to a rebase of the branch bugfix/GH-2569-add-digest-for-dex-config and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

bert-e · 2020-05-27T06:48:02Z

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:

tests/post/features/service_configuration.feature

bert-e · 2020-05-27T06:58:40Z

History mismatch

Merge commit #2fa0e1317e6f6057b4f931a273c35e940f02b126 on the integration branch
w/2.6/bugfix/GH-2569-add-digest-for-dex-config is merging a branch which is neither the current
branch bugfix/GH-2569-add-digest-for-dex-config nor the development branch
development/2.6.

It is likely due to a rebase of the branch bugfix/GH-2569-add-digest-for-dex-config and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

bert-e · 2020-05-27T07:02:22Z

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:

TeddyAndrieux · 2020-05-27T07:07:10Z

/approve

bert-e · 2020-05-27T07:20:04Z

Build failed

The build for commit did not succeed in branch bugfix/GH-2569-add-digest-for-dex-config.

The following options are set: approve

bert-e · 2020-05-27T07:32:47Z

Build failed

The build for commit did not succeed in branch w/2.6/bugfix/GH-2569-add-digest-for-dex-config.

The following options are set: approve

bert-e · 2020-05-27T07:38:42Z

Build failed

The build for commit did not succeed in branch bugfix/GH-2569-add-digest-for-dex-config.

The following options are set: approve

tests/post/features/service_configuration.feature

bert-e · 2020-05-27T08:57:23Z

History mismatch

Merge commit #12028b4d4a80582f85874dd74a650204b31be2dd on the integration branch
w/2.6/bugfix/GH-2569-add-digest-for-dex-config is merging a branch which is neither the current
branch bugfix/GH-2569-add-digest-for-dex-config nor the development branch
development/2.6.

It is likely due to a rebase of the branch bugfix/GH-2569-add-digest-for-dex-config and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

The following options are set: approve

Currently Salt 3000.3 only support slots on root of salt states, but in MetalK8s context we need to render slot in nested dictionnary, for example in some kubernetes manifests. This commit add a really simple slot implementation for nested dictionnary

Use our `format_slots` implementation on the kubernetes manifest so that we can use simple salt slot in kubernetes manifest nested dictionnary

Simple helper to get a kubernetes object and then compute the digest from this object, it's mainly needed to use in slot call in manifest to get the digest from a config file in a config map or a secret

To change default user password we need it to be present in the default CSC ConfigMap so that you not need to get the user ID and other information from Dex Secret to change it in the CSC ConfigMap

We need Dex Pods to restart when secret content change, so use the `checksum/config` annotation on the Dex Pods. This annotation is equal to the digest of the secret content This chart is re-rendered using: ``` ./charts/render.py dex --namespace metalk8s-auth charts/dex.yaml \ --service-config dex metalk8s-dex-config \ charts/dex/ > salt/metalk8s/addons/dex/deployed/chart.sls ``` Fixes: #2569

This reverts commit 1fb6804. This rollout is not longer needed since it's automatically handled by the salt states Refs: #2569

(cherry picked from commit 1e110a3)

Add single `then` to handle Dex authentication success or fail, so that we can add a single `then` to check Dex authentication result, and move this `then` to conftest so that we can use it for other tests (like CSC)

Add ability to give int in the path so that we can take an element of the list `mydict.0` to get the first element of the list in `mydict`

Use CSC to change the password for the Admin static user and test login with this new password on Dex Refs: #2569

bert-e · 2020-05-27T14:08:57Z

History mismatch

Merge commit #945c524d165c7ec58926729df830cecd3bf71b8f on the integration branch
w/2.6/bugfix/GH-2569-add-digest-for-dex-config is merging a branch which is neither the current
branch bugfix/GH-2569-add-digest-for-dex-config nor the development branch
development/2.6.

It is likely due to a rebase of the branch bugfix/GH-2569-add-digest-for-dex-config and the
merge is not possible until all related w/* branches are deleted or updated.

Please use the reset command to have me reinitialize these branches.

The following options are set: approve

bert-e · 2020-05-27T14:26:54Z

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 · 2020-05-27T15:53:31Z

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.5
✔️ development/2.6

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

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 · 2020-05-27T16:47:34Z

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

✔️ development/2.5
✔️ development/2.6

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

Please check the status of the associated issue GH-2569.

Goodbye teddyandrieux.

TeddyAndrieux requested a review from a team May 26, 2020 09:10

gdemonet reviewed May 26, 2020

View reviewed changes

TeddyAndrieux force-pushed the bugfix/GH-2569-add-digest-for-dex-config branch from dbb25c4 to e1f1cd1 Compare May 26, 2020 12:47

gdemonet previously approved these changes May 27, 2020

View reviewed changes

TeddyAndrieux dismissed gdemonet’s stale review via 2fa0e13 May 27, 2020 06:45

TeddyAndrieux force-pushed the bugfix/GH-2569-add-digest-for-dex-config branch from e1f1cd1 to 2fa0e13 Compare May 27, 2020 06:45

gdemonet previously approved these changes May 27, 2020

View reviewed changes

alexandre-allard reviewed May 27, 2020

View reviewed changes

tests/post/features/service_configuration.feature Outdated Show resolved Hide resolved

TeddyAndrieux dismissed gdemonet’s stale review via 119a88d May 27, 2020 06:58

TeddyAndrieux force-pushed the bugfix/GH-2569-add-digest-for-dex-config branch from 2fa0e13 to 119a88d Compare May 27, 2020 06:58

gdemonet previously approved these changes May 27, 2020

View reviewed changes

alexandre-allard previously approved these changes May 27, 2020

View reviewed changes

alexandre-allard reviewed May 27, 2020

View reviewed changes

tests/post/features/service_configuration.feature Outdated Show resolved Hide resolved

alexandre-allard self-requested a review May 27, 2020 07:55

TeddyAndrieux dismissed stale reviews from alexandre-allard and gdemonet via f74e47d May 27, 2020 08:57

TeddyAndrieux force-pushed the bugfix/GH-2569-add-digest-for-dex-config branch from 119a88d to f74e47d Compare May 27, 2020 08:57

TeddyAndrieux and others added 10 commits May 27, 2020 14:52

salt: Add ability to use slots in kubernetes manifest

cf0a481

Use our `format_slots` implementation on the kubernetes manifest so that we can use simple salt slot in kubernetes manifest nested dictionnary

salt: Add an helper to get k8s object digest

6a434bb

Simple helper to get a kubernetes object and then compute the digest from this object, it's mainly needed to use in slot call in manifest to get the digest from a config file in a config map or a secret

salt: Add dex userlist by default to CSC ConfigMap

6f04b35

To change default user password we need it to be present in the default CSC ConfigMap so that you not need to get the user ID and other information from Dex Secret to change it in the CSC ConfigMap

Revert "docs: Add step to restart dex deployment after config changes"

458bf6b

This reverts commit 1fb6804. This rollout is not longer needed since it's automatically handled by the salt states Refs: #2569

tests: add a get_grain helper and use it everywhere

ebccfda

(cherry picked from commit 1e110a3)

tests: Refactor Authentication to Dex test

0dd9235

Add single `then` to handle Dex authentication success or fail, so that we can add a single `then` to check Dex authentication result, and move this `then` to conftest so that we can use it for other tests (like CSC)

tests: Handle list element in {get,set}_dict_element function

3f597f8

Add ability to give int in the path so that we can take an element of the list `mydict.0` to get the first element of the list in `mydict`

tests: Add scenario to test Admin static user password change

9fa09f8

Use CSC to change the password for the Admin static user and test login with this new password on Dex Refs: #2569

TeddyAndrieux force-pushed the bugfix/GH-2569-add-digest-for-dex-config branch from f74e47d to 9fa09f8 Compare May 27, 2020 14:08

gdemonet approved these changes May 27, 2020

View reviewed changes

alexandre-allard approved these changes May 27, 2020

View reviewed changes

bert-e merged commit ef488b2 into development/2.5 May 27, 2020

bert-e deleted the bugfix/GH-2569-add-digest-for-dex-config branch May 27, 2020 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add digest for dex config #2573

Add digest for dex config #2573

TeddyAndrieux commented May 26, 2020 •

edited

Loading

bert-e commented May 26, 2020

bert-e commented May 26, 2020

bert-e commented May 26, 2020

gdemonet left a comment

TeddyAndrieux commented May 26, 2020

bert-e commented May 26, 2020

TeddyAndrieux commented May 26, 2020

bert-e commented May 26, 2020

bert-e commented May 26, 2020

bert-e commented May 26, 2020

gdemonet left a comment

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

TeddyAndrieux commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

bert-e commented May 27, 2020

Add digest for dex config #2573

Add digest for dex config #2573

Conversation

TeddyAndrieux commented May 26, 2020 • edited Loading

bert-e commented May 26, 2020

Hello teddyandrieux,

bert-e commented May 26, 2020

Conflict

bert-e commented May 26, 2020

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

TeddyAndrieux commented May 26, 2020

bert-e commented May 26, 2020

History mismatch

TeddyAndrieux commented May 26, 2020

bert-e commented May 26, 2020

Reset complete

bert-e commented May 26, 2020

Conflict

bert-e commented May 26, 2020

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented May 27, 2020

History mismatch

bert-e commented May 27, 2020

Waiting for approval

bert-e commented May 27, 2020

History mismatch

bert-e commented May 27, 2020

Waiting for approval

TeddyAndrieux commented May 27, 2020

bert-e commented May 27, 2020

Build failed

bert-e commented May 27, 2020

Build failed

bert-e commented May 27, 2020

Build failed

bert-e commented May 27, 2020

History mismatch

bert-e commented May 27, 2020

History mismatch

bert-e commented May 27, 2020

Waiting for approval

bert-e commented May 27, 2020

In the queue

bert-e commented May 27, 2020

TeddyAndrieux commented May 26, 2020 •

edited

Loading