Schneegans / dynamic-badges-action

This action allows you to create badges for your README.md with shields.io which may change with every commit. To do this, this action does not need to push anything to your repository!
https://schneegans.github.io/tutorials/2022/04/18/badges
MIT License
257 stars 37 forks source link

How to use it with organizations accounts ? #14

Closed LexABzH closed 1 year ago

LexABzH commented 2 years ago

Hello,

I just came accross this project and I was wondering how you would use it with an organization account ?

As I understand, you can't create gists with organizations accounts, so do you need to create a gist with a user account ? Does that mean that the token will also have access to all the other user's gists ? What if this user leaves the organization ? Any advice would be appreciated :)

Schneegans commented 2 years ago

That's an interesting question to which I do not have a good answer. A user once tried to use the project's wiki repository instead of a gist (#8), but this feels pretty hacky.

Maybe it could be a solution to create a custom user account just for this purpose?

Edit: TLDR; The discussion in this issue digressed a bit. After all, I think there's currently no better solution for organizations than to use the gist of a user account. Maybe even a user created specifically for this purpose.

Organization gists are a frequently requested feature so it's not unlikely that we will get this one day!

LexABzH commented 2 years ago

Thanks for your reply.

I was thinking the same, but it's far from perfect. I will try to explore other approaches, and I let you know if I find something interesting :)

codebydant commented 1 year ago

Hi @LexABzH, I need this as well for my private repository on my organization. Are there any updates regarding this feature?

LexABzH commented 1 year ago

Hi. Unfortunately, I couldn't find a good solution and I haven't come back to this problem since.

An idea might be to create a "fake" user for your organization :

Hope it helps.

codebydant commented 1 year ago

Hi. Unfortunately, I couldn't find a good solution and I haven't come back to this problem since.

An idea might be to create a "fake" user for your organization :

  • Create a new gist with this user
  • Create a token with the gist scope
  • Add the token as a Secret of your repository

Hope it helps.

I don't think this works. Since my user is already in the organization and despite I have setup the token and the gist ID I keep getting the error:

Run schneegans/dynamic-badges-action@v1.6.0
Failed to get gist, response status code: 401, status message: Unauthorized
/home/runner/work/_actions/schneegans/dynamic-badges-action/v1.6.0/index.js:209
        if (oldGist.body.files[filename]) {
                              ^

TypeError: Cannot read properties of undefined (reading 'version.json')
    at /home/runner/work/_actions/schneegans/dynamic-badges-action/v1.6.0/index.js:209:31
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

I noticed that the gits_ID parameter is not taking into account the user account to which the ID belongs.

jvfresco commented 1 year ago

Hello, I have the same problem with enterprise accounts. This is what I found:

On enterprise accounts gists are created on https://gist.github-enterprise-hostname Github standard api gists endpoint is api.github.com/gists In Github enterprise the api endpoint is github-enterprise-hostname/api/v3/gists

Allowing a host as input parameter and change conditionally the url where the request to get the gist is performed could be a solution. In my case even with that would be useless because the link to the raw gist is unnaccessible outside enterprise network.

LucBerge commented 1 year ago

@LexABzH @Schneegans @jvfresco @dtcMLOps Hello everybody, I also need to use it in a private owned organization I found how to do it.

The issue of using the current structure is that shield.io cannot access the organization gist. The firewall won't accept the request. Shield.io cannot access the orga but your orga can make a request to shield.io. So instead of saving a JSON based object, you save the SVG file in a gist. The README then display an image from gist.


Before:

  1. Action: Compute the result
  2. Action: Create a gist with a JSON object
  3. README: Get request to Shield.io with gist url
  4. Shield.io: Cannot access the orga gist => ERROR

After:

  1. Action: Compute the result
  2. Action: Get request to shield.io to create the badge
  3. Action: Save the SVG badge in gist
  4. README: Get request to gist

Customer usage:

The following code publish a JSON based object compatible with Shield.io

- name: Create Awesome Badge
  uses: schneegans/dynamic-badges-action@v1.6.0
  with:
    auth: ${{ secrets.GIST_SECRET }}
    gistID: <gist-ID>
    filename: test.json
    label: Hello
    message: World
    color: orange

The following code publish an SVG image

- name: Create Awesome Badge
  uses: schneegans/dynamic-badges-action@v1.6.0
  with:
    auth: ${{ secrets.GIST_SECRET }}
    host: github-enterprise-hostname/api/v3/gists
    gistID: <gist-ID>
    filename: test.svg
    label: Hello
    message: World
    color: orange

In other word, if the filename extension is .svg, the action makes a request to Shield.io to create the badge and the result is saved to gist. Regarding the API request from your action to gist enterprise, you just have to make the host parameter as a variable.

Schneegans commented 1 year ago

@LucBerge That is indeed a great idea! Have you tried this already so that you could create a corresponding merge request? If not, I will try this. I think in order to stay backwards-compatible, we could check the filename and if it ends with ".svg" we could use the proposed approach and else use the old behavior.

LucBerge commented 1 year ago

Have you tried this already so that you could create a corresponding merge request?

Not really. The only thing I have tried is to display an svg file hosted on gist from a README.

I think in order to stay backwards-compatible, we could check the filename and if it ends with ".svg" we could use the proposed approach and else use the old behavior.

Yes

Keep us in touch!

LucBerge commented 1 year ago

@Schneegans Do you work on it?

Schneegans commented 1 year ago

Not yet. And given on how many other open-source projects I am currently working in my spare time I doubt that I will have the time to implement this in the coming days. It'll be rather a couple of weeks. Hence I would happily accept a pull request :wink:

LucBerge commented 1 year ago

Since I need it at work, I'll do a PR on my work time.

LucBerge commented 1 year ago

@Schneegans I have questions:

  1. I know it is working as this but is it an error? [filename] is an array key of a dict. The doc says it should just be a string.
  2. Why don't you update the gist, whether the content is different or not? Don't you care to update something if it does not change? Removing the forceUpdate parameter would simplify the code and remove this branch.
  3. An object oriented code would be easier to maintain. Do you agree?
  4. In the README, for each optional parameter, we should add what happens if not defined.
  5. Do you have tests to verify the code works as expected? Would be nice to have some to make sure I don't break something while working on it. The tests must cover all the possibilities.

I'll work step by step:

  1. Add tests => PR
  2. Create a function for getGist (like updateGist)
  3. Add host parameter
  4. Add a function for getBadge and change behavior based on the filename extension => PR
  5. Migrate to object oriented code => PR
Schneegans commented 1 year ago

Thank you for looking into this! Your overall plan looks very good. The code dates back to a time where I did not know much about Node / JavaScript / async programming / etc :smile: When looking at it today, I think that there is much room for improvement!

Regarding your points:

  1. Looks weird. I guess just check if it works if replaced by a simple string. I cannot remember any reason for the array.
  2. Gists are version-controlled and updating them creates a new revision even if the content did not change. If a workflow runs frequently, this can create a huge number of revisions quickly. The corresponding issue was #16.
  3. Definitely, yes!
  4. Yeah. When looking at the current docs of shields.io it seems that many of the parameters are actually not exposed for the badges which we could retrieve with a GET request (like logoPosition or logoSvg). This would make the documentation and the code even more complicated... I am wondering if we should drop support for the JSON-endpoint variant altogether? I do not see a disadvantage in pushing the SVG to the gist. Well, it's a bit larger in terms of file-size but this shouldn't mater much.... This would break backwards-compatibility but this would also be fine if we released a new major version. What do you think?
  5. No. Well, there are some badges in the README which are generated by this workflow but I would not consider this a thorough test :sweat_smile:

What do you think? Thanks again!

LucBerge commented 1 year ago
  1. Ok
  2. Ok, keeping the forceUpdate parameter
  3. Ok
  4. I agree, I don't see why you should keep the JSON endpoint, it is exactly the same as SVG.
  5. Ok
LucBerge commented 1 year ago

@Schneegans Static badges and Endpoint badges do have differencies.

Endpoint badges have the following fields static badges don't have

Endpoint badges field nameLogo = Static badges field logo

Do we still keep the json endpoint? At least for backward compatibility?

Schneegans commented 1 year ago

Well, I would say that supporting custom logos is not worth the added code complexity. I doubt that many people are actually using this. And it makes not only the code but also the documentation much more complex.

LucBerge commented 1 year ago

Are you saying we drop the json endpoint once for all ? It will break backward compatibility for beforehand fields.

Schneegans commented 1 year ago

Yes. It will be a breaking change, but we will create a new major version (v2.0.0) for indicating this change. As I said before, I think that the number of people using this feature is very low (if any at all).

I also just realized that despite being not documented, logoWidth seems to work. Also, it seems to be possible to pass base64 encoded images to the logo parameter:

https://img.shields.io/badge/any_text-you_like-blue?logoWidth=200&logo=data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4NCjwhLS0gR2VuZXJhdG9yOiBB%0D%0AZG9iZSBJbGx1c3RyYXRvciAxNS4xLjAsIFNWRyBFeHBvcnQgUGx1Zy1JbiAuIFNWRyBWZXJzaW9u%0D%0AOiA2LjAwIEJ1aWxkIDApICAtLT4NCjwhRE9DVFlQRSBzdmcgUFVCTElDICItLy9XM0MvL0RURCBT%0D%0AVkcgMS4xLy9FTiIgImh0dHA6Ly93d3cudzMub3JnL0dyYXBoaWNzL1NWRy8xLjEvRFREL3N2ZzEx%0D%0ALmR0ZCI+DQo8c3ZnIHZlcnNpb249IjEuMSIgaWQ9ImNpcmNsZV9waWVjZXMiIHhtbG5zPSJodHRw%0D%0AOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5%0D%0AOTkveGxpbmsiIHg9IjBweCINCgkgeT0iMHB4IiB3aWR0aD0iNTExLjg3NXB4IiBoZWlnaHQ9IjUx%0D%0AMS44MjRweCIgdmlld0JveD0iMCAwIDUxMS44NzUgNTExLjgyNCIgZW5hYmxlLWJhY2tncm91bmQ9%0D%0AIm5ldyAwIDAgNTExLjg3NSA1MTEuODI0Ig0KCSB4bWw6c3BhY2U9InByZXNlcnZlIj4NCjxjaXJj%0D%0AbGUgaWQ9ImNpcmNsZSIgZmlsbD0iI0ZGRkZGRiIgY3g9IjI1Ni4yNTIiIGN5PSIyNTUuOTg2IiBy%0D%0APSIyNTMuMDkzIi8+DQo8cGF0aCBpZD0iYmx1ZS1waWVjZSIgZmlsbD0iIzNFNUJBOSIgZD0iTTQ1%0D%0ANS4zOTgsNDEyLjE5N2MzMy43OTItNDMuMDIxLDUzLjk0Ni05Ny4yNjIsNTMuOTQ2LTE1Ni4yMTEN%0D%0ACgljMC0xMzkuNzc5LTExMy4zMTMtMjUzLjA5My0yNTMuMDkzLTI1My4wOTNjLTMwLjQwNiwwLTU5%0D%0ALjU1OCw1LjM2Ny04Ni41NjYsMTUuMTk3QzI3Mi40MzUsNzEuOTg5LDQwOC4zNDksMjQ3LjgzOSw0%0D%0ANTUuMzk4LDQxMi4xOTd6DQoJIi8+DQo8cGF0aCBpZD0ibGVmdC1yZWQtcGllY2UiIGZpbGw9IiM5%0D%0ARjFEMjAiIGQ9Ik0yMjAuMDAzLDE2NC4zMzdjLTM5LjQ4MS00Mi41MzMtODMuNjk1LTc2LjMxMi0x%0D%0AMzAuNTIzLTk4LjcxNQ0KCUMzNi41NzMsMTEyLjAxMSwzLjE1OSwxODAuMDkyLDMuMTU5LDI1NS45%0D%0AODZjMCw2My44MTQsMjMuNjI2LDEyMi4xMDQsNjIuNTk3LDE2Ni42MjMNCglDMTAwLjExMSwzMTku%0D%0AMzkyLDE2NC42OTcsMjE5LjkwNywyMjAuMDAzLDE2NC4zMzd6Ii8+DQo8cGF0aCBpZD0iYm90dG9t%0D%0ALXJlZC1waWVjZSIgZmlsbD0iIzlGMUQyMCIgZD0iTTI2Ni42MzgsMjIxLjcyN2MtNTQuNzkyLDU5%0D%0ALjA1MS0xMDkuMzkyLDE2Mi40MjItMTI5LjE1MiwyNTcuNzk0DQoJYzM1LjQxOSwxOC44NTcsNzUu%0D%0AODQsMjkuNTU5LDExOC43NjYsMjkuNTU5YzQ0LjEzMiwwLDg1LjYxOC0xMS4zMDYsMTIxLjc0LTMx%0D%0ALjE2M0MzNTcuMTcxLDM4MS43MTIsMzE3Ljg2OCwyOTMuNjA0LDI2Ni42MzgsMjIxLjcyNw0KCXoi%0D%0ALz4NCjwvc3ZnPg0K

So after all, the change wouldn't be too breaking :smile:. Maybe we could leave the logoWidth parameter there, indicating that it is not officially supported...

runarberg commented 1 year ago

I should have read this thread before posting #24

In my fork I simply drop an SVG image if the filename ends with .svg. And then only use parameters supported to generate it, meaning it is completely backwards compatible.

Schneegans commented 1 year ago

Sorry for the late reply, I am very busy these days :sweat_smile:

Anyways, it's an even more awesome idea to use this library instead of the GET request to shields.io. @LucBerge how far is your refactoring? Do you want to incorporate the idea by @runarberg or how shall we proceed? I guess we could also simply use the approach by @runarberg, but maybe we would miss an opportunity for cleaning up the code :smiley:

LucBerge commented 1 year ago

Are you saying we drop the json endpoint once for all ? It will break backward compatibility for beforehand fields.

Yes. It will be a breaking change [...]

...


@LucBerge how far is your refactoring?

Not so far

Do you want to incorporate the idea by @runarberg or how shall we proceed?

Would love it.

maybe we would miss an opportunity for cleaning up the code 😃

We can do it later

runarberg commented 1 year ago

I actually did do a fair amount of refactoring in my fork, if you want I can send a PR

runarberg commented 1 year ago

Although most of my refactoring changes were for selfish reasons, e.g. I know the newer fetch API better then the older http API, so I swapped them.

runarberg commented 1 year ago

I opened #25 It may contain more refactoring than hoped. For example node has been bumped to version 20, there is a lot of whitespace changes caused by prettier, etc. But I cleaned up my fork in a way that it can at least be merged if desired.

Schneegans commented 1 year ago

I just exposed a host parameter in the latest master branch of the action. Can anybody with a GitHub enterprise instance test whether this works? Maybe even together with the new SVG option? If it works, I would tag a new release!

I would also close this issue even if this is not really a solution to the original issue reported here by @LexABzH. However, I think there's no better solution for organizations than to use the gist of a user account. Maybe even a user created specifically for this purpose.

Organization gists are a frequently requested feature so maybe we will get this one day!

LucBerge commented 1 year ago

I just exposed a host parameter in the latest master branch of the action. Can anybody with a GitHub enterprise instance test whether this works? Maybe even together with the new SVG option? If it works, I would tag a new release!

I'll test it next week.


Not sure I can call the action with last commit

uses: schneegans/dynamic-badges-action@<last_commit>

EDIT: I can uses: https://docs.github.com/en/actions/creating-actions/about-custom-actions#using-branches-for-release-management

Schneegans commented 1 year ago

As it is in master, you can also do uses: schneegans/dynamic-badges-action@master at this point.

Schneegans commented 1 year ago

@LucBerge any update on this one?

LucBerge commented 1 year ago

@Schneegans My company must white list the action before I can use it. Long process.

Schneegans commented 1 year ago

@LucBerge I now such companies :sweat_smile: Do you have a rough estimation how long this will take? Else I would simply release the new version and fix any issues with the host parameter thereafter. Everything else seems to work decently.

LucBerge commented 1 year ago

Do you have a rough estimation how long this will take?

Absolutely no idea

Schneegans commented 1 year ago

Alright. I just published v1.7.0. If you encounter any problems, please open a new issue. I guess we have digressed this issue here enough :laughing:

I added a TLDR; to the first comment above to prominently show the results of the discussion. Thank you all for your contributions!