Skip to content

fix: prevent race conditions causing plain text communication#246

Merged
arjan-bal merged 14 commits intogrpc:mainfrom
AgraVator:fix-plain-text-race-condition
Apr 29, 2026
Merged

fix: prevent race conditions causing plain text communication#246
arjan-bal merged 14 commits intogrpc:mainfrom
AgraVator:fix-plain-text-race-condition

Conversation

@AgraVator
Copy link
Copy Markdown
Contributor

@AgraVator AgraVator commented Apr 15, 2026

Currently, a race condition exists where pods connect to Traffic Director before security policies are fully propagated, causing them to fallback to plaintext configurations. This leads to unrecoverable protocol mismatches (e.g., SSL wrong version number errors) in the initial attemps.

To eliminate test flakiness and avoid the need for the assertTestAppSecurityWithRetry retry loop, we must ensure that the client and server receive their intended TLS/mTLS security configurations in their very first xDS responses from Traffic Director.

Security Test Run
Authz Test Run

@AgraVator AgraVator requested a review from a team as a code owner April 15, 2026 05:11
@AgraVator AgraVator closed this Apr 15, 2026
@AgraVator AgraVator reopened this Apr 15, 2026
@pawbhard pawbhard requested a review from arjan-bal April 16, 2026 15:20
@pawbhard
Copy link
Copy Markdown
Contributor

@eshitachandwani and @arjan-bal Please review the PR.
Abhishek, lets make sure to run securtity and authz test run with the changes.

@arjan-bal
Copy link
Copy Markdown
Contributor

Apologies for the delay, have been busy last week. Will leave a review within the next couple of days.

Comment thread framework/infrastructure/traffic_director.py Outdated
Comment thread framework/infrastructure/traffic_director.py Outdated
Comment thread framework/infrastructure/traffic_director.py Outdated
Comment thread framework/infrastructure/traffic_director.py Outdated
Comment thread framework/infrastructure/traffic_director.py Outdated
@AgraVator
Copy link
Copy Markdown
Contributor Author

Latest Test Run

@AgraVator AgraVator requested a review from arjan-bal April 23, 2026 03:19
Comment thread framework/infrastructure/traffic_director.py Outdated
@arjan-bal
Copy link
Copy Markdown
Contributor

@pawbhard should we remove the retires introduced in #237?

@pawbhard
Copy link
Copy Markdown
Contributor

@pawbhard should we remove the retires introduced in #237?

Will be done after this PR

@eshitachandwani
Copy link
Copy Markdown
Member

The changes look good to me. But since we are adding a field in the function that is used by almost all the tests , can you please run one more test, other than security to make sure it is not failing anything else. Since it is an optional field, it should not affect other tests, but we should still verify.
Also please fix the pylint.

@AgraVator
Copy link
Copy Markdown
Contributor Author

@AgraVator AgraVator requested a review from arjan-bal April 29, 2026 04:16
Copy link
Copy Markdown
Contributor

@arjan-bal arjan-bal left a comment

Choose a reason for hiding this comment

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

LGTM!

@arjan-bal arjan-bal merged commit 14ff198 into grpc:main Apr 29, 2026
6 checks passed
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.

4 participants