Skip to content
This repository has been archived by the owner on Jan 21, 2024. It is now read-only.

Sync README.md with docker hub #385

Merged
merged 3 commits into from
Aug 3, 2023
Merged

Conversation

NotMyFault
Copy link
Member

@NotMyFault NotMyFault commented Aug 1, 2023

Recently, @timja updated the readmes of a few repositories on docker hub, to be up-to-date. The change proposed keeps their presence in sync with what's used on GitHub.

Depending on the seats available, we could add a machine account, or one of the docker hub admins would need to create a PAT and add that as secret. Keep the token in mind, we could reuse it in other repositories too.

I did a quick demo earlier with https://github.com/NotMyFault/dockerhub-sync-em pushing to https://hub.docker.com/r/notmyfault1/sync-demo to ensure the presence looks as intended with our formatting.

@NotMyFault NotMyFault requested a review from a team as a code owner August 1, 2023 10:30
@dduportal
Copy link
Contributor

That is a good idea!

However there are some elements to discuss first:

  • Which "version" should be mapped to the DockerHub? Should we always map the "latest master branch README" to the DockerHub? Or the README of the last built (and deployed) tag? Or something else?
  • Should we use trusted.ci.jenkins.io to update the description (which already uses ad DockerHub PAT with "write" access to the repositories)? Or GitHub action? Or another system?

Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

My personal vote is to use trusted.ci.jenkins.io and trigger the "description update" as the last step of a release (e.g. sync. the README of the last tagged + deployed version) under the following arguments:

  • No need to create additional PAT (or to store existing PAT on different locations suchs as GitHub Actions secrets in addition to trusted.ci.jenkins.io)
  • Simplified workflows
  • The DockerHub description always map the latest released version (no shift between what is on the main branch and what is the really last version) and can specify this "version" so we can always monitor which one it is (build failure ont trusted, "freshness" of the released version, etc.)

The cons of my vote is that we'll have to run the same code as the proposed GHA inside Jenkins (but it does not seems a problem as we have both NodeJS and Docker available on the trusted.ci.jenkins.io build agents).

@hervelemeur
Copy link

hervelemeur commented Aug 1, 2023

I've also looked into this these past days, we don't need nodejs nor docker at all, the gist of the GHA is just an API call.

@NotMyFault
Copy link
Member Author

NotMyFault commented Aug 1, 2023

That is a good idea!

However there are some elements to discuss first:

  • Which "version" should be mapped to the DockerHub? Should we always map the "latest master branch README" to the DockerHub? Or the README of the last built (and deployed) tag? Or something else?
  • Should we use trusted.ci.jenkins.io to update the description (which already uses ad DockerHub PAT with "write" access to the repositories)? Or GitHub action? Or another system?

To 1; I updated the workflow to run once a GH release is cut, matching with what is deployed, or someone triggered the workflow manually.
To 2; Given it's a simple GH action, it would make sense to add the couple of secrets to the repository, no?
How would you run a GH action on trusted.ci? That sounds a bit overkill for such a simple workflow 🤔

@dduportal
Copy link
Contributor

That is a good idea!
However there are some elements to discuss first:

  • Which "version" should be mapped to the DockerHub? Should we always map the "latest master branch README" to the DockerHub? Or the README of the last built (and deployed) tag? Or something else?
  • Should we use trusted.ci.jenkins.io to update the description (which already uses ad DockerHub PAT with "write" access to the repositories)? Or GitHub action? Or another system?

To 1; I updated the workflow to run once a GH release is cut, matching with what is deployed, or someone triggered the workflow manually. To 2; Given it's a simple GH action, it would make sense to add the couple of secrets to the repository, no? How would you run a GH action on trusted.ci? That sounds a bit overkill for such a simple workflow 🤔

It’s a matter of risk management and maintenance overhead:

  • if we have to rotate the PAT credential, it means another location to track, document and update
  • adding another element (a GHA workflow) means another elements which could run unexpectedly (eg should it wait for the image to be deployed by trusted ? In parallel ? Etc.)

i would also add that using jenkins for jenkins project, even if not a hard constraint, should preferred when possible.

Personal opinion : one or two curl requests looks way easier for me than a bunch of yaml loading someone else implementation

I do not mind spending time asap on implementing it in a jenkins pipeline.

Additionally : running the github action in jenkins should not be complicated as it is only a docker image or nodejs script with standardized input and outputs : idea for a plugin ?

@NotMyFault
Copy link
Member Author

NotMyFault commented Aug 1, 2023

Personal opinion : one or two curl requests looks way easier for me than a bunch of yaml loading someone else implementation

I do not mind spending time asap on implementing it in a jenkins pipeline.

If that's simpler and fine for you to implement over a GH action, we can surely do that. I could make it work using curl and jq locally:

DOCKERHUB_USERNAME="notmyfault1"
DOCKERHUB_PASSWORD="a"
GITHUB_README_URL="https://raw.githubusercontent.com/NotMyFault/dockerhub-sync-em/main/README.md"
DOCKERHUB_REPO="notmyfault1/sync-demo"

response=$(curl -s -X POST "https://hub.docker.com/v2/users/login/" -H "Content-Type: application/json" -d "{\"username\":\"$DOCKERHUB_USERNAME\",\"password\":\"$DOCKERHUB_PASSWORD\"}")
token=$(echo "$response" | jq -r '.token')

readme_content=$(curl -sL "$GITHUB_README_URL" | jq -sR .)
curl -X PATCH -H "Authorization: JWT $token" -H "Content-Type: application/json" -d "{\"full_description\": $readme_content}" "https://hub.docker.com/v2/repositories/$DOCKERHUB_REPO/"

@lemeurherve
Copy link
Member

Personal opinion : one or two curl requests looks way easier for me than a bunch of yaml loading someone else implementation

I do not mind spending time asap on implementing it in a jenkins pipeline.

If that's simpler and fine for you to implement over a GH action, we can surely do that. I could make it work using curl and jq locally:

DOCKERHUB_USERNAME="notmyfault1"
DOCKERHUB_PASSWORD="a"
GITHUB_README_URL="https://raw.githubusercontent.com/NotMyFault/dockerhub-sync-em/main/README.md"
DOCKERHUB_REPO="notmyfault1/sync-demo"

response=$(curl -s -X POST "https://hub.docker.com/v2/users/login/" -H "Content-Type: application/json" -d "{\"username\":\"$DOCKERHUB_USERNAME\",\"password\":\"$DOCKERHUB_PASSWORD\"}")
token=$(echo "$response" | jq -r '.token')

readme_content=$(curl -sL "$GITHUB_README_URL" | jq -sR .)
curl -X PATCH -H "Authorization: JWT $token" -H "Content-Type: application/json" -d "{\"full_description\": $readme_content}" "https://hub.docker.com/v2/repositories/$DOCKERHUB_REPO/"

That's what I had in mind, I've opened a PR (WIP) for that: jenkins-infra/pipeline-library#714

@timja
Copy link
Member

timja commented Aug 1, 2023

It's not just a simple API call you need to rewrite links and image URLs which the github action will handle for you.

Also it's not just a PAT with write scope it requires read/write/delete

imo GitHub action that's already built for this is simpler to use than maintaining our own thing.

@dduportal
Copy link
Contributor

It's not just a simple API call you need to rewrite links and image URLs which the github action will handle for you.

That one is not a problem but make sense

Also it's not just a PAT with write scope it requires read/write/delete

True, I discovered this requirement after my previous messages: docker/hub-feedback#2321

I feel really uncomfortable with providing PAT with the delete permission but that change my initial assumption as it
means we'll need highly privileged PAT to be authored.
And we don't want to have these PAT in trusted.ci.jenkins.io to avoid any accidental deletion of images or tags due to automation error which means GHA might be the solution (my other arguments are nitpicks but this safety measure is critical).

Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

LGTM under the following assumption:

An admin PAT need to be set up and added in this repository's secrets (or at the organization level but with restriction to the repositories mapped to an image: jenkinsci/docker, jenkinsci/docker-agent, jenkinsci/inbound-agent, jenkinsci/ssh-agent, jenkinsci/inbound-agents and jenkinsci/ath as far as I can tell)

@timja
Copy link
Member

timja commented Aug 3, 2023

I've done on all but docker-inbound-agents that don't seem to have any README's in GitHub for the images.

@NotMyFault can always add it to that repo if he plans to add it there

@NotMyFault NotMyFault enabled auto-merge (squash) August 3, 2023 14:23
@NotMyFault NotMyFault merged commit 1ee1ace into jenkinsci:master Aug 3, 2023
5 checks passed
@NotMyFault NotMyFault deleted the sync-readme branch August 3, 2023 14:46
@NotMyFault
Copy link
Member Author

I've done on all but docker-inbound-agents that don't seem to have any README's in GitHub for the images.

NotMyFault can always add it to that repo if he plans to add it there

@timja Does the PAT have the proper scopes (read/write/delete)? Sync failed with "Forbidden".

@timja
Copy link
Member

timja commented Aug 3, 2023

I've done on all but docker-inbound-agents that don't seem to have any README's in GitHub for the images.
NotMyFault can always add it to that repo if he plans to add it there

@timja Does the PAT have the proper scopes (read/write/delete)? Sync failed with "Forbidden".

it does

image

@NotMyFault
Copy link
Member Author

That's weird 🤔

@timja
Copy link
Member

timja commented Aug 3, 2023

image

@timja
Copy link
Member

timja commented Aug 3, 2023

I've recreated it

@NotMyFault
Copy link
Member Author

I've recreated it

Same outcome 😔

Is the jenkinsinfraadmin account an org owner or part of a team that has "Admin" access to a repository? I'm uncertain whether teams with Read/Write suffice to modify the overview, given

Screenshot 2023-08-03 at 17 37 16

@dduportal
Copy link
Contributor

I've recreated it

Same outcome 😔

Is the jenkinsinfraadmin account an org owner or part of a team that has "Admin" access to a repository? I'm uncertain whether teams with Read/Write suffice to modify the overview, given

Screenshot 2023-08-03 at 17 37 16

Good catch: jenkinsinfraadmin is not admin (and should not).

@timja
Copy link
Member

timja commented Aug 3, 2023

Why should not it have admin? if we want to manage the description through automation it will need that.

We can manage the token scope that's given out to limit the places where it has that access?

@dduportal
Copy link
Contributor

Why should not it have admin? if we want to manage the description through automation it will need that.

We can manage the token scope that's given out to limit the places where it has that access?

Since it is in the jenkinsci area i will let @Wadeck have the last word.

I think the automation is not worth the risk.

@Wadeck
Copy link

Wadeck commented Aug 4, 2023

(not time to dig into that topic right now, will try to come back to it soonish)

lemeurherve pushed a commit to lemeurherve/jenkinsci-docker-inbound-agent that referenced this pull request Nov 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants