From: DenisGNoonan Date: Tue, 20 Feb 2024 16:29:24 +0000 (+0000) Subject: NONRTRIC-946: Fix panics in Capif X-Git-Tag: 1.3.1~16 X-Git-Url: https://gerrit.o-ran-sc.org/r/gitweb?a=commitdiff_plain;h=4974b9d1c7256e90cb206b327b0c81f7364beeab;p=nonrtric%2Fplt%2Fsme.git NONRTRIC-946: Fix panics in Capif Issue-ID: NONRTRIC-946 Change-Id: Iff56e2443feb8514f919f278c8743c3195a14b73 Signed-off-by: DenisGNoonan --- diff --git a/capifcore/internal/invokermanagement/invokermanagement.go b/capifcore/internal/invokermanagement/invokermanagement.go index ee7030a..f0a4155 100644 --- a/capifcore/internal/invokermanagement/invokermanagement.go +++ b/capifcore/internal/invokermanagement/invokermanagement.go @@ -31,10 +31,9 @@ import ( "oransc.org/nonrtric/capifcore/internal/common29122" invokerapi "oransc.org/nonrtric/capifcore/internal/invokermanagementapi" - "oransc.org/nonrtric/capifcore/internal/publishservice" - "github.com/labstack/echo/v4" + echo "github.com/labstack/echo/v4" ) //go:generate mockery --name InvokerRegister @@ -103,17 +102,18 @@ func (im *InvokerManager) GetInvokerApiList(invokerId string) *invokerapi.APILis // Creates a new individual API Invoker profile. func (im *InvokerManager) PostOnboardedInvokers(ctx echo.Context) error { - var newInvoker invokerapi.APIInvokerEnrolmentDetails errMsg := "Unable to onboard invoker due to %s" - if err := ctx.Bind(&newInvoker); err != nil { - return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, "invalid format for invoker")) + + newInvoker, err := getInvokerFromRequest(ctx) + if err != nil { + return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } - if err := im.isInvokerOnboarded(newInvoker); err != nil { + if err = im.isInvokerOnboarded(newInvoker); err != nil { return sendCoreError(ctx, http.StatusForbidden, fmt.Sprintf(errMsg, err)) } - if err := im.validateInvoker(newInvoker, ctx); err != nil { + if err = im.validateInvoker(newInvoker, ctx); err != nil { return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } @@ -123,7 +123,8 @@ func (im *InvokerManager) PostOnboardedInvokers(ctx echo.Context) error { uri := ctx.Request().Host + ctx.Request().URL.String() ctx.Response().Header().Set(echo.HeaderLocation, ctx.Scheme()+`://`+path.Join(uri, *newInvoker.ApiInvokerId)) - err := ctx.JSON(http.StatusCreated, newInvoker) + + err = ctx.JSON(http.StatusCreated, newInvoker) if err != nil { // Something really bad happened, tell Echo that our handler failed return err @@ -185,29 +186,40 @@ func (im *InvokerManager) deleteInvoker(onboardingId string) { delete(im.onboardedInvokers, onboardingId) } +func getInvokerFromRequest(ctx echo.Context) (invokerapi.APIInvokerEnrolmentDetails, error) { + var invoker invokerapi.APIInvokerEnrolmentDetails + if err := ctx.Bind(&invoker); err != nil { + return invokerapi.APIInvokerEnrolmentDetails{}, fmt.Errorf("invalid format for invoker") + } + return invoker, nil +} + // Updates an individual API invoker details. func (im *InvokerManager) PutOnboardedInvokersOnboardingId(ctx echo.Context, onboardingId string) error { - var invoker invokerapi.APIInvokerEnrolmentDetails errMsg := "Unable to update invoker due to %s" - if err := ctx.Bind(&invoker); err != nil { - return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, "invalid format for invoker")) + + newInvoker, err := getInvokerFromRequest(ctx) + if err != nil { + return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } - if onboardingId != *invoker.ApiInvokerId { - return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, "ApiInvokerId not matching")) + // Additional validation for PUT + if (newInvoker.ApiInvokerId == nil) || (*newInvoker.ApiInvokerId != onboardingId) { + errMismatch := "APIInvokerEnrolmentDetails ApiInvokerId doesn't match path parameter" + return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, errMismatch)) } - if err := im.validateInvoker(invoker, ctx); err != nil { + if err := im.validateInvoker(newInvoker, ctx); err != nil { return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } if _, ok := im.onboardedInvokers[onboardingId]; ok { - im.updateInvoker(invoker) + im.updateInvoker(newInvoker) } else { return sendCoreError(ctx, http.StatusNotFound, "The invoker to update has not been onboarded") } - err := ctx.JSON(http.StatusOK, invoker) + err = ctx.JSON(http.StatusOK, newInvoker) if err != nil { // Something really bad happened, tell Echo that our handler failed return err diff --git a/capifcore/internal/invokermanagement/invokermanagement_test.go b/capifcore/internal/invokermanagement/invokermanagement_test.go index 7b92972..b46e925 100644 --- a/capifcore/internal/invokermanagement/invokermanagement_test.go +++ b/capifcore/internal/invokermanagement/invokermanagement_test.go @@ -166,6 +166,29 @@ func TestDeleteInvoker(t *testing.T) { } } +func TestFailedUpdateInvoker(t *testing.T) { + publishRegisterMock := publishmocks.PublishRegister{} + publishRegisterMock.On("GetAllPublishedServices").Return([]publishserviceapi.ServiceAPIDescription{}) + serviceUnderTest, _, requestHandler := getEcho(&publishRegisterMock, nil) + + invokerInfo := "invoker a" + invokerId := "api_invoker_id_" + strings.Replace(invokerInfo, " ", "_", 1) + + // Attempt to update with an invoker without the ApiInvokerId provided in the parameter body. We should get 400 with problem details. + invalidInvoker := getInvoker(invokerInfo) + serviceUnderTest.onboardedInvokers[invokerId] = invalidInvoker + + result := testutil.NewRequest().Put("/onboardedInvokers/"+invokerId).WithJsonBody(invalidInvoker).Go(t, requestHandler) + assert.Equal(t, http.StatusBadRequest, result.Code()) + + var problemDetails common29122.ProblemDetails + err := result.UnmarshalBodyToObject(&problemDetails) + assert.NoError(t, err, "error unmarshaling response") + assert.Equal(t, http.StatusBadRequest, *problemDetails.Status) + + assert.Contains(t, *problemDetails.Cause, "APIInvokerEnrolmentDetails ApiInvokerId doesn't match path parameter") +} + func TestUpdateInvoker(t *testing.T) { publishRegisterMock := publishmocks.PublishRegister{} publishRegisterMock.On("GetAllPublishedServices").Return([]publishserviceapi.ServiceAPIDescription{}) @@ -236,8 +259,7 @@ func TestUpdateInvoker(t *testing.T) { err = result.UnmarshalBodyToObject(&problemDetails) assert.NoError(t, err, "error unmarshaling response") assert.Equal(t, http.StatusBadRequest, *problemDetails.Status) - assert.Contains(t, *problemDetails.Cause, "not matching") - assert.Contains(t, *problemDetails.Cause, "ApiInvokerId") + assert.Contains(t, *problemDetails.Cause, "APIInvokerEnrolmentDetails ApiInvokerId doesn't match path parameter") // Update an invoker that has not been onboarded, should get 404 with problem details missingId := "1" diff --git a/capifcore/internal/publishservice/publishservice.go b/capifcore/internal/publishservice/publishservice.go index b4e38e7..0fa125f 100644 --- a/capifcore/internal/publishservice/publishservice.go +++ b/capifcore/internal/publishservice/publishservice.go @@ -233,7 +233,8 @@ func (ps *PublishService) GetApfIdServiceApisServiceApiId(ctx echo.Context, apfI func getServiceDescription(serviceApiId string, descriptions []publishapi.ServiceAPIDescription) (int, *publishapi.ServiceAPIDescription) { for pos, description := range descriptions { - if serviceApiId == *description.ApiId { + // Check for nil as we had a failure here when running unit tests in parallel against a single Capifcore instance + if (description.ApiId != nil) && (serviceApiId == *description.ApiId) { return pos, &description } }