Skip to content

Commit

Permalink
Add ngrok enriched errors (#555)
Browse files Browse the repository at this point in the history
* Add ngrok enriched errors

* Fix staticcheck
  • Loading branch information
hjkatz authored Jan 23, 2025
1 parent ba5c219 commit 554bb8b
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 29 deletions.
1 change: 0 additions & 1 deletion api/bindings/v1alpha1/boundendpoint_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions internal/controller/base_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"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"
Expand Down Expand Up @@ -163,6 +164,12 @@ func CtrlResultForErr(err error) (ctrl.Result, error) {
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
Expand Down
43 changes: 28 additions & 15 deletions internal/controller/bindings/boundendpoint_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ import (
"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"
)

Expand All @@ -61,12 +63,6 @@ const (
// 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 (
Expand Down Expand Up @@ -223,13 +219,15 @@ func (r *BoundEndpointReconciler) createTargetService(ctx context.Context, owner
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")
Expand All @@ -245,13 +243,15 @@ func (r *BoundEndpointReconciler) createUpstreamService(ctx context.Context, own
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")
Expand Down Expand Up @@ -589,13 +589,15 @@ func (r *BoundEndpointReconciler) tryToBindEndpoint(ctx context.Context, boundEn

// 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
Expand All @@ -610,5 +612,16 @@ func (r *BoundEndpointReconciler) tryToBindEndpoint(ctx context.Context, boundEn

// 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
}
Expand Down
16 changes: 8 additions & 8 deletions internal/controller/bindings/boundendpoint_poller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand All @@ -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: "",
Expand All @@ -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: "",
Expand Down
6 changes: 5 additions & 1 deletion internal/controller/ngrok/kubernetesoperator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ var featureMap = map[string]string{
ngrokv1alpha1.KubernetesOperatorFeatureGateway: "Gateway",
}

const (
NgrokErrorFailedToCreateCSR = "ERR_NGROK_20006"
)

// KubernetesOperatorReconciler reconciles a KubernetesOperator object
type KubernetesOperatorReconciler struct {
client.Client
Expand Down Expand Up @@ -149,7 +153,7 @@ func (r *KubernetesOperatorReconciler) create(ctx context.Context, ko *ngrokv1al
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{
Expand Down
45 changes: 45 additions & 0 deletions internal/ngrokapi/enriched_errors.go
Original file line number Diff line number Diff line change
@@ -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,
}
}

0 comments on commit 554bb8b

Please sign in to comment.