diff --git a/cmd/boulder-observer/main.go b/cmd/boulder-observer/main.go index ee802cf0deb..9bbe3032839 100644 --- a/cmd/boulder-observer/main.go +++ b/cmd/boulder-observer/main.go @@ -1,7 +1,6 @@ package main import ( - "errors" "flag" "io/ioutil" @@ -12,7 +11,7 @@ import ( func main() { configPath := flag.String( - "config", "config.yaml", "Path to boulder-observer configuration file") + "config", "config.yml", "Path to boulder-observer configuration file") flag.Parse() configYAML, err := ioutil.ReadFile(*configPath) @@ -22,11 +21,12 @@ func main() { var config observer.ObsConf err = yaml.Unmarshal(configYAML, &config) if err != nil { - cmd.FailOnError(err, "failed to parse yaml config") + cmd.FailOnError(err, "failed to parse YAML config") } - if config.DebugAddr == "" { - cmd.FailOnError(errors.New(""), "debugaddr is not defined") + err = config.ValidateDebugAddr() + if err != nil { + cmd.FailOnError(err, "config") } // start monitoring and logging diff --git a/observer/monitor.go b/observer/monitor.go index a515c74c45e..887308e580b 100644 --- a/observer/monitor.go +++ b/observer/monitor.go @@ -32,11 +32,11 @@ func (m monitor) start() *time.Ticker { case <-ticker.C: result, dur := m.prober.Do(m.period) statObservations.WithLabelValues( - m.prober.Name(), m.prober.Type(), strconv.FormatBool(result)). + m.prober.Name(), m.prober.Kind(), strconv.FormatBool(result)). Observe(dur.Seconds()) m.logger.Infof( - "type=[%s] result=[%v] duration=[%f] name=[%s]", - m.prober.Type(), result, dur.Seconds(), m.prober.Name()) + "kind=[%s] result=[%v] duration=[%f] name=[%s]", + m.prober.Kind(), result, dur.Seconds(), m.prober.Name()) } } }() diff --git a/observer/obs_conf.go b/observer/obs_conf.go index 8290b267f74..3fe34d19e50 100644 --- a/observer/obs_conf.go +++ b/observer/obs_conf.go @@ -3,6 +3,8 @@ package observer import ( "errors" "fmt" + "regexp" + "strconv" "github.com/letsencrypt/boulder/cmd" blog "github.com/letsencrypt/boulder/log" @@ -15,61 +17,78 @@ type ObsConf struct { MonConfs []*MonConf `yaml:"monitors"` } -// validateMonConfs calls the validate method for each of the received -// `MonConf` objects. If an error is encountered, this is appended to a -// slice of errors. If no `MonConf` remain, the list of errors is -// returned along with `false`, indicating there are 0 valid `MonConf`. -// Otherwise, the list of errors is returned to be presented to the -// end-user, and true is returned to indicate that there is at least one -// valid `MonConf` +// validateMonConfs calls the validate method for each `MonConf`. If a +// valiation error is encountered, this is appended to a slice of +// errors. If no valid `MonConf` remain, the slice of errors is returned +// along with `false`, indicating that observer should not start func (c *ObsConf) validateMonConfs() ([]error, bool) { - var validationErrs []error + var errs []error for _, m := range c.MonConfs { err := m.validate() if err != nil { - validationErrs = append(validationErrs, err) + errs = append(errs, err) } } + if len(c.MonConfs) == len(errs) { + // all configured monitors are invalid, cannot continue + return errs, false + } + return errs, true +} - // all configured monitors are invalid, cannot continue - if len(c.MonConfs) == len(validationErrs) { - return validationErrs, false +// ValidateDebugAddr ensures the the `debugAddr` received by `ObsConf` +// is properly formatted and a valid port +func (c *ObsConf) ValidateDebugAddr() error { + addrExp := regexp.MustCompile("^:([[:digit:]]{1,5})$") + if !addrExp.MatchString(c.DebugAddr) { + return fmt.Errorf( + "invalid `debugaddr`, %q, not expected format", c.DebugAddr) + } + addrExpMatches := addrExp.FindAllStringSubmatch(c.DebugAddr, -1) + port, _ := strconv.Atoi(addrExpMatches[0][1]) + if !(port > 0 && port < 65535) { + return fmt.Errorf( + "invalid `debugaddr`, %q, is not a valid port", port) } - return validationErrs, true + return nil } -// validate normalizes and validates the observer config as well as each -// `MonConf`. If no valid `MonConf` remain, an error indicating that -// Observer cannot be started is returned. In all instances the the -// rationale for invalidating a 'MonConf' will logged to stderr +// validate normalizes then validates the config received the `ObsConf` +// and each of it's `MonConf`. If no valid `MonConf` remain, an error +// indicating that Observer cannot be started is returned. In all +// instances the rationale for invalidating a 'MonConf' will logged to +// stderr func (c *ObsConf) validate(log blog.Logger) error { if c == nil { return errors.New("observer config is empty") } - if len(c.MonConfs) == 0 { - return errors.New("observer config is invalid, 0 monitors configured") + // validate `debugaddr` + err := c.ValidateDebugAddr() + if err != nil { + return err } - logErrs := func(errs []error, lenMons int) { - log.Errf("%d of %d monitors failed validation", len(errs), lenMons) - for _, err := range errs { - log.Errf("invalid monitor: %s", err) - } + // operator failed to provide any monitors + if len(c.MonConfs) == 0 { + return errors.New("observer config is invalid, no monitors provided") } errs, ok := c.validateMonConfs() + for mon, err := range errs { + log.Errf("monitor %q is invalid: %s", mon, err) + } - // if no valid `MonConf` remain, log validation errors, return error - if len(errs) != 0 && !ok { - logErrs(errs, len(c.MonConfs)) - return fmt.Errorf("no valid mons, cannot continue") + if len(errs) != 0 { + log.Errf("%d of %d monitors failed validation", len(errs), len(c.MonConfs)) + } else { + log.Info("all monitors passed validation") } - // if at least one valid `MonConf` remains, only log validation - // errors - if len(errs) != 0 && ok { - logErrs(errs, len(c.MonConfs)) + // if 0 `MonConfs` passed validation, return error + if !ok { + return fmt.Errorf("no valid mons, cannot continue") } + return nil } diff --git a/observer/obs_conf_test.go b/observer/obs_conf_test.go new file mode 100644 index 00000000000..df505048445 --- /dev/null +++ b/observer/obs_conf_test.go @@ -0,0 +1,69 @@ +package observer + +import ( + "testing" + + "github.com/letsencrypt/boulder/cmd" + blog "github.com/letsencrypt/boulder/log" +) + +func TestObsConf_ValidateDebugAddr(t *testing.T) { + type fields struct { + DebugAddr string + } + tests := []struct { + name string + fields fields + wantErr bool + }{ + // valid + {"valid", fields{":8080"}, false}, + // invalid + {"out of range high", fields{":65536"}, true}, + {"out of range low", fields{":0"}, true}, + {"not even a port", fields{":foo"}, true}, + {"missing :", fields{"foo"}, true}, + {"missing port", fields{"foo:"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &ObsConf{ + DebugAddr: tt.fields.DebugAddr, + } + if err := c.ValidateDebugAddr(); (err != nil) != tt.wantErr { + t.Errorf("ObsConf.ValidateDebugAddr() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestObsConf_validate(t *testing.T) { + type fields struct { + Syslog cmd.SyslogConfig + DebugAddr string + MonConfs []*MonConf + } + type args struct { + log blog.Logger + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &ObsConf{ + Syslog: tt.fields.Syslog, + DebugAddr: tt.fields.DebugAddr, + MonConfs: tt.fields.MonConfs, + } + if err := c.validate(tt.args.log); (err != nil) != tt.wantErr { + t.Errorf("ObsConf.validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/observer/observer.go b/observer/observer.go index af759d2883b..7e6fa737a3b 100644 --- a/observer/observer.go +++ b/observer/observer.go @@ -18,7 +18,7 @@ var ( Name: "obs_monitors", Help: "count of configured monitors", }, - []string{"name", "type", "valid"}, + []string{"name", "kind", "valid"}, ) statObservations = prometheus.NewHistogramVec( prometheus.HistogramOpts{ @@ -26,7 +26,7 @@ var ( Help: "time taken for a monitor to perform a request/query", Buckets: metrics.InternetFacingBuckets, }, - []string{"name", "type", "result"}, + []string{"name", "kind", "result"}, ) ) diff --git a/observer/probes/dns/dns.go b/observer/probes/dns/dns.go index 4d38e6f6691..614e4e119ca 100644 --- a/observer/probes/dns/dns.go +++ b/observer/probes/dns/dns.go @@ -22,8 +22,8 @@ func (p DNSProbe) Name() string { return fmt.Sprintf("%s-%s-%s-%s", p.Proto, p.Server, p.QName, dns.TypeToString[p.QType]) } -// Type returns a name that uniquely identifies the monitor -func (p DNSProbe) Type() string { +// Kind returns a name that uniquely identifies the prober +func (p DNSProbe) Kind() string { return "DNS" } diff --git a/observer/probes/dns/dns_conf.go b/observer/probes/dns/dns_conf.go index 4bc73400f43..bdbc712e342 100644 --- a/observer/probes/dns/dns_conf.go +++ b/observer/probes/dns/dns_conf.go @@ -4,6 +4,7 @@ import ( "fmt" "net" "regexp" + "strconv" "strings" p "github.com/letsencrypt/boulder/observer/probes" @@ -49,25 +50,31 @@ func (c DNSConf) validateServer() error { // ensure `server` does not contain scheme if schemeExp.MatchString(c.Server) { return fmt.Errorf( - "invalid server, %q, remove %q", c.Server, + "invalid `server`, %q, remove %q", c.Server, strings.SplitAfter(c.Server, "://")[0]) } - serverExp := regexp.MustCompile("^(.*)+([[:alnum:]])+(:)([[:digit:]]{1,5})$") + servExp := regexp.MustCompile("^(.*)+([[:alnum:]])+(:)([[:digit:]]{1,5})$") // ensure `server` contains a port - if !serverExp.MatchString(c.Server) { + if !servExp.MatchString(c.Server) { return fmt.Errorf( - "invalid server, %q, is missing a port", c.Server) + "invalid `server`, %q, missing a port", c.Server) } - - // ensure `server` is a valid fqdn, ipv4, or ipv6 address - host := serverExp.FindAllStringSubmatch(c.Server, -1)[0][1] + // ensure the `server` port provided is a valid port + servExpMatches := servExp.FindAllStringSubmatch(c.Server, -1) + port, _ := strconv.Atoi(servExpMatches[0][4]) + if !(port > 0 && port < 65535) { + return fmt.Errorf( + "invalid `server`, %q, is not a valid port", port) + } + // ensure `server` is a valid fqdn or ipv4/ipv6 address + host := servExpMatches[0][1] ipv6 := net.ParseIP(host).To16() ipv4 := net.ParseIP(host).To4() fqdn := dns.IsFqdn(dns.Fqdn(host)) if ipv6 == nil && ipv4 == nil && fqdn != true { return fmt.Errorf( - "invalid server, %q, is not an fqdn or ipv4/6 address", c.Server) + "invalid `server`, %q, is not an fqdn or ipv4/6 address", c.Server) } return nil } @@ -79,7 +86,7 @@ func (c DNSConf) validateProto() error { } } return fmt.Errorf( - "invalid protocol, got: %q, expected one in: %s", c.Proto, validProtos) + "invalid `protocol`, got: %q, expected one in: %s", c.Proto, validProtos) } func (c DNSConf) validateQType() error { @@ -91,7 +98,7 @@ func (c DNSConf) validateQType() error { } } return fmt.Errorf( - "invalid query_type, got: %q, expected one in %s", c.QType, q) + "invalid `query_type`, got: %q, expected one in %s", c.QType, q) } // Validate normalizes and validates the received `DNSConf`. If the @@ -102,7 +109,7 @@ func (c DNSConf) Validate() error { // validate `query_name` if !dns.IsFqdn(dns.Fqdn(c.QName)) { - return fmt.Errorf("invalid query_name, %q is not an fqdn", c.QName) + return fmt.Errorf("invalid `query_name`, %q is not an fqdn", c.QName) } // validate `server` diff --git a/observer/probes/http/http.go b/observer/probes/http/http.go index 8442d5d8647..846f9acfc42 100644 --- a/observer/probes/http/http.go +++ b/observer/probes/http/http.go @@ -18,8 +18,8 @@ func (p HTTPProbe) Name() string { return fmt.Sprintf("%s-%d", p.URL, p.RCodes) } -// Type returns the type of prober as a string -func (p HTTPProbe) Type() string { +// Kind returns a name that uniquely identifies the prober +func (p HTTPProbe) Kind() string { return "HTTP" } diff --git a/observer/probes/prober.go b/observer/probes/prober.go index 611e484fe2c..d70f19edd3e 100644 --- a/observer/probes/prober.go +++ b/observer/probes/prober.go @@ -17,7 +17,7 @@ var ( // Prober is the expected interface for Prober types type Prober interface { Name() string - Type() string + Kind() string Do(time.Duration) (bool, time.Duration) }