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 on minikube #1719

Merged
merged 9 commits into from
Oct 8, 2021
Merged

Deploy dex on minikube #1719

merged 9 commits into from
Oct 8, 2021

Conversation

tolusha
Copy link
Collaborator

@tolusha tolusha commented Sep 29, 2021

What does this PR do?

Deploy dex on minikube

Screenshot/screencast of this PR

  ✔ Deploy Dex
    ✔ Create Dex namespace...[OK]
    ✔ Create Dex certificate
      ✔ Check Cert Manager deployment...not deployed
      ✔ Deploy Cert Manager...done
      ✔ Wait for Cert Manager...ready
      ✔ Check Cert Manager CA certificate...generating new one
      ✔ Set up Eclipse Che certificates issuer...done
      ✔ Request self-signed certificate...done
      ✔ Wait for self-signed certificate...ready
      ✔ Save Dex certificate...[OK: /tmp/dex-ca.crt]
    ✔ Create Dex service account...[OK]
    ✔ Create Dex cluster role...[OK]
    ✔ Create Dex cluster role binding...[OK]
    ✔ Create Dex service...[OK]
    ✔ Create Dex configmap...[OK]
    ✔ Create Dex deployment...[OK]
    ✔ Wait for Dex is ready...[OK]
    ✔ Configure API server
      ✔ Configure Minikube API server...[OK]
      ✔ Wait for Minikube API server...[OK]

What issues does this PR fix or reference?

eclipse-che/che#19366

How to test this PR?

# deploy dex
cat >/tmp/patch.yaml <<EOF                                                          
spec:
  auth:
    nativeUserMode: true
EOF

yarn && cd bin && ./run server:deploy --platform minikube --che-operator-cr-patch-yaml /tmp/patch.yaml

# check dex pod logs for errors
DEX_POD=$(kubectl get pods -n dex | grep dex | awk '{print $1}')
kubectl logs $DEX_POD -n dex

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@openshift-ci
Copy link

openshift-ci bot commented Sep 29, 2021

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2021

Codecov Report

Merging #1719 (dafdb54) into main (f8acd58) will decrease coverage by 0.19%.
The diff coverage is 6.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1719      +/-   ##
==========================================
- Coverage   11.01%   10.81%   -0.20%     
==========================================
  Files          63       64       +1     
  Lines        6839     7141     +302     
  Branches     1175     1202      +27     
==========================================
+ Hits          753      772      +19     
- Misses       6086     6369     +283     
Impacted Files Coverage Δ
src/commands/cacert/export.ts 0.00% <0.00%> (ø)
src/commands/server/deploy.ts 0.00% <0.00%> (ø)
src/tasks/component-installers/dex.ts 0.00% <0.00%> (ø)
src/tasks/installers/helm.ts 9.69% <0.00%> (-0.22%) ⬇️
src/tasks/installers/olm.ts 0.00% <0.00%> (ø)
src/tasks/installers/operator.ts 0.00% <0.00%> (ø)
src/tasks/platforms/platform.ts 0.00% <0.00%> (ø)
src/api/kube.ts 5.76% <4.09%> (-0.13%) ⬇️
src/tasks/platforms/minikube.ts 20.58% <6.66%> (-5.09%) ⬇️
src/tasks/che.ts 3.14% <10.00%> (+0.23%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8acd58...dafdb54. Read the comment docs.

@sparkoo
Copy link
Member

sparkoo commented Oct 1, 2021

I can't yarn the branch, is it issue on my side? Sorry if it is obvious issue, I don't work with yarn or javascript projects very often...

[~/dev/chectl] (dex ✘)✭ λ yarn
yarn install v1.22.4
[1/5] Validating package.json...
[2/5] Resolving packages...
[3/5] Fetching packages...
error Couldn't find match for "b8cc02bf9a6a0d6f4eb5d64e02feae8b51653bcd" in "refs/heads/1.x,refs/heads/19284,refs/heads/19978,refs/heads/20337,refs/heads/6.18.x,refs/heads/7.10.0,refs/heads/7.12.0,refs/heads/7.12.1,refs/heads/7.12.2,refs/heads/7.12.x,refs/heads/7.13.0,refs/heads/7.13.1,refs/heads/7.13.x,refs/heads/7.14.x,refs/heads/7.15.x,refs/heads/7.16.x,refs/heads/7.17.x,refs/heads/7.18.x,refs/heads/7.19.x,refs/heads/7.20.x,refs/heads/7.21.x,refs/heads/7.22.x,refs/heads/7.23.x,refs/heads/7.24.x,refs/heads/7.25.x,refs/heads/7.26.x,refs/heads/7.27.x,refs/heads/7.28.x,refs/heads/7.29.x,refs/heads/7.3.0,refs/heads/7.3.1,refs/heads/7.3.2,refs/heads/7.30.x,refs/heads/7.31.x,refs/heads/7.32.x,refs/heads/7.33.x,refs/heads/7.34.x,refs/heads/7.35.x,refs/heads/7.36.x,refs/heads/7.37.x,refs/heads/7.4.0,refs/heads/7.6.0,refs/heads/7.7.0,refs/heads/7.8.0,refs/heads/7.9.1,refs/heads/7.9.2,refs/heads/7.9.3,refs/heads/7.9.x,refs/heads/7.test,refs/heads/AddWorkspaceLogsToTheCIChecks,refs/heads/che-17090,refs/heads/che-19837,refs/heads/che-20200,refs/heads/che-20547,refs/heads/che-ci,refs/heads/cheos,refs/heads/crw-2.0,refs/heads/defaultProtocol,refs/heads/disc_back,refs/heads/dwo080-released-main,refs/heads/fix-18393,refs/heads/fix-pr-gh,refs/heads/fixCheOperatorArchCompilation,refs/heads/fixUsingCustomImageWithDeployDevCommand,refs/heads/fix_disc,refs/heads/fix_mem,refs/heads/fixes_ci,refs/heads/fixxxa,refs/heads/gh-release,refs/heads/improvetest,refs/heads/keycloak.11.0.2.Test,refs/heads/keycloakInternalNetwork,refs/heads/main,refs/heads/managedDWO,refs/heads/newerOperatorSDK-before-squash,refs/heads/ocp_ci,refs/heads/openshiftci,refs/heads/operator-tls,refs/heads/operatorPRgenFix,refs/heads/opmBundle,refs/heads/opmBundle2,refs/heads/opt_tel,refs/heads/platforms-pr,refs/heads/pr-update-base-images-1627071868,refs/heads/pr-update-base-images-1627073652,refs/heads/pr-update-base-images-1628900332,refs/heads/pr-update-base-images-1629035233,refs/heads/refactor-dir-variables,refs/heads/release-fix,refs/heads/reuseMakeFileTemplateGenerationInTheCITests,refs/heads/test_ci,refs/heads/test_release,refs/tags/1.0.1.GA,refs/tags/1.0.2.GA,refs/tags/1.1.0.GA,refs/tags/1.2.0.GA,refs/tags/1.2.2.GA,refs/tags/2.0.0.GA,refs/tags/2.1.0.GA,refs/tags/2.1.1.GA,refs/tags/2.2.0.GA,refs/tags/2.3.0.GA,refs/tags/6.19.2,refs/tags/7.0.0,refs/tags/7.0.0-RC-2.0,refs/tags/7.0.0-rc-4.0,refs/tags/7.1.0,refs/tags/7.10.0,refs/tags/7.11.0,refs/tags/7.12.0,refs/tags/7.12.1,refs/tags/7.19.0,refs/tags/7.19.1,refs/tags/7.19.2,refs/tags/7.2.0,refs/tags/7.20.0,refs/tags/7.20.1,refs/tags/7.21.0,refs/tags/7.21.1,refs/tags/7.21.2,refs/tags/7.22.0,refs/tags/7.22.1,refs/tags/7.22.2,refs/tags/7.23.0,refs/tags/7.23.1,refs/tags/7.23.2,refs/tags/7.24.0,refs/tags/7.24.1,refs/tags/7.24.2,refs/tags/7.25.0,refs/tags/7.25.1,refs/tags/7.25.2,refs/tags/7.26.0,refs/tags/7.26.1,refs/tags/7.26.2,refs/tags/7.27.0,refs/tags/7.27.1,refs/tags/7.27.2,refs/tags/7.28.0,refs/tags/7.28.1,refs/tags/7.28.2,refs/tags/7.29.0,refs/tags/7.29.1,refs/tags/7.29.2,refs/tags/7.3.0,refs/tags/7.3.1,refs/tags/7.3.2,refs/tags/7.30.0,refs/tags/7.30.1,refs/tags/7.30.2,refs/tags/7.31.0,refs/tags/7.31.1,refs/tags/7.31.2,refs/tags/7.32.0,refs/tags/7.32.1,refs/tags/7.32.2,refs/tags/7.33.0,refs/tags/7.33.1,refs/tags/7.33.2,refs/tags/7.34.0,refs/tags/7.34.1,refs/tags/7.34.2,refs/tags/7.35.0,refs/tags/7.35.1,refs/tags/7.35.2,refs/tags/7.36.0,refs/tags/7.36.1,refs/tags/7.36.2,refs/tags/7.37.0,refs/tags/7.4.0,refs/tags/7.6.0,refs/tags/7.7.0,refs/tags/7.7.1,refs/tags/7.8.0,refs/tags/7.9.0,refs/tags/7.9.1,refs/tags/7.9.2,refs/tags/7.9.3,refs/tags/v7.12.2,refs/tags/v7.13.0,refs/tags/v7.13.1,refs/tags/v7.13.2,refs/tags/v7.14.1,refs/tags/v7.14.2,refs/tags/v7.14.3,refs/tags/v7.15.0,refs/tags/v7.15.1,refs/tags/v7.15.2,refs/tags/v7.16.0,refs/tags/v7.16.1,refs/tags/v7.16.2,refs/tags/v7.17.0,refs/tags/v7.17.1,refs/tags/v7.17.2,refs/tags/v7.18.0,refs/tags/v7.18.1,refs/tags/v7.18.2" for "git://github.com/eclipse-che/che-operator".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

@tolusha
Copy link
Collaborator Author

tolusha commented Oct 1, 2021

@sparkoo
try yarn cache clean

Comment on lines 32 to 33
# bcrypt hash of the string "password"
hash: "$2a$10$2b2cU8CPhOTaGrs1HRQuAueS7JTT5ZHsHSzYiFPm1leZck7Mc8T4W"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we generate the password dynamically?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sparkoo
Could you answer pls?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's needed. It's never meant to be used in production. It's like the admin:admin we now have in keycloak...

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK, we don't have admin:admin in Keycloak for considerable amount of time. All the passwords are generated and stored in secrets.

Copy link
Member

Choose a reason for hiding this comment

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

really? I remember logging into Che with admin:admin last time I've tested on minikube

Copy link
Contributor

Choose a reason for hiding this comment

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

We are talking about different things. I was saying that we generate password for Keycloak realm, but for the user login we keep admin:admin.
So the question is: do we use this to login a Che user or Dex admin?

installers/dex/cluster-role.yaml Outdated Show resolved Hide resolved
installers/dex/service.yaml Outdated Show resolved Hide resolved
src/commands/server/deploy.ts Outdated Show resolved Hide resolved
src/tasks/component-installers/dex.ts Show resolved Hide resolved
src/tasks/installers/helm.ts Outdated Show resolved Hide resolved
src/tasks/platforms/minikube.ts Outdated Show resolved Hide resolved
src/tasks/platforms/minikube.ts Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
src/tasks/platforms/platform.ts Show resolved Hide resolved
@sparkoo
Copy link
Member

sparkoo commented Oct 4, 2021

Fails to deploy Dex for me.

    ✔ Create Dex deployment...[OK]
    ✖ Wait for Dex is ready
      → ERR_TIMEOUT: Timeout set to pod ready timeout 600000
      Configure API server
    🏃‍  Running the Eclipse Che operator
    Error: Command server:deploy failed. Error log: /home/mvala/.cache/chectl/error.log.
[~/dev/chectl/bin] (dex ✘)✹✭ λ kc get pods -n dex
NAME                 READY   STATUS             RESTARTS   AGE
dex-d79c6f7d-pltwq   0/1     CrashLoopBackOff   7          12m
[~/dev/chectl/bin] (dex ✘)✹✭ λ kc logs dex-d79c6f7d-pltwq -n dex
time="2021-10-04T11:47:09Z" level=info msg="config issuer: https://dex.192.168.39.130.nip.io:32000"
failed to initialize storage: failed to inspect service account token: jwt claim "kubernetes.io/serviceaccount/namespace" not found

@tolusha
Copy link
Collaborator Author

tolusha commented Oct 4, 2021

@sparkoo
What is your minikube version?

@sparkoo
Copy link
Member

sparkoo commented Oct 4, 2021

@sparkoo What is your minikube version?

[~/dev/chectl] (dex ✘)✹✭ λ minikube version
minikube version: v1.23.0
commit: 5931455374810b1bbeb222a9713ae2c756daee10
[~/dev/chectl] (dex ✘)✹✭ λ kubectl version
Client Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.2", GitCommit:"8b5a19147530eaac9476b0ab82980b4088bbc1b2", GitTreeState:"clean", BuildDate:"2021-09-15T21:38:50Z", GoVersion:"go1.16.8", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.1", GitCommit:"632ed300f2c34f6d6d15ca4cef3d3c7073412212", GitTreeState:"clean", BuildDate:"2021-08-19T15:39:34Z", GoVersion:"go1.16.7", Compiler:"gc", Platform:"linux/amd64"}

@tolusha
Copy link
Collaborator Author

tolusha commented Oct 4, 2021

I tested on minikube v1.21.0 and reproduced issue with v1.23.0

@tolusha
Copy link
Collaborator Author

tolusha commented Oct 4, 2021

To be honest initial scripts [1] don't work with minikube v1.23 as well
[1] https://github.com/che-incubator/che-auth-playground/tree/master/minikube_dex

@sparkoo
Copy link
Member

sparkoo commented Oct 4, 2021

@tolusha ok. I've initially run with older kubernetes version on latest minikube --kubernetes-version='1.21.4', but that doesn't help. I'll try older minikube then, but we need to make it work on latest versions as well.

@sparkoo
Copy link
Member

sparkoo commented Oct 5, 2021

works with minikube 1.21

@sparkoo
Copy link
Member

sparkoo commented Oct 5, 2021

@tolusha I believe you're using different self-signed certificates for Dex and Che. Does it deploy CA of Dex self signed cert anywhere in eclipse-che namespace? I believe we need that in order to communicate with Dex over https from Che.

staticClients:
- id: {{CLIENT_ID}}
redirectURIs:
- 'https://{{NAMESPACE}}.{{DOMAIN}}/oauth2/callback'
Copy link
Member

Choose a reason for hiding this comment

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

this is not where che is exposed

    ✔ Users Dashboard           : https://che-eclipse-che.192.168.39.106.nip.io

config:
inCluster: true
web:
https: 0.0.0.0:5556
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to listen on http as tls is already manager in ingress

@openshift-ci openshift-ci bot removed the lgtm label Oct 8, 2021
@tolusha
Copy link
Collaborator Author

tolusha commented Oct 8, 2021

@sparkoo
addressed

@sparkoo
Copy link
Member

sparkoo commented Oct 8, 2021

@tolusha thank you. Works for me now as expected.

Copy link
Member

@sparkoo sparkoo left a comment

Choose a reason for hiding this comment

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

I don't know the code enough to review it, but it's doing what is suppose to. I'm able to login, get the tokens and use them against kubernetes API.

@openshift-ci
Copy link

openshift-ci bot commented Oct 8, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AndrienkoAleksandr, mmorhun, sparkoo, tolusha

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Oct 8, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Oct 8, 2021
@tolusha tolusha marked this pull request as ready for review October 8, 2021 13:25
@tolusha
Copy link
Collaborator Author

tolusha commented Oct 8, 2021

/retest

1 similar comment
@tolusha
Copy link
Collaborator Author

tolusha commented Oct 8, 2021

/retest

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha tolusha merged commit af390a5 into main Oct 8, 2021
@tolusha tolusha deleted the dex branch October 8, 2021 17:23
@che-bot che-bot added this to the 7.38 milestone Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants