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

Fix issue where terminating gateway service resolvers weren't properly cleaned up #16498

Merged
merged 4 commits into from
Mar 3, 2023
Merged

Fix issue where terminating gateway service resolvers weren't properly cleaned up #16498

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e14b430
Fix issue where terminating gateway service resolvers weren't properl…
Mar 2, 2023
5255013
Add integration test for cleaning up resolvers
Mar 2, 2023
b1b2abc
Add changelog entry
Mar 2, 2023
ecaeff2
Use state test and drop integration test
Mar 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion agent/proxycfg/terminating_gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,13 @@ func (s *handlerTerminatingGateway) handleUpdate(ctx context.Context, u UpdateEv
// There should only ever be one entry for a service resolver within a namespace
if resolver, ok := resp.Entry.(*structs.ServiceResolverConfigEntry); ok {
snap.TerminatingGateway.ServiceResolvers[sn] = resolver
snap.TerminatingGateway.ServiceResolversSet[sn] = true
} else {
// we likely have a deleted service resolver, and our cast is a nil
// cast, so clear this out
delete(snap.TerminatingGateway.ServiceResolvers, sn)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to handle this similar to here? This allows us to differentiate between nil and a bad config entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that should be fine, in the case of a bad cast though, we're likely going to want to clean this up, no? That or return an error about casting?

Though, not sure what use returning an error would do if when we're requesting a service resolver we get something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that seems reasonable to me. This isn't a huge deal either way.

snap.TerminatingGateway.ServiceResolversSet[sn] = false
}
snap.TerminatingGateway.ServiceResolversSet[sn] = true

case strings.HasPrefix(u.CorrelationID, serviceIntentionsIDPrefix):
resp, ok := u.Result.(structs.Intentions)
Expand Down