diff --git a/pkg/metrics/ntp.go b/pkg/metrics/ntp.go index d985ea422..7d3106e9f 100644 --- a/pkg/metrics/ntp.go +++ b/pkg/metrics/ntp.go @@ -84,12 +84,23 @@ func (p *Ntp) Offset() (time.Duration, error) { var ntpQueryErr error for _, host := range p.pool { response, err := p.ntpQuery(host, ntp.QueryOptions{Timeout: p.timeout}) - if err == nil { - offsets = append(offsets, response.ClockOffset) - } else { + if err != nil { ntpQueryErr = multierr.Append(ntpQueryErr, err) syslog.WithError(err).WithField("ntp_host", host).WithField("timeout", p.timeout).Debug("error getting ntp offset") + + continue + } + + err = response.Validate() + if err != nil { + ntpQueryErr = multierr.Append(ntpQueryErr, err) + syslog.WithError(err).WithField("ntp_host", host).WithField("timeout", p.timeout).Debug("error validating ntp response, skipping") + + continue } + + syslog.WithField("ntp_host", host).WithField("response", response).Trace("valid ntp response retrieved") + offsets = append(offsets, response.ClockOffset) } if len(offsets) == 0 { diff --git a/pkg/metrics/ntp_test.go b/pkg/metrics/ntp_test.go index 6b37ce348..a31057790 100644 --- a/pkg/metrics/ntp_test.go +++ b/pkg/metrics/ntp_test.go @@ -108,7 +108,7 @@ func TestOffset_Cache(t *testing.T) { name: "first request should not use cached value", now: nowMock("2022-09-28 16:02:45"), pool: []string{"one", "two"}, - ntpResponses: []ntpResp{{&ntp.Response{ClockOffset: 50 * time.Millisecond}, nil}, {&ntp.Response{ClockOffset: 10 * time.Millisecond}, nil}}, + ntpResponses: []ntpResp{{buildValidNtpResponse(50 * time.Millisecond), nil}, {buildValidNtpResponse(10 * time.Millisecond), nil}}, expectedOffset: 30 * time.Millisecond, expectedUpdatedAt: mustParse("2006-01-02 15:04:05", "2022-09-28 16:02:45"), // same as now }, @@ -127,7 +127,7 @@ func TestOffset_Cache(t *testing.T) { updatedAt: mustParse("2006-01-02 15:04:05", "2022-09-28 15:42:45"), pool: []string{"one", "two"}, offset: 20 * time.Millisecond, - ntpResponses: []ntpResp{{&ntp.Response{ClockOffset: 50 * time.Millisecond}, nil}, {&ntp.Response{ClockOffset: 10 * time.Millisecond}, nil}}, + ntpResponses: []ntpResp{{buildValidNtpResponse(50 * time.Millisecond), nil}, {buildValidNtpResponse(10 * time.Millisecond), nil}}, expectedOffset: 30 * time.Millisecond, expectedUpdatedAt: mustParse("2006-01-02 15:04:05", "2022-09-28 16:02:45"), // same as now }, @@ -153,15 +153,15 @@ func TestOffset_OffsetAverage(t *testing.T) { ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval) ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{ { - resp: &ntp.Response{ClockOffset: 110 * time.Millisecond}, + resp: buildValidNtpResponse(110 * time.Millisecond), err: nil, }, { - resp: &ntp.Response{ClockOffset: 20 * time.Millisecond}, + resp: buildValidNtpResponse(20 * time.Millisecond), err: nil, }, { - resp: &ntp.Response{ClockOffset: 23 * time.Millisecond}, + resp: buildValidNtpResponse(23 * time.Millisecond), err: nil, }, }...) @@ -177,14 +177,14 @@ func TestOffset_AnyHostErrorShouldNotReturnError(t *testing.T) { ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval) ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{ { - resp: &ntp.Response{ClockOffset: 50 * time.Millisecond}, + resp: buildValidNtpResponse(50 * time.Millisecond), err: nil, }, { err: errors.New("this is an error"), }, { - resp: &ntp.Response{ClockOffset: 40 * time.Millisecond}, + resp: buildValidNtpResponse(40 * time.Millisecond), err: nil, }, }...) @@ -215,6 +215,58 @@ func TestOffset_AllHostErrorShouldReturnError(t *testing.T) { assert.ErrorAs(t, err, &ErrGettingNtpOffset) } +func TestOffset_InvalidNtpResponse(t *testing.T) { + t.Parallel() + + timeout := uint(5000) + interval := uint(15) + ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval) + ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{ + { + resp: buildInvalidNtpResponse(), + err: nil, + }, + { + resp: buildInvalidNtpResponse(), + err: nil, + }, + { + resp: buildInvalidNtpResponse(), + err: nil, + }, + }...) + ntpMonitor.now = nowMock("2022-09-28 16:02:45") + offset, err := ntpMonitor.Offset() + assert.Equal(t, time.Duration(0), offset) + assert.ErrorAs(t, err, &ErrGettingNtpOffset) +} + +func TestOffset_AnyHostInvalidShouldNotReturnError(t *testing.T) { + t.Parallel() + + timeout := uint(5000) + interval := uint(15) + ntpMonitor := NewNtp([]string{"one", "two", "three"}, timeout, interval) + ntpMonitor.ntpQuery = ntpQueryMock([]ntpResp{ + { + resp: buildValidNtpResponse(50 * time.Millisecond), + err: nil, + }, + { + resp: buildInvalidNtpResponse(), + err: nil, + }, + { + resp: buildValidNtpResponse(40 * time.Millisecond), + err: nil, + }, + }...) + ntpMonitor.now = nowMock("2022-09-28 16:02:45") + offset, err := ntpMonitor.Offset() + assert.Equal(t, time.Millisecond*45, offset) + assert.Equal(t, nil, err) +} + // nolint:unparam func nowMock(dateTime string) func() time.Time { return func() time.Time { @@ -243,3 +295,25 @@ func mustParse(layout string, value string) time.Time { return t } + +func buildValidNtpResponse(offset time.Duration) *ntp.Response { + // A response that should pass the ntp.Validate() check without errors + return &ntp.Response{ //nolint:exhaustruct + // Stratum should be not 0 and lower than 16 + Stratum: 1, + // Leap should not be ntp.LeapNotInSync + Leap: ntp.LeapNoWarning, + + ClockOffset: offset, + } +} + +func buildInvalidNtpResponse() *ntp.Response { + // A response that should fail the ntp.Validate() check + return &ntp.Response{ //nolint:exhaustruct + // Stratum should be not 0 and lower than 16 + Stratum: 0, + // Leap should not be ntp.LeapNotInSync + Leap: ntp.LeapNotInSync, + } +}