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

Expand the raw devfile url resolver pattern with token query parameter #616

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Nov 23, 2023

What does this PR do?

Rework the accept() check in the raw devfile url resolver to respect token query parameters e.g. https://raw.githubusercontent.com/vinokurig/private/main/devfile.yaml?token=<token>

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes eclipse-che/che#22685
fixes eclipse-che/che#22684

How to test this PR?

  1. Deploy che with the PR image: quay.io/eclipse/che-server:pr-616
  2. Create a workspace from a raw devfile url located in a private GitHub repository.

see: devfile is resolved and the workspace starts successfully.

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.

@vinokurig
Copy link
Contributor Author

/retest

@@ -40,6 +41,7 @@ public class RawDevfileUrlFactoryParameterResolver extends BaseFactoryParameterR
implements FactoryParametersResolver {

private static final String PROVIDER_NAME = "raw-devfile-url";
private static final Pattern PATTERN = Pattern.compile("^https?://.*\\.ya?ml(\\?token=.*)?$");
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this token used later?

Copy link
Contributor Author

@vinokurig vinokurig Nov 24, 2023

Choose a reason for hiding this comment

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

We have not respected it yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean the token is a part of the URL, we do not use it but it is needed for the http request.

@artaleks9
Copy link
Contributor

Verified on Che with quay.io/eclipse/che-server:pr-616 for raw devfile public/private repository from github.com and GitHub Enterprise server, works as expected.

Copy link

openshift-ci bot commented Nov 28, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: artaleks9, tolusha, vinokurig

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

Needs approval from an approver in each of these files:

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

@vinokurig vinokurig merged commit f34e804 into main Nov 28, 2023
@vinokurig vinokurig deleted the che-22685 branch November 28, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants