chrysom bump, validation config fix (#91)

* updated errors to have more information

* change validation config field name to match

* bumped argus version

* add mapstructure tag for proper yaml unmarshaling

* updated changelog

* added chrysom version

* fixed error string

* prep for release

Author: kristinapathak

CHANGELOG.md

@@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+## [v0.3.4]
+- Added more details to some validation errors. [#91](https://github.com/xmidt-org/ancla/pull/91)
+- Fixed naming mismatch for Validator config: changed name in Config struct to what we send in yaml. [#91](https://github.com/xmidt-org/ancla/pull/91)
+- Bumped chrysom to v0.6.0: updated code to create basic and listener clients. [#91](https://github.com/xmidt-org/ancla/pull/91)
+
 ## [v0.3.3]
 - Removed legacy logic condition for inserting a webhook. [#85](https://github.com/xmidt-org/ancla/pull/85)
 - Changed webhook address handling. [#84](https://github.com/xmidt-org/ancla/pull/84)
@@ -78,7 +83,8 @@ internalWebhooks. [#80](https://github.com/xmidt-org/ancla/pull/80)
 ## [v0.1.0]
 - Initial release
 
-[Unreleased]: https://github.com/xmidt-org/ancla/compare/v0.3.3...HEAD
+[Unreleased]: https://github.com/xmidt-org/ancla/compare/v0.3.4...HEAD
+[v0.3.4]: https://github.com/xmidt-org/ancla/compare/0.3.3...v0.3.4
 [v0.3.3]: https://github.com/xmidt-org/ancla/compare/0.3.2...v0.3.3
 [v0.3.2]: https://github.com/xmidt-org/ancla/compare/0.3.1...v0.3.2
 [v0.3.1]: https://github.com/xmidt-org/ancla/compare/0.3.0...v0.3.1

go.mod

@@ -5,9 +5,9 @@ go 1.15
 require (
 	github.com/go-kit/kit v0.10.0
 	github.com/golang-jwt/jwt v3.2.1+incompatible
-	github.com/spf13/cast v1.3.1
+	github.com/spf13/cast v1.4.1
 	github.com/stretchr/testify v1.7.0
-	github.com/xmidt-org/argus v0.5.1
+	github.com/xmidt-org/argus v0.6.0
 	github.com/xmidt-org/bascule v0.10.2
 	github.com/xmidt-org/httpaux v0.2.1
 	github.com/xmidt-org/webpa-common/v2 v2.0.1

go.sum

@@ -59,7 +59,7 @@ type Service interface {
 // Config contains information needed to initialize the webhook service.
 type Config struct {
 	// Argus contains configuration to initialize an Argus client.
-	Argus chrysom.ClientConfig
+	Argus ArgusConfig
 
 	// Logger for this package.
 	// Gets passed to Argus config before initializing the client.
@@ -81,12 +81,17 @@ type Config struct {
 	// checking the validity of the partnerIDs in the request
 	DisablePartnerIDs bool
 
-	// WebhookValidationConfig provides options for validating the webhook's URL and TTL
+	// Validation provides options for validating the webhook's URL and TTL
 	// related fields. Some validation happens regardless of the configuration:
 	// URLs must be a valid URL structure, the Matcher.DeviceID values must
 	// compile into regular expressions, and the Events field must have at
 	// least one value and all values must compile into regular expressions.
-	WebhookValidationConfig ValidatorConfig
+	Validation ValidatorConfig
+}
+
+type ArgusConfig struct {
+	chrysom.BasicClientConfig `mapstructure:",squash"`
+	Listen                    chrysom.ListenerClientConfig
 }
 
 type service struct {
@@ -193,21 +198,25 @@ func Initialize(cfg Config, getLogger func(ctx context.Context) log.Logger, setL
 	m := &chrysom.Measures{
 		Polls: cfg.MetricsProvider.NewCounterVec(chrysom.PollCounter),
 	}
-	argus, err := chrysom.NewClient(cfg.Argus, m, getLogger, setLogger)
+	basic, err := chrysom.NewBasicClient(cfg.Argus.BasicClientConfig, getLogger, setLogger)
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to create chrysom basic client: %v", err)
+	}
+	listener, err := chrysom.NewListenerClient(cfg.Argus.Listen, setLogger, m, basic)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to create chrysom listener client: %v", err)
 	}
 
 	svc := &service{
 		logger: cfg.Logger,
-		argus:  argus,
+		argus:  basic,
 		config: cfg,
 		now:    time.Now,
 	}
 
-	argus.Start(context.Background())
+	listener.Start(context.Background())
 
-	return svc, func() { argus.Stop(context.Background()) }, nil
+	return svc, func() { listener.Stop(context.Background()) }, nil
 }
 
 func prepArgusConfig(cfg *Config, watches ...Watch) error {

service.go

@@ -96,7 +96,7 @@ func TestAdd(t *testing.T) {
 		{
 			Description: "Unknown push result",
 			PushItemResults: pushItemResults{
-				result: "unknownResult",
+				result: chrysom.UnknownPushResult,
 			},
 			ExpectedErr: errNonSuccessPushResult,
 		},

service_test.go

@@ -30,13 +30,14 @@ var (
 	errInvalidFailureURL     = errors.New("invalid Failure URL")
 	errInvalidAlternativeURL = errors.New("invalid Alternative URL(s)")
 	errURLIsNotHTTPS         = errors.New("URL scheme is not HTTPS")
-	errInvalidHost           = errors.New("invalid host")
+	errInvalidHost           = errors.New("host is blocked")
 	errIPGivenAsHost         = errors.New("cannot use IP as host")
 	errLoopbackGivenAsHost   = errors.New("cannot use loopback host")
 	errIPinInvalidSubnets    = errors.New("IP is within a blocked subnet")
 	errInvalidSubnet         = errors.New("invalid subnet")
 	errNoSuchHost            = errors.New("host does not exist")
 	errBadURLProtocol        = errors.New("bad URL protocol")
+	errEmptyURL              = errors.New("url cannot be an empty string")
 )
 
 // filterNil takes out all entries of Nil value from the slice.
@@ -55,6 +56,10 @@ func filterNil(vs []ValidURLFunc) (filtered []ValidURLFunc) {
 func GoodConfigURL(vs []ValidURLFunc) ValidatorFunc {
 	vs = filterNil(vs)
 	return func(w Webhook) error {
+		if w.Config.URL == "" {
+			return fmt.Errorf("%w: %v",
+				errInvalidURL, errEmptyURL)
+		}
 		parsedURL, err := url.ParseRequestURI(w.Config.URL)
 		if err != nil {
 			return fmt.Errorf("%w: %v", errInvalidURL, err)
@@ -99,16 +104,19 @@ func GoodAlternativeURLs(vs []ValidURLFunc) ValidatorFunc {
 	return func(w Webhook) error {
 		for _, u := range w.Config.AlternativeURLs {
 			if u == "" {
-				return errInvalidAlternativeURL
+				return fmt.Errorf("%w: %v",
+					errInvalidAlternativeURL, errEmptyURL)
 			}
 			parsedAlternativeURL, err := url.ParseRequestURI(u)
 			if err != nil {
-				return fmt.Errorf("%w: %v", errInvalidAlternativeURL, err)
+				return fmt.Errorf("%w '%s': %v",
+					errInvalidAlternativeURL, u, err)
 			}
 			for _, f := range vs {
 				err = f(parsedAlternativeURL)
 				if err != nil {
-					return fmt.Errorf("%w: %v", errInvalidAlternativeURL, err)
+					return fmt.Errorf("%w '%s': %v",
+						errInvalidAlternativeURL, u, err)
 				}
 			}
 		}
@@ -122,10 +130,10 @@ func GoodAlternativeURLs(vs []ValidURLFunc) ValidatorFunc {
 func GoodURLScheme(httpsOnly bool) ValidURLFunc {
 	return func(u *url.URL) error {
 		if u.Scheme != "https" && u.Scheme != "http" {
-			return errBadURLProtocol
+			return fmt.Errorf("%w: %s", errBadURLProtocol, u.Scheme)
 		}
 		if httpsOnly && u.Scheme != "https" {
-			return errURLIsNotHTTPS
+			return fmt.Errorf("%w: %s", errURLIsNotHTTPS, u.Scheme)
 		}
 		return nil
 	}
@@ -144,7 +152,7 @@ func RejectHosts(invalidHosts []string) ValidURLFunc {
 	return func(u *url.URL) error {
 		for _, v := range ih {
 			if strings.Contains(u.Host, v) {
-				return errInvalidHost
+				return fmt.Errorf("%w: %s", errInvalidHost, u.Host)
 			}
 		}
 		return nil
@@ -158,7 +166,7 @@ func RejectAllIPs() ValidURLFunc {
 		host := u.Hostname()
 		ip := net.ParseIP(host)
 		if ip != nil {
-			return errIPGivenAsHost
+			return fmt.Errorf("%w: %v", errIPGivenAsHost, host)
 		}
 		return nil
 	}
@@ -171,15 +179,16 @@ func RejectLoopback() ValidURLFunc {
 		host := u.Hostname()
 		ip := net.ParseIP(host)
 		if ip != nil && ip.IsLoopback() {
-			return errLoopbackGivenAsHost
+			return fmt.Errorf("%w: %v", errLoopbackGivenAsHost, ip)
 		}
 		ips, err := net.LookupIP(host)
 		if err != nil {
 			return fmt.Errorf("%w: %v", errNoSuchHost, err)
 		}
 		for _, i := range ips {
 			if i.IsLoopback() {
-				return errLoopbackGivenAsHost
+				return fmt.Errorf("%w: %v lookup includes %v",
+					errLoopbackGivenAsHost, host, i)
 			}
 		}
 		return nil
@@ -193,19 +202,20 @@ func InvalidSubnets(i []string) (ValidURLFunc, error) {
 	for _, sp := range i {
 		_, n, err := net.ParseCIDR(sp)
 		if err != nil {
-			return nil, errInvalidSubnet
+			return nil, fmt.Errorf("%w %s: %v", errInvalidSubnet, sp, err)
 		}
 		invalidSubnets = append(invalidSubnets, n)
 	}
 	return func(u *url.URL) error {
 		ips, err := net.LookupIP(u.Hostname())
 		if err != nil {
-			return errInvalidURL
+			return fmt.Errorf("%w: %v", errInvalidURL, err)
 		}
 		for _, d := range ips {
 			for _, s := range invalidSubnets {
 				if s.Contains(d) {
-					return errIPinInvalidSubnets
+					return fmt.Errorf("%w: ip %s in %s",
+						errIPinInvalidSubnets, d, s)
 				}
 			}
 		}

webhookURLValidator.go

@@ -91,6 +91,13 @@ func TestGoodConfigURL(t *testing.T) {
 			expectedErr:   errInvalidURL,
 			validURLFuncs: simpleFuncs,
 		},
+		{
+			desc: "Parse Failure",
+			webhook: Webhook{Config: DeliveryConfig{URL: "\\\\"},
+				FailureURL: ""},
+			expectedErr:   errInvalidURL,
+			validURLFuncs: simpleFuncs,
+		},
 		{
 			desc: "No https url Failure",
 			webhook: Webhook{Config: DeliveryConfig{URL: "http://www.google.com/"},

webhookURLValidator_test.go

@@ -19,6 +19,7 @@ package ancla
 
 import (
 	"errors"
+	"fmt"
 	"net/url"
 	"regexp"
 	"time"
@@ -31,8 +32,8 @@ var (
 	errInvalidDuration     = errors.New("duration value of webhook is out of bounds")
 	errInvalidUntil        = errors.New("until value of webhook is out of bounds")
 	errUntilDurationAbsent = errors.New("until and duration are both absent")
-	errInvalidTTL          = errors.New("invalid TTL")
-	errInvalidJitter       = errors.New("invalid jitter")
+	errInvalidTTL          = errors.New("TTL must be non-negative")
+	errInvalidJitter       = errors.New("jitter must be non-negative")
 )
 
 // Validator is a WebhookValidator that allows access to the Validate function.
@@ -108,7 +109,8 @@ func CheckDuration(maxTTL time.Duration) (ValidatorFunc, error) {
 	}
 	return func(w Webhook) error {
 		if maxTTL < w.Duration || w.Duration < 0 {
-			return errInvalidDuration
+			return fmt.Errorf("%w: %v not between 0 and %v",
+				errInvalidDuration, w.Duration, maxTTL)
 		}
 		return nil
 	}, nil
@@ -131,7 +133,8 @@ func CheckUntil(jitter time.Duration, maxTTL time.Duration, now func() time.Time
 		limit := (now().Add(maxTTL)).Add(jitter)
 		proposed := (w.Until)
 		if proposed.After(limit) {
-			return errInvalidUntil
+			return fmt.Errorf("%w: %v after %v",
+				errInvalidUntil, proposed.String(), limit.String())
 		}
 		return nil
 	}, nil