Use correct severity for logs by using apierrors.LogAndReturn

Co-authored-by: Kieron Browne <kbrowne@vmware.com>

api/actions/app_logs.go

@@ -33,20 +33,17 @@ func (a *AppLogs) Read(ctx context.Context, logger logr.Logger, authInfo authori
 
 	app, err := a.appRepo.GetApp(ctx, authInfo, appGUID)
 	if err != nil {
-		logger.Error(err, "Failed to fetch app from Kubernetes", "AppGUID", appGUID)
-		return nil, apierrors.ForbiddenAsNotFound(err)
+		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "Failed to fetch app from Kubernetes", "AppGUID", appGUID)
 	}
 
 	build, err := a.buildRepo.GetLatestBuildByAppGUID(ctx, authInfo, app.SpaceGUID, appGUID)
 	if err != nil {
-		logger.Error(err, "Failed to fetch latest app CFBuild from Kubernetes", "AppGUID", appGUID)
-		return nil, apierrors.ForbiddenAsNotFound(err)
+		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "Failed to fetch latest app CFBuild from Kubernetes", "AppGUID", appGUID)
 	}
 
 	buildLogs, err := a.buildRepo.GetBuildLogs(ctx, authInfo, app.SpaceGUID, build.GUID)
 	if err != nil {
-		logger.Error(err, "Failed to fetch build logs", "AppGUID", appGUID, "BuildGUID", build.GUID)
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch build logs", "AppGUID", appGUID, "BuildGUID", build.GUID)
 	}
 
 	logLimit := int64(defaultLogLimit)
@@ -61,8 +58,7 @@ func (a *AppLogs) Read(ctx context.Context, logger logr.Logger, authInfo authori
 		Limit:       logLimit,
 	})
 	if err != nil {
-		logger.Error(err, "Failed to fetch app runtime logs from Kubernetes", "AppGUID", appGUID)
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch app runtime logs from Kubernetes", "AppGUID", appGUID)
 	}
 
 	logs := append(buildLogs, runtimeLogs...)

api/handlers/auth_aware_handler.go

@@ -73,25 +73,27 @@ func (h *AuthAwareHandlerFuncWrapper) Wrap(delegate AuthAwareHandlerFunc) http.H
 		authInfo, ok := h.authInfoProvider(r.Context())
 		if !ok {
 			logger.Error(nil, "unable to get auth info")
-			presentError(w, nil)
+			presentError(h.logger, w, nil)
 			return
 		}
 
 		handlerResponse, err := delegate(ctx, logger, authInfo, r)
 		if err != nil {
 			logger.Info("handler returned error", "error", err)
-			presentError(w, err)
+			presentError(h.logger, w, err)
 			return
 		}
 
-		handlerResponse.writeTo(w)
+		if err := handlerResponse.writeTo(w); err != nil {
+			_ = apierrors.LogAndReturn(logger, err, "failed to write result to the HTTP response", "handlerResponse", handlerResponse, "method", r.Method, "URL", r.URL)
+		}
 	}
 }
 
-func presentError(w http.ResponseWriter, err error) {
+func presentError(logger logr.Logger, w http.ResponseWriter, err error) {
 	var apiError apierrors.ApiError
 	if errors.As(err, &apiError) {
-		NewHandlerResponse(apiError.HttpStatus()).
+		writeErr := NewHandlerResponse(apiError.HttpStatus()).
 			WithBody(presenter.ErrorsResponse{
 				Errors: []presenter.PresentedError{
 					{
@@ -102,20 +104,25 @@ func presentError(w http.ResponseWriter, err error) {
 				},
 			}).
 			writeTo(w)
+
+		if writeErr != nil {
+			_ = apierrors.LogAndReturn(logger, writeErr, "failed to write error to the HTTP response")
+		}
+
 		return
 	}
 
-	presentError(w, apierrors.NewUnknownError(err))
+	presentError(logger, w, apierrors.NewUnknownError(err))
 }
 
-func (response *HandlerResponse) writeTo(w http.ResponseWriter) {
+func (response *HandlerResponse) writeTo(w http.ResponseWriter) error {
 	for k, v := range response.headers {
 		w.Header().Set(k, v)
 	}
 
 	if response.body == nil {
 		w.WriteHeader(response.httpStatus)
-		return
+		return nil
 	}
 
 	w.Header().Set(headers.ContentType, "application/json")
@@ -126,6 +133,8 @@ func (response *HandlerResponse) writeTo(w http.ResponseWriter) {
 
 	err := encoder.Encode(response.body)
 	if err != nil {
-		Logger.Error(err, "failed to encode and write response")
+		return fmt.Errorf("failed to encode and write response: %w", err)
 	}
+
+	return nil
 }

api/handlers/authentication_middleware.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"net/http"
 
+	"code.cloudfoundry.org/korifi/api/apierrors"
 	"code.cloudfoundry.org/korifi/api/authorization"
 	"code.cloudfoundry.org/korifi/api/correlation"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -48,17 +49,16 @@ func (a *AuthenticationMiddleware) Middleware(next http.Handler) http.Handler {
 
 		authInfo, err := a.authInfoParser.Parse(r.Header.Get(headers.Authorization))
 		if err != nil {
-			logger.Error(err, "failed to parse auth info")
-			presentError(w, err)
+			logger.Info("failed to parse auth info", "err", err)
+			presentError(logger, w, err)
 			return
 		}
 
 		r = r.WithContext(authorization.NewContext(r.Context(), &authInfo))
 
 		_, err = a.identityProvider.GetIdentity(r.Context(), authInfo)
 		if err != nil {
-			logger.Error(err, "failed to get identity")
-			presentError(w, err)
+			presentError(logger, w, apierrors.LogAndReturn(logger, err, "failed to get identity"))
 			return
 		}
 

api/handlers/build_handler.go

@@ -57,8 +57,7 @@ func (h *BuildHandler) buildGetHandler(ctx context.Context, logger logr.Logger,
 
 	build, err := h.buildRepo.GetBuild(ctx, authInfo, buildGUID)
 	if err != nil {
-		logger.Error(err, fmt.Sprintf("Failed to fetch %s from Kubernetes", repositories.BuildResourceType), "guid", buildGUID)
-		return nil, apierrors.ForbiddenAsNotFound(err)
+		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), fmt.Sprintf("Failed to fetch %s from Kubernetes", repositories.BuildResourceType), "guid", buildGUID)
 	}
 
 	return NewHandlerResponse(http.StatusOK).WithBody(presenter.ForBuild(build, h.serverURL)), nil
@@ -67,25 +66,27 @@ func (h *BuildHandler) buildGetHandler(ctx context.Context, logger logr.Logger,
 func (h *BuildHandler) buildCreateHandler(ctx context.Context, logger logr.Logger, authInfo authorization.Info, r *http.Request) (*HandlerResponse, error) {
 	var payload payloads.BuildCreate
 	if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil {
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload")
 	}
 
 	packageRecord, err := h.packageRepo.GetPackage(r.Context(), authInfo, payload.Package.GUID)
 	if err != nil {
-		logger.Info("Error finding Package", "Package GUID", payload.Package.GUID)
-		return nil, apierrors.AsUnprocessableEntity(err,
-			"Unable to use package. Ensure that the package exists and you have access to it.",
-			apierrors.ForbiddenError{},
-			apierrors.NotFoundError{},
+		return nil, apierrors.LogAndReturn(
+			logger,
+			apierrors.AsUnprocessableEntity(err,
+				"Unable to use package. Ensure that the package exists and you have access to it.",
+				apierrors.ForbiddenError{},
+				apierrors.NotFoundError{},
+			),
+			"Error finding Package", "Package GUID", payload.Package.GUID,
 		)
 	}
 
 	buildCreateMessage := payload.ToMessage(packageRecord)
 
 	record, err := h.buildRepo.CreateBuild(r.Context(), authInfo, buildCreateMessage)
 	if err != nil {
-		logger.Info("Error creating build with repository", "error", err.Error())
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Error creating build with repository")
 	}
 
 	return NewHandlerResponse(http.StatusCreated).WithBody(presenter.ForBuild(record, h.serverURL)), nil

api/handlers/buildpack_handler.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/url"
 
+	"code.cloudfoundry.org/korifi/api/apierrors"
 	"code.cloudfoundry.org/korifi/api/authorization"
 	"code.cloudfoundry.org/korifi/api/payloads"
 	"code.cloudfoundry.org/korifi/api/presenter"
@@ -43,21 +44,18 @@ func NewBuildpackHandler(
 
 func (h *BuildpackHandler) buildpackListHandler(ctx context.Context, logger logr.Logger, authInfo authorization.Info, r *http.Request) (*HandlerResponse, error) {
 	if err := r.ParseForm(); err != nil {
-		logger.Error(err, "Unable to parse request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters")
 	}
 
 	buildpackListFilter := new(payloads.BuildpackList)
 	err := payloads.Decode(buildpackListFilter, r.Form)
 	if err != nil {
-		logger.Error(err, "Unable to decode request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters")
 	}
 
 	buildpacks, err := h.buildpackRepo.ListBuildpacks(ctx, authInfo)
 	if err != nil {
-		logger.Error(err, "Failed to fetch buildpacks from Kubernetes")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch buildpacks from Kubernetes")
 	}
 
 	return NewHandlerResponse(http.StatusOK).WithBody(presenter.ForBuildpackList(buildpacks, h.serverURL, *r.URL)), nil

api/handlers/domain_handler.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/url"
 
+	"code.cloudfoundry.org/korifi/api/apierrors"
 	"code.cloudfoundry.org/korifi/api/authorization"
 	"code.cloudfoundry.org/korifi/api/payloads"
 	"code.cloudfoundry.org/korifi/api/presenter"
@@ -46,21 +47,18 @@ func NewDomainHandler(
 
 func (h *DomainHandler) DomainListHandler(ctx context.Context, logger logr.Logger, authInfo authorization.Info, r *http.Request) (*HandlerResponse, error) { //nolint:dupl
 	if err := r.ParseForm(); err != nil {
-		logger.Error(err, "Unable to parse request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters")
 	}
 
 	domainListFilter := new(payloads.DomainList)
 	err := payloads.Decode(domainListFilter, r.Form)
 	if err != nil {
-		logger.Error(err, "Unable to decode request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters")
 	}
 
 	domainList, err := h.domainRepo.ListDomains(ctx, authInfo, domainListFilter.ToMessage())
 	if err != nil {
-		logger.Error(err, "Failed to fetch domain(s) from Kubernetes")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch domain(s) from Kubernetes")
 	}
 
 	return NewHandlerResponse(http.StatusOK).WithBody(presenter.ForDomainList(domainList, h.serverURL, *r.URL)), nil

api/handlers/droplet_handler.go

@@ -49,8 +49,12 @@ func (h *DropletHandler) dropletGetHandler(ctx context.Context, logger logr.Logg
 
 	droplet, err := h.dropletRepo.GetDroplet(ctx, authInfo, dropletGUID)
 	if err != nil {
-		logger.Error(err, fmt.Sprintf("Failed to fetch %s from Kubernetes", repositories.DropletResourceType), "guid", dropletGUID)
-		return nil, apierrors.ForbiddenAsNotFound(err)
+		return nil, apierrors.LogAndReturn(
+			logger,
+			apierrors.ForbiddenAsNotFound(err),
+			fmt.Sprintf("Failed to fetch %s from Kubernetes", repositories.DropletResourceType),
+			"guid", dropletGUID,
+		)
 	}
 
 	return NewHandlerResponse(http.StatusOK).WithBody(presenter.ForDroplet(droplet, h.serverURL)), nil

api/handlers/job_handler.go

@@ -46,8 +46,11 @@ func (h *JobHandler) jobGetHandler(ctx context.Context, logger logr.Logger, auth
 	jobType, resourceGUID, match := parseJobGUID(jobGUID)
 
 	if !match {
-		logger.Info("Invalid Job GUID")
-		return nil, apierrors.NewNotFoundError(fmt.Errorf("invalid job guid: %s", jobGUID), JobResourceType)
+		return nil, apierrors.LogAndReturn(
+			logger,
+			apierrors.NewNotFoundError(fmt.Errorf("invalid job guid: %s", jobGUID), JobResourceType),
+			"Invalid Job GUID",
+		)
 	}
 
 	var jobResponse presenter.JobResponse
@@ -58,8 +61,11 @@ func (h *JobHandler) jobGetHandler(ctx context.Context, logger logr.Logger, auth
 	case appDeletePrefix, orgDeletePrefix, spaceDeletePrefix, routeDeletePrefix:
 		jobResponse = presenter.ForDeleteJob(jobGUID, jobType, h.serverURL)
 	default:
-		logger.Info(fmt.Sprintf("Invalid Job type: %s", jobType))
-		return nil, apierrors.NewNotFoundError(fmt.Errorf("invalid job type: %s", jobType), JobResourceType)
+		return nil, apierrors.LogAndReturn(
+			logger,
+			apierrors.NewNotFoundError(fmt.Errorf("invalid job type: %s", jobType), JobResourceType),
+			fmt.Sprintf("Invalid Job type: %s", jobType),
+		)
 	}
 
 	return NewHandlerResponse(http.StatusOK).WithBody(jobResponse), nil

api/handlers/log_cache_handler.go

@@ -60,21 +60,22 @@ func (h *LogCacheHandler) logCacheInfoHandler(ctx context.Context, logger logr.L
 
 func (h *LogCacheHandler) logCacheReadHandler(ctx context.Context, logger logr.Logger, authInfo authorization.Info, r *http.Request) (*HandlerResponse, error) {
 	if err := r.ParseForm(); err != nil {
-		logger.Error(err, "Unable to parse request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters")
 	}
 
 	logReadPayload := new(payloads.LogRead)
 	err := payloads.Decode(logReadPayload, r.Form)
 	if err != nil {
-		logger.Error(err, "Unable to decode request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters")
 	}
 
 	v := validator.New()
 	if logReadPayloadErr := v.Struct(logReadPayload); logReadPayloadErr != nil {
-		logger.Error(logReadPayloadErr, "Error validating log read request query parameters")
-		return nil, apierrors.NewUnprocessableEntityError(logReadPayloadErr, "error validating log read query parameters")
+		return nil, apierrors.LogAndReturn(
+			logger,
+			apierrors.NewUnprocessableEntityError(logReadPayloadErr, "error validating log read query parameters"),
+			"Error validating log read request query parameters",
+		)
 	}
 
 	vars := mux.Vars(r)

api/handlers/org_handler.go

@@ -57,15 +57,13 @@ func NewOrgHandler(apiBaseURL url.URL, orgRepo CFOrgRepository, domainRepo CFDom
 func (h *OrgHandler) orgCreateHandler(ctx context.Context, logger logr.Logger, authInfo authorization.Info, r *http.Request) (*HandlerResponse, error) {
 	var payload payloads.OrgCreate
 	if err := h.decoderValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil {
-		logger.Info("invalid-payload-for-create-org", "err", err)
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "invalid-payload-for-create-org")
 	}
 
 	org := payload.ToMessage()
 	record, err := h.orgRepo.CreateOrg(ctx, authInfo, org)
 	if err != nil {
-		logger.Error(err, "Failed to create org", "Org Name", payload.Name)
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Failed to create org", "Org Name", payload.Name)
 	}
 
 	return NewHandlerResponse(http.StatusCreated).WithBody(presenter.ForCreateOrg(record, h.apiBaseURL)), nil
@@ -80,8 +78,7 @@ func (h *OrgHandler) orgDeleteHandler(ctx context.Context, logger logr.Logger, a
 	}
 	err := h.orgRepo.DeleteOrg(ctx, authInfo, deleteOrgMessage)
 	if err != nil {
-		logger.Error(err, "Failed to delete org", "OrgGUID", orgGUID)
-		return nil, apierrors.ForbiddenAsNotFound(err)
+		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "Failed to delete org", "OrgGUID", orgGUID)
 	}
 
 	return NewHandlerResponse(http.StatusAccepted).WithHeader("Location", presenter.JobURLForRedirects(orgGUID, presenter.OrgDeleteOperation, h.apiBaseURL)), nil
@@ -92,8 +89,7 @@ func (h *OrgHandler) orgListHandler(ctx context.Context, logger logr.Logger, aut
 
 	orgs, err := h.orgRepo.ListOrgs(ctx, authInfo, repositories.ListOrgsMessage{Names: names})
 	if err != nil {
-		logger.Error(err, "failed to fetch orgs")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "failed to fetch orgs")
 	}
 
 	resp := NewHandlerResponse(http.StatusOK).WithBody(presenter.ForOrgList(orgs, h.apiBaseURL, *r.URL))
@@ -110,26 +106,22 @@ func (h *OrgHandler) orgListDomainHandler(ctx context.Context, logger logr.Logge
 	orgGUID := vars["guid"]
 
 	if _, err := h.orgRepo.GetOrg(ctx, authInfo, orgGUID); err != nil {
-		logger.Error(err, "Unable to get organization")
-		return nil, apierrors.ForbiddenAsNotFound(err)
+		return nil, apierrors.LogAndReturn(logger, apierrors.ForbiddenAsNotFound(err), "Unable to get organization")
 	}
 
 	if err := r.ParseForm(); err != nil {
-		logger.Error(err, "Unable to parse request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to parse request query parameters")
 	}
 
 	domainListFilter := new(payloads.DomainList)
 	err := payloads.Decode(domainListFilter, r.Form)
 	if err != nil {
-		logger.Error(err, "Unable to decode request query parameters")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Unable to decode request query parameters")
 	}
 
 	domainList, err := h.domainRepo.ListDomains(ctx, authInfo, domainListFilter.ToMessage())
 	if err != nil {
-		logger.Error(err, "Failed to fetch domain(s) from Kubernetes")
-		return nil, err
+		return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch domain(s) from Kubernetes")
 	}
 
 	return NewHandlerResponse(http.StatusOK).WithBody(presenter.ForDomainList(domainList, h.apiBaseURL, *r.URL)), nil