Deny BoundEndpoints based on allowedURLs

api/bindings/v1alpha1/boundendpoint_types.go

@@ -130,7 +130,7 @@ type BindingEndpoint struct {
 	v6.Ref `json:",inline"`
 
 	// +kubebuilder:validation:Required
-	// +kube:validation:Enum=provisioning;bound;error;unknown
+	// +kube:validation:Enum=provisioning;bound;denied;error;unknown
 	// +kubebuilder:default="unknown"
 	Status BindingEndpointStatus `json:"status"`
 
@@ -148,12 +148,13 @@ type BindingEndpoint struct {
 
 // BindingEndpointStatus is an enum that represents the status of a BindingEndpoint
 // TODO(https://github.com/ngrok-private/ngrok/issues/32666)
-// +kubebuilder:validation:Enum=unknown;provisioning;bound;error
+// +kubebuilder:validation:Enum=unknown;provisioning;denied;bound;error
 type BindingEndpointStatus string
 
 const (
 	StatusUnknown      BindingEndpointStatus = "unknown"
 	StatusProvisioning BindingEndpointStatus = "provisioning"
+	StatusDenied       BindingEndpointStatus = "denied"
 	StatusBound        BindingEndpointStatus = "bound"
 	StatusError        BindingEndpointStatus = "error"
 )

api/ngrok/v1alpha1/kubernetesoperator_types.go

@@ -62,9 +62,9 @@ type KubernetesOperatorBinding struct {
 	//   - All services in a namespace: `*.namespace`
 	//   - All namespaces: `*.*`
 	//   - Named service in all namespaces: `service.*`
-	// See: https://regex101.com/r/APbE3G/2
+	// See: https://regex101.com/r/APbE3G/4
 	// +kubebuilder:validation:Required
-	// +kubebuilder:validation:items:Pattern=`^([*]|(https?|tls|tcp)://([*]|([*]|[a-z]([-a-z0-9]{0,61}[a-z0-9])?)[.]([*]|[a-z]([-a-z0-9]{0,61}[a-z0-9])?)))$`
+	// +kubebuilder:validation:items:Pattern=`^(([*]|(https?|tls|tcp)://)?([*]|([*]|[a-z]([-a-z0-9]{0,61}[a-z0-9])?)[.]([*]|[a-z]([-a-z0-9]{0,61}[a-z0-9])?)))$`
 	AllowedURLs []string `json:"allowedURLs,omitempty"`
 
 	// The public ingress endpoint for this Kubernetes Operator

cmd/api/main.go

@@ -526,6 +526,7 @@ func enableBindingsFeatureSet(_ context.Context, opts managerOpts, mgr ctrl.Mana
 		Recorder:                     mgr.GetEventRecorderFor("endpoint-binding-poller"),
 		Namespace:                    opts.namespace,
 		KubernetesOperatorConfigName: opts.releaseName,
+		AllowedURLs:                  opts.bindings.allowedURLs,
 		TargetServiceAnnotations:     targetServiceAnnotations,
 		TargetServiceLabels:          targetServiceLabels,
 		PollingInterval:              10 * time.Second,
@@ -577,6 +578,8 @@ func createKubernetesOperator(ctx context.Context, client client.Client, opts ma
 			}
 		}
 		k8sOperator.Spec.EnabledFeatures = features
+
+		setupLog.Info("created KubernetesOperator", "name", k8sOperator.Name, "namespace", k8sOperator.Namespace, "op", fmt.Sprintf("%+v", k8sOperator.Spec.Binding))
 		return nil
 	})
 	return err

helm/ngrok-operator/tests/controller-deployment_test.yaml

@@ -222,6 +222,19 @@ tests:
   - contains:
       path: spec.template.spec.containers[0].args
       content: --zap-log-level=error
+- it: Should pass bindings allowed urls if set
+  set:
+    bindings:
+      enabled: true
+      allowedURLs:
+      - test.example
+      - http://*
+  template: controller-deployment.yaml
+  documentIndex: 0 # Document 0 is the deployment since its the first template
+  asserts:
+  - contains:
+      path: spec.template.spec.containers[0].args
+      content: --bindings-allowed-urls=test.example,http://*
 - it: Should pass log format argument if set
   set:
     log:

helm/ngrok-operator/values.yaml

@@ -281,7 +281,7 @@ bindings:
   name: ""
   description: "Created by ngrok-operator"
   allowedURLs:
-  - "*" # TODO(hkatz) only supporting this for now, implement others later
+  - "*"
   serviceAnnotations: {}
   serviceLabels: {}
   ingressEndpoint: "kubernetes-binding-ingress.ngrok.io:443"

internal/controller/bindings/boundendpoint_controller.go

@@ -65,6 +65,7 @@ const (
 	NgrokErrorUpstreamServiceCreateFailed = "ERR_NGROK_0001"
 	NgrokErrorTargetServiceCreateFailed   = "ERR_NGROK_0002"
 	NgrokErrorFailedToBind                = "ERR_NGROK_003"
+	NgrokErrorNotAllowed                  = "ERR_NGROK_004"
 )
 
 var (
@@ -179,6 +180,11 @@ func (r *BoundEndpointReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 func (r *BoundEndpointReconciler) create(ctx context.Context, cr *bindingsv1alpha1.BoundEndpoint) error {
 	targetService, upstreamService := r.convertBoundEndpointToServices(cr)
 
+	// binding is not allowed to be created
+	if !cr.Spec.Allowed {
+		return r.denyBoundEndpoint(ctx, cr)
+	}
+
 	if err := r.createUpstreamService(ctx, cr, upstreamService); err != nil {
 		return r.controller.ReconcileStatus(ctx, cr, err)
 	}
@@ -259,6 +265,15 @@ func (r *BoundEndpointReconciler) createUpstreamService(ctx context.Context, own
 func (r *BoundEndpointReconciler) update(ctx context.Context, cr *bindingsv1alpha1.BoundEndpoint) error {
 	log := ctrl.LoggerFrom(ctx)
 
+	// binding is not allowed to be created
+	if !cr.Spec.Allowed {
+		if err := r.deleteBoundEndpointServices(ctx, cr); err != nil {
+			return r.controller.ReconcileStatus(ctx, cr, err)
+		}
+
+		return r.denyBoundEndpoint(ctx, cr)
+	}
+
 	desiredTargetService, desiredUpstreamService := r.convertBoundEndpointToServices(cr)
 
 	var existingTargetService v1.Service
@@ -333,9 +348,26 @@ func (r *BoundEndpointReconciler) update(ctx context.Context, cr *bindingsv1alph
 }
 
 func (r *BoundEndpointReconciler) delete(ctx context.Context, cr *bindingsv1alpha1.BoundEndpoint) error {
+	return r.deleteBoundEndpointServices(ctx, cr)
+}
+
+// deleteBoundEndpointServices deletes the Target and Upstream Services for the BoundEndpoint
+func (r *BoundEndpointReconciler) deleteBoundEndpointServices(ctx context.Context, cr *bindingsv1alpha1.BoundEndpoint) error {
 	log := ctrl.LoggerFrom(ctx)
 
 	targetService, upstreamService := r.convertBoundEndpointToServices(cr)
+
+	targetNamespace := &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: targetService.Namespace}}
+
+	if err := r.Client.Get(ctx, types.NamespacedName{Name: targetNamespace.Name}, targetNamespace); err != nil {
+		if client.IgnoreNotFound(err) == nil {
+			return nil
+		} else {
+			log.Error(err, "Failed to get Target Namespace")
+			return err
+		}
+	}
+
 	if err := r.Client.Delete(ctx, targetService); err != nil {
 		if client.IgnoreNotFound(err) == nil {
 			return nil
@@ -555,6 +587,7 @@ func (r *BoundEndpointReconciler) tryToBindEndpoint(ctx context.Context, boundEn
 				bindErr = err
 			} else {
 				// success case: we dialed and closed the connection
+				bindErr = nil
 				break
 			}
 		}
@@ -586,3 +619,17 @@ func (r *BoundEndpointReconciler) tryToBindEndpoint(ctx context.Context, boundEn
 	setEndpointsStatus(boundEndpoint, desired)
 	return bindErr
 }
+
+// denyBoundEndpoint sets the status of the BoundEndpoint to denied
+func (r *BoundEndpointReconciler) denyBoundEndpoint(ctx context.Context, boundEndpoint *bindingsv1alpha1.BoundEndpoint) error {
+	reason := "Endpoint URI is not allowed by KubernetesOperator allowedURLs configuration"
+
+	setEndpointsStatus(boundEndpoint, &bindingsv1alpha1.BindingEndpoint{
+		Status:       bindingsv1alpha1.StatusDenied,
+		ErrorCode:    NgrokErrorNotAllowed,
+		ErrorMessage: reason,
+	})
+
+	r.Recorder.Event(boundEndpoint, v1.EventTypeWarning, "Denied", reason)
+	return r.controller.ReconcileStatus(ctx, boundEndpoint, nil)
+}

internal/controller/bindings/boundendpoint_poller.go

@@ -209,6 +209,9 @@ func (r *BoundEndpointPoller) reconcileBoundEndpointsFromAPI(ctx context.Context
 		return err
 	}
 
+	// modify the desired BoundEndpoints updating their Allow/Deny status
+	allowDenyEndpointByURL(ctx, desiredBoundEndpoints, r.allowedUrlRegexes)
+
 	// Get all current BoundEndpoint resources in the cluster.
 	var epbList bindingsv1alpha1.BoundEndpointList
 	if err := r.List(ctx, &epbList); err != nil {
@@ -377,6 +380,7 @@ func (r *BoundEndpointPoller) createBinding(ctx context.Context, desired binding
 			Namespace: r.Namespace,
 		},
 		Spec: bindingsv1alpha1.BoundEndpointSpec{
+			Allowed:     desired.Spec.Allowed,
 			EndpointURI: desired.Spec.EndpointURI,
 			Scheme:      desired.Spec.Scheme,
 			Port:        port,
@@ -455,8 +459,8 @@ func (r *BoundEndpointPoller) updateBinding(ctx context.Context, desired binding
 	desired.Spec.Target.Metadata.Annotations = r.TargetServiceAnnotations
 	desired.Spec.Target.Metadata.Labels = r.TargetServiceLabels
 
-	var existing bindingsv1alpha1.BoundEndpoint
-	err := r.Get(ctx, client.ObjectKey{Namespace: r.Namespace, Name: desiredName}, &existing)
+	existing := &bindingsv1alpha1.BoundEndpoint{}
+	err := r.Get(ctx, client.ObjectKey{Namespace: r.Namespace, Name: desiredName}, existing)
 	if err != nil {
 		if client.IgnoreNotFound(err) == nil {
 			// BoundEndpoint doesn't exist, create it on the next polling loop
@@ -469,14 +473,15 @@ func (r *BoundEndpointPoller) updateBinding(ctx context.Context, desired binding
 		}
 	}
 
-	if !boundEndpointNeedsUpdate(ctx, existing, desired) {
+	if !boundEndpointNeedsUpdate(ctx, *existing, desired) {
 		log.Info("BoundEndpoint already matches existing state, skipping update...", "name", desiredName, "uri", desired.Spec.EndpointURI)
 		return nil
 	}
 
 	// found existing endpoint
 	// now let's merge them together
-	toUpdate := &existing
+	toUpdate := existing
+	toUpdate.Spec.Allowed = desired.Spec.Allowed
 	toUpdate.Spec.Port = existing.Spec.Port // keep the same port
 	toUpdate.Spec.Scheme = desired.Spec.Scheme
 	toUpdate.Spec.Target = desired.Spec.Target
@@ -618,7 +623,8 @@ func allowDenyEndpointByURL(ctx context.Context, endpoints ngrokapi.AggregatedEn
 func boundEndpointNeedsUpdate(ctx context.Context, existing bindingsv1alpha1.BoundEndpoint, desired bindingsv1alpha1.BoundEndpoint) bool {
 	log := ctrl.LoggerFrom(ctx)
 
-	hasSpecChanged := existing.Spec.Scheme != desired.Spec.Scheme ||
+	hasSpecChanged := existing.Spec.Allowed != desired.Spec.Allowed ||
+		existing.Spec.Scheme != desired.Spec.Scheme ||
 		existing.Spec.Target.Port != desired.Spec.Target.Port ||
 		existing.Spec.Target.Protocol != desired.Spec.Target.Protocol ||
 		existing.Spec.Target.Service != desired.Spec.Target.Service ||