Skip to content

fix: handle multi-architecture images in registry lookup#631

Merged
csatib02 merged 1 commit intobank-vaults:mainfrom
Tolsto:fix-arm64-only-manifest
Feb 10, 2025
Merged

fix: handle multi-architecture images in registry lookup#631
csatib02 merged 1 commit intobank-vaults:mainfrom
Tolsto:fix-arm64-only-manifest

Conversation

@Tolsto
Copy link
Contributor

@Tolsto Tolsto commented Jan 30, 2025

The webhook was failing when trying to inspect images that only had arm64 variants available, as it defaulted to looking up amd64 images first. This was problematic because the webhook runs before pod scheduling, so we don't yet know which architecture the pod will run on.

The fix modifies the image lookup logic to take the first available image manifest from the registry's manifest list, regardless of architecture. This works because the core image configuration (entrypoint, env vars, etc.) that we need to inspect is typically identical across architectures.

Fixes
could not mutate object: cannot convert image descriptor to v1.Image: no child with platform linux/amd64 in index ...

@Tolsto Tolsto requested a review from a team as a code owner January 30, 2025 09:34
@Tolsto Tolsto requested review from lgecse and removed request for a team January 30, 2025 09:34
@github-actions github-actions bot added the size/S Denotes a PR that changes 10-99 lines label Jan 30, 2025
@Tolsto Tolsto force-pushed the fix-arm64-only-manifest branch 2 times, most recently from fbb3a79 to e861db9 Compare January 30, 2025 09:39
The webhook was failing when trying to inspect images that only had arm64
variants available, as it defaulted to looking up amd64 images first. This
was problematic because the webhook runs before pod scheduling, so we don't
yet know which architecture the pod will run on.

The fix modifies the image lookup logic to take the first available image
manifest from the registry's manifest list, regardless of architecture.
This works because the core image configuration (entrypoint, env vars, etc.)
that we need to inspect is typically identical across architectures.

Signed-off-by: Nils Mueller <20240901+Tolsto@users.noreply.github.com>
@Tolsto Tolsto force-pushed the fix-arm64-only-manifest branch from e861db9 to e096cc0 Compare January 30, 2025 10:15
@csatib02 csatib02 requested review from csatib02 and removed request for lgecse February 2, 2025 08:58
@csatib02 csatib02 added the kind/bug Categorizes issue or PR as related to a bug. label Feb 2, 2025
Copy link
Member

@csatib02 csatib02 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution!

Could you please open a PR in the new Webhook's repo too? https://github.com/bank-vaults/secrets-webhook

@csatib02 csatib02 merged commit bf98fb0 into bank-vaults:main Feb 10, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-99 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants