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

CHE-14527: Support git clone for repos with self-signed SSL certs #15084

Closed
wants to merge 19 commits into from

Conversation

vparfonov
Copy link
Contributor

@vparfonov vparfonov commented Nov 4, 2019

What does this PR do?

Support git clone for repos with self-signed SSL certs
CHE-14527: Support git clone for repos with self-signed SSL certs

How it works:

For testing on my laptop I setup local git server which support cloning my https (e.g. https://gogs.io/) pretty easy for installation.
After successfully deploy Eclipse on minikube need to execute next command:
kubectl create configmap che-git-self-signed-cert --from-file=ca.crt --from-literal=githost={host}:{port} -n=che
where:

ca.crt - your self-signed certificate 
{host}:{port} - host and port for HTTPS connection on your git server.

Now you can create and start new workspace.
On each container of your workspace will be mount special volume that contains file with your self-signed certificate and to the gitconfig section with information about git server host and path to the certificate. E.g.

[http "https://10.33.177.118:3000"]
        sslCAInfo = /etc/che/git/cert/cert.pem

What issues does this PR fix or reference?

#14527

Release Notes

Docs PR

@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Nov 4, 2019
@che-bot
Copy link
Contributor

che-bot commented Nov 4, 2019

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 4, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@che-bot
Copy link
Contributor

che-bot commented Nov 5, 2019

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 5, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

In general LGTM.

The idea with multiple self-signed certs definitely can be implemented in the scope of a separate issue if it's valid and important.

Let me know when my inline comments are addressed and PR is ready to review.


# If git-self-signed-cert is used then configure Che Server with certificate content
# to propagate it to trust store
{{- if .Values.global.useGitSelfSignedCerts }}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can with minimal changes support multiple self-signed certs for different hosts?
On one hand it's quite easy to imagine config map like

data:
  my-git.com:9232: --BEGINCERT adssaxxxxxx --
  my-another-git.org:3716: --BEGINCERT adssaxxxxxx --

On another hand, it's not clear how to inject each property to the server. Ways I see:

  1. Just configure SecretName for Che Server, and then Che Server will ready it from K8s API and propagate to workspaces.
  2. Mount whole secret as a folder, where files names are hosts, and content - is ca.cert. And let Che Server know where these folder is mount.

private final PreviewUrlExposer<KubernetesEnvironment> previewUrlExposer;
private VcsSslCertificateProvisioner vcsSslCertificateProvisioner;
Copy link
Member

Choose a reason for hiding this comment

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

please add final

static final String CHE_GIT_SELF_SIGNED_CERT_CONFIG_MAP_SUFFIX = "-che-git-self-signed-cert";
static final String CHE_GIT_SELF_SIGNED_VOLUME = "che-git-self-signed-cert";
static final String CERT_MOUNT_PATH = "/etc/che/git/cert/";
static final String CA_CERT_FILE = "cert.pem";
Copy link
Member

Choose a reason for hiding this comment

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

Could you elaborate on what is the purpose of injecting ca.crt and propagate it as cert.pem.
Would not it be more straightforward to propagate certificate as initially configured ca.crt?

this.host = host;
}

/** @return true only if */
Copy link
Member

Choose a reason for hiding this comment

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

true only if what? 😄


public String getGitServerHost() {
if (isNullOrEmpty(host)) {
return nullToEmpty(host);
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate more about what will happen if host is empty? Is it a valid case? Then git will use such a certificate for everything?

return CERT_MOUNT_PATH + CA_CERT_FILE;
}

public String getGitServerHost() {
Copy link
Member

Choose a reason for hiding this comment

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

I would not expect from this method to return server host with escaped ".
I think it's specific to gitconfig constructing and you append " in GitConfigProvisioner instead

@@ -1,10 +1,13 @@
{
"name": "@eclipse-che/api",
"version": "7.4.0-SNAPSHOT",
"version": "7.4.0-7a47579",
Copy link
Member

Choose a reason for hiding this comment

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

Seems it's not related to changes that you described in PR description

@che-bot
Copy link
Contributor

che-bot commented Nov 13, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

Signed-off-by: Vitalii Parfonov <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Nov 13, 2019

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 13, 2019

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 13, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@vparfonov vparfonov closed this Nov 21, 2019
@vparfonov vparfonov deleted the che#14527 branch February 18, 2020 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants