-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
fix(ci): Fix the automatic linking of Docker images #6821
Conversation
There were a few issues with the previous implementation. Some of the links were not generated correctly and Docker Official Images were not supported at all. This should also automatically clean up non-links in the sources list which is causing linting issues. After this runs there are still some issues where existing Chart.yaml files may include http instead of https URLs or other artifacts that should be cleaned up manually.
0b223ac
to
41c49a9
Compare
*ghcr*|*quay*) | ||
# Both ghcr.io and quay.io have nice redirection. | ||
prefix="https://" | ||
;; | ||
*lscr*) | ||
# Images hosted on lscr.io seem to all be available on Docker Hub under the linuxserver user. | ||
prefix="https://hub.docker.com/r/linuxserver/" | ||
# Get the image name without any pathing. | ||
container="${container##*/}" | ||
;; | ||
*tccr*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't really confident in this matching, it seemed very susceptible to collisions, like if I made an opello/ghcr
image and pushed it to Docker Hub, I think a bad link would be generated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran through all of the substrings and didn't find a problem in the generated output. I still think it's worth thinking about a little though.
This PR is locked to prevent necro-posting on closed PRs. Please create a issue or contact staff on discord if you want to further discuss this |
Description
There were a few issues with the previous implementation. Some of the links were not generated correctly and Docker Official Images were not supported at all.
This should also automatically clean up non-links in the sources list which is causing linting issues.
After this runs there are still some issues where existing Chart.yaml files may include http instead of https URLs or other artifacts that should be cleaned up manually.
⚒️ Fixes
Numerous issues with Chart.yaml sources lists that contain dead links and non-URLs.
⚙️ Type of change
🧪 How Has This Been Tested?
Cut out the sync_tag function and ran it in its own script. Tested against individual charts as well as running against all charts as the CI job would.
📃 Notes:
There is a change of indent to be consistent with what seemed prevalent in the file. There is a move from an if-ladder to a case statement for nicer string matching. There is the use of parameter expansion to manipulate strings. Any of these might be worth careful review or an alternative, more accessible approach.
✔️ Checklist:
➕ App addition
If this PR is an app addition please make sure you have done the following.
icon.png
Please don't blindly check all the boxes. Read them and only check those that apply.
Those checkboxes are there for the reviewer to see what is this all about and
the status of this PR with a quick glance.