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

ca: fix a masked bug in leaf cert generation that would not be notified of root cert rotation after the first one #15005

Merged
merged 2 commits into from Oct 17, 2022
Merged

ca: fix a masked bug in leaf cert generation that would not be notified of root cert rotation after the first one #15005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16e8570
ca: fix a masked bug in leaf cert generation that would not be notifi…
rboyer Oct 14, 2022
78800cb
changelog
rboyer Oct 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/15005.txt
View file
Open in desktop
@@ -0,0 +1,3 @@
```release-note:bug
ca: fix a masked bug in leaf cert generation that would not be notified of root cert rotation after the first one
```
1 change: 1 addition & 0 deletions agent/cache-types/connect_ca_leaf.go
View file
Open in desktop
Expand Up @@ -165,6 +165,7 @@ func (c *ConnectCALeaf) fetchDone(rootUpdateCh chan struct{}) {
if len(c.rootWatchSubscribers) == 0 && c.rootWatchCancel != nil {
// This was the last request. Stop the root watcher.
c.rootWatchCancel()
c.rootWatchCancel = nil
Copy link
Member Author

Choose a reason for hiding this comment

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

fetchStart uses the nil-ness of rootWatchCancel to decide if it needs to launch the goroutine again, so when we invoke the cancel and do not nil it out it means that the goroutine will never run again.

The fix for #14956 mostly caused the cache requests for things like ca roots to act more like the responses were cached or at least singleflighted better, and apparently that rugged "just try again" spirit kept this bug from manifesting in practice in leaf cert generation.

}
}

Expand Down