Skip to content
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

Add 5 second timeout to Auth dialer #26650

Merged
merged 2 commits into from
Jul 18, 2023
Merged

Add 5 second timeout to Auth dialer #26650

merged 2 commits into from
Jul 18, 2023

Conversation

meyskens
Copy link
Member

@meyskens meyskens commented Jul 5, 2023

The TCP level dialer had no timeout before. This could cause some issues. This change adds a configurable timeout of default 5 seconds to the TCP dailer.

Add a 5 second timeout to the Mutual Auth TCP handshake

@meyskens meyskens requested a review from mhofstetter July 5, 2023 15:18
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 5, 2023
@meyskens meyskens marked this pull request as ready for review July 5, 2023 15:18
@meyskens meyskens requested a review from a team as a code owner July 5, 2023 15:18
@meyskens meyskens added release-note/bug This PR fixes an issue in a previous release of Cilium. area/servicemesh GH issues or PRs regarding servicemesh labels Jul 5, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 5, 2023
mhofstetter
mhofstetter requested changes Jul 6, 2023
View reviewed changes
Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

thanks for this enhancement! ⌛ 🎉

some requested changes inline.

pkg/auth/mutual_authhandler.go Outdated Show resolved Hide resolved
pkg/auth/mutual_authhandler.go Outdated Show resolved Hide resolved
pkg/auth/mutual_authhandler.go Outdated Show resolved Hide resolved
@meyskens meyskens requested review from a team as code owners July 12, 2023 07:02
mhofstetter
mhofstetter requested changes Jul 12, 2023
View reviewed changes
Copy link
Member

@mhofstetter mhofstetter left a comment

Choose a reason for hiding this comment

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

helm values needs to be reflected in the docs -> make -C Documentation update-helm-values 🚀

install/kubernetes/cilium/templates/cilium-configmap.yaml Outdated Show resolved Hide resolved
kaworu
kaworu approved these changes Jul 12, 2023
View reviewed changes
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Helm changes LGTM.

install/kubernetes/cilium/templates/cilium-configmap.yaml Outdated Show resolved Hide resolved
@mhofstetter
Copy link
Member

/test

@meyskens
Copy link
Member Author

/test

@mhofstetter
Copy link
Member

mhofstetter commented Jul 17, 2023 •
edited

@meyskens looks like this needs another round of make -C Documentation update-helm-values

@meyskens
Copy link
Member Author

Seems the merge un-did the cmdref this time. updating...

@meyskens
Add 5 second timeout to Auth dialer
213d189 
The TCP level dialer had no timeout before. This could cause some issues.
This change adds a configurable timeout of deault 5 seconds to the TCP
dailer.

Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>
@meyskens
Expose mesh-auth-mutual-connect-timeout in Helm
6d62dc5 
Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>
@meyskens
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 17, 2023
@aditighag aditighag merged commit ffc60dd into cilium:main Jul 18, 2023
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaworu kaworu kaworu approved these changes

@joamaki joamaki joamaki approved these changes

@mhofstetter mhofstetter mhofstetter approved these changes

Assignees
No one assigned
Labels
area/servicemesh GH issues or PRs regarding servicemesh ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@meyskens @mhofstetter @kaworu @joamaki @aditighag