Skip to content

enable supported conformance test #7291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Oct 24, 2025
Merged

enable supported conformance test #7291

merged 12 commits into from
Oct 24, 2025

Conversation

@zirain
Copy link
Member

@zirain zirain commented Oct 21, 2025

xref: #7290

@zirain zirain requested a review from a team as a code owner October 21, 2025 03:02
@codecov
Copy link

codecov bot commented Oct 21, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 89.79592% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.01%. Comparing base (67f9b5d) to head (6e718d0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/conformance/suite.go 0.00% 5 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7291      +/-   ##
==========================================
+ Coverage   71.92%   72.01%   +0.08%     
==========================================
  Files         229      229              
  Lines       33189    33198       +9     
==========================================
+ Hits        23870    23906      +36     
+ Misses       7585     7561      -24     
+ Partials     1734     1731       -3

☔ 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.

@zirain zirain force-pushed the fix-conformance-test branch 2 times, most recently from fcb8605 to 6c57d80 Compare October 21, 2025 07:22
@zirain zirain mentioned this pull request Oct 21, 2025
Open
6 tasks
@arkodg arkodg added this to the v1.6.0-rc.1 Release milestone Oct 21, 2025
@zirain zirain force-pushed the fix-conformance-test branch 3 times, most recently from f9d423d to 07e2372 Compare October 22, 2025 03:15
arkodg
arkodg reviewed Oct 22, 2025
View reviewed changes
internal/xds/translator/translator.go
// 3. If SubjectAltNames are specified, Hostname can be used for certificate selection
// but MUST NOT be used for authentication. If you want to use the value
// of the Hostname field for authentication, you MUST add it to the SubjectAltNames list.
if tlsConfig.SNI != nil && len(tlsConfig.SubjectAltNames) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

looks good to me, cc @guydc

Copy link
Member Author

Choose a reason for hiding this comment

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

ref: https://github.com/kubernetes-sigs/gateway-api/blob/fefb2037d7ce7519984fb9222bbfd1367fdbba6e/apis/v1/backendtlspolicy_types.go#L200

@zirain zirain force-pushed the fix-conformance-test branch from 45c82ff to 0e77c7a Compare October 22, 2025 03:43
@zirain zirain mentioned this pull request Oct 22, 2025
Open
zirain
zirain commented Oct 22, 2025
View reviewed changes
internal/gatewayapi/route.go Outdated

protocol := inspectAppProtocolByRouteKind(routeType)

// Always processing BackendTLSPolicy if possible.
Copy link
Member Author

Choose a reason for hiding this comment

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

key changes: always processing BackendTLSPolicy

zirain
zirain commented Oct 22, 2025
View reviewed changes
internal/gatewayapi/backendtlspolicy.go

if err != nil {
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
status.SetConditionForPolicyAncestors(&policy.Status,
Copy link
Member Author

Choose a reason for hiding this comment

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

key changes: set BackendTLSPolicyReasonNoValidCACertificate status

arkodg
arkodg reviewed Oct 23, 2025
View reviewed changes
arkodg
arkodg reviewed Oct 23, 2025
View reviewed changes
arkodg
arkodg reviewed Oct 23, 2025
View reviewed changes
arkodg
arkodg reviewed Oct 23, 2025
View reviewed changes
arkodg
arkodg approved these changes Oct 24, 2025
View reviewed changes
Copy link
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

@arkodg arkodg requested review from a team October 24, 2025 00:06
zhaohuabing
zhaohuabing reviewed Oct 24, 2025
View reviewed changes
@zirain zirain enabled auto-merge (squash) October 24, 2025 01:20
zirain added 7 commits October 24, 2025 09:21
@zirain
BackendTLSPolicy
6165e04 
Signed-off-by: zirain <[email protected]>
@zirain
fix gen
88755a5 
Signed-off-by: zirain <[email protected]>
@zirain
BackendTLSPolicyObservedGenerationBump
d059cdb 
Signed-off-by: zirain <[email protected]>
@zirain
nit
0ca551a 
Signed-off-by: zirain <[email protected]>
@zirain
update
d7ab5bf 
Signed-off-by: zirain <[email protected]>
@zirain
update
c1b3a04 
Signed-off-by: zirain <[email protected]>
@zirain
fix gen
6e718d0 
Signed-off-by: zirain <[email protected]>
@zirain zirain force-pushed the fix-conformance-test branch from 5c5deab to 6e718d0 Compare October 24, 2025 01:21
@zirain zirain requested a review from zhaohuabing October 24, 2025 01:26
zhaohuabing
zhaohuabing approved these changes Oct 24, 2025
View reviewed changes
Copy link
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!

@zirain zirain merged commit dacf790 into envoyproxy:main Oct 24, 2025
60 of 65 checks passed
@zirain zirain deleted the fix-conformance-test branch October 24, 2025 03:22
kkk777-7
kkk777-7 reviewed Oct 24, 2025
View reviewed changes
internal/gatewayapi/route.go
protocol := inspectAppProtocolByRouteKind(routeType)

// Process BackendTLSPolicy first to ensure status is set.
tls, tlsErr := t.applyBackendTLSSetting(
Copy link
Member

Choose a reason for hiding this comment

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

thanks for making this change so that BackendTLSPolicyObservedGenerationBump passes.

@kkk777-7
Copy link
Member

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kkk777-7 kkk777-7 kkk777-7 left review comments

@zhaohuabing zhaohuabing zhaohuabing approved these changes

@arkodg arkodg arkodg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.6.0-rc.1 Release

Development

Successfully merging this pull request may close these issues.

4 participants

@zirain @kkk777-7 @zhaohuabing @arkodg