Skip to content

Commit

Permalink
Fix network resolve introduced with linting (#1340)
Browse files Browse the repository at this point in the history
* Fix network resolve introduced with linting

* Restore test
  • Loading branch information
Victor Castell authored May 29, 2023
1 parent 3055367 commit ed8cd8d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
2 changes: 1 addition & 1 deletion dkron/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (a *Agent) Start() error {
rand.Seed(time.Now().UnixNano())

// Normalize configured addresses
if err := a.config.normalizeAddrs(); err != nil {
if err := a.config.normalizeAddrs(); err != nil && !errors.Is(err, ErrResolvingHost) {
return err
}

Expand Down
10 changes: 6 additions & 4 deletions dkron/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ const (
DefaultRetryInterval time.Duration = time.Second * 30
)

var ErrResolvingHost = errors.New("error resolving hostname")

// DefaultConfig returns a Config struct pointer with sensible
// default settings.
func DefaultConfig() *Config {
Expand Down Expand Up @@ -344,14 +346,14 @@ func (c *Config) normalizeAddrs() error {
if c.HTTPAddr != "" {
ipStr, err := ParseSingleIPTemplate(c.HTTPAddr)
if err != nil {
return fmt.Errorf("bind address resolution failed: %v", err)
return fmt.Errorf("HTTP address resolution failed: %v", err)
}
c.HTTPAddr = ipStr
}

addr, err := normalizeAdvertise(c.AdvertiseAddr, c.BindAddr, DefaultBindPort, c.DevMode)
if err != nil {
return fmt.Errorf("failed to parse HTTP advertise address (%v, %v, %v, %v): %v", c.AdvertiseAddr, c.BindAddr, DefaultBindPort, c.DevMode, err)
return fmt.Errorf("failed to parse advertise address (%v, %v, %v, %v): %w", c.AdvertiseAddr, c.BindAddr, DefaultBindPort, c.DevMode, err)
}
c.AdvertiseAddr = addr

Expand Down Expand Up @@ -410,10 +412,10 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string
return addr, nil
}

// Fallback to bind address first, and then try resolving the local hostname
// TODO: Revisit this as the lookup doesn't work with IP addresses
ips, err := net.LookupIP(bind)
if err != nil {
return "", fmt.Errorf("Error resolving bind address %q: %v", bind, err)
return "", ErrResolvingHost //fmt.Errorf("Error resolving bind address %q: %v", bind, err)
}

// Return the first non-localhost unicast address
Expand Down
27 changes: 25 additions & 2 deletions dkron/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,31 @@ func Test_normalizeAdvertise(t *testing.T) {
}

tests := []test{
{addr: "192.168.1.1", bind: ":8946", want: "192.168.1.1:8946", dev: false},
{addr: "", bind: "127.0.0.1", want: "127.0.0.1:8946", dev: true},
{
addr: "192.168.1.1",
bind: ":8946",
dev: false,
want: "192.168.1.1:8946",
},
{
addr: "",
bind: "127.0.0.1",
dev: true,
want: "127.0.0.1:8946",
},
// TODO: Revisit this test
// {
// addr: "",
// bind: "127.0.0.1:8946",
// dev: true,
// want: "127.0.0.1:8946",
// },
// {
// addr: "",
// bind: "localhost:8946",
// dev: true,
// want: "127.0.0.1:8946",
// },
}

for _, tc := range tests {
Expand Down

0 comments on commit ed8cd8d

Please sign in to comment.