Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deploy dex in metalk8s from helm charts #2025

Merged

Conversation

Ebaneck
Copy link
Contributor

@Ebaneck Ebaneck commented Nov 7, 2019

Component:

'authentication', 'salt', 'kubernetes'

Context:

See #2007

Summary:

Acceptance criteria:

  • Have Dex pods running in MetalK8s cluster deployed from helm charts
  • Access the dex URL exposed by ingress and verify that it works : https://{{ control_plane_address }}:8443/oidc/.well-known/openid-configuration
{
  "issuer": "https://172.21.254.3:8443/oidc",
  "authorization_endpoint": "https://172.21.254.3:8443/oidc/auth",
  "token_endpoint": "https://172.21.254.3:8443/oidc/token",
  "jwks_uri": "https://172.21.254.3:8443/oidc/keys",
  "userinfo_endpoint": "https://172.21.254.3:8443/oidc/userinfo",
  "response_types_supported": [
    "code",
    "id_token",
    "token"
  ],
  "subject_types_supported": [
    "public"
  ],
  "id_token_signing_alg_values_supported": [
    "RS256"
  ],
  "scopes_supported": [
    "openid",
    "email",
    "groups",
    "profile",
    "offline_access"
  ],
  "token_endpoint_auth_methods_supported": [
    "client_secret_basic"
  ],
  "claims_supported": [
    "aud",
    "email",
    "email_verified",
    "exp",
    "iat",
    "iss",
    "locale",
    "name",
    "sub"
  ]
}

Closes: #2007

@bert-e
Copy link
Contributor

bert-e commented Nov 7, 2019

Hello ebaneck,

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
Copy link
Contributor

bert-e commented Nov 7, 2019

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:

@Ebaneck Ebaneck requested a review from a team November 7, 2019 15:32
@Ebaneck Ebaneck changed the title WIP: Feature/2007 deploy dex in metalk8s from helm charts WIP: Deploy dex in metalk8s from helm charts Nov 7, 2019
@Ebaneck Ebaneck force-pushed the feature/2007-deploy-dex-in-metalk8s-from-helm-charts branch 2 times, most recently from 646e460 to ebc9116 Compare November 7, 2019 16:24
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
salt/metalk8s/addons/dex/deployed/namespace.sls Outdated Show resolved Hide resolved
@bert-e
Copy link
Contributor

bert-e commented Nov 7, 2019

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:

@Ebaneck Ebaneck force-pushed the feature/2007-deploy-dex-in-metalk8s-from-helm-charts branch from ebc9116 to 45d5892 Compare November 8, 2019 09:12
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Outdated Show resolved Hide resolved
charts/dex.yaml Show resolved Hide resolved
charts/dex.yaml Show resolved Hide resolved
charts/dex.yaml Show resolved Hide resolved
salt/metalk8s/addons/dex/deployed/chart.sls Outdated Show resolved Hide resolved
salt/metalk8s/addons/dex/deployed/chart.sls Outdated Show resolved Hide resolved
@Ebaneck Ebaneck force-pushed the feature/2007-deploy-dex-in-metalk8s-from-helm-charts branch 2 times, most recently from c9255d9 to 58aece3 Compare November 12, 2019 09:25
@Ebaneck Ebaneck force-pushed the feature/2007-deploy-dex-in-metalk8s-from-helm-charts branch 6 times, most recently from c692de0 to 8bc02b2 Compare November 14, 2019 10:45
@bert-e
Copy link
Contributor

bert-e commented Nov 15, 2019

Branches have diverged

This pull request's source branch feature/2007-deploy-dex-in-metalk8s-from-helm-charts has diverged from
development/2.4 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

  • Merge origin/development/2.4 into feature/2007-deploy-dex-in-metalk8s-from-helm-charts
  • Rebase feature/2007-deploy-dex-in-metalk8s-from-helm-charts onto origin/development/2.4

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

gdemonet
gdemonet previously approved these changes Nov 21, 2019
Copy link
Contributor

@gdemonet gdemonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - added a wait to not merge this in 2.4.

In the meantime (waiting for 2.5 branch), could you please:

  • start adding some tests (e.g. try to reach the openid-configuration and check the issuer_url)
  • investigate SSL verification from the Ingress (the thing that we removed) - does it work? or is it just ignored?

@gdemonet gdemonet changed the base branch from development/2.4 to development/2.5 November 21, 2019 22:41
@bert-e
Copy link
Contributor

bert-e commented Nov 21, 2019

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
Copy link
Contributor

/after_pull_request=2063

@bert-e
Copy link
Contributor

bert-e commented Nov 21, 2019

Waiting for other pull request(s)

The current pull request is locked by the after_pull_request option.

In order for me to merge this pull request, run the following actions first:

➡️ Merge the OPEN pull request:

Alternatively, delete all the after_pull_request comments from this pull request.

The following options are set: after_pull_request

Ebaneck and others added 8 commits November 22, 2019 12:18
```
$ helm fetch -d charts --untar stable/dex
```

Closes: #2007
That prevented a proper dumping of the Secret data
This commit adds support for ClusterIP in the Dex charts.
This changed is added because multi-line strings is an issue in
rendered charts since Jinja "commands" are split.

This fix is needed to properly add and render Dex charts as done
in the next commit
This commit adds the following:

Adds Dex Image to the buildchain

Adds method to obtain OIDC service IP and binds this
static IP as the ClusterIP address for Dex service

Adds `metalk8-auth` namespace which holds OIDC/authentication
cluster resources(PODS)

Automatically generate Dex deployment, service account,
cluster role and clusterrolebindings

The Dex chart.sls is generated from the Helm charts using:

```
$ ./charts/render.py dex metalk8s-auth charts/dex.yaml charts/dex/ >
salt/metalk8s/addons/dex/deployed/chart.sls

Add states to deploy Dex and related server certificates

Closes: #2007
Closes: #2011
The newly created Dex static user has no access to any
resources in a Metalk8s cluster.

This commit adds a clusterrolebinding which binds the Dex static
user to the Metalk8s Cluster admin Clusterrole.

Closes: #2055
This commit adds the following:

Add configuration parameters to the APIserver manifest which is required
by Dex

Define a way to find the Ingress external IP required by Dex config flags

If a minion wants to reference the control-plane Ingress by its external
IP(in our case the Dex service), it needs to know the control-plane IP of
the bootstrap minion (as it is the one used by Salt master when creating the Service).

For posterity, we define a helper that should work even during
the initial boostrap.

Closes: #2010
We need to backup the Dex CAfile and key in other to be able
to run a cluster restore.

This commit adds the Dex CA file and Key to the backup and restore
scripts.
@Ebaneck Ebaneck force-pushed the feature/2007-deploy-dex-in-metalk8s-from-helm-charts branch from b52a215 to d310f34 Compare November 22, 2019 11:19
@bert-e
Copy link
Contributor

bert-e commented Nov 22, 2019

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:

The following options are set: after_pull_request

@Ebaneck
Copy link
Contributor Author

Ebaneck commented Nov 22, 2019

/approve

@bert-e
Copy link
Contributor

bert-e commented Nov 22, 2019

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:

The following options are set: approve, after_pull_request

@Ebaneck
Copy link
Contributor Author

Ebaneck commented Nov 22, 2019

/status

@bert-e
Copy link
Contributor

bert-e commented Nov 22, 2019

Status

Status report is not available.

The following options are set: approve, after_pull_request

@bert-e
Copy link
Contributor

bert-e commented Nov 22, 2019

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:

The following options are set: approve, after_pull_request

@bert-e
Copy link
Contributor

bert-e commented Nov 22, 2019

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

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, after_pull_request

@bert-e
Copy link
Contributor

bert-e commented Nov 22, 2019

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

  • ✔️ development/2.5

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 None.

Goodbye ebaneck.

@bert-e bert-e merged commit d310f34 into development/2.5 Nov 22, 2019
@bert-e bert-e deleted the feature/2007-deploy-dex-in-metalk8s-from-helm-charts branch November 22, 2019 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants