Consistent Logging Severity

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/apierrors/errors.go

@@ -25,7 +25,8 @@ type ApiError interface {
 // error level since api errors are expected recoverable conditions.
 // It returns the error for convenience.
 func LogAndReturn(logger logr.Logger, err error, msg string, keysAndValues ...interface{}) error {
-	if _, ok := err.(ApiError); ok {
+	var apiError ApiError
+	if errors.As(err, &apiError) {
 		keysAndValues = append(keysAndValues, "err", err)
 		logger.Info(msg, keysAndValues...)
 	} else {
@@ -103,10 +104,18 @@ func NewMessageParseError(cause error) MessageParseError {
 	}
 }
 
+// UnknownError is a generic wrapper over an error Korifi cannot recover from.
+// Unknown errors should be only used by the presentation layer to present such
+// an error to the user. Other components (handlers, repositories, etc.) should
+// simply return the incoming error, it would be mapped to `UnknownError` by
+// the presentation layer
 type UnknownError struct {
 	apiError
 }
 
+// NewUnknownError creates an UnknownError. One should generally not create
+// unknown errors as generic errors are automatically presented as unknown
+// errors to the user
 func NewUnknownError(cause error) UnknownError {
 	return UnknownError{
 		apiError{

api/apierrors/errors_test.go

@@ -1,6 +1,8 @@
 package apierrors_test
 
 import (
+	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 
@@ -9,6 +11,7 @@ import (
 	. "github.com/onsi/gomega"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 var _ = Describe("FromK8sError", func() {
@@ -245,6 +248,64 @@ var _ = Describe("AsUnprocessibleEntity", func() {
 	})
 })
 
+var _ = Describe("LogAndReturn", func() {
+	var (
+		originalErr error
+		logBuf      *bytes.Buffer
+		logEntry    map[string]interface{}
+	)
+
+	BeforeEach(func() {
+		logBuf = new(bytes.Buffer)
+	})
+
+	JustBeforeEach(func() {
+		logger := zap.New(zap.WriteTo(logBuf))
+		returnedErr := apierrors.LogAndReturn(logger, originalErr, "some message", "some-key", "some-value")
+		Expect(returnedErr).To(Equal(originalErr))
+		Expect(json.Unmarshal(logBuf.Bytes(), &logEntry)).To(Succeed())
+	})
+
+	When("the erorr is not an ApiError", func() {
+		BeforeEach(func() {
+			originalErr = errors.New("not-api-error")
+		})
+
+		It("logs an error", func() {
+			Expect(logEntry["level"]).To(Equal("error"))
+			Expect(logEntry["msg"]).To(Equal("some message"))
+			Expect(logEntry["some-key"]).To(Equal("some-value"))
+			Expect(logEntry["error"]).To(Equal("not-api-error"))
+		})
+	})
+
+	When("the error is an ApiError", func() {
+		BeforeEach(func() {
+			originalErr = apierrors.NewForbiddenError(errors.New("cause-err"), "my-resource")
+		})
+
+		It("logs an info", func() {
+			Expect(logEntry["level"]).To(Equal("info"))
+			Expect(logEntry["msg"]).To(Equal("some message"))
+			Expect(logEntry["some-key"]).To(Equal("some-value"))
+			Expect(logEntry["err"]).To(Equal("cause-err"))
+		})
+	})
+
+	When("the error is a wrapped ApiError", func() {
+		BeforeEach(func() {
+			originalErr = fmt.Errorf("wrapping: %w", apierrors.NewForbiddenError(errors.New("cause-err"), "my-resource"))
+		})
+
+		It("logs an info", func() {
+			Expect(logEntry["level"]).To(Equal("info"))
+			Expect(logEntry["msg"]).To(Equal("some message"))
+			Expect(logEntry["some-key"]).To(Equal("some-value"))
+			Expect(logEntry["err"]).To(Equal("wrapping: cause-err"))
+		})
+	})
+})
+
 type testApiError struct {
 	apierrors.ApiError
 }

api/authorization/cert_inspector.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 
 	"code.cloudfoundry.org/korifi/api/apierrors"
@@ -25,17 +26,17 @@ func NewCertInspector(restConfig *rest.Config) *CertInspector {
 func (c *CertInspector) WhoAmI(ctx context.Context, certPEM []byte) (Identity, error) {
 	certBlock, rst := pem.Decode(certPEM)
 	if certBlock == nil {
-		return Identity{}, fmt.Errorf("failed to decode cert PEM")
+		return Identity{}, apierrors.NewInvalidAuthError(errors.New("failed to decode cert PEM"))
 	}
 
 	cert, err := x509.ParseCertificate(certBlock.Bytes)
 	if err != nil {
-		return Identity{}, fmt.Errorf("failed to parse certificate: %w", err)
+		return Identity{}, apierrors.NewInvalidAuthError(fmt.Errorf("failed to parse certificate: %w", err))
 	}
 
 	keyBlock, _ := pem.Decode(rst)
 	if keyBlock == nil {
-		return Identity{}, fmt.Errorf("failed to decode key PEM")
+		return Identity{}, apierrors.NewInvalidAuthError(errors.New("failed to decode key PEM"))
 	}
 
 	config := rest.AnonymousClientConfig(c.restConfig)

api/authorization/cert_inspector_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/pem"
+	"errors"
 
 	"code.cloudfoundry.org/korifi/api/apierrors"
 	"code.cloudfoundry.org/korifi/api/authorization"
@@ -57,7 +58,7 @@ var _ = Describe("CertInspector", func() {
 		})
 
 		It("returns an error", func() {
-			Expect(inspectorErr).To(MatchError("failed to decode cert PEM"))
+			Expect(inspectorErr).To(Equal(apierrors.NewInvalidAuthError(errors.New("failed to decode cert PEM"))))
 		})
 	})
 
@@ -78,7 +79,10 @@ var _ = Describe("CertInspector", func() {
 		})
 
 		It("returns an error", func() {
-			Expect(inspectorErr).To(MatchError(ContainSubstring("failed to parse certificate")))
+			Expect(inspectorErr).To(SatisfyAll(
+				matchers.WrapErrorAssignableToTypeOf(apierrors.InvalidAuthError{})),
+				MatchError(ContainSubstring("failed to parse certificate")),
+			)
 		})
 	})
 })

api/handlers/apis_suite_test.go

@@ -11,12 +11,15 @@ import (
 	"testing"
 
 	"code.cloudfoundry.org/korifi/api/authorization"
+	"code.cloudfoundry.org/korifi/api/correlation"
 	"github.com/go-http-utils/headers"
 	"github.com/google/uuid"
 	"github.com/gorilla/mux"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gstruct"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 const (
@@ -25,21 +28,27 @@ const (
 )
 
 var (
-	rr        *httptest.ResponseRecorder
-	router    *mux.Router
-	serverURL *url.URL
-	ctx       context.Context
-	authInfo  authorization.Info
+	rr            *httptest.ResponseRecorder
+	router        *mux.Router
+	serverURL     *url.URL
+	ctx           context.Context
+	authInfo      authorization.Info
+	correlationID string
 )
 
 func TestApis(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Apis Suite")
 }
 
+var _ = BeforeSuite(func() {
+	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter)))
+})
+
 var _ = BeforeEach(func() {
 	authInfo = authorization.Info{Token: "a-token"}
-	ctx = authorization.NewContext(context.Background(), &authInfo)
+	correlationID = generateGUID("corrID")
+	ctx = correlation.ContextWithId(authorization.NewContext(context.Background(), &authInfo), correlationID)
 	rr = httptest.NewRecorder()
 	router = mux.NewRouter()