fix(chart): use fully qualified container image names #4288#4289
fix(chart): use fully qualified container image names #4288#4289volker-raschek wants to merge 1 commit intoopen-policy-agent:masterfrom
Conversation
volker-raschek
force-pushed
the
master
branch
2 times, most recently
from
fc73bee to
835a9a3
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for fully qualified container image names in the Gatekeeper Helm chart to address CRI-O v1.34+ requirements, where unqualified image names are no longer pulled from docker.io by default.
- Adds
registryfield to all image configurations in values.yaml withdocker.ioas the default - Updates Helm templates to construct image references using
printf "%s/%s:%s"format to include registry prefix - Applies changes consistently across both
manifest_stagingandcmd/build/helmify/staticdirectories
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| manifest_staging/charts/gatekeeper/values.yaml | Adds registry fields for all image configurations (main image, CRD images, hook images) with docker.io defaults |
| manifest_staging/charts/gatekeeper/templates/webhook-configs-pre-delete.yaml | Updates pre-delete webhook configuration image reference to include registry prefix |
| manifest_staging/charts/gatekeeper/templates/upgrade-crds-hook.yaml | Updates CRD upgrade hook image references to include registry prefix for both custom and default CRD repositories |
| manifest_staging/charts/gatekeeper/templates/namespace-post-upgrade.yaml | Updates post-upgrade namespace labeling job image references to include registry prefix |
| manifest_staging/charts/gatekeeper/templates/namespace-post-install.yaml | Updates post-install namespace labeling job image references to include registry prefix |
| manifest_staging/charts/gatekeeper/templates/_helpers.tpl | Updates webhook probe container helper template to include registry prefix |
| cmd/build/helmify/static/values.yaml | Mirror of manifest_staging values.yaml for chart generation source |
| cmd/build/helmify/static/templates/webhook-configs-pre-delete.yaml | Mirror of manifest_staging template for chart generation source |
| cmd/build/helmify/static/templates/upgrade-crds-hook.yaml | Mirror of manifest_staging template for chart generation source |
| cmd/build/helmify/static/templates/namespace-post-upgrade.yaml | Mirror of manifest_staging template for chart generation source |
| cmd/build/helmify/static/templates/namespace-post-install.yaml | Mirror of manifest_staging template for chart generation source |
| cmd/build/helmify/static/templates/_helpers.tpl | Mirror of manifest_staging template for chart generation source |
| preInstall: | ||
| crdRepository: | ||
| image: | ||
| registry: null |
There was a problem hiding this comment.
Setting registry: null is problematic. If a user sets preInstall.crdRepository.image.repository to a custom value but leaves registry as null, the template at lines 100 and 102 in upgrade-crds-hook.yaml will render the string "null" literally, producing invalid image names like null/custom-repo. Consider using an empty string "" or a default value like docker.io instead.
| registry: null | |
| registry: docker.io |
JaydipGabani
left a comment
JaydipGabani
left a comment
There was a problem hiding this comment.
Thanks for raising this pr. You will need to make changes to Makefile and elsewhere in the repository where image strings are being used to construct image url.
| extraVolumeMounts: [] | ||
| extraVolumes: [] | ||
| image: | ||
| registry: docker.io |
There was a problem hiding this comment.
where is this variable being used?
There was a problem hiding this comment.
can you add variable information to
at appropriate location?
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #4289 +/- ##
===========================================
- Coverage 54.49% 40.67% -13.83%
===========================================
Files 134 251 +117
Lines 12329 17723 +5394
===========================================
+ Hits 6719 7209 +490
- Misses 5116 9889 +4773
- Partials 494 625 +131
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
fixes - #4288 |
|
@volker-raschek are you still working on this? |
|
Hi @JaydipGabani, If anyone else is more familiar with the source code, they are welcome to adopt my changes and continue the work independently. |
The container runtime CRI-O changed their default behavior for unqualified container images names, also named short-names. In previous versions of CRI-O v1.34 has unqualified container images been pulled from `docker.io`. Starting with CRI-O 1.34 it is necessary to define in `/etc/containers/registry.conf` the property like the example below: ```conf unqualified-search-registries = [ "docker.io" ] ``` Based on the fact that this configuration is not enabled by default, the deployment of OPA failes. This patch extends the `values.yaml` file of the attribute `registry`. Other users should not be affected by this change. It should only help users with CRI-O >= v1.34 to be able to deploy OPA successfully again. Signed-off-by: Markus Pesch <markus.pesch@cryptic.systems>
volker-raschek
force-pushed
the
master
branch
2 times, most recently
from
cebe2cb to
90cd3f4
Compare
|
Hi @JaydipGabani, |
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
The container runtime CRI-O changed their default behavior for unqualified container images names, also named short-names.
In privious versions of CRI-O v1.34 has unqualified container images been pulled from docker.io. Starting with CRI-O 1.34 it is necessary to define in
/etc/containers/registry.confthe property like the example below:Based on the fact that this configuration is not enabled by default, the deployment of OPA failes.
This patch extends the
values.yamlfile of the attributeregistry. Other users should not be affected by this change. It should only help users with CRI-O >= v1.34 to be able to deploy OPA successfully again.