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

[build] feat: add env to set endpoints for non standard oidc auth in … #596

Closed
wants to merge 1 commit into from

Conversation

dymart
Copy link
Contributor

@dymart dymart commented Aug 2, 2022

…kubernetes

What does this PR do?

Add env vars ( KUBERNETES_PORT, KUBERNETES_PORT_443_TCP_ADDR, KUBERNETES_PORT_443_TCP, KUBERNETES_SERVICE_HOST ) to the dashboard so that one can set the endpoints for custom oidc auth in a kubernetes deployment. This also goes with the changes to che-operator that set these env vars in the deployment. This allows custom oidc endpoints for deployments in gke and other cloud providers that do not have dex setup.

goes with: eclipse-che/che-operator#1465

What issues does this PR fix or reference?

eclipse-che/che#21260

Is it tested? How?

The environment variables are added to the dashboard deployment if they are present in the operator manifest. Then is the env vars are set in the dashboard deployment the entrypoint.sh will check and see if they are present and is so set them. If not default values will stay with those env vars. Currently no tests for this.

Release Notes

Add env var to dashboard to change KUBERNETES_PORT, KUBERNETES_PORT_443_TCP_ADDR, KUBERNETES_PORT_443_TCP, KUBERNETES_SERVICE_HOST based on env vars from che-operator.

Docs PR eclipse-che/che-docs#2413

@openshift-ci
Copy link

openshift-ci bot commented Aug 2, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dymart

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

@openshift-ci
Copy link

openshift-ci bot commented Aug 2, 2022

Hi @dymart. Thanks for your PR.

I'm waiting for a eclipse-che member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@che-bot
Copy link
Contributor

che-bot commented Aug 2, 2022

Click here to review and test in web IDE: Contribute

@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #596 (ba0955a) into main (5f8108f) will decrease coverage by 0.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #596      +/-   ##
==========================================
- Coverage   56.16%   55.96%   -0.21%     
==========================================
  Files         219      223       +4     
  Lines        7449     7505      +56     
  Branches     1273     1269       -4     
==========================================
+ Hits         4184     4200      +16     
- Misses       3080     3119      +39     
- Partials      185      186       +1     
Flag Coverage Δ
unittests 55.96% <ø> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../src/pages/GetStarted/GetStartedTab/SampleCard.tsx 67.50% <0.00%> (-32.50%) ⬇️
...es/GetStarted/GetStartedTab/SamplesListGallery.tsx 65.33% <0.00%> (-9.25%) ⬇️
packages/dashboard-frontend/src/preload/index.ts 82.05% <0.00%> (-1.29%) ⬇️
...dashboard-backend/src/devworkspace-client/index.ts 39.28% <0.00%> (-0.72%) ⬇️
...rd-frontend/src/components/DevfileEditor/index.tsx 27.95% <0.00%> (-0.39%) ⬇️
packages/dashboard-frontend/src/Routes/index.tsx 73.91% <0.00%> (ø)
...kages/dashboard-backend/src/api/devworkspaceApi.ts 0.00% <0.00%> (ø)
...kages/dashboard-backend/src/api/dockerConfigApi.ts 0.00% <0.00%> (ø)
...rd-frontend/src/containers/FactoryLoader/index.tsx 81.35% <0.00%> (ø)
...frontend/src/store/Plugins/chePlugins/selectors.ts 100.00% <0.00%> (ø)
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@dymart
Copy link
Contributor Author

dymart commented Aug 7, 2022

Can test this with: eclipse-che/che-operator#1468

@Jonher937
Copy link

Jonher937 commented Oct 5, 2022

Can test this with: eclipse-che/che-operator#1468

It seems to work overriding the KUBERNETES_SERVICE_HOST via spec.components.dashboard.containers.env
However certificates are not trusted from either the CHE issuer or the custom-ca bundle attached via the optional ConfigMap.

@ibuziuk
Copy link
Member

ibuziuk commented Dec 12, 2022

@Jonher937 @dymart folks, are you ok with closing this PR? the env vars should have been set on the operator level

@ibuziuk ibuziuk closed this Jan 20, 2023
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.

5 participants