janus-idp / backstage-plugins

Plugins for Backstage
https://janus-idp.io
Apache License 2.0
151 stars 150 forks source link

Question in regards to JFROG Plugin 🐸 #429

Closed Parsifal-M closed 2 months ago

Parsifal-M commented 1 year ago

Hey Team! 👋

Apologies if I use the wrong issue type but I couldn't really find anything that matches what I wanted to ask 😅

I stumbled across the JFROG Plugin you have and tried it but it did not work for me I kept getting 404 errors when trying to get what I wanted out of Jfrog (a docker image).

So I forked the project and went to work on the plugin, I was just wondering if this is something you'd like me to push back here or not as I made a fair amount of changes across the whole plugin and didn't want to just submit a big PR and expect it to be merged with little context 😆

The key changes I made:

There are still some additional changes I think need to be done, I am not sure the config.d.ts is needed as you would almost always use the proxy as well but I have not got to that part yet.

So the question is, would these changes be something you'd like me to try and merge? I will not be offended if not of course 😄

Please LMK what you think!

tumido commented 1 year ago

Hi @Parsifal-M!

We definitely invite and encourage all contributions and collaborations on any of our plugins! Please do not hesitate and submit a pull request! 🙂

IMO, there's no point in maintaining multiple Artifactory plugins and if we can converge on a single solution that satisfies all the use cases, it's the best. 🙂

I really appreciate that you've listed all the changes and ideas that you've implemented, however, it's quite hard for me to comprehend all aspects of it. After all, I'm a software engineer, and for me, reading code is easier than text about said code. 😄

I think if you submit a pull request we can have a great discussion in comments and reviews. In the end, we may decide that it's not a good fit for this project (which I doubt in this particular case, but there's always the possibility) and even if so, we've at least learned something along the way.

tumido commented 1 year ago

👀 cc @fmenesesg

Parsifal-M commented 1 year ago

Hi @Parsifal-M!

We definitely invite and encourage all contributions and collaborations on any of our plugins! Please do not hesitate and submit a pull request! 🙂

IMO, there's no point in maintaining multiple Artifactory plugins and if we can converge on a single solution that satisfies all the use cases, it's the best. 🙂

I really appreciate that you've listed all the changes and ideas that you've implemented, however, it's quite hard for me to comprehend all aspects of it. After all, I'm a software engineer, and for me, reading code is easier than text about said code. 😄

I think if you submit a pull request we can have a great discussion in comments and reviews. In the end, we may decide that it's not a good fit for this project (which I doubt in this particular case, but there's always the possibility) and even if so, we've at least learned something along the way.

Awesome! Let me put it together in a PR and I will submit it at soon as I have some spare time 👍

timdittler commented 1 year ago

@Parsifal-M Thank you very much for working on this! Did you figure out which part of the original code triggered the 404s? I'm running into the same problem and would like to understand.

Parsifal-M commented 1 year ago

@Parsifal-M Thank you very much for working on this! Did you figure out which part of the original code triggered the 404s? I'm running into the same problem and would like to understand.

Hey @timdittler! 👋

No worries! That is a good question my memory is hazy at the best of times. In hindsight, I should have probably mentioned how I fixed it when I did 😓

If I swap back now to before the changes I get 200's when I use this as a url in the proxy:

  '/jfrog-artifactory/api':
    target: 'https://<somecompany>.jfrog.io'
    headers:
      Authorization: Bearer <sometoken>
      secure: true

But the response query is empty when using an image name

I think the issue might have been a combination of the url I was using and possibly the query. I will remember next time to add some steps to reproduce 😞 sorry!

timdittler commented 1 year ago

Thank you for the feedback @Parsifal-M . While I like the connected PR because of the added functionality, the current state also works if configured correctly. The GraphQL query still work. I just verified the hard way :laughing:

Parsifal-M commented 1 year ago

Thank you for the feedback @Parsifal-M . While I like the connected PR because of the added functionality, the current state also works if configured correctly. The GraphQL query still work. I just verified the hard way :laughing:

Ah nice! I must have made a mistake in my configuration 😵‍💫.

Where was the issue?

As I mentioned I understand if its not something you'd like to merge and that's fine 😁.

Thanks!

jasondiazg commented 5 months ago

Any update on this guys?

rhdh-bot commented 2 months ago

This issue has been closed due to the fact that the Janus community is being sunset.

For future plugin issues, please use https://github.com/backstage/community-plugins/issues

For future showcase issues, please use https://issues.redhat.com/browse/RHIDP

For more information on the sunset, see:

https://janus-idp.io/blog/2024/07/05/future-of-janus-community https://issues.redhat.com/browse/RHIDP-3690 https://issues.redhat.com/browse/RHIDP-1018