NONRTRIC-946: Fix panics in Capif

Issue-ID: NONRTRIC-946
Change-Id: Iff56e2443feb8514f919f278c8743c3195a14b73
Signed-off-by: DenisGNoonan <denis.noonan@est.tech>

diff --git a/capifcore/internal/invokermanagement/invokermanagement.go b/capifcore/internal/invokermanagement/invokermanagement.go
index ee7030a..f0a4155 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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
                }
        }