Init operations for workspaces with Git Init as initial implemetation #69 #182

jfaltermeier · 2023-05-22T14:01:11Z

CRD, RBAC changes: https://github.com/eclipsesource/theia-cloud-helm/pull/26/files

add a new docker image (theiacloud/theia-cloud-git-init) that can be used to clone a git repository and checkout a specific pointer
- support HTTPS and SSH auth
- can be tested outside of Theia Cloud with the instructions in python/git-init/README.md (you may have to create/find approriate Git repositories to test though. Note: https://gitlab.eclipse.org allows to create private forks of existing projects in your user namespace!)
arguments to create a init container using the new image are passed via the new initOperations array of a session. The container is created in GitInitOperationHandler
- for private repositories the handler currently expects that the secret is available as a kubernetes secret already
- besides that the secret has to be annotated to be used by the operator. Otherwise it will not be exposed. See the sample secrets in python/git-init/README.md
- since the secrets are only available in the init container, they cannot be read from the main IDE
Via the InitOperationHandler new init operation handlers may be added to the Operator
extends the REST Service API and Typescript Common API to allow creating sessions with init operations

Open issues / Known limitations are:

the actual IDE has no git config with the user email yet
If we did a clone using SSH the private key is not made available to the main IDE yet
Does not work with ephemeral sessions, because we have no volume we could mount in the init container
We have no API to create the required Kubernetes Secrets. Currently it is expected that an admin user adds the credentials to kubernetes with the required annotations

For the credential handling I am not sure if this should be a first level Theia Cloud Feature. We could offer some library/util that helps with creating the secrets/adding the annotations, but I think this should be used from client code, e.g. from a dashboard implementation.

lucas-koehler

Initial code review. I will test this later.
Code structure and code looks mostly good to me already :) . I added some inline comments.

doc/docs/Building-Internal.md

dockerfiles/git-init/Dockerfile

lucas-koehler · 2023-06-06T14:27:04Z

...heia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/SessionSpec.java

-		+ ", error=" + error + ", workspace=" + workspace + ", lastActivity=" + lastActivity + "]";
+		+ ", error=" + error + ", workspace=" + workspace + ", lastActivity=" + lastActivity
+		+ ", sessionSecret=" + sessionSecret + ", envVars=" + envVars + ", envVarsFromConfigMaps="
+		+ envVarsFromConfigMaps + ", envVarsFromSecrets=" + envVarsFromSecrets + ", initOperations="


Should we omit envVarsFromSecrets here to avoid leaking secrets in logs, etc.?

Those are just the names of the secrets under which they are stored in k8s and not their actual values

lucas-koehler · 2023-06-06T14:45:18Z

...heia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/SessionSpec.java

    }

    public static boolean isEphemeral(String workspace) {
 	return workspace == null || workspace.isBlank();
    }

+    public static class InitOperation {
+
+	@JsonProperty("id")


Should this be required?

I've added id as required in the CRD: https://github.com/eclipsesource/theia-cloud-helm/blob/fbbceb8196189367e76106504a03c2f02bfc737e/charts/theia-cloud-crds/templates/session-spec-resource.yaml#L70C25-L70C25

...heia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/resource/SessionSpec.java

...tor/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/GitInitOperationHandler.java

lucas-koehler · 2023-06-06T15:10:30Z

...tor/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/GitInitOperationHandler.java

+	if (isHTTP(repository)) {
+	    if (!injectHTTPRepoCredentials(correlationId, secret, secretName, repository, gitInitContainer)) {
+		return;
+	    }
+	} else {
+	    if (!injectSSHRepoCredentials(correlationId, secret, secretName, repository, gitInitContainer, volumes)) {
+		return;
+	    }
+	}


What is the purpose of the returns? There is nothing after this. If this is some kind of error, should something be logged?

All the logging is done in the inject methods itself.
The returns are executed when there was a problem.
Reason is that if we add more init code at the end at some point (e.g. injecting the SSH Key in the IDE) or if an adopter overrides the handler and calls super, we only run this new code if there were no issues.
I've added more comments, because it looks weird indeed.

lucas-koehler · 2023-06-06T15:11:52Z

...tor/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/GitInitOperationHandler.java

+	    String repositoryWithoutProtocol = split[1];
+	    if (repositoryWithoutProtocol.contains("@")) {
+		if (repositoryWithoutProtocol.split(Pattern.quote("@"))[0].contains(":")) {
+		    /* username and password part of URL */


Is this if empty on purpose?

Yes, injectUsername and injectPassword are false by default and we won't inject either if both username and password are part of the repo url.
I've adopted the comment

lucas-koehler · 2023-06-06T15:12:42Z

...tor/src/main/java/org/eclipse/theia/cloud/operator/handler/impl/GitInitOperationHandler.java

+	LOGGER.info(LogMessageUtil.formatLogMessage(correlationId,
+		MessageFormat.format("Inject username: {0}; Inject password: {1}", injectUsername, injectPassword)));


Should we log credentials? This might expose secrets in the log.

This just a boolean whether they will be injected or not

....eclipse.theia.cloud.service/src/main/java/org/eclipse/theia/cloud/service/BaseResource.java

* create init container that can be used to checkout git repositories with https and ssh

* add ability to checkout branches/commits/tags

* add list of InitOperations to Session * introduce binding of multiple InitOperationHandler * implement GitInitOperationHandler * add init container with required environment variables for HTTP(S) Git checkout * checks that the secret is allowed to be used by this user and by this InitOperationHandler

* implement injecting ssh key and password to init container * adjust git init script to work with any user id

* make auth info optional (for public repos) * regenerate open api schema * regenerate typescript API

* Extend public API * add verification job for new docker image * do not update all references to @eclipse-theiacloud/common: 0.8.1-alpha.2 yet, since this is not released yet

* remove osweek branches from workflow files * fix typos * explicitly use python3

github-actions · 2024-05-26T09:34:34Z

This PR is stale because it has been open 180 days with no activity.

lucas-koehler · 2024-05-28T07:45:53Z

keep open

parzival418 · 2024-08-05T23:32:57Z

Any word on this? This would be amazing for some work we are looking to use Theia Cloud for.

jfaltermeier · 2024-08-07T06:31:44Z

Any word on this? This would be amazing for some work we are looking to use Theia Cloud for.

Thank you for your interest in this feature! Currently, our main priority is transferring the codebase to the Eclipse Foundation. Once this process is complete, we plan to revisit and continue development on earlier work, including this ticket.

jfaltermeier force-pushed the jf/git-init branch 2 times, most recently from c0fe595 to daaeb3f Compare May 22, 2023 14:11

jfaltermeier marked this pull request as ready for review May 22, 2023 14:36

jfaltermeier requested a review from lucas-koehler May 22, 2023 14:37

lucas-koehler reviewed Jun 6, 2023

View reviewed changes

jfaltermeier added 6 commits November 23, 2023 15:24

Git Init Container for cloning #69

f84e9de

* create init container that can be used to checkout git repositories with https and ssh

Git Init Container checkout #69

a873805

* add ability to checkout branches/commits/tags

SSH Auth for Git Init #69

819a61a

* implement injecting ssh key and password to init container * adjust git init script to work with any user id

Don't clone if directory exists and is not empty #69

17e75b6

Add new API to Service #69

b78952a

* make auth info optional (for public repos) * regenerate open api schema * regenerate typescript API

jfaltermeier force-pushed the jf/git-init branch from daaeb3f to f06f15f Compare November 24, 2023 11:34

Add new API to Service #69

81269b9

* Extend public API * add verification job for new docker image * do not update all references to @eclipse-theiacloud/common: 0.8.1-alpha.2 yet, since this is not released yet

jfaltermeier force-pushed the jf/git-init branch 4 times, most recently from cc41923 to 01db0af Compare November 27, 2023 11:59

Cleanup #69

2ca1612

* remove osweek branches from workflow files * fix typos * explicitly use python3

jfaltermeier force-pushed the jf/git-init branch from 01db0af to 2ca1612 Compare November 27, 2023 12:01

github-actions bot added the stale label May 26, 2024

jfaltermeier removed the stale label Jul 3, 2024

iyannsch mentioned this pull request Sep 5, 2024

Initialize session with git repo #348

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Init operations for workspaces with Git Init as initial implemetation #69 #182

Init operations for workspaces with Git Init as initial implemetation #69 #182

jfaltermeier commented May 22, 2023 •

edited

Loading

lucas-koehler left a comment

lucas-koehler Jun 6, 2023

jfaltermeier Nov 24, 2023

lucas-koehler Jun 6, 2023

jfaltermeier Nov 27, 2023

lucas-koehler Jun 6, 2023

jfaltermeier Nov 27, 2023 •

edited

Loading

lucas-koehler Jun 6, 2023

jfaltermeier Nov 27, 2023

lucas-koehler Jun 6, 2023

jfaltermeier Nov 24, 2023

github-actions bot commented May 26, 2024

lucas-koehler commented May 28, 2024

parzival418 commented Aug 5, 2024

jfaltermeier commented Aug 7, 2024

		LOGGER.info(LogMessageUtil.formatLogMessage(correlationId,
		MessageFormat.format("Inject username: {0}; Inject password: {1}", injectUsername, injectPassword)));

Init operations for workspaces with Git Init as initial implemetation #69 #182

Are you sure you want to change the base?

Init operations for workspaces with Git Init as initial implemetation #69 #182

Conversation

jfaltermeier commented May 22, 2023 • edited Loading

lucas-koehler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfaltermeier Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 26, 2024

lucas-koehler commented May 28, 2024

parzival418 commented Aug 5, 2024

jfaltermeier commented Aug 7, 2024

jfaltermeier commented May 22, 2023 •

edited

Loading

jfaltermeier Nov 27, 2023 •

edited

Loading