From 0555925a02ba4524a460db4f16086aa12c7a75c2 Mon Sep 17 00:00:00 2001 From: Joel Unzain Date: Thu, 28 Sep 2017 15:25:23 -0700 Subject: [PATCH 1/2] Fixed mux issues and webhook logic --- src/tr1d1um/tr1d1um.go | 40 ++++++++++++------------------------- src/tr1d1um/tr1d1um_test.go | 2 +- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/src/tr1d1um/tr1d1um.go b/src/tr1d1um/tr1d1um.go index de30262a..44b6bec9 100644 --- a/src/tr1d1um/tr1d1um.go +++ b/src/tr1d1um/tr1d1um.go @@ -1,10 +1,10 @@ package main import ( + "bytes" "fmt" "io/ioutil" "net/http" - "net/url" "os" "time" @@ -51,7 +51,7 @@ func tr1d1um(arguments []string) (exitCode int) { infoLogger.Log("configurationFile", v.ConfigFileUsed()) tConfig := new(Tr1d1umConfig) - err = v.Unmarshal(tConfig) //todo: decide best way to get current unexported fields from viper + err = v.Unmarshal(tConfig) if err != nil { fmt.Fprintf(os.Stderr, "Unable to unmarshall config data into struct: %s\n", err.Error()) @@ -71,12 +71,12 @@ func tr1d1um(arguments []string) (exitCode int) { AddRoutes(r, preHandler, conversionHandler) - if exitCode = ConfigureWebHooks(r, preHandler, v, logger); exitCode != 0 { + if exitCode = ConfigureWebHooks(r, preHandler, v); exitCode != 0 { return } var ( - _, tr1d1umServer = webPA.Prepare(logger, nil, conversionHandler) + _, tr1d1umServer = webPA.Prepare(logger, nil, r) signals = make(chan os.Signal, 1) ) @@ -89,7 +89,7 @@ func tr1d1um(arguments []string) (exitCode int) { } //ConfigureWebHooks sets route paths, initializes and synchronizes hook registries for this tr1d1um instance -func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper, logger log.Logger) int { +func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper) int { webHookFactory, err := webhook.NewFactory(v) if err != nil { @@ -97,38 +97,26 @@ func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper, l return 1 } - webHookRegistry, webHookHandler := webHookFactory.NewRegistryAndHandler() + webHookRegistry, _ := webHookFactory.NewRegistryAndHandler() // register webHook end points for api r.Handle("/hook", preHandler.ThenFunc(webHookRegistry.UpdateRegistry)) r.Handle("/hooks", preHandler.ThenFunc(webHookRegistry.GetRegistry)) - selfURL := &url.URL{ - Scheme: "https", - Host: v.GetString("fqdn") + v.GetString("primary.address"), - } - - webHookFactory.Initialize(r, selfURL, webHookHandler, logger, nil) - webHookFactory.PrepareAndStart() - - startChan := make(chan webhook.Result, 1) - webHookFactory.Start.GetCurrentSystemsHooks(startChan) - - if webHookStartResults := <-startChan; webHookStartResults.Error == nil { - webHookFactory.SetList(webhook.NewList(webHookStartResults.Hooks)) - } else { - logging.Error(logger).Log(logging.ErrorKey(), webHookStartResults.Error) - } - return 0 } //AddRoutes configures the paths and connection rules to TR1D1UM -func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *ConversionHandler) *mux.Router { +func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *ConversionHandler) { var BodyNonEmpty = func(request *http.Request, match *mux.RouteMatch) (accept bool) { if request.Body != nil { + var tmp bytes.Buffer p, err := ioutil.ReadAll(request.Body) - accept = err == nil && len(p) > 0 + if accept = err == nil && len(p) > 0; accept { + //place back request's body + tmp.Write(p) + request.Body = ioutil.NopCloser(&tmp) + } } return } @@ -146,8 +134,6 @@ func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *Conver apiHandler.Handle("/device/{deviceid}/{service}/{parameter}", preHandler.Then(conversionHandler)). Methods(http.MethodPut, http.MethodPost).MatcherFunc(BodyNonEmpty) - - return r } //SetUpHandler prepares the main handler under TR1D1UM which is the ConversionHandler diff --git a/src/tr1d1um/tr1d1um_test.go b/src/tr1d1um/tr1d1um_test.go index 93e8d4ad..d6fa917a 100644 --- a/src/tr1d1um/tr1d1um_test.go +++ b/src/tr1d1um/tr1d1um_test.go @@ -40,7 +40,7 @@ func TestRouteConfigurations(t *testing.T) { fakePreHandler := new(alice.Chain) fakeHandler := &ConversionHandler{} - r = AddRoutes(r, fakePreHandler, fakeHandler) + AddRoutes(r, fakePreHandler, fakeHandler) var nonEmpty bytes.Buffer nonEmpty.WriteString(`{empty: false}`) From 404adb9436047e2cd86fabdfdef364972e02d942 Mon Sep 17 00:00:00 2001 From: Joel Unzain Date: Thu, 28 Sep 2017 17:15:11 -0700 Subject: [PATCH 2/2] Use viper more, add back needed webhook logic, run goimports --- src/tr1d1um/communication.go | 2 +- src/tr1d1um/http.go | 16 +++++++------ src/tr1d1um/http_test.go | 2 +- src/tr1d1um/tr1d1um.go | 44 +++++++++++++++++++----------------- src/tr1d1um/tr1d1um_test.go | 26 ++++++++++++--------- src/tr1d1um/tr1d1um_type.go | 31 ++----------------------- 6 files changed, 51 insertions(+), 70 deletions(-) diff --git a/src/tr1d1um/communication.go b/src/tr1d1um/communication.go index aad04a64..cdc592e1 100644 --- a/src/tr1d1um/communication.go +++ b/src/tr1d1um/communication.go @@ -38,7 +38,7 @@ func (tr1 *Tr1SendAndHandle) Send(ch *ConversionHandler, resp http.ResponseWrite return } - fullPath := ch.targetURL + baseURI + "/" + version + "/" + wrpMsg.Destination + fullPath := ch.targetURL + baseURI + "/" + ch.serverVersion + "/" + wrpMsg.Destination requestToServer, err := tr1.NewHTTPRequest(http.MethodPost, fullPath, bytes.NewBuffer(wrpPayload)) if err != nil { diff --git a/src/tr1d1um/http.go b/src/tr1d1um/http.go index 6ba1901c..5b3e786b 100644 --- a/src/tr1d1um/http.go +++ b/src/tr1d1um/http.go @@ -10,9 +10,9 @@ import ( //ConversionHandler wraps the main WDMP -> WRP conversion method type ConversionHandler struct { - infoLogger log.Logger - errorLogger log.Logger + logger log.Logger targetURL string + serverVersion string wdmpConvert ConversionTool sender SendAndHandle encodingHelper EncodingTool @@ -20,9 +20,11 @@ type ConversionHandler struct { //ConversionHandler handles the different incoming tr1 requests func (ch *ConversionHandler) ServeHTTP(origin http.ResponseWriter, req *http.Request) { - var err error - var wdmp interface{} - var urlVars = mux.Vars(req) + var ( + err error + wdmp interface{} + urlVars = mux.Vars(req) + ) switch req.Method { case http.MethodGet: @@ -49,7 +51,7 @@ func (ch *ConversionHandler) ServeHTTP(origin http.ResponseWriter, req *http.Req if err != nil { origin.WriteHeader(http.StatusInternalServerError) - ch.errorLogger.Log(logging.MessageKey(), ErrUnsuccessfulDataParse, logging.ErrorKey(), err.Error()) + logging.Error(ch.logger).Log(logging.MessageKey(), ErrUnsuccessfulDataParse, logging.ErrorKey(), err.Error()) return } @@ -57,7 +59,7 @@ func (ch *ConversionHandler) ServeHTTP(origin http.ResponseWriter, req *http.Req if err != nil { origin.WriteHeader(http.StatusInternalServerError) - ch.errorLogger.Log(logging.ErrorKey(), err.Error()) + logging.Error(ch.logger).Log(logging.ErrorKey(), err.Error()) return } diff --git a/src/tr1d1um/http_test.go b/src/tr1d1um/http_test.go index d7049802..38932e85 100644 --- a/src/tr1d1um/http_test.go +++ b/src/tr1d1um/http_test.go @@ -22,7 +22,7 @@ var ( wdmpConvert: mockConversion, sender: mockSender, encodingHelper: mockEncoding, - errorLogger: fakeLogger, + logger: fakeLogger, } ) diff --git a/src/tr1d1um/tr1d1um.go b/src/tr1d1um/tr1d1um.go index 44b6bec9..767dc425 100644 --- a/src/tr1d1um/tr1d1um.go +++ b/src/tr1d1um/tr1d1um.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/url" "os" "time" @@ -28,7 +29,6 @@ const ( applicationName = "tr1d1um" DefaultKeyID = "current" baseURI = "/api" - version = "v2" // TODO: Should these values change? ) func tr1d1um(arguments []string) (exitCode int) { @@ -50,9 +50,6 @@ func tr1d1um(arguments []string) (exitCode int) { infoLogger.Log("configurationFile", v.ConfigFileUsed()) - tConfig := new(Tr1d1umConfig) - err = v.Unmarshal(tConfig) - if err != nil { fmt.Fprintf(os.Stderr, "Unable to unmarshall config data into struct: %s\n", err.Error()) return 1 @@ -65,13 +62,13 @@ func tr1d1um(arguments []string) (exitCode int) { return 1 } - conversionHandler := SetUpHandler(tConfig, logger) + conversionHandler := SetUpHandler(v, logger) r := mux.NewRouter() - AddRoutes(r, preHandler, conversionHandler) + AddRoutes(r, preHandler, conversionHandler, v) - if exitCode = ConfigureWebHooks(r, preHandler, v); exitCode != 0 { + if exitCode = ConfigureWebHooks(r, preHandler, v, logger); exitCode != 0 { return } @@ -89,7 +86,7 @@ func tr1d1um(arguments []string) (exitCode int) { } //ConfigureWebHooks sets route paths, initializes and synchronizes hook registries for this tr1d1um instance -func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper) int { +func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper, logger log.Logger) int { webHookFactory, err := webhook.NewFactory(v) if err != nil { @@ -97,17 +94,25 @@ func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper) i return 1 } - webHookRegistry, _ := webHookFactory.NewRegistryAndHandler() + webHookRegistry, webHookHandler := webHookFactory.NewRegistryAndHandler() // register webHook end points for api r.Handle("/hook", preHandler.ThenFunc(webHookRegistry.UpdateRegistry)) r.Handle("/hooks", preHandler.ThenFunc(webHookRegistry.GetRegistry)) + selfURL := &url.URL{ + Scheme: "https", + Host: v.GetString("fqdn") + v.GetString("primary.address"), + } + + webHookFactory.Initialize(r, selfURL, webHookHandler, logger, nil) + webHookFactory.PrepareAndStart() + return 0 } //AddRoutes configures the paths and connection rules to TR1D1UM -func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *ConversionHandler) { +func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *ConversionHandler, v *viper.Viper) { var BodyNonEmpty = func(request *http.Request, match *mux.RouteMatch) (accept bool) { if request.Body != nil { var tmp bytes.Buffer @@ -121,7 +126,7 @@ func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *Conver return } - apiHandler := r.PathPrefix(fmt.Sprintf("%s/%s", baseURI, version)).Subrouter() + apiHandler := r.PathPrefix(fmt.Sprintf("%s/%s", baseURI, v.GetString("version"))).Subrouter() apiHandler.Handle("/device/{deviceid}/{service}", preHandler.Then(conversionHandler)). Methods(http.MethodGet) @@ -137,22 +142,20 @@ func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *Conver } //SetUpHandler prepares the main handler under TR1D1UM which is the ConversionHandler -func SetUpHandler(tConfig *Tr1d1umConfig, logger log.Logger) (cHandler *ConversionHandler) { - timeOut, err := time.ParseDuration(tConfig.HTTPTimeout) +func SetUpHandler(v *viper.Viper, logger log.Logger) (cHandler *ConversionHandler) { + timeOut, err := time.ParseDuration(v.GetString("requestTimeout")) if err != nil { timeOut = time.Second * 60 //default val } cHandler = &ConversionHandler{ - wdmpConvert: &ConversionWDMP{&EncodingHelper{}}, - sender: &Tr1SendAndHandle{log: logger, timedClient: &http.Client{Timeout: timeOut}, - NewHTTPRequest: http.NewRequest}, + wdmpConvert: &ConversionWDMP{&EncodingHelper{}}, + sender: &Tr1SendAndHandle{log: logger, timedClient: &http.Client{Timeout: timeOut}, NewHTTPRequest: http.NewRequest}, encodingHelper: &EncodingHelper{}, + logger: logger, + targetURL: v.GetString("targetURL"), + serverVersion: v.GetString("version"), } - //pass loggers - cHandler.errorLogger = logging.Error(logger) - cHandler.infoLogger = logging.Info(logger) - cHandler.targetURL = tConfig.TargetURL return } @@ -210,7 +213,6 @@ func GetValidator(v *viper.Viper) (validator secure.Validator, err error) { ) } - // TODO: This should really be part of the unmarshalled validators somehow basicAuth := v.GetStringSlice("authHeader") for _, authValue := range basicAuth { validators = append( diff --git a/src/tr1d1um/tr1d1um_test.go b/src/tr1d1um/tr1d1um_test.go index d6fa917a..05744db6 100644 --- a/src/tr1d1um/tr1d1um_test.go +++ b/src/tr1d1um/tr1d1um_test.go @@ -11,27 +11,30 @@ import ( "github.com/Comcast/webpa-common/logging" "github.com/gorilla/mux" "github.com/justinas/alice" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" ) func TestSetUpHandler(t *testing.T) { assert := assert.New(t) - tConfig := &Tr1d1umConfig{HTTPTimeout: "10s", TargetURL: "https://someCoolURL.com"} + v := viper.New() + v.Set("requestTimeout", "60s") + v.Set("targetURL", "https://someCoolURL.com") logger := logging.DefaultLogger() t.Run("NormalSetUp", func(t *testing.T) { - actualHandler := SetUpHandler(tConfig, logger) - AssertCommon(actualHandler, tConfig, assert) + actualHandler := SetUpHandler(v, logger) + AssertCommon(actualHandler, v, assert) }) t.Run("IncompleteConfig", func(t *testing.T) { - tConfig.HTTPTimeout = "" //make config struct incomplete + v.Set("requestTimeOut", "") //make config incomplete - actualHandler := SetUpHandler(tConfig, logger) + actualHandler := SetUpHandler(v, logger) realSender := actualHandler.sender.(*Tr1SendAndHandle) assert.EqualValues(time.Second*60, realSender.timedClient.Timeout) //turn to default value - AssertCommon(actualHandler, tConfig, assert) + AssertCommon(actualHandler, v, assert) }) } @@ -39,8 +42,10 @@ func TestRouteConfigurations(t *testing.T) { r := mux.NewRouter() fakePreHandler := new(alice.Chain) fakeHandler := &ConversionHandler{} + v := viper.New() + v.Set("version", "v2") - AddRoutes(r, fakePreHandler, fakeHandler) + AddRoutes(r, fakePreHandler, fakeHandler, v) var nonEmpty bytes.Buffer nonEmpty.WriteString(`{empty: false}`) @@ -94,12 +99,11 @@ func AssertConfiguredRoutes(r *mux.Router, t *testing.T, testCases []RouteTestBu } } -func AssertCommon(actualHandler *ConversionHandler, tConfig *Tr1d1umConfig, assert *assert.Assertions) { +func AssertCommon(actualHandler *ConversionHandler, v *viper.Viper, assert *assert.Assertions) { assert.NotNil(actualHandler.wdmpConvert) assert.NotNil(actualHandler.encodingHelper) - assert.NotNil(actualHandler.errorLogger) - assert.NotNil(actualHandler.infoLogger) - assert.EqualValues(tConfig.TargetURL, actualHandler.targetURL) + assert.NotNil(actualHandler.logger) + assert.EqualValues(v.Get("targetURL"), actualHandler.targetURL) assert.NotNil(actualHandler.sender) realizedSender := actualHandler.sender.(*Tr1SendAndHandle) diff --git a/src/tr1d1um/tr1d1um_type.go b/src/tr1d1um/tr1d1um_type.go index 561cbdfd..f74be4cc 100644 --- a/src/tr1d1um/tr1d1um_type.go +++ b/src/tr1d1um/tr1d1um_type.go @@ -5,39 +5,12 @@ import ( "github.com/Comcast/webpa-common/secure/key" ) -//Tr1d1umConfig wraps all the config -type Tr1d1umConfig struct { - Addr string `json:"addr"` - PprofAddr string `json:"pprofaddr"` - Cert string `json:"cert"` - Key string `json:"key"` - AuthKey []string `json:"authKey"` - HandlerTimeout string `json:"handlerTimeout"` - HTTPTimeout string `json:"httpTimeout"` - HealthInterval string `json:"healthInterval"` - Version string `json:"version"` - MaxTCPConns int64 `json:"maxApiTcpConnections"` - MaxHealthTCPConns int64 `json:"maxHealthTcpConnections"` - ServiceList []string `json:"serviceList"` - WrpSource string `json:"wrpSource"` - TargetURL string `json:"targetURL"` - - JWTValidators []struct { - // JWTKeys is used to create the key.Resolver for JWT verification keys - Keys key.ResolverFactory `json:"keys"` - - // Custom is an optional configuration section that defines - // custom rules for validation over and above the standard RFC rules. - Custom secure.JWTValidatorFactory `json:"custom"` - } `json:"jwtValidators"` -} - //JWTValidator provides a convenient way to define jwt validator through config files type JWTValidator struct { // JWTKeys is used to create the key.Resolver for JWT verification keys - Keys key.ResolverFactory + Keys key.ResolverFactory `json:"keys"` // Custom is an optional configuration section that defines // custom rules for validation over and above the standard RFC rules. - Custom secure.JWTValidatorFactory + Custom secure.JWTValidatorFactory `json:"custom"` }