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

Omit scm-username annotation from the PAT secret #533

Merged
merged 12 commits into from
Jul 25, 2023
Merged

Omit scm-username annotation from the PAT secret #533

merged 12 commits into from
Jul 25, 2023

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Jul 19, 2023

What does this PR do?

Ignore the che.eclipse.org/scm-username annotation when fetching token from a PAT secret. Instead fetch the username from the isValid(<token>) request.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#22344

How to test this PR?

  1. Deploy che using the PR image: quay.io/ivinokur/che-server:che-22344
  2. Create a PAT of any of the supported scm providers:
  • Go to dashboard -> user-preferences -> Personal Access Tokens tab and create a token but enter a random string in the username input.
  • Create a PAT secret in the user namespace manually but either omit the che.eclipse.org/scm-username annotation or use a random string value
  1. Create a workspace from a private repository of the used scm provider.

PR Checklist

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

Reviewers

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

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

I'm confused by the three versions of isValid in this PR -- one returns a bool, one returns a string, and one returns pair<bool, string>

@artaleks9
Copy link
Contributor

/retest-required

1 similar comment
@artaleks9
Copy link
Contributor

/retest-required

@tolusha
Copy link
Contributor

tolusha commented Jul 20, 2023

Tested for Azure DevOps

@artaleks9
Copy link
Contributor

/retest-required

@vinokurig
Copy link
Contributor Author

/retest

@artaleks9
Copy link
Contributor

@vinokurig
Verified for GitLab.com - works properly.
But for GitHub.com - there is a problem:

  • The github-with-pat-setup-flow test crashes
    with PAT configured for private GitHub repo, factory resolver doesn't return 200..
  • I tried to manually set up PAT for GitHub.com and run factory - and I get an error..

@vinokurig
Copy link
Contributor Author

Rolled back the PAT scopes validation for GitHub and GitLab, created a separate issue for that: eclipse-che/che#22381

@vinokurig
Copy link
Contributor Author

/retest-required

// belongs to Bitbucket.
bitbucketServerApiClient.getPersonalAccessToken("", 0L);
bitbucketServerApiClient.getUser(null);
Copy link
Member

Choose a reason for hiding this comment

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

why do we pass null as an argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If null is passed the client fetches OAuth token:

/**
* @param token token to override. Pass {@code null} to use token from the authentication flow.
* @return - Retrieve the {@link BitbucketUser} matching the supplied userSlug.
* @throws ScmItemNotFoundException
* @throws ScmUnauthorizedException
* @throws ScmCommunicationException
*/
BitbucketUser getUser(@Nullable String token)
throws ScmItemNotFoundException, ScmUnauthorizedException, ScmCommunicationException;

@artaleks9
Copy link
Contributor

/retest-required

@artaleks9
Copy link
Contributor

@vinokurig
It seems, there is a some problem with deploy Che using the last quay.io/eclipse/che-server:pr-533 image of PR:

[22:31:41] → Failed to start a pod, reason: Error, exitCode: 1
[22:31:41] Eclipse Che Server pod bootstrap [failed]
[22:31:41] → Failed to start a pod, reason: Error, exitCode: 1
[22:31:41] Wait for Eclipse Che ready [failed]
[22:31:41] → Failed to start a pod, reason: Error, exitCode: 1
    Error: Command server:deploy failed with the error: Failed to start a pod,

I tried to install Che manually, but got the error with start che-server pod, here is the che-pod log:

Using custom assembly from /home/user/eclipse-che
Found a custom cert. Adding it to java trust store /home/user/cacerts based on /usr/lib/jvm/jre/lib/security/cacerts
Exception in thread "main" java.lang.Error: java.io.FileNotFoundException: /usr/lib/jvm/java-11-openjdk-11.0.20.0.8-2.el8.x86_64/lib/tzdb.dat (No such file or directory)
	at java.base/sun.util.calendar.ZoneInfoFile$1.run(ZoneInfoFile.java:261)
	at java.base/sun.util.calendar.ZoneInfoFile$1.run(ZoneInfoFile.java:251)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/sun.util.calendar.ZoneInfoFile.<clinit>(ZoneInfoFile.java:251)
	at java.base/sun.util.calendar.ZoneInfo.getTimeZone(ZoneInfo.java:588)
	at java.base/java.util.TimeZone.getTimeZone(TimeZone.java:577)
	at java.base/java.util.TimeZone.setDefaultZone(TimeZone.java:682)
	at java.base/java.util.TimeZone.getDefaultRef(TimeZone.java:653)
	at java.base/java.util.Date.normalize(Date.java:1198)
	at java.base/java.util.Date.toString(Date.java:1031)
	at java.base/sun.security.tools.keytool.Main.printX509Cert(Main.java:3537)
	at java.base/sun.security.tools.keytool.Main.addTrustedCert(Main.java:3317)
	at java.base/sun.security.tools.keytool.Main.doCommands(Main.java:1222)
	at java.base/sun.security.tools.keytool.Main.run(Main.java:421)
	at java.base/sun.security.tools.keytool.Main.main(Main.java:414)
Caused by: java.io.FileNotFoundException: /usr/lib/jvm/java-11-openjdk-11.0.20.0.8-2.el8.x86_64/lib/tzdb.dat (No such file or directory)
	at java.base/java.io.FileInputStream.open0(Native Method)
	at java.base/java.io.FileInputStream.open(FileInputStream.java:219)
	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157)
	at java.base/sun.util.calendar.ZoneInfoFile$1.run(ZoneInfoFile.java:255)
	... 14 more

@devstudio-release
Copy link

Build 3.8 :: update-digests_3.8/132: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.8 :: operator-bundle_3.8/56: Console, Changes, Git Data

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.8 :: sync-to-downstream_3.8/67: SUCCESS

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.8/65 triggered; /job/DS_CI/job/dsc_3.8 triggered;

@devstudio-release
Copy link

Build 3.8 :: operator-bundle_3.8/56: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.8/67 triggered

@devstudio-release
Copy link

Build 3.8 :: copyIIBsToQuay/1552: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.8 :: dsc_3.8/15: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.8 :: update-digests_3.8/132: SUCCESS

Detected new images: rebuild operator-bundle
* server; /DS_CI/operator-bundle_3.8/56 triggered

@devstudio-release
Copy link

Build 3.8 :: dsc_3.8/15: SUCCESS

3.8.0 CI

@devstudio-release
Copy link

Build 3.9 :: server_3.x/190: Console, Changes, Git Data

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.9 :: server_3.x/190: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/3821 triggered

@devstudio-release
Copy link

Build 3.9 :: update-digests_3.x/3785: Console, Changes, Git Data

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.9 :: copyIIBsToQuay/1563: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.9 :: sync-to-downstream_3.x/3827: SUCCESS

Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/3678 triggered; /job/DS_CI/job/dsc_3.x triggered;

@devstudio-release
Copy link

Build 3.9 :: operator-bundle_3.x/1630: SUCCESS

Upstream sync done; /DS_CI/sync-to-downstream_3.x/3827 triggered

@devstudio-release
Copy link

Build 3.9 :: dsc_3.x/1055: Console, Changes, Git Data

@devstudio-release
Copy link

Build 3.9 :: update-digests_3.x/3785: SUCCESS

Detected new images: rebuild operator-bundle
* configbump
* devfileregistry
* devspaces-operator
* machineexec
* pluginregistry
* server
* traefik
* udi; /DS_CI/operator-bundle_3.x/1630 triggered

@devstudio-release
Copy link

Build 3.9 :: copyIIBsToQuay/1563: SUCCESS

3.9
arches = x86_64, s390x, ppc64le;
  * LATEST DS OPERATOR BUNDLE = <a href=https://quay.io/repository/devspaces/devspaces-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devspaces-operator-bundle:3.9-8
  * LATEST DWO OPERATOR BUNDLE = <a href=https://quay.io/repository/devworkspace/devworkspace-operator-bundle?tab=tags>registry-proxy.engineering.redhat.com/rh-osbs/devworkspace-operator-bundle:0.21-7
+ s390x-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.9-v4.14-544362-533291-s390x
  + quay.io/devspaces/iib:3.9-v4.14-s390x
  + quay.io/devspaces/iib:next-v4.14-s390x
  + quay.io/devspaces/iib:3.9-v4.13-545208-533286-s390x
  + quay.io/devspaces/iib:3.9-v4.13-s390x
  + quay.io/devspaces/iib:next-v4.13-s390x
  + quay.io/devspaces/iib:3.9-v4.12-545205-533281-s390x
  + quay.io/devspaces/iib:3.9-v4.12-s390x
  + quay.io/devspaces/iib:next-v4.12-s390x
  + quay.io/devspaces/iib:3.9-v4.11-545204-533277-s390x
  + quay.io/devspaces/iib:3.9-v4.11-s390x
  + quay.io/devspaces/iib:next-v4.11-s390x
  + quay.io/devspaces/iib:3.9-v4.10-545203-533276-s390x
  + quay.io/devspaces/iib:3.9-v4.10-s390x
  + quay.io/devspaces/iib:next-v4.10-s390x
+ ppc64le-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.9-v4.14-544362-533291-ppc64le
  + quay.io/devspaces/iib:3.9-v4.14-ppc64le
  + quay.io/devspaces/iib:next-v4.14-ppc64le
  + quay.io/devspaces/iib:3.9-v4.13-545208-533286-ppc64le
  + quay.io/devspaces/iib:3.9-v4.13-ppc64le
  + quay.io/devspaces/iib:next-v4.13-ppc64le
  + quay.io/devspaces/iib:3.9-v4.12-545205-533281-ppc64le
  + quay.io/devspaces/iib:3.9-v4.12-ppc64le
  + quay.io/devspaces/iib:next-v4.12-ppc64le
  + quay.io/devspaces/iib:3.9-v4.11-545204-533277-ppc64le
  + quay.io/devspaces/iib:3.9-v4.11-ppc64le
  + quay.io/devspaces/iib:next-v4.11-ppc64le
  + quay.io/devspaces/iib:3.9-v4.10-545203-533276-ppc64le
  + quay.io/devspaces/iib:3.9-v4.10-ppc64le
  + quay.io/devspaces/iib:next-v4.10-ppc64le
+ x86_64-rhel8 IIB(s) copied:
  + quay.io/devspaces/iib:3.9-v4.14-544362-533291-x86_64
  + quay.io/devspaces/iib:3.9-v4.14-x86_64
  + quay.io/devspaces/iib:next-v4.14-x86_64
  + quay.io/devspaces/iib:3.9-v4.13-545208-533286-x86_64
  + quay.io/devspaces/iib:3.9-v4.13-x86_64
  + quay.io/devspaces/iib:next-v4.13-x86_64
  + quay.io/devspaces/iib:3.9-v4.12-545205-533281-x86_64
  + quay.io/devspaces/iib:3.9-v4.12-x86_64
  + quay.io/devspaces/iib:next-v4.12-x86_64
  + quay.io/devspaces/iib:3.9-v4.11-545204-533277-x86_64
  + quay.io/devspaces/iib:3.9-v4.11-x86_64
  + quay.io/devspaces/iib:next-v4.11-x86_64
  + quay.io/devspaces/iib:3.9-v4.10-545203-533276-x86_64
  + quay.io/devspaces/iib:3.9-v4.10-x86_64
  + quay.io/devspaces/iib:next-v4.10-x86_64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants