-
Notifications
You must be signed in to change notification settings - Fork 71
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
fix: Adding ability to use kubernetes API proxy on Kubernetes #6
Conversation
Signed-off-by: xbaran4 <pbaran@redhat.com>
Signed-off-by: xbaran4 <pbaran@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
Can you please build docker image and write some better "how to test"? How to deploy it, with what CheCluster patch and how to test actual endpoint (with curl? some example?)
assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
Show resolved
Hide resolved
Ideally, you should not ask for it PR author. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Yes, ideally. I agree :)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @sparkoo that the config property description needs to mention that it poses no security risk on OpenShift. Otherwise LGTM, good job!
Can one of the admins verify this patch? |
Signed-off-by: xbaran4 <pbaran@redhat.com>
Signed-off-by: xbaran4 <pbaran@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - just a small comment about getAuthenticatedHttpClient
but feel free to merge without addressing it.
...c/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesClientFactory.java
Outdated
Show resolved
Hide resolved
Signed-off-by: xbaran4 <pbaran@redhat.com>
assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
Outdated
Show resolved
Hide resolved
assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
Outdated
Show resolved
Hide resolved
assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties
Outdated
Show resolved
Hide resolved
I'm trying to run it on minikube with Even though it tries to request k8s api, it fails with Is image up to date? full exception log:
|
…/che/che.properties Co-authored-by: Fabrice Flore-Thébault <ffloreth@redhat.com>
…/che/che.properties Co-authored-by: Fabrice Flore-Thébault <ffloreth@redhat.com>
…/che/che.properties Co-authored-by: Fabrice Flore-Thébault <ffloreth@redhat.com>
had the same thing as @sparkoo Will take a look tomorrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems https://www.eclipse.org/che/docs/che-7/installation-guide/enabling-dev-workspace-engine/ needs to be adapted
after enabling devworspaces on minikube I got.
|
I think we do need support of headers. At least content-type. Request PATCH devworkspacetemplate
|
Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Signed-off-by: xbaran4 pbaran@redhat.com
What does this PR do?
Adds property, that will allow the use of /unsupported/k8s even when on Kubernetes infrastructure.
Screenshot/screencast of this PR
What issues does this PR fix or reference?
eclipse-che/che#19813
How to test this PR?
kubectl patch checluster/eclipse-che -n eclipse-che --type=merge -p "$(cat patch.yaml)"
where patch.yaml contains:
3.run
where CHE_HOST is the URL of your che-server instance and TOKEN is your users keycloak token
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or reference
andHow to test this PR
completedReviewers
Reviewers, please comment how you tested the PR when approving it.