Add Azure DevOps Server support by vinokurig · Pull Request #754 · eclipse-che/che-server

vinokurig · 2025-01-20T14:28:14Z

What does this PR do?

Add Azure DevOps Server support to use with PAT ONLY, as the azure Devops Api does not support oauth2, see: Support for Microsoft Azure DevOps Server (TFS) che#23306 (comment)
Change the gitconfig storage configmap to a secret because we are going to mount sensitive data to the devworkspace system scope gitconfig, see: Change the git-userdata configmap to secret che-dashboard#1313

Depends on eclipse-che/che-dashboard#1313

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes eclipse-che/che#23306

How to test this PR?

Deploy che with the pull request image: quay.io/eclipse/che-server:pr-754.
In the Dev azure Server instance create a personal access token with full access.
Create a personal access token, use the Organization input to enter the Collection name.
Start a workspace from an azure devops server repository with a devfile.

See: workspace starts with the devfile resolve.

PR Checklist

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

Release Notes

Reviewers

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

vinokurig · 2025-01-21T15:10:25Z

/retest

vinokurig · 2025-01-22T07:37:09Z

/retest

tolusha · 2025-01-23T11:24:08Z

...ure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsUrl.java

  }

+  public AzureDevOpsUrl withServerUrl(String serverUrl) {
+    this.serverUrl = serverUrl;


When use set serverUrl, then hostname is null, which is used later in getRepositoryLocation

The hostName does not rely on serverUrl, we set it independently:

che-server/wsmaster/che-core-api-factory-azure-devops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsURLParser.java

Lines 198 to 200 in 4c865ce

return new AzureDevOpsUrl()

.withHostName(

url.startsWith("git@ssh.") ? azureDevOpsScmApiEndpointHost : URI.create(url).getHost())

tolusha · 2025-01-23T11:34:31Z

...a/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsPersonalAccessTokenFetcher.java

    if (!isValidScmServerUrl(params.getScmProviderUrl())) {
-      LOG.debug("not a valid url {} for current fetcher ", params.getScmProviderUrl());
-      return Optional.empty();
+      if (OAUTH_PROVIDER_NAME.equals(params.getScmProviderName())) {


Can we refactor this fuction?
For instance, move if (OAUTH_PROVIDER_NAME.equals(params.getScmProviderName())) { before if (!isValidScmServerUrl(params.getScmProviderUrl())) {

We need to check if the token belongs to Azure DevOps SAAS or Server first. The isValidScmServerUrl function name is a bit misleading, so I renamed it.

tolusha · 2025-01-23T11:45:18Z

...rc/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsUserDataFetcher.java

-        azureDevOpsApiClient.getUserWithPAT(
-            personalAccessToken.getToken(), personalAccessToken.getScmOrganization());
-    return new GitUserData(user.getDisplayName(), user.getEmailAddress());
+    if (personalAccessToken.getScmProviderUrl().equals("https://dev.azure.com")) {


We have AzureDevOps class for contants

tolusha · 2025-01-27T09:52:45Z

...-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/AbstractGitlabUrlParser.java

      } catch (ScmUnauthorizedException e) {
-        return true;
+        // the error message is a JSON if it is a response from Gitlab.
+        return e.getMessage().startsWith("{");


Do we really need this changes in context of azure?

Yes, we need to specify the unauthorized exception as Azure also returns an unauthorized exception in this case.

what is the response from Azure in this case?

I moved the json validation check to a separate private function, enhanced the check with JSON parser and updated the Javadoc.

...vops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsURLParser.java

+
+  private Optional<Matcher> getPatternMatcherByUrl(String url) {
+    String host = URI.create(url).getHost();
+    Matcher matcher = compile(format(azureDevOpsPatternTemplate, host)).matcher(url);


...vops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsURLParser.java

+    if (matcher.matches()) {
+      return Optional.of(matcher);
+    } else {
+      matcher = compile(format(azureSSHDevOpsPatternTemplate, host)).matcher(url);


...vops/src/main/java/org/eclipse/che/api/factory/server/azure/devops/AzureDevOpsURLParser.java

+      if (matcher.matches()) {
+        return Optional.of(matcher);
+      } else {
+        matcher = compile(format(azureSSHDevOpsServerPatternTemplate, host)).matcher(url);


...e/infrastructure/kubernetes/namespace/configurator/GitconfigAutomauntSecretConfigurator.java

...-factory-azure-devops/src/test/resources/__files/azure-devops-server/rest/user/response.json

...i-factory-azure-devops/src/test/resources/__files/azure-devops/rest/user/email/response.json

...ore-api-factory-azure-devops/src/test/resources/__files/azure-devops/rest/user/response.json

ibuziuk · 2025-02-28T11:51:41Z

...-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/AbstractGitlabUrlParser.java

-        return true;
+        // Some Git providers e.g. Azure Devops Server, may return unauthorized exception on invalid
+        // API request, but Gitlab API returns unauthorized error message in JSON format, so to be
+        // sure that the URL belongs to Gitlab, we need to check if the error message is a valid
+        // JSON.
+        return isJsonValid(e.getMessage());


@vinokurig I'm a bit lost with Gitlab change in general.
Could you clarify why are we changing AbstractGitlabUrlParser.java in the context of the Azure TFS onboarding? How come GitLab flow is affected by the Azure Devops Server ?

When we resolve a factory url we iterate over provider implementations and check if the url corresponds to any known implemented provider.
In the Gitlab implementation, when we resolve a public Gitlab Server repo url but neither oauth is configured nor a PAT is added, we use the isApiRequestRelevant()function:
The function extracts the server url from the repository url and with this server url we make a test Gitlab Server API call. The idea is: if the test api call returns unauthorised error, it means that the extracted server url is a Gitlab Server url and we treat the repository url as a Gitlab Server url.

The problem is that Azure Devops Server Api returns unauthorized response on any invalid API request, so when we iterate the Azure repo url through the Gitlab implementation, the url is tested by the isApiRequestRelevant()function and according to the response status it becomes a Gitlab url.

To be able to distinguish the unauthorized response of Azure Devops Server url and Gitlab Server url, we check the error message format, if it is in JSON format, we can assume that the url is a Gitlab repo, but if the error message is a plain text, we continue the iteration because the url is an Azure Devops Server repo.

ibuziuk

@artaleks9 @dmytro-ndp folks, please provide your approval before merging as well. Looks like Gitlab test is failing right now (please make sure that all PR checks are merged before we merge)

openshift-ci · 2025-03-03T13:48:59Z

@vinokurig: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/v14-gitlab-with-oauth-setup-flow	`7681e71`	link	true	`/test v14-gitlab-with-oauth-setup-flow`

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

dmytro-ndp · 2025-03-04T15:34:38Z

@vinokurig : after the review of this feature demo, I can admit that adding extraheader under the [http] section manually, with need to encode the personal access token and enter it in the 'Import Git Config' text area of User Preferences, could be a challenge for regular user.
Do you think it's possible to get rid of this step by reusing existing Personal Access Token entered on the 'User Preferences > Personal Access Token' page?

vinokurig · 2025-03-04T15:41:33Z

@dmytro-ndp
That's a good point, we have discussed it with @ibuziuk and decided to keep the manual flow to be sure that this logic will not break the factory flow with other providers. Since we are adding authorization header to all git requests we want user to be aware of that and if something goes wrong, the user will be able to revert the header.
When we are sure that this is safe we will add a mechanism that will automatically add the configuration to the workspace gitconfig.

SkorikSergey · 2025-03-12T11:46:37Z

@vinokurig : hello
FYI:

verification of eclipse-che/che#23306 issue fix up has been passed.

See test details below:

Test scenario to check eclipse-che/che#23306 issue:

Deploy che with the pull request image: quay.io/eclipse/che-server:pr-754.
In the Dev azure Server instance create a personal access token with full access.
Create a personal access token, use the Organization input to enter the Collection name.
Change gitconfig according to documentation
Start a workspace from an azure devops server repository with a devfile.
Create new file and commit changes to new brach.
Push changes and check them on Azure Server test repo page.

openshift-ci · 2025-03-12T11:47:24Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ibuziuk, SkorikSergey, vinokurig

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

SkorikSergey · 2025-03-12T12:24:31Z

@vinokurig After adding token according to this documentation I have also started workspace from private repo GitHub Enterprise Server(OAuth accepted). Workspace started successfully but project was not cloned. After removing token from gitconfig works as expected.

ibuziuk · 2025-03-12T15:04:54Z

@vinokurig I think we should at very least mention this caveat in the docs

devspacesbuild · 2025-03-13T07:26:46Z

Build 3.20 :: server_3.x/389: Console, Changes, Git Data

devspacesbuild · 2025-03-13T07:37:37Z

Build 3.20 :: sync-to-downstream_3.x/8902: Console, Changes, Git Data

devspacesbuild · 2025-03-13T07:41:41Z

Build 3.20 :: get-sources-rhpkg-container-build_3.x/9021: Console, Changes, Git Data

devspacesbuild · 2025-03-13T07:47:08Z

Build 3.20 :: get-sources-rhpkg-container-build_3.x/9021:

server : 3.x :: Failed in 66972204 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

Fix the build failure caused by merging #754

vinokurig requested review from SDawley, ibuziuk and tolusha as code owners January 20, 2025 14:28

vinokurig marked this pull request as draft January 20, 2025 14:40

openshift-ci bot added the do-not-merge/work-in-progress label Jan 20, 2025

vinokurig force-pushed the che-23306 branch 2 times, most recently from 259113e to 4c865ce Compare January 21, 2025 12:58

vinokurig marked this pull request as ready for review January 21, 2025 13:35

openshift-ci bot removed the do-not-merge/work-in-progress label Jan 21, 2025

tolusha reviewed Jan 23, 2025

View reviewed changes

tolusha reviewed Jan 27, 2025

View reviewed changes

github-advanced-security bot found potential problems Jan 29, 2025

View reviewed changes

vinokurig closed this Feb 5, 2025

vinokurig deleted the che-23306 branch February 5, 2025 14:01

vinokurig restored the che-23306 branch February 5, 2025 14:01

vinokurig reopened this Feb 5, 2025

vinokurig force-pushed the che-23306 branch 2 times, most recently from 80c8b96 to 12721bc Compare February 6, 2025 12:00

vinokurig marked this pull request as draft February 7, 2025 08:43

openshift-ci bot added the do-not-merge/work-in-progress label Feb 7, 2025

github-advanced-security bot found potential problems Feb 8, 2025

View reviewed changes

...e/infrastructure/kubernetes/namespace/configurator/GitconfigAutomauntSecretConfigurator.java Fixed Show fixed Hide fixed

...e/infrastructure/kubernetes/namespace/configurator/GitconfigAutomauntSecretConfigurator.java Fixed Show fixed Hide fixed

vinokurig force-pushed the che-23306 branch 4 times, most recently from cfaf300 to 95d556a Compare February 10, 2025 08:40

github-advanced-security bot found potential problems Feb 10, 2025

View reviewed changes

...e/infrastructure/kubernetes/namespace/configurator/GitconfigAutomauntSecretConfigurator.java Fixed Show fixed Hide fixed

...e/infrastructure/kubernetes/namespace/configurator/GitconfigAutomauntSecretConfigurator.java Fixed Show fixed Hide fixed

vinokurig force-pushed the che-23306 branch 2 times, most recently from 2e1f709 to 3531467 Compare February 10, 2025 10:53

vinokurig force-pushed the che-23306 branch from 27bcfca to 9e52d56 Compare February 27, 2025 13:09

ibuziuk reviewed Feb 28, 2025

View reviewed changes

...-factory-azure-devops/src/test/resources/__files/azure-devops-server/rest/user/response.json Outdated Show resolved Hide resolved

ibuziuk reviewed Feb 28, 2025

View reviewed changes

...i-factory-azure-devops/src/test/resources/__files/azure-devops/rest/user/email/response.json Outdated Show resolved Hide resolved

ibuziuk reviewed Feb 28, 2025

View reviewed changes

...ore-api-factory-azure-devops/src/test/resources/__files/azure-devops/rest/user/response.json Outdated Show resolved Hide resolved

ibuziuk reviewed Feb 28, 2025

View reviewed changes

ibuziuk requested review from artaleks9 and dmytro-ndp February 28, 2025 12:26

ibuziuk approved these changes Feb 28, 2025

View reviewed changes

openshift-ci bot added lgtm and removed lgtm labels Feb 28, 2025

fixup! Add Azure DevOps Server support

7681e71

vinokurig force-pushed the che-23306 branch from d689ed0 to 7681e71 Compare March 3, 2025 12:34

SkorikSergey approved these changes Mar 12, 2025

View reviewed changes

openshift-ci bot assigned SkorikSergey Mar 12, 2025

openshift-ci bot added the lgtm label Mar 12, 2025

vinokurig merged commit 870aed1 into main Mar 13, 2025
26 of 28 checks passed

vinokurig deleted the che-23306 branch March 13, 2025 07:20

vinokurig mentioned this pull request Mar 13, 2025

Fix build #782

Merged

9 tasks

vinokurig added a commit that referenced this pull request Mar 13, 2025

Fix build (#782)

1a582ce

Fix the build failure caused by merging #754

	return new AzureDevOpsUrl()
	.withHostName(
	url.startsWith("git@ssh.") ? azureDevOpsScmApiEndpointHost : URI.create(url).getHost())

Conversation

vinokurig commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Release Notes

Reviewers

Uh oh!

vinokurig commented Jan 21, 2025

Uh oh!

vinokurig commented Jan 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Check failure

Uh oh!

Uh oh!

Uh oh!

Check failure

Uh oh!

Uh oh!

Uh oh!

Check failure

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ibuziuk left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Mar 3, 2025

Uh oh!

dmytro-ndp commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vinokurig commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SkorikSergey commented Mar 12, 2025

Uh oh!

openshift-ci bot commented Mar 12, 2025

Uh oh!

SkorikSergey commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ibuziuk commented Mar 12, 2025

Uh oh!

Uh oh!

devspacesbuild commented Mar 13, 2025

vinokurig commented Jan 20, 2025 •

edited

Loading

dmytro-ndp commented Mar 4, 2025 •

edited

Loading

vinokurig commented Mar 4, 2025 •

edited

Loading

SkorikSergey commented Mar 12, 2025 •

edited

Loading