fix: do not panic on an invalid image name regexp#6184
Conversation
IsImageMatched interpolates the image name from kustomization images[].name straight into a regexp and discarded the compile error. When the name is not a valid regexp (for example "["), regexp.Compile returns a nil *Regexp and the following MatchString call dereferences it, so kustomize build crashes with a SIGSEGV. Capture the compile error and return false when it is set. A name that can't compile matches no image, which leaves the resource untouched (the same result you get for any name that doesn't match). Adds a unit test for the invalid name and a krusty end-to-end case that builds without panicking. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
|
Welcome @arpitjain099! |
|
Hi @arpitjain099. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
@arpitjain099 /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arpitjain099, koba1t The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Hi, I ran into a crash while working with an images override and traced it back to IsImageMatched in the api module.
The image name that comes from a kustomization's
images[].nameis interpolated straight into a regexp, and the compile error was discarded:If the name isn't a valid regexp (for example
[),regexp.Compilereturns(nil, err)and the followingpattern.MatchStringdereferences the nil *Regexp, sokustomize buildpanics with a SIGSEGV instead of failing cleanly. This is reachable in practice: a name like that in a remote base that you don't control (a common GitOps setup) takes down the whole build.The fix keeps the exact same regex string and just checks the error, returning false when the name can't compile. That preserves the existing semantics, since a name that matches nothing leaves the image untouched, which is the same outcome you already get for any name that doesn't match a container.
Verification:
go test ./internal/image/ ./filters/imagetag/passes.internal/imagewith the name[.images: [{name: "[", newTag: v2}]over a Deployment) that panics on master and builds through cleanly with this change.Thanks for taking a look.