Skip to content

chore(installation): early validate presence of tag in image and imageRepository#6354

Merged
rudrakhp merged 5 commits intoenvoyproxy:mainfrom
sudiptob2:installation/6350/validate-tag
Jun 24, 2025
Merged

chore(installation): early validate presence of tag in image and imageRepository#6354
rudrakhp merged 5 commits intoenvoyproxy:mainfrom
sudiptob2:installation/6350/validate-tag

Conversation

@sudiptob2
Copy link
Member

@sudiptob2 sudiptob2 commented Jun 19, 2025

PR #6296 added support for customizing the EnvoyProxy image using the image and imageRepository fields. In summary:

  • If users want to explicitly specify the image tag, they should use the format: image: repo:tag. This has been the standard approach in all versions up to this PR.
  • If users prefer to define only the image repository and let Envoy Gateway decide the appropriate tag, they should use the format: imageRepository: repo_url.

To ensure these fields are used correctly, it's helpful to validate the presence or absence of a tag depending on which field is set.

This PR improves the resolveProxyImage function by adding the following validations: (5a85f82)
This PR adds following CEL validation:

  • If imageRepository is set, it must not include a tag.
  • If image is set, it must include a valid tag.

Idea for more validation logic is appreciated.

Fixes #6350

@sudiptob2 sudiptob2 force-pushed the installation/6350/validate-tag branch from 671cb4f to bb71c3a Compare June 19, 2025 03:39
@codecov
Copy link

codecov bot commented Jun 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.82%. Comparing base (9453572) to head (ba8b2f4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6354      +/-   ##
==========================================
- Coverage   70.82%   70.82%   -0.01%     
==========================================
  Files         220      220              
  Lines       37132    37132              
==========================================
- Hits        26300    26299       -1     
  Misses       9293     9293              
- Partials     1539     1540       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sudiptob2 sudiptob2 marked this pull request as ready for review June 19, 2025 14:25
@sudiptob2 sudiptob2 requested a review from a team as a code owner June 19, 2025 14:25
@sudiptob2 sudiptob2 force-pushed the installation/6350/validate-tag branch from bb71c3a to c0fdbf5 Compare June 20, 2025 17:13
@arkodg
Copy link
Contributor

arkodg commented Jun 21, 2025

can this be solved with CEL ? so the user hit this error during apply and won't have to find the error in logs

@sudiptob2 sudiptob2 force-pushed the installation/6350/validate-tag branch from c0fdbf5 to 8ca03db Compare June 21, 2025 17:14
@sudiptob2
Copy link
Member Author

sudiptob2 commented Jun 21, 2025

can this be solved with CEL ? So the user hit this error during apply and won't have to find the error in logs

Updated with CEL validation

@sudiptob2 sudiptob2 force-pushed the installation/6350/validate-tag branch from db914a4 to 2b57d13 Compare June 22, 2025 16:27
Copy link
Member

@rudrakhp rudrakhp Jun 23, 2025

Choose a reason for hiding this comment

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

Suggested change
// +kubebuilder:validation:XValidation:rule="has(self.image) ? self.image.matches('^[^:]+:[^:]+$') : true",message="Image must include a tag (e.g., 'image:tag')."
// +kubebuilder:validation:XValidation:rule="self.image.matches('^[a-zA-Z0-9._/-]+:[a-zA-Z0-9._-]+$') : true",message="Image must include a tag and allowed characters only (e.g., 'image:tag')."

Added some suggestions to make regex stricter, let me know what you think! Also can we remove has checks and move them to the respective fields, so kube validations apply on non-null values of the fields only?

Copy link
Member Author

Choose a reason for hiding this comment

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

Stricter regex sounds good!

can we remove has checks and move them to the respective fields

Yeah, moving validation to the respective field will be a cleaner approach, but we wont be able to provide a custom message. But a generic message would be fine, I guess.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the generic message does not look very good. Let me know your thougts

EnvoyProxy.gateway.envoyproxy.io "proxy-1750699777609469000" is invalid: spec.provider.kubernetes.envoyDeployment.container.image: Invalid value: "envoyproxy/envoy": spec.provider.kubernetes.envoyDeployment.container.image in body should match '^[a-zA-Z0-9._/-]+:[a-zA-Z0-9._-]+$'

Copy link
Member

Choose a reason for hiding this comment

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

@sudiptob2 can we do something like this?

// +kubebuilder:validation:XValidation:message="loadBalancerIP must be a valid IPv4 address",rule="self.matches(r\"^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$\")"

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

Copy link
Member

@rudrakhp rudrakhp Jun 23, 2025

Choose a reason for hiding this comment

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

Suggested change
// +kubebuilder:validation:XValidation:rule="has(self.imageRepository) ? !self.imageRepository.contains(':') : true",message="ImageRepository must not include a tag or any colons."
// +kubebuilder:validation:XValidation:rule="self.imageRepository.matches('^[a-zA-Z0-9._/-]+$')",message="ImageRepository must include allowed characters only and not include a tag or any colons."

@sudiptob2 sudiptob2 force-pushed the installation/6350/validate-tag branch 2 times, most recently from 40f0a49 to 0086edd Compare June 23, 2025 20:37
…mageRepository` field in KubernetesContainerSpec

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>
…epository

Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>
Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>
Signed-off-by: sudipto baral <sudiptobaral.me@gmail.com>
@sudiptob2 sudiptob2 force-pushed the installation/6350/validate-tag branch from 0086edd to c53ec24 Compare June 23, 2025 20:37
@sudiptob2 sudiptob2 requested a review from rudrakhp June 23, 2025 21:41
@rudrakhp rudrakhp requested review from a team June 24, 2025 03:00
@shawnh2 shawnh2 enabled auto-merge (squash) June 24, 2025 04:12
@shawnh2
Copy link
Contributor

shawnh2 commented Jun 24, 2025

/retest

1 similar comment
@rudrakhp
Copy link
Member

/retest

@rudrakhp rudrakhp disabled auto-merge June 24, 2025 17:12
@rudrakhp rudrakhp merged commit 6f42531 into envoyproxy:main Jun 24, 2025
61 of 66 checks passed
@sudiptob2 sudiptob2 deleted the installation/6350/validate-tag branch June 24, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate proxy image and repository

4 participants