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

Publish packages to GitHub Registry #1410

Closed
wants to merge 2 commits into from
Closed

Publish packages to GitHub Registry #1410

wants to merge 2 commits into from

Conversation

yordis
Copy link

@yordis yordis commented Dec 12, 2020

Missing Steps

  1. Create a new personal access token (PAT) with the appropriate scopes for the tasks you want to accomplish
  • Select the read:packages scope to download container images and read their metadata.
  • Select the write:packages scope to download and upload container images and read and write their metadata.
  • Select the delete:packages scope to delete container images.
  1. Create a repository secret called CR_PAT with the value of the PAT created before.

More details at: Migrating to GitHub Container Registry for Docker images

Description

Publish the images to Github Docker Registry.

closes #1392

Motivation and Context

Docker is rate-limiting pulling the images from Docker Hub.

Testing Details

  • Make sure the images are being published correctly

Types of changes

  • Documentation
  • Version change (Update, remove or add more Node.js versions)
  • Variant change (Update, remove or add more variants, or versions of variants)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Others (non of above)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • All new and existing tests passed.

@yordis
Copy link
Author

yordis commented Dec 12, 2020

I am not sure how we would like to document such a feature, suggestions are welcome.

@PeterDaveHello PeterDaveHello requested a review from a team December 16, 2020 08:29
Copy link
Member

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

Blocking as we'd need some consensus from the nodejs/TSC and CommComm before adding another official channel, and also to get a new PAT that allows publishing packages.
If someone wants to do the work, they can dismiss the review once they've sorted it out

with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.CR_PAT }}
Copy link
Member

Choose a reason for hiding this comment

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

This would need to be requested in nodejs/admin

@PeterDaveHello
Copy link
Member

Blocking as we'd need some consensus from the nodejs/TSC and CommComm before adding another official channel, and also to get a new PAT that allows publishing packages.

Or maybe just treat the registry on GitHub as development build, instead of another official channel? Like the unofficial build of Alpine Linux binary we used?

cc @nodejs/tsc

@mcollina
Copy link
Member

I'm positive on adding it.

@mhdawson
Copy link
Member

@yordis does github rebuild the images when the underlying image changes. For some reason I think the official images in the docker repo automatically get updates when there are updates in things like the debian base images we us?

@mmarchini
Copy link

mmarchini commented Dec 17, 2020

I'm +1 to publish to Docker Registry. As a reminder, new PAT or changes to existing PATs must be requested on https://github.com/nodejs/admin following our policy.

@yordis
Copy link
Author

yordis commented Dec 17, 2020

@yordis does github rebuild the images when the underlying image changes. For some reason I think the official images in the docker repo automatically get updates when there are updates in things like the debian base images we use?

@mhdawson From my understanding it should, the only concern is that for that to happen you suppose to change the Dockerfile since this workflow will trigger based on some files changes (check the on setup)

@mmarchini
Copy link

PAT token request: nodejs/admin#581

@mhdawson
Copy link
Member

This was discussed in the TSC meeting today. The TSC does not have any concerns provided the docker team is agreed that it is something we should do.

@nschonni
Copy link
Member

@yordis does github rebuild the images when the underlying image changes. For some reason I think the official images in the docker repo automatically get updates when there are updates in things like the debian base images we us?

As far as I can tell GHCR is just a dumb storage bucket and API endpoint. The image building we currently do here is just smoke testing that is then done on the various architectures by the Docker groups Jenkins setup. That platform is also what regenerates the images when the base image is changed.

Overall I'm -0 on this, as it's positioned as a development courtesy copy, I'm not sure that would be something that rate limits even hit https://www.docker.com/blog/rate-limiting-by-the-numbers/

/cc @tianon @yosifkit to see if any of the other official images have setup this or any other mirror-like setup

@tianon
Copy link
Contributor

tianon commented Jan 14, 2021

We have no current plans to push any official images to GHCR (or any other registry).

@yordis
Copy link
Author

yordis commented Mar 25, 2021

https://github.blog/changelog/2021-03-24-packages-container-registry-now-supports-github_token/

@PeterDaveHello PeterDaveHello requested a review from a team April 4, 2021 16:30
@nschonni
Copy link
Member

nschonni commented Apr 4, 2021

Still 👎 on this. If people want to have a non-security patched copy in the GitHub container registry, they should do it on their own

@SimenB
Copy link
Member

SimenB commented Apr 4, 2021

Still 👎 on this. If people want to have a non-security patched copy in the GitHub container registry, they should do it on their own

Agreed 👍

@yordis
Copy link
Author

yordis commented Apr 4, 2021

Still 👎 on this. If people want to have a non-security patched copy in the GitHub container registry, they should do it on their own

@nschonni would you mind elaborating?

Trying to see where you shared something related to security, I don't support any insecure images either, but I am not sure what piece you are referring to.

@nschonni
Copy link
Member

nschonni commented Apr 4, 2021

All changes and rebuilds for base image security is done by the Docker Hub CI setup. All images built here is do some sanity checking at Node release time, and then PR upstream to fully build out on the multiple architectures and are kept up to date when base image changes by them

@nschonni nschonni closed this Jul 4, 2021
@yordis yordis deleted the yordis/adding-docker-image-publishing branch July 5, 2021 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add GitHub Action to Publish to Github Docker Registry
9 participants