diff --git a/src/tr1d1um/conversion_utils.go b/src/tr1d1um/conversion_utils.go index 6e23e338..c3831451 100644 --- a/src/tr1d1um/conversion_utils.go +++ b/src/tr1d1um/conversion_utils.go @@ -52,7 +52,7 @@ type ConversionWDMP struct { func (cw *ConversionWDMP) GetFlavorFormat(req *http.Request, pathVars Vars, attr, namesKey, sep string) (wdmp *GetWDMP, err error) { wdmp = new(GetWDMP) - if service, _ := cw.GetFromURLPath("service", pathVars); service == "stat"{ + if service, _ := cw.GetFromURLPath("service", pathVars); service == "stat" { return //todo: maybe we need more validation here } diff --git a/src/tr1d1um/conversion_utils_test.go b/src/tr1d1um/conversion_utils_test.go index 47b85e87..49a257a7 100644 --- a/src/tr1d1um/conversion_utils_test.go +++ b/src/tr1d1um/conversion_utils_test.go @@ -37,7 +37,7 @@ func TestGetFlavorFormat(t *testing.T) { t.Run("IdealGet", func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "http://api/device/config?names=p1,p2", nil) - wdmp, err := c.GetFlavorFormat(req, nil,"attributes", "names", ",") + wdmp, err := c.GetFlavorFormat(req, nil, "attributes", "names", ",") assert.Nil(err) assert.EqualValues(wdmpGet, wdmp) @@ -46,7 +46,7 @@ func TestGetFlavorFormat(t *testing.T) { t.Run("IdealGetStat", func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "http://api/device/mac:112233445566/stat", nil) - wdmp, err := c.GetFlavorFormat(req, map[string]string{"service":"stat"},"attributes", "names", ",") + wdmp, err := c.GetFlavorFormat(req, map[string]string{"service": "stat"}, "attributes", "names", ",") assert.Nil(err) assert.EqualValues(new(GetWDMP), wdmp) @@ -57,7 +57,7 @@ func TestGetFlavorFormat(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "http://api/device/config?names=p1,p2&attributes=attr1", nil) - wdmp, err := c.GetFlavorFormat(req, nil,"attributes", "names", ",") + wdmp, err := c.GetFlavorFormat(req, nil, "attributes", "names", ",") assert.Nil(err) assert.EqualValues(wdmpGetAttrs, wdmp) @@ -67,7 +67,7 @@ func TestGetFlavorFormat(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "http://api/device/config?names=", nil) - _, err := c.GetFlavorFormat(req, nil,"attributes", "names", ",") + _, err := c.GetFlavorFormat(req, nil, "attributes", "names", ",") assert.NotNil(err) assert.True(strings.HasPrefix(err.Error(), "names is a required")) diff --git a/src/tr1d1um/http.go b/src/tr1d1um/http.go index 3a46b87f..6ba1901c 100644 --- a/src/tr1d1um/http.go +++ b/src/tr1d1um/http.go @@ -64,4 +64,3 @@ func (ch *ConversionHandler) ServeHTTP(origin http.ResponseWriter, req *http.Req response, err := ch.sender.Send(ch, origin, wdmpPayload, req) ch.sender.HandleResponse(ch, err, response, origin) } - diff --git a/src/tr1d1um/http_test.go b/src/tr1d1um/http_test.go index 992bcf68..d7049802 100644 --- a/src/tr1d1um/http_test.go +++ b/src/tr1d1um/http_test.go @@ -17,7 +17,7 @@ var ( payload, body = []byte("SomePayload"), bytes.NewBufferString("body") resp = &http.Response{} mockConversion, mockEncoding, mockSender = &MockConversionTool{}, &MockEncodingTool{}, &MockSendAndHandle{} - fakeLogger = &LightFakeLogger{} + fakeLogger = &LightFakeLogger{} ch = &ConversionHandler{ wdmpConvert: mockConversion, sender: mockSender, @@ -57,7 +57,7 @@ func TestConversionHandler(t *testing.T) { t.Run("IdealGet", func(t *testing.T) { mockConversion.On("GetFlavorFormat", commonRequest, vars, "attributes", "names", ","). - Return(wdmpGet, nil).Once() + Return(wdmpGet, nil).Once() SetUpTest(wdmpGet, commonRequest) AssertCommonCalls(t) @@ -121,7 +121,7 @@ func AssertCommonCalls(t *testing.T) { mockSender.AssertExpectations(t) } -type LightFakeLogger struct {} +type LightFakeLogger struct{} func (fake *LightFakeLogger) Log(_ ...interface{}) error { return nil diff --git a/src/tr1d1um/mocks_test.go b/src/tr1d1um/mocks_test.go index bc78378e..ddefa079 100644 --- a/src/tr1d1um/mocks_test.go +++ b/src/tr1d1um/mocks_test.go @@ -14,7 +14,7 @@ type MockConversionTool struct { mock.Mock } -func (m *MockConversionTool) GetFlavorFormat(req *http.Request, vars Vars,i string, i2 string, i3 string) (*GetWDMP, error) { +func (m *MockConversionTool) GetFlavorFormat(req *http.Request, vars Vars, i string, i2 string, i3 string) (*GetWDMP, error) { args := m.Called(req, vars, i, i2, i3) return args.Get(0).(*GetWDMP), args.Error(1) } diff --git a/src/tr1d1um/tr1d1um.go b/src/tr1d1um/tr1d1um.go index 7c78edb0..de30262a 100644 --- a/src/tr1d1um/tr1d1um.go +++ b/src/tr1d1um/tr1d1um.go @@ -2,24 +2,25 @@ package main import ( "fmt" + "io/ioutil" "net/http" + "net/url" "os" "time" - "net/url" + "github.com/Comcast/webpa-common/concurrent" "github.com/Comcast/webpa-common/logging" "github.com/Comcast/webpa-common/secure" "github.com/Comcast/webpa-common/secure/handler" "github.com/Comcast/webpa-common/secure/key" "github.com/Comcast/webpa-common/server" + "github.com/Comcast/webpa-common/webhook" "github.com/SermoDigital/jose/jwt" "github.com/go-kit/kit/log" "github.com/gorilla/mux" "github.com/justinas/alice" "github.com/spf13/pflag" "github.com/spf13/viper" - "github.com/Comcast/webpa-common/concurrent" - "github.com/Comcast/webpa-common/webhook" ) //convenient global values @@ -42,7 +43,7 @@ func tr1d1um(arguments []string) (exitCode int) { fmt.Fprintf(os.Stderr, "Unable to initialize viper: %s\n", err.Error()) return 1 } - + var ( infoLogger = logging.Info(logger) ) @@ -65,18 +66,18 @@ func tr1d1um(arguments []string) (exitCode int) { } conversionHandler := SetUpHandler(tConfig, logger) - + r := mux.NewRouter() AddRoutes(r, preHandler, conversionHandler) - if exitCode = ConfigureWebHooks(r,preHandler,v,logger); exitCode != 0 { + if exitCode = ConfigureWebHooks(r, preHandler, v, logger); exitCode != 0 { return } var ( _, tr1d1umServer = webPA.Prepare(logger, nil, conversionHandler) - signals = make(chan os.Signal, 1) + signals = make(chan os.Signal, 1) ) if err := concurrent.Await(tr1d1umServer, signals); err != nil { @@ -116,17 +117,20 @@ func ConfigureWebHooks(r *mux.Router, preHandler *alice.Chain, v *viper.Viper, l if webHookStartResults := <-startChan; webHookStartResults.Error == nil { webHookFactory.SetList(webhook.NewList(webHookStartResults.Hooks)) } else { - logging.Error(logger).Log(logging.ErrorKey(),webHookStartResults.Error) + 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 { - var BodyNonNil = func(request *http.Request, match *mux.RouteMatch) bool { - return request.Body != nil + var BodyNonEmpty = func(request *http.Request, match *mux.RouteMatch) (accept bool) { + if request.Body != nil { + p, err := ioutil.ReadAll(request.Body) + accept = err == nil && len(p) > 0 + } + return } apiHandler := r.PathPrefix(fmt.Sprintf("%s/%s", baseURI, version)).Subrouter() @@ -135,14 +139,14 @@ func AddRoutes(r *mux.Router, preHandler *alice.Chain, conversionHandler *Conver Methods(http.MethodGet) apiHandler.Handle("/device/{deviceid}/{service}", preHandler.Then(conversionHandler)). - Methods(http.MethodPatch).MatcherFunc(BodyNonNil) + Methods(http.MethodPatch).MatcherFunc(BodyNonEmpty) apiHandler.Handle("/device/{deviceid}/{service}/{parameter}", preHandler.Then(conversionHandler)). Methods(http.MethodDelete) apiHandler.Handle("/device/{deviceid}/{service}/{parameter}", preHandler.Then(conversionHandler)). - Methods(http.MethodPut, http.MethodPost).MatcherFunc(BodyNonNil) - + Methods(http.MethodPut, http.MethodPost).MatcherFunc(BodyNonEmpty) + return r } @@ -154,15 +158,15 @@ func SetUpHandler(tConfig *Tr1d1umConfig, logger log.Logger) (cHandler *Conversi } 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{}, } //pass loggers cHandler.errorLogger = logging.Error(logger) cHandler.infoLogger = logging.Info(logger) - cHandler.targetURL = "https://api-cd.xmidt.comcast.net:8090" + cHandler.targetURL = tConfig.TargetURL return } diff --git a/src/tr1d1um/tr1d1um_test.go b/src/tr1d1um/tr1d1um_test.go new file mode 100644 index 00000000..93e8d4ad --- /dev/null +++ b/src/tr1d1um/tr1d1um_test.go @@ -0,0 +1,115 @@ +package main + +import ( + "bytes" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/Comcast/webpa-common/logging" + "github.com/gorilla/mux" + "github.com/justinas/alice" + "github.com/stretchr/testify/assert" +) + +func TestSetUpHandler(t *testing.T) { + assert := assert.New(t) + tConfig := &Tr1d1umConfig{HTTPTimeout: "10s", TargetURL: "https://someCoolURL.com"} + logger := logging.DefaultLogger() + + t.Run("NormalSetUp", func(t *testing.T) { + actualHandler := SetUpHandler(tConfig, logger) + AssertCommon(actualHandler, tConfig, assert) + }) + + t.Run("IncompleteConfig", func(t *testing.T) { + tConfig.HTTPTimeout = "" //make config struct incomplete + + actualHandler := SetUpHandler(tConfig, logger) + + realSender := actualHandler.sender.(*Tr1SendAndHandle) + assert.EqualValues(time.Second*60, realSender.timedClient.Timeout) //turn to default value + AssertCommon(actualHandler, tConfig, assert) + }) +} + +func TestRouteConfigurations(t *testing.T) { + r := mux.NewRouter() + fakePreHandler := new(alice.Chain) + fakeHandler := &ConversionHandler{} + + r = AddRoutes(r, fakePreHandler, fakeHandler) + + var nonEmpty bytes.Buffer + nonEmpty.WriteString(`{empty: false}`) + + requests := []*http.Request{ + //0: case no base uri + httptest.NewRequest(http.MethodGet, "http://someurl.com/", nil), + + //1: case method get but no service + httptest.NewRequest(http.MethodGet, "http://server.com/api/v2/device/mac:11223344/", nil), + + //2: case method get normal + httptest.NewRequest(http.MethodGet, "http://server.com/api/v2/device/mac:11223344/serv1", nil), + + //3: case method patch body is nil + httptest.NewRequest(http.MethodPatch, "http://server.com/api/v2/device/mac:11223344/serv1", nil), + + //4: case method (put | post) body is nil + httptest.NewRequest(http.MethodPost, "http://server.com/api/v2/device/mac:11223344/serv1/param1", nil), + + //5: No parameter. Applicable to methods delete, put and post + httptest.NewRequest(http.MethodPost, "http://server.com/api/v2/device/mac:11223344/serv1", &nonEmpty), + + //6: Normal Case. Applicable to methods delete, put and post + httptest.NewRequest(http.MethodPost, "http://server.com/api/v2/device/mac:11223344/serv1/param", + &nonEmpty), + } + + expectedResults := map[int]bool{ //a map for reading ease with respect to ^ + 0: false, 1: false, 2: true, 3: false, 4: false, 5: false, 6: true, + } + + testsCases := make([]RouteTestBundle, len(requests)) + + for i, request := range requests { + testsCases[i] = RouteTestBundle{request, expectedResults[i]} + } + AssertConfiguredRoutes(r, t, testsCases) +} + +//AssertConfiguredRoutes checks that all given tests cases pass with regards to requests that should be +//allowed to hit our handler +func AssertConfiguredRoutes(r *mux.Router, t *testing.T, testCases []RouteTestBundle) { + for _, bundle := range testCases { + var match mux.RouteMatch + if bundle.passing != r.Match(bundle.req, &match) { + fmt.Printf("Expecting request with URL='%s' and method='%s' to have a route?: %v but got %v", + bundle.req.URL, bundle.req.Method, bundle.passing, !bundle.passing) + t.FailNow() + } + } +} + +func AssertCommon(actualHandler *ConversionHandler, tConfig *Tr1d1umConfig, 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.sender) + + realizedSender := actualHandler.sender.(*Tr1SendAndHandle) + + //assert necessary inner methods are set in the method under test + assert.NotNil(realizedSender.timedClient) + assert.NotNil(realizedSender.NewHTTPRequest) +} + +type RouteTestBundle struct { + req *http.Request + passing bool +} diff --git a/src/tr1d1um/tr1d1um_type.go b/src/tr1d1um/tr1d1um_type.go index 249bcf00..561cbdfd 100644 --- a/src/tr1d1um/tr1d1um_type.go +++ b/src/tr1d1um/tr1d1um_type.go @@ -20,7 +20,7 @@ type Tr1d1umConfig struct { MaxHealthTCPConns int64 `json:"maxHealthTcpConnections"` ServiceList []string `json:"serviceList"` WrpSource string `json:"wrpSource"` - targetURL string `json:"targetUrl"` + TargetURL string `json:"targetURL"` JWTValidators []struct { // JWTKeys is used to create the key.Resolver for JWT verification keys