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

Fix download URL for JF_RELEASES_REPO case #691

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

chkp-roniz
Copy link

Using Artifactory remote repository JF_RELEASES_REPO produces an invalid URL:

https://artifactory.company.com/artifactory/myrepo/artifactory/frogbot/v2/[RELEASE]/frogbot-linux-amd64/frogbot

twice artifactory keyword and two repositories name in the URL

The fix includes a modification of the the URL in a way that supports both cases

Using Artifactory remote repository JF_RELEASES_REPO produces an invalid URL:
```
https://artifactory.company.com/artifactory/myrepo/artifactory/frogbot/v2/[RELEASE]/frogbot-linux-amd64/frogbot
```
(twice artifactory keyword and two repositories name in the URL)

The fix simply format the URL in a way to support both cases
Copy link
Contributor

github-actions bot commented Apr 30, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@chkp-roniz chkp-roniz marked this pull request as ready for review April 30, 2024 11:33
@chkp-roniz
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@chkp-roniz
Copy link
Author

recheck

@eranturgeman
Copy link
Contributor

Hello @chkp-roniz and thank you for bringing this issue for our attention
We will review it shortly and will update you

@eranturgeman eranturgeman added bug Something isn't working safe to test Approve running integration tests on a pull request labels May 28, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label May 28, 2024
@eranturgeman
Copy link
Contributor

Hey @chkp-roniz I have looked into this and the script works as it should, I'll explain:
JF_RELEASES_REPO should contain ONLY the name of the remote repository
Another important thing to note is that this repository MUST point to releases.jfrog.io for the switch to happen in a correct way (not releases.jfrog.io/artifactory). Ill explain with an example:
lets say your platform url is https://artifactory.company.com, and the remote repo is called 'myRepo' and it points to releases.jfrog.io.
The full url you get is (according to your example): https://artifactory.company.com/artifactory/myRepo/artifactory/frogbot/v2/[RELEASE]/frogbot-linux-amd64/frogbot

Now since https://artifactory.company.com/artifactory/myRepo = releases.jfrog.io the final url you address to is:
https://releases.jfrog.io/artifactory/frogbot/v2/[RELEASE]/frogbot-linux-amd64/frogbot

which is exactly where you should address.
Please make sure your remote repository points to the correct url. If so - please contact us again (I'll leave this PR open for a while so you can tell me if it worked for you. If it doesn't we will look into it again and fix the bug is such exist)

Copy link
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@chkp-roniz
Copy link
Author

Hi @eranturgeman,
Thank you for the detailed description. It is working when referring only to https://releases.jfrog.io
However, by this method, we lose Smart Repository features...

@eranturgeman
Copy link
Contributor

Thanks @chkp-roniz for letting me know it works!
Can you give me an example for a feature you are missing due to this way?
I'd love to look deeper into it so we can consider a change if necessary and if it is valuable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants