Add ngrok enriched errors

api/bindings/v1alpha1/boundendpoint_types.go

@@ -132,7 +132,6 @@ type BindingEndpoint struct {
 	// ErrorCode is the ngrok API error code if the status is error
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:validation:Pattern=`^ERR_NGROK_\d+$`
-	// TODO(hkatz) Define error codes and implement in the API
 	ErrorCode string `json:"errorCode,omitempty"`
 
 	// ErrorMessage is a free-form error message if the status is error

internal/controller/base_controller.go

@@ -9,6 +9,7 @@
 
 	"github.com/go-logr/logr"
 	"github.com/ngrok/ngrok-api-go/v7"
+	"github.com/ngrok/ngrok-operator/internal/ngrokapi"
 	"github.com/ngrok/ngrok-operator/internal/util"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/record"
@@ -163,6 +164,12 @@
 			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		case nerr.StatusCode == http.StatusNotFound:
 			return ctrl.Result{}, err
+		case ngrok.IsErrorCode(nerr, ngrokapi.NgrokOpErrFailedToCreateCSR.Code):
+			return ctrl.Result{RequeueAfter: 30 * time.Second}, nerr
+		case ngrok.IsErrorCode(nerr, ngrokapi.NgrokOpErrFailedToCreateUpstreamService.Code, ngrokapi.NgrokOpErrFailedToCreateTargetService.Code):
+			return ctrl.Result{RequeueAfter: 1 * time.Minute}, nerr
+		case ngrok.IsErrorCode(nerr, ngrokapi.NgrokOpErrEndpointDenied.Code):
+			return ctrl.Result{}, nil // do not retry, endpoint poller will take care of this
 		default:
 			// the rest are client errors, we don't retry by default
 			return ctrl.Result{}, nil

internal/controller/bindings/boundendpoint_controller.go

@@ -44,8 +44,10 @@
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/go-logr/logr"
+	"github.com/ngrok/ngrok-api-go/v7"
 	bindingsv1alpha1 "github.com/ngrok/ngrok-operator/api/bindings/v1alpha1"
 	"github.com/ngrok/ngrok-operator/internal/controller"
+	"github.com/ngrok/ngrok-operator/internal/ngrokapi"
 	"github.com/ngrok/ngrok-operator/internal/util"
 )
 
@@ -61,12 +63,6 @@
 	// Used for indexing BoundEndpoints by their target namespace. Not an actual
 	// field on the BoundEndpoint object.
 	BoundEndpointTargetNamespacePath = ".spec.targetNamespace"
-
-	// TODO(hkatz) ngrok-error-codes
-	NgrokErrorUpstreamServiceCreateFailed = "ERR_NGROK_0001"
-	NgrokErrorTargetServiceCreateFailed   = "ERR_NGROK_0002"
-	NgrokErrorFailedToBind                = "ERR_NGROK_003"
-	NgrokErrorNotAllowed                  = "ERR_NGROK_004"
 )
 
 var (
@@ -223,13 +219,15 @@
 		r.Recorder.Event(owner, v1.EventTypeWarning, "Created", "Failed to create Target Service")
 		log.Error(err, "Failed to create Target Service")
 
+		ngrokErr := ngrokapi.NewNgrokError(err, ngrokapi.NgrokOpErrFailedToCreateTargetService, "Failed to create Target Service")
+
 		setEndpointsStatus(owner, &bindingsv1alpha1.BindingEndpoint{
 			Status:       bindingsv1alpha1.StatusError,
-			ErrorCode:    NgrokErrorTargetServiceCreateFailed,
-			ErrorMessage: fmt.Sprintf("Failed to create Target Service: %s", err),
+			ErrorCode:    ngrokErr.ErrorCode,
+			ErrorMessage: ngrokErr.Error(),
 		})
 
-		return err
+		return ngrokErr
 	}
 
 	r.Recorder.Event(service, v1.EventTypeNormal, "Created", "Created Target Service")
@@ -245,13 +243,15 @@
 		r.Recorder.Event(owner, v1.EventTypeWarning, "Created", "Failed to create Upstream Service")
 		log.Error(err, "Failed to create Upstream Service")
 
+		ngrokErr := ngrokapi.NewNgrokError(err, ngrokapi.NgrokOpErrFailedToCreateUpstreamService, "Failed to create Upstream Service")
+
 		setEndpointsStatus(owner, &bindingsv1alpha1.BindingEndpoint{
 			Status:       bindingsv1alpha1.StatusError,
-			ErrorCode:    NgrokErrorUpstreamServiceCreateFailed,
-			ErrorMessage: fmt.Sprintf("Failed to create Upstream Service: %s", err),
+			ErrorCode:    ngrokErr.ErrorCode,
+			ErrorMessage: ngrokErr.Error(),
 		})
 
-		return err
+		return ngrokErr
 	}
 
 	r.Recorder.Event(service, v1.EventTypeNormal, "Created", "Created Upstream Service")
@@ -589,13 +589,15 @@
 
 	// update statuses
 	var desired *bindingsv1alpha1.BindingEndpoint
+	var ngrokErr *ngrok.Error
 	if bindErr != nil {
 		// error
 		log.Error(bindErr, "Failed to bind BoundEndpoint, moving to error")
+		ngrokErr = ngrokapi.NewNgrokError(bindErr, ngrokapi.NgrokOpErrFailedToConnectServices, "Failed to bind BoundEndpoint")
 		desired = &bindingsv1alpha1.BindingEndpoint{
 			Status:       bindingsv1alpha1.StatusError,
-			ErrorCode:    NgrokErrorFailedToBind,
-			ErrorMessage: fmt.Sprintf("Failed to bind BoundEndpoint: %s", bindErr),
+			ErrorCode:    ngrokErr.ErrorCode,
+			ErrorMessage: ngrokErr.Error(),
 		}
 	} else {
 		// success
@@ -610,5 +612,16 @@
 
 	// set status
 	setEndpointsStatus(boundEndpoint, desired)
-	return bindErr
+
+	// Why do we check for nil here and return nil explicitly instead of just `return ngrokErr`?
+	// Because *ngrok.Error meets the interface of error means that error is actually (T=*ngrok.Error, V=nil)
+	// so outer function signature is (error) and not (*ngrok.Error)
+	// and an interface _pointing_ to nil is != to nil itself
+	// therefore *ngrok.Error.(error) is _always_ != nil
+	//
+	// See: https://go.dev/doc/faq#nil_error
+	if ngrokErr != nil {
+		return ngrokErr
+	}
+	return nil
 }

internal/controller/bindings/boundendpoint_controller_test.go

@@ -140,7 +140,7 @@ func Test_setEndpointsStatus(t *testing.T) {
 			desired: &bindingsv1alpha1.BindingEndpoint{
 				Status:       bindingsv1alpha1.StatusError,
 				ErrorCode:    "ERR_NGROK_1234",
-				ErrorMessage: "Exampl Error Message",
+				ErrorMessage: "Example Error Message",
 			},
 		},
 	}

internal/controller/bindings/boundendpoint_poller_test.go

@@ -5,7 +5,7 @@ import (
 	"testing"
 
 	"github.com/go-logr/logr"
-	v6 "github.com/ngrok/ngrok-api-go/v7"
+	"github.com/ngrok/ngrok-api-go/v7"
 	bindingsv1alpha1 "github.com/ngrok/ngrok-operator/api/bindings/v1alpha1"
 	"github.com/ngrok/ngrok-operator/internal/ngrokapi"
 	"github.com/stretchr/testify/assert"
@@ -184,7 +184,7 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 			HashedName: hashURI(uriExample1),
 			Endpoints: []bindingsv1alpha1.BindingEndpoint{
 				{
-					Ref:          v6.Ref{ID: "ep_abc123", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_abc123", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",
@@ -218,7 +218,7 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 			HashedName: hashURI(uriExample1),
 			Endpoints: []bindingsv1alpha1.BindingEndpoint{
 				{
-					Ref:          v6.Ref{ID: "ep_abc123", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_abc123", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",
@@ -248,7 +248,7 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 			HashedName: hashURI(uriExample1),
 			Endpoints: []bindingsv1alpha1.BindingEndpoint{
 				{
-					Ref:          v6.Ref{ID: "ep_abc123", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_abc123", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",
@@ -275,7 +275,7 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 			HashedName: hashURI(uriExample2),
 			Endpoints: []bindingsv1alpha1.BindingEndpoint{
 				{
-					Ref:          v6.Ref{ID: "ep_def456", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_def456", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",
@@ -301,19 +301,19 @@ func Test_BoundEndpointPoller_boundEndpointNeedsUpdate(t *testing.T) {
 			HashedName: hashURI(uriExample2),
 			Endpoints: []bindingsv1alpha1.BindingEndpoint{
 				{
-					Ref:          v6.Ref{ID: "ep_def456", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_def456", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",
 				},
 				{
-					Ref:          v6.Ref{ID: "ep_xyz999", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_xyz999", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",
 				},
 				{
-					Ref:          v6.Ref{ID: "ep_www000", URI: "example-uri"},
+					Ref:          ngrok.Ref{ID: "ep_www000", URI: "example-uri"},
 					Status:       bindingsv1alpha1.StatusProvisioning,
 					ErrorCode:    "",
 					ErrorMessage: "",

internal/controller/ngrok/kubernetesoperator_controller.go

@@ -62,6 +62,10 @@
 	ngrokv1alpha1.KubernetesOperatorFeatureGateway:  "Gateway",
 }
 
+const (
+	NgrokErrorFailedToCreateCSR = "ERR_NGROK_20006"
+)
+
 // KubernetesOperatorReconciler reconciles a KubernetesOperator object
 type KubernetesOperatorReconciler struct {
 	client.Client
@@ -149,7 +153,7 @@
 	if bindingsEnabled {
 		tlsSecret, err := r.findOrCreateTLSSecret(ctx, ko)
 		if err != nil {
-			return err
+			return ngrokapi.NewNgrokError(err, ngrokapi.NgrokOpErrFailedToCreateCSR, "failed to create TLS secret for CSR")
 		}
 
 		createParams.Binding = &ngrok.KubernetesOperatorBindingCreate{

internal/ngrokapi/enriched_errors.go

@@ -0,0 +1,45 @@
+package ngrokapi
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/ngrok/ngrok-api-go/v7"
+)
+
+// Enriched Errors utilities to help interact with ngrok.Error and ngrok.IsErrorCode() returned by the ngrok API
+// For right now these are all manually defined
+
+type EnrichedError struct {
+	name       string
+	Code       int
+	statusCode int32
+}
+
+// list of supported ngrok error codes
+var (
+	NgrokOpErrInternalServerError           = EnrichedError{"ERR_NGROK_20000", 20000, http.StatusInternalServerError}
+	NgrokOpErrConfigurationError            = EnrichedError{"ERR_NGROK_20001", 20001, http.StatusBadRequest}
+	NgrokOpErrFailedToCreateUpstreamService = EnrichedError{"ERR_NGROK_20002", 20002, http.StatusServiceUnavailable}
+	NgrokOpErrFailedToCreateTargetService   = EnrichedError{"ERR_NGROK_20003", 20003, http.StatusServiceUnavailable}
+	NgrokOpErrFailedToConnectServices       = EnrichedError{"ERR_NGROK_20004", 20004, http.StatusServiceUnavailable}
+	NgrokOpErrEndpointDenied                = EnrichedError{"ERR_NGROK_20005", 20005, http.StatusForbidden}
+	NgrokOpErrFailedToCreateCSR             = EnrichedError{"ERR_NGROK_20006", 20006, http.StatusInternalServerError}
+)
+
+func NewNgrokError(origErr error, ee EnrichedError, msg string) *ngrok.Error {
+	if ngrokErr, ok := origErr.(*ngrok.Error); ok {
+		// already have an ngrok.Error
+		// overwrite the message and return
+		return &ngrok.Error{
+			Msg:     ngrokErr.Msg,
+			Details: ngrokErr.Details,
+		}
+	}
+
+	return &ngrok.Error{
+		Msg:        fmt.Sprintf("%s: %s", msg, origErr),
+		ErrorCode:  ee.name,
+		StatusCode: ee.statusCode,
+	}
+}