gitlab icon indicating copy to clipboard operation
gitlab copied to clipboard

feat(generics): allow upload of build assets as generic packages #174

Open JonasSchubert opened this issue 2 years ago • 6 comments

Progress of #283 which I closed due to missing time on my side

Gitlab allows to add generic packages as assets attached to a release. #174 This is a suggestion to define a list of generics to be published into the generics repository of a project and then to be used as assets for the release.

JonasSchubert avatar Mar 22 '22 13:03 JonasSchubert

@JonasSchubert Could you check if it makes sense to pull the changes from #322 in this MR? It seems to touch the very same code paths.

fgreinacher avatar Apr 13 '22 10:04 fgreinacher

Will check

JonasSchubert avatar Apr 13 '22 14:04 JonasSchubert

@JonasSchubert Could you check if it makes sense to pull the changes from #322 in this MR? It seems to touch the very same code paths.

I would tackle this in a separate PR. I checked the code and in my opinion it needs major adjustments regarding assets handling.

JonasSchubert avatar Apr 16 '22 16:04 JonasSchubert

I would wait for #322 and merge afterwards.

JonasSchubert avatar Apr 28 '22 17:04 JonasSchubert

I would wait for #322 and merge afterwards.

Fine for me! Ping me again in case you change your mind :)

fgreinacher avatar Apr 28 '22 18:04 fgreinacher

any update on this?

mfoti avatar Jun 23 '22 09:06 mfoti

@fgreinacher @JonasSchubert #322 has been merged. Can you please merge this PR?

bgoareguer avatar Dec 23 '22 08:12 bgoareguer

Happy to merge once conflicts are resolved. @JonasSchubert are you still willing to work on this?

fgreinacher avatar Jan 09 '23 14:01 fgreinacher

Sorry everyone! I lost track of some stuff... Fixed the conflicts and ready to merge. Will keep an eye in case any issues should occur

JonasSchubert avatar Jan 22 '23 09:01 JonasSchubert

@fgreinacher Rebased to v10 and fixed lint errors. Should be fine now.

JonasSchubert avatar Jan 28 '23 16:01 JonasSchubert

:tada: This PR is included in version 10.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] avatar Jan 30 '23 09:01 github-actions[bot]

Thanks for the work to all, and for merging this.

I've tried this feature and I guess there are two bugs:

  1. the encoded url should end with the file name, not the label here: https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L91 as in documentation (https://docs.gitlab.com/ee/user/packages/generic_packages/#publish-a-package-file)

The url should be like this:

PUT /projects/:id/packages/generic/:package_name/:package_version/:file_name?status=:status

This code should fix: /projects/${encodedRepoId}/packages/generic/release/${encodedGitTag}/${encodedFileName}

I got this error creating labels like this: "MyApp x86 (.dmg)"

  1. The url should not be taken from the response body (https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L105) but should be the value of uploadEndpoint. Actually when it does a release, it publish the package with an url like this:
"links": [
      {
        "name": "MyApp-v2.9-x64.dmg",
        "url": "https://storage.googleapis.com/gitlab-gprd-package-repo/25/87/2587b9f64886c595ba69e1e67434ca7c69a120154ad2d3d5a4a10ddca78a978e/packages/12549696/files/68634567/MyApp-v2.9-x64.dmg?GoogleAccessId=gitlab-object-storage-prd@gitlab-production.iam.gserviceaccount.com&Signature=M03DQehdSKyLvpdTBKAuxiveI4yANOMgKcFs5etCp%2FTwGoj%2BoQy%2FRpoiSPGd%0Ak6leI4uVtOv55w%2BZa764o%2FENpOQ2eFTOfIr6H45Hyi5QupPArB0OL0164Wul%0AgmhqQtcP%2FZ%2FQEVm%2F40Vq%2Fa50MPoPgOa4%2BRt%2FDS9CjWVPXz2PvKz9Zzy0P114%0AXiEyaMxSCQmPcai6%2BYFetwTI%2Fi%2Fm4HaSkOqG%2BLhHd%2BswJDNZNBucZo7gf2fg%0APbGQkJ3wQxzVY%2BukhxjMm%2BjXw8kR%2F%2FPsu8F810gjLzMsnCYNt2O5pwpCY5fr%0AzDP%2BQ1YosqKiTuBhWR6kPUUh43j6mdKcjmbGv7vJcA%3D%3D&Expires=1676411480",
        "link_type": "package"
      }

After few minutes the package is no longer available, loading the package url I got this response:

<Error>
<Code>ExpiredToken</Code>
<Message>The provided token has expired.</Message>
<Details>Request signature expired at: 2023-02-14T21:51:20+00:00</Details>
</Error>

I guess that this can fix the bug: assetsList.push({ label, alt: "release", uploadEndpoint, type: "package", filepath, target });

I can do the PRs if needed

mfoti avatar Feb 14 '23 22:02 mfoti

Hi @mfoti and thanks for your report.

  1. the encoded url should end with the file name, not the label here: https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L91 as in documentation (https://docs.gitlab.com/ee/user/packages/generic_packages/#publish-a-package-file)

The url should be like this:

PUT /projects/:id/packages/generic/:package_name/:package_version/:file_name?status=:status

This code should fix: /projects/${encodedRepoId}/packages/generic/release/${encodedGitTag}/${encodedFileName}

I got this error creating labels like this: "MyApp x86 (.dmg)"

As the README describes, the label represents the file name. IMO the current implementation is here correct.

  1. The url should not be taken from the response body (https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L105) but should be the value of uploadEndpoint. Actually when it does a release, it publish the package with an url like this:
"links": [
      {
        "name": "MyApp-v2.9-x64.dmg",
        "url": "https://storage.googleapis.com/gitlab-gprd-package-repo/25/87/2587b9f64886c595ba69e1e67434ca7c69a120154ad2d3d5a4a10ddca78a978e/packages/12549696/files/68634567/MyApp-v2.9-x64.dmg?GoogleAccessId=gitlab-object-storage-prd@gitlab-production.iam.gserviceaccount.com&Signature=M03DQehdSKyLvpdTBKAuxiveI4yANOMgKcFs5etCp%2FTwGoj%2BoQy%2FRpoiSPGd%0Ak6leI4uVtOv55w%2BZa764o%2FENpOQ2eFTOfIr6H45Hyi5QupPArB0OL0164Wul%0AgmhqQtcP%2FZ%2FQEVm%2F40Vq%2Fa50MPoPgOa4%2BRt%2FDS9CjWVPXz2PvKz9Zzy0P114%0AXiEyaMxSCQmPcai6%2BYFetwTI%2Fi%2Fm4HaSkOqG%2BLhHd%2BswJDNZNBucZo7gf2fg%0APbGQkJ3wQxzVY%2BukhxjMm%2BjXw8kR%2F%2FPsu8F810gjLzMsnCYNt2O5pwpCY5fr%0AzDP%2BQ1YosqKiTuBhWR6kPUUh43j6mdKcjmbGv7vJcA%3D%3D&Expires=1676411480",
        "link_type": "package"
      }

After few minutes the package is no longer available, loading the package url I got this response:

<Error>
<Code>ExpiredToken</Code>
<Message>The provided token has expired.</Message>
<Details>Request signature expired at: 2023-02-14T21:51:20+00:00</Details>
</Error>

I guess that this can fix the bug: assetsList.push({ label, alt: "release", uploadEndpoint, type: "package", filepath, target });

This is a little bit more tricky then 1..

For me it looks like you use Google Storage to store some assets and packages and this provider returns an URL with an expiration date. Not sure whether this is for the URL only or also for the file. Interesting would be what happens if we use the uploadEndpoint? Even if we use this URL isn't the returned URL still active in the background and by this probably causing the same issue? Would it be possible for you to test this in your project?

BTW the currently used URL is the one provided by the API call response according to the docs. With this said, the docs also highlight how to download a generic package.

But:

If multiple packages have the same name, version, and filename, then the most recent one is retrieved.

With the current approach we have direct access to the uploaded file without worrying about this drawback, which we might loose using the Gitlab URL.

@mfoti what are your thoughts on this?

JonasSchubert avatar Feb 15 '23 01:02 JonasSchubert

As the README describes, the label represents the file name. IMO the current implementation is here correct.

I guess defaults to the filename, but allow to put a string as package description, as in my case: "MacOS x64 (.dmg)"

BTW I can confirm that using the filename as label works:

       // This works
       {
          label: `MyApp-${nextRelease.version}-x64.dmg`,
          path: `release/build/MyApp-${nextRelease.version}-x64.dmg`,
          type: "image",
          target: "generic_package"
        },

But if you use a different label it doesn't

       // This doesn't works
       {
          label: 'MacOS x64 (.dmg)',
          path: `release/build/MyApp-${nextRelease.version}-x64.dmg`,
          type: "image",
          target: "generic_package"
        },

I guess to avoid errors (release is correctly generated with a label different of the filename using the default target) we can just ignore the label in the code and use the filename here:

https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L88

IMO is better to give the ability to users to use their own label. BTW This is not a blocking behavior, when the target is to generic_package using the filename as label create the release correctly.

This is a little bit more tricky then 1..

yep man, it is lol 🤣

For me it looks like you use Google Storage to store some assets and packages and this provider returns an URL with an expiration date. Not sure whether this is for the URL only or also for the file. Interesting would be what happens if we use the uploadEndpoint? Even if we use this URL isn't the returned URL still active in the background and by this probably causing the same issue? Would it be possible for you to test this in your project?

yes, I already tested it and it works :) I have a fork of the project with this fix and I can confirm resolve the issue.

BTW the currently used URL is the one provided by the API call response according to the docs. With this said, the docs also highlight how to download a generic package.

Yes you are right, but I guess is not well documented, I'll try to find the code on gitlab repo to check better the behavior. What can I say is that my builds are greater the 200mb, probably gitlab uses google storage if the size of the package is bigger than a certain value. I don't know. But the url provided in filename is broken in my case, and this is a blocking bug that doesn't allow me to use this kind of release

But:

If multiple packages have the same name, version, and filename, then the most recent one is retrieved.

With the current approach we have direct access to the uploaded file without worrying about this drawback, which we might loose using the Gitlab URL.

To be honest I don't understand which is this case, let's assume I'll build two different packages with the same name, for example:

  • /build/x86/MyApp.dmg <-for x86
  • /build/arm64/MyApp.dmg <-for arm64

And try to publish them. In this case I will have an url like: /projects/${encodedRepoId}/packages/generic/release/my-tag-v1.0/MyApp.dmg

That of course will store the last published file to that path. At the end you will always get the most recent (last published) package downloading the content from the package path

The main problem is that it doesn't works for me as is now, I have to use my fork.

mfoti avatar Feb 15 '23 09:02 mfoti

I guess there is another feature that can be used too, the package type.

At the moment it is hardcoded:

assetsList.push({ label, alt: "release", url, type: "package", filepath, target }); (https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L107)

This type value should be taken from the configuration, this should fix:

const type = asset.type ? template(asset.type)(context) : "package";
assetsList.push({ label, alt: "release", url, type, filepath, target });

The final effect, without hardcoding the type, will be this: Screen Shot 2023-02-15 at 12 04 03

mfoti avatar Feb 15 '23 11:02 mfoti

@mfoti You already created a fork with a fix on your side. Feel free to create the PR ;) Already thanks for your work!

JonasSchubert avatar Feb 16 '23 13:02 JonasSchubert

I should squash, but is a really easy fix: https://github.com/semantic-release/gitlab/pull/504, this fixes only the main "bug", but not the package type and the filename/label

mfoti avatar Feb 16 '23 16:02 mfoti

I guess there is another feature that can be used too, the package type.

At the moment it is hardcoded:

assetsList.push({ label, alt: "release", url, type: "package", filepath, target }); (https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L107)

IMO the current implementation is correct here. Generic packages only should have the type packages. The other type options (runbook, image and other) are for assets and not packages. This can be used with either not setting the property target at all or setting the (default) value project_upload.

JonasSchubert avatar Feb 17 '23 07:02 JonasSchubert

I guess defaults to the filename, but allow to put a string as package description, as in my case: "MacOS x64 (.dmg)"

BTW I can confirm that using the filename as label works:

       // This works
       {
          label: `MyApp-${nextRelease.version}-x64.dmg`,
          path: `release/build/MyApp-${nextRelease.version}-x64.dmg`,
          type: "image",
          target: "generic_package"
        },

But if you use a different label it doesn't

       // This doesn't works
       {
          label: 'MacOS x64 (.dmg)',
          path: `release/build/MyApp-${nextRelease.version}-x64.dmg`,
          type: "image",
          target: "generic_package"
        },

I guess to avoid errors (release is correctly generated with a label different of the filename using the default target) we can just ignore the label in the code and use the filename here:

https://github.com/semantic-release/gitlab/blob/master/lib/publish.js#L88

IMO is better to give the ability to users to use their own label. BTW This is not a blocking behavior, when the target is to generic_package using the filename as label create the release correctly.

I think this is a create idea. Would you like to add it to your PR too?

JonasSchubert avatar Feb 17 '23 07:02 JonasSchubert