Skip to content

fix: validation behavior for multiple certificate refs#7909

Merged
arkodg merged 5 commits intoenvoyproxy:mainfrom
kkk777-7:fix-multiple-cert-refs
Jan 14, 2026
Merged

fix: validation behavior for multiple certificate refs#7909
arkodg merged 5 commits intoenvoyproxy:mainfrom
kkk777-7:fix-multiple-cert-refs

Conversation

@kkk777-7
Copy link
Copy Markdown
Member

@kkk777-7 kkk777-7 commented Jan 9, 2026

What this PR does / why we need it:
Fix validation behavior for multiple certificate refs.

currently, when both an invalid certificate ref and a valid certificate ref are present in listener TLS, we set ResolvedRefs=False and Programmed=False.
As a result, listener translation fails and no xDS listeners are generated.

we update the behavior by introducing new PartiallyInvalidCertificateRef condition reason.

  • If both valid and invalid certificate refs exist:
    • ResolvedRefs=False with reason PartiallyInvalidCertificateRef
    • Programmed=True

Which issue(s) this PR fixes:

Fixes #5730

Release Notes: Yes

@netlify
Copy link
Copy Markdown

netlify bot commented Jan 9, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 503a781
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69670db03f5491000890a5ae
😎 Deploy Preview https://deploy-preview-7909--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 9, 2026

Codecov Report

❌ Patch coverage is 69.50355% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.79%. Comparing base (a2c0944) to head (503a781).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/validate.go 72.85% 18 Missing and 1 partial ⚠️
internal/gatewayapi/status/error.go 42.85% 10 Missing and 2 partials ⚠️
internal/gatewayapi/tls.go 76.00% 11 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7909   +/-   ##
=======================================
  Coverage   72.79%   72.79%           
=======================================
  Files         235      235           
  Lines       35214    35295   +81     
=======================================
+ Hits        25635    25694   +59     
- Misses       7760     7778   +18     
- Partials     1819     1823    +4     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

// with a Reason method that returns the specific condition reason.
//
// TODO kkk777-7: consider using generics to make Error interface resource-agnostic to support all resource types.
type ListenerError interface {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we should use generics in error interface to avoid duplication.

But this needs wide changes. so I want to handle this in follow up PR.

@kkk777-7 kkk777-7 marked this pull request as ready for review January 10, 2026 05:17
@kkk777-7 kkk777-7 requested a review from a team as a code owner January 10, 2026 05:17
@kkk777-7 kkk777-7 requested a review from zhaohuabing January 10, 2026 05:17
@arkodg arkodg added this to the v1.7.0-rc.1 Release milestone Jan 10, 2026
arkodg
arkodg previously approved these changes Jan 11, 2026
Copy link
Copy Markdown
Contributor

@arkodg arkodg 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

@kkk777-7
Copy link
Copy Markdown
Member Author

gentle ping @zhaohuabing

@zhaohuabing zhaohuabing force-pushed the fix-multiple-cert-refs branch from 9a5cbde to f3cc480 Compare January 14, 2026 02:32
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing 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!

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
@zhaohuabing zhaohuabing force-pushed the fix-multiple-cert-refs branch from f3cc480 to 503a781 Compare January 14, 2026 03:29
@arkodg arkodg merged commit 5aadb86 into envoyproxy:main Jan 14, 2026
33 of 36 checks passed
andreik-n2 pushed a commit to andreik-n2/gateway that referenced this pull request Jan 15, 2026
* fix: multiple certificates validation behavior

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
SadmiB pushed a commit to SadmiB/gateway that referenced this pull request Jan 30, 2026
* fix: multiple certificates validation behavior

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: Sadmi Bouhafs <sadmibouhafs@gmail.com>
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.

certificate in gateway

4 participants