Repository navigation
Conversation
There was a problem hiding this comment.
Seems like it would work as it is. I think there could be some improvements though
|
|
||
| docker pull ${base_image_name}:latest | ||
|
|
||
| latest_base_image_url=$(docker inspect ${base_image_name}:latest | jq -r '.[0].Config.Labels.url') |
There was a problem hiding this comment.
It is possible to access the manifests without downloading the images. It's a little tricky, as each repo seems to implement the dockerhttp API slightly differently (particularly around the access tokens), but it could save a lot of time.
Quay needs an anonymous token with the right service and scope parameters:
host=quay.io
repository=keycloak/keycloak
tag=18.0.0
token="$(curl -sSfL 'https://$host/v2/auth?service=$host&scope=repository:$repository:pull' | jq -r '.token')"
curl -sSfL -H "Authorization: Bearer $token" "https://$host/v2/$repository/manifests/$tag" | jq -r '.history[0].v1Compatibility' | jq -r '.config.Labels.url'Which returns https://access.redhat.com/containers/#/registry.access.redhat.com/ubi8-minimal/images/8.6-751
Red Hat's repo provides tokenless anonymous access:
```bash
host=registry.access.redhat.com
repository=ubi8-minimal
tag=latest
curl -sSfL "https://$host/v2/$repository/manifests/$tag" | jq -r '.history[0].v1Compatibility' | jq -r '.config.Labels.url'
Which returns https://access.redhat.com/containers/#/registry.access.redhat.com/ubi8-minimal/images/8.6-751
So you could make your approach to the comparison faster based of off this method. It won't be as generic an approach as you're trying to write. However, the assumption around the url label isn't generic either, it's something that happens to work for UBI
That got me thinking, this check could also be done by comparing the layer hashes. The code for that is simpler, compared to parsing the label, but does require you to hardcode the parent URL:
curl -sSfL -H 'Authorization: Bearer $token' https://quay.io/v2/keycloak/keycloak/manifests/18.0.0 | jq -r '.fsLayers | .[] | .blobSum' > /tmp/layers-kc
curl -sSfL https://registry.access.redhat.com/v2/ubi8-minimal/manifests/latest | jq -r '.fsLayers | .[] | .blobSum' > /tmp/layers-ubi
grep -qxF -f /tmp/layers-kc /tmp/layers-ubiWhich would return exit code 0 if the latest ubi layers were present in the keycloak layers (no respin required), or exit code 1 if they were not (respin required)
There was a problem hiding this comment.
Thanks for the accurate review @ASzc !
With this implementation, I was working under the assumption that the job will run pretty seldom and I was preferring (as you noticed) "portability" over "speed".
You are correct that the code is not completely portable as it relies on RedHat specific UBI image labels but, at least, it will run on both quay.io and dockerhub which is convenient for testing IMHO; also, we are not likely to use an image that is not a UBI any time soon AFAIK.
I think I'm still inclined to keep the original implementation using docker pull, mostly because it still looks simpler (more dumb 🙂 ) to me (i.e. doesn't require knowledge about the docker HTTP API nor tokens etc.), but I'm happy to hear more feedback in this regard.
There was a problem hiding this comment.
Right, it's a tradeoff :)
I think for a once-per-day check, the performance hit of downloading several hundred megabytes of data shouldn't be too bad
| tags: | ||
| - '[0-9]+.[0-9]+.[0-9]+' | ||
| schedule: | ||
| - cron: '0 0 * * *' |
There was a problem hiding this comment.
It's recommended not to run crons at the top of an hour: "To decrease the chance of delay, schedule your workflow to run at a different time of the hour"
So something like '7 0 * * *' might work more reliably
There was a problem hiding this comment.
You might want to consider something more frequent than nightly, perhaps hourly. This would be possible without a lot of resource consumption, using the more efficient check as I suggested
There was a problem hiding this comment.
This was not a requirement in my understanding.
From other discussions, I got the impression that we want to re-spin images a maximum of once a day.
I'm happy to re-consider your proposal if this assumption has changed! cc. @abstractj @stianst
There was a problem hiding this comment.
The reason I suggest a more frequent check is to more quickly respond to new versions of UBI, and limit the potential time any CVEs go unfixed. Ideally this would be event driven, but I don't know of any API to enable that. Checking more frequently doesn't necessarily mean more than one image is made per day
There was a problem hiding this comment.
Assuming that everyone is ok with a once-per-day check, this is sufficient for that. As noted in my other comments, a more frequent check is possible with some changes
|
|
||
| latest_base_image_url=$(docker inspect ${base_image_name}:latest | jq -r '.[0].Config.Labels.url') | ||
|
|
||
| if [ "${base_image_url}" = "${latest_base_image_url}" ]; then |
There was a problem hiding this comment.
@andreaTP, thanks for submitting those changes. If I understood it correctly, the script will check if the base image had changed to perform the respin on the daily basis.
If that's the case, I'd suggest to respin the build of those images more frequently to fix known CVEs that will eventually emerge.
The latest UBI was released 27 days ago, that means that if there's a new CVE, the only way to remediate those CVEs is waiting until the next release to benefit from the fix, instead of trigger microdnf update.
Does it make some sense?
//cc @ASzc
There was a problem hiding this comment.
I might have understood it wrongly, but, my impression was that @stianst expressed an opinion toward less frequent re-spins (e.g. max once a day) without quickly reacting to upstream to give users a more "stable" experience.
I'm open to discuss the tradeoffs and eventually steer the PR in the desired direction.
There was a problem hiding this comment.
@andreaTP no problem, we can wait for more feedback. Even with less frequent respins, which is a valid concern. We need some agreement about when to patch those CVEs. As you are already aware, we have 2 alternatives:
- Patch CVEs only when there's a new UBI image
- Patch when there are some dependency updates + a new UBI image
There was a problem hiding this comment.
@abstractj agreed, my personal vote goes to option 1, leaving 2 "covered" by manual triggers.
A third option would be to check for dependencies updates at a "slower pace" e.g. once a week/month.
There was a problem hiding this comment.
In theory UBI should be maintained at a Health Score of A at all times, so there shouldn't be significant public CVEs whenever you happen to pull the latest UBI, even though it could be many weeks old (29 days as of this comment, with no CVEs listed).
I don't think microdnf update would be required at any point to fix a CVE from UBI. Anything that's embargoed wouldn't show up in the updates, and it would be embargoed until a new UBI is published
Draft PR to automate images respinning in CI. The logic is as follows:
latesturls if they change upstream image has been respun/has changes