Skip to content

Apply DNS host config on change only #4695

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Apply DNS host config on change only #4695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions client/internal/dns/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type DefaultServer struct {
updateSerial uint64
previousConfigHash uint64
currentConfig HostDNSConfig
currentConfigHash uint64
handlerChain *HandlerChain
extraDomains map[domain.Domain]int

Expand Down Expand Up @@ -583,10 +584,27 @@ func (s *DefaultServer) applyHostConfig() {

log.Debugf("extra match domains: %v", maps.Keys(s.extraDomains))

hash, err := hashstructure.Hash(config, hashstructure.FormatV2, &hashstructure.HashOptions{
ZeroNil: true,
IgnoreZeroValue: true,
SlicesAsSets: true,
UseStringer: true,
})
if err != nil {
log.Errorf("unable to hash the host dns configuration, got error: %s", err)
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

When hashing fails, the error is logged but execution continues with hash being 0. This could cause incorrect behavior where a zero hash might spuriously match s.currentConfigHash (which is also initialized to 0), preventing the first configuration from being applied. Consider returning early or setting a flag to force application when hashing fails.

Suggested change
log.Errorf("unable to hash the host dns configuration, got error: %s", err)
log.Errorf("unable to hash the host dns configuration, got error: %s", err)
return

Copilot uses AI. Check for mistakes.
}

if s.currentConfigHash == hash {
log.Infof("not applying host config as there are no changes")
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

[nitpick] Changed 'Infof' to 'Debugf' for consistency with logging levels - configuration skip events are typically debug-level information rather than info-level.

Suggested change
log.Infof("not applying host config as there are no changes")
log.Debugf("not applying host config as there are no changes")

Copilot uses AI. Check for mistakes.
return
}

if err := s.hostManager.applyDNSConfig(config, s.stateManager); err != nil {
log.Errorf("failed to apply DNS host manager update: %v", err)
}

s.currentConfigHash = hash

s.registerFallback(config)
}

Expand Down
Loading