Merge pull request #16 from Comcast/webhook-config-change

Modified how we load acquirer configs

Author: kristinapathak

svalinn.yaml

@@ -46,10 +46,9 @@ webhook:
   timeout: "1m"
   registrationURL: "https://127.0.0.1:6000/hook"
   eventsToWatch: ["device-status.*"]
-  acquirerConfig:
-    basic: 
-    sat:
-      client: ""
-      secret: ""
-      satURL: ""
-      timeout: "1m"
+  basic: 
+  sat:
+    client: ""
+    secret: ""
+    satURL: ""
+    timeout: "1m"

webhook.go

@@ -18,7 +18,6 @@
 package main
 
 import (
-	"errors"
 	"net/http"
 	"time"
 
@@ -29,12 +28,6 @@ import (
 	"github.com/go-kit/kit/log"
 )
 
-var (
-	errParseSat   = errors.New("Couldn't parse SAT acquirer config")
-	errParseBasic = errors.New("Couldn't parse basic acquirer config")
-	errEmptyBasic = errors.New("Empty basic credentials")
-)
-
 type webhookRegisterer interface {
 	Register(client *http.Client, secret string) error
 }
@@ -50,7 +43,8 @@ type WebhookConfig struct {
 	Timeout              time.Duration
 	RegistrationURL      string
 	EventsToWatch        []string
-	AcquirerConfig       map[string]interface{}
+	Sat                  webhook.SatAcquirer
+	Basic                string
 	Secret               string
 }
 
@@ -63,11 +57,9 @@ type periodicRegisterer struct {
 }
 
 func newPeriodicRegisterer(wc WebhookConfig, secretGetter secretGetter, logger log.Logger) periodicRegisterer {
-	acquirer, err := determineTokenAcquirer(wc.AcquirerConfig)
-	if err != nil {
-		logging.Error(logger, emperror.Context(err)...).Log(logging.MessageKey(), "Failed to parse token acquirer config, using default acquirer",
-			logging.ErrorKey(), err.Error(), "config", wc.AcquirerConfig)
-	}
+	acquirer := determineTokenAcquirer(wc)
+	logging.Debug(logger).Log(logging.MessageKey(), "determined token acquirer", "acquirer", acquirer,
+		"config", wc)
 	webhook := webhook.Webhook{
 		URL:             wc.URL,
 		RegistrationURL: wc.RegistrationURL,
@@ -84,26 +76,17 @@ func newPeriodicRegisterer(wc WebhookConfig, secretGetter secretGetter, logger l
 }
 
 // determineTokenAcquirer always returns a valid TokenAcquirer, but may also return an error
-func determineTokenAcquirer(config map[string]interface{}) (webhook.TokenAcquirer, error) {
+func determineTokenAcquirer(config WebhookConfig) webhook.TokenAcquirer {
 	defaultAcquirer := &webhook.DefaultAcquirer{}
-	if value, ok := config["sat"]; ok {
-		acquirer, ok := value.(webhook.SatAcquirer)
-		if !ok {
-			return defaultAcquirer, errParseSat
-		}
-		return &acquirer, nil
+	if config.Sat.Client != "" && config.Sat.SatURL != "" && config.Sat.Secret != "" && config.Sat.Timeout != time.Duration(0)*time.Second {
+		return &config.Sat
 	}
-	if value, ok := config["basic"]; ok {
-		str, ok := value.(string)
-		if !ok {
-			return defaultAcquirer, errParseBasic
-		}
-		if str == "" {
-			return defaultAcquirer, errEmptyBasic
-		}
-		return webhook.NewBasicAcquirer(str), nil
+
+	if config.Basic != "" {
+		return webhook.NewBasicAcquirer(config.Basic)
 	}
-	return defaultAcquirer, nil
+
+	return defaultAcquirer
 }
 
 func (p *periodicRegisterer) registerAtInterval() {

webhook_test.go

@@ -90,39 +90,20 @@ func TestDetermineTokenAcquirer(t *testing.T) {
 	}
 	tests := []struct {
 		description           string
-		satVal                interface{}
-		basicVal              interface{}
+		satVal                webhook.SatAcquirer
+		basicVal              string
 		expectedTokenAcquirer webhook.TokenAcquirer
-		expectedErr           error
 	}{
 		{
 			description:           "Sat Success",
 			satVal:                goodSatAcquirer,
 			expectedTokenAcquirer: &goodSatAcquirer,
 		},
-		{
-			description:           "Sat Error",
-			satVal:                "",
-			expectedTokenAcquirer: defaultAcquirer,
-			expectedErr:           errParseSat,
-		},
 		{
 			description:           "Basic Success",
 			basicVal:              "test basic",
 			expectedTokenAcquirer: goodBasicAcquirer,
 		},
-		{
-			description:           "Basic Parse Error",
-			basicVal:              goodSatAcquirer,
-			expectedTokenAcquirer: defaultAcquirer,
-			expectedErr:           errParseBasic,
-		},
-		{
-			description:           "Basic Empty Creds Error",
-			basicVal:              "",
-			expectedTokenAcquirer: defaultAcquirer,
-			expectedErr:           errEmptyBasic,
-		},
 		{
 			description:           "Default Success",
 			expectedTokenAcquirer: defaultAcquirer,
@@ -132,19 +113,11 @@ func TestDetermineTokenAcquirer(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.description, func(t *testing.T) {
 			assert := assert.New(t)
-			config := make(map[string]interface{})
-			if tc.satVal != nil {
-				config["sat"] = tc.satVal
-			}
-			if tc.basicVal != nil {
-				config["basic"] = tc.basicVal
-			}
-			tokenAcquirer, err := determineTokenAcquirer(config)
-			if tc.expectedErr == nil || err == nil {
-				assert.Equal(tc.expectedErr, err)
-			} else {
-				assert.Contains(err.Error(), tc.expectedErr.Error())
+			config := WebhookConfig{
+				Sat:   tc.satVal,
+				Basic: tc.basicVal,
 			}
+			tokenAcquirer := determineTokenAcquirer(config)
 			assert.Equal(tc.expectedTokenAcquirer, tokenAcquirer)
 		})
 	}