Return user friendly error when package doesn't exist

Signed-off-by: rcmadhankumar <[email protected]>

pkg/apiserver/registry/datapackaging/package_crd_rest_test.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/internalpackaging/v1alpha1"
 	"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging"
 	datapkgreg "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/registry/datapackaging"
@@ -146,6 +147,7 @@ func TestPackageVersionGetPresentInOnlyNS(t *testing.T) {
 func TestPackageVersionGetNotFound(t *testing.T) {
 	namespacedPackageVersion := excludedNonGlobalIntPackageVersion()
 	name := namespacedPackageName
+	expectedError := "package.data.packaging.carvel.dev \"" + name + "\" not found"
 
 	internalClient := fake.NewSimpleClientset(namespacedPackageVersion)
 	fakeCoreClient := k8sfake.NewSimpleClientset(namespace())
@@ -157,10 +159,8 @@ func TestPackageVersionGetNotFound(t *testing.T) {
 		t.Fatalf("Expected get operation to fail, but it didn't")
 	}
 
-	if !errors.IsNotFound(err) {
-		t.Fatalf("Expected a not found error, got: %v", err)
-	}
-
+	assert.True(t, errors.IsNotFound(err))
+	assert.ErrorContains(t, err, expectedError)
 }
 
 func TestPackageVersionGetPreferNS(t *testing.T) {

pkg/apiserver/registry/datapackaging/package_storage_client.go

@@ -6,6 +6,7 @@ package datapackaging
 import (
 	"context"
 	"encoding/base32"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -14,7 +15,7 @@ import (
 	datapkgingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1"
 	"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/watchers"
 	internalclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/watch"
@@ -65,7 +66,7 @@ func (t PackageTranslator) ToExternalObj(intObj *internalpkgingv1alpha1.Internal
 	var err error
 	obj.Name, err = t.ToExternalName(intObj.Name)
 	if err != nil {
-		return nil, errors.NewInternalError(fmt.Errorf("decoding internal obj name '%s': %v", intObj.Name, err))
+		return nil, apierrors.NewInternalError(fmt.Errorf("decoding internal obj name '%s': %v", intObj.Name, err))
 	}
 
 	// Self link is deprecated and planned for removal, so we don't translate it
@@ -122,10 +123,10 @@ func (t PackageTranslator) ToExternalWatcher(intObjWatcher watch.Interface, fiel
 			evt.Object, err = t.ToExternalObj(intpkg)
 			if err != nil {
 				var status metav1.Status
-				if statusErr, ok := err.(*errors.StatusError); ok {
+				if statusErr, ok := err.(*apierrors.StatusError); ok {
 					status = statusErr.Status()
 				} else {
-					status = errors.NewInternalError(err).Status()
+					status = apierrors.NewInternalError(err).Status()
 				}
 				return watch.Event{Type: watch.Error, Object: &status}
 			}
@@ -152,7 +153,32 @@ func (t PackageTranslator) ToExternalWatcher(intObjWatcher watch.Interface, fiel
 }
 
 func (t PackageTranslator) ToExternalError(err error) error {
-	// TODO: implement
+
+	status, ok := err.(apierrors.APIStatus)
+	if (ok || errors.As(err, &status)) &&
+		status.Status().Details != nil &&
+		status.Status().Reason != "" {
+
+		details := status.Status().Details
+		if details.Kind == "internalpackages" && details.Group == internalpkgingv1alpha1.SchemeGroupVersion.Group {
+			packageName, translateErr := t.ToExternalName(details.Name)
+			if translateErr != nil {
+				return err
+			}
+
+			switch status.Status().Reason {
+			case metav1.StatusReasonNotFound:
+				return apierrors.NewNotFound(datapkgingv1alpha1.Resource("package"), packageName)
+			case metav1.StatusReasonAlreadyExists:
+				return apierrors.NewAlreadyExists(datapkgingv1alpha1.Resource("package"), packageName)
+			// TODO Handle other types of errors
+			default:
+				return err
+			}
+		}
+
+	}
+
 	return err
 }
 

test/e2e/kappcontroller/package_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1"
 	"github.com/vmware-tanzu/carvel-kapp-controller/test/e2e"
 	"sigs.k8s.io/yaml"
@@ -409,3 +410,23 @@ spec:
 		}
 	})
 }
+
+func TestPackageNotFound(t *testing.T) {
+	env := e2e.BuildEnv(t)
+	logger := e2e.Logger{}
+	k := e2e.Kubectl{t, env.Namespace, logger}
+	packageName := "foo.1.0.0"
+	expectedError := "stderr: 'Error from server (NotFound): package.data.packaging.carvel.dev \"foo.1.0.0\" not found"
+
+	logger.Section("Get Package", func() {
+		_, err := k.RunWithOpts([]string{"get", "package", packageName}, e2e.RunOpts{AllowError: true})
+		assert.NotNil(t, err)
+		assert.ErrorContains(t, err, expectedError)
+	})
+
+	logger.Section("delete Package", func() {
+		_, err := k.RunWithOpts([]string{"delete", "package", packageName}, e2e.RunOpts{AllowError: true})
+		assert.NotNil(t, err)
+		assert.ErrorContains(t, err, expectedError)
+	})
+}