Skip to content

Commit

Permalink
Add conversion support from ingress to endpoints (#562)
Browse files Browse the repository at this point in the history
* update edges calculation to ignore endpoint ingresses/gateways

Signed-off-by: Alice-Lilith <[email protected]>

* add conversion support from ingress to endpoints

Signed-off-by: Alice-Lilith <[email protected]>

* move common helpers from driver member funcs to standalone util funcs

Signed-off-by: Alice-Lilith <[email protected]>

* add traffic policy phase name

Signed-off-by: Alice-Lilith <[email protected]>

* add rule name support to traffic policy pkg

Signed-off-by: Alice-Lilith <[email protected]>

* add ir package to assist with translation

Signed-off-by: Alice-Lilith <[email protected]>

* move endpoints annotation util to common package

Signed-off-by: Alice-Lilith <[email protected]>

* add new translator to take over config translation
I will likely pull this out into it's own package after decoupling
some more of the driver

Signed-off-by: Alice-Lilith <[email protected]>

* rework manager driver to use new config translator

Signed-off-by: Alice-Lilith <[email protected]>

* clean up manager driver utils and tests

Signed-off-by: Alice-Lilith <[email protected]>

* add translator tests

Signed-off-by: Alice-Lilith <[email protected]>

* fix: Add ingress/gateway domains to returned map

Fixes and issue where the last domain would win

(cherry picked from commit 36d332e)

* test: Add test to reproduce behavior

(cherry picked from commit ff44b31)

* fix endpoint translation merging bugs and add more test coverage

Signed-off-by: Alice-Lilith <[email protected]>

---------

Signed-off-by: Alice-Lilith <[email protected]>
Co-authored-by: Jonathan Stacks <[email protected]>
  • Loading branch information
Alice-Lilith and jonstacks authored Jan 14, 2025
1 parent 47d2d90 commit 0162b41
Show file tree
Hide file tree
Showing 22 changed files with 3,515 additions and 291 deletions.
15 changes: 15 additions & 0 deletions api/common/v1alpha1/common_types.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package common

import "strings"

type ApplicationProtocol string

const (
Expand Down Expand Up @@ -34,4 +36,17 @@ func (t ProxyProtocolVersion) IsKnown() bool {

const (
DefaultClusterDomain = "svc.cluster.local"

// When this annotation is present on an Ingress/Gateway resource and set to "true", that Ingress/Gateway
// will cause an endpoint to be created instead of an edge
AnnotationUseEndpoints = "k8s.ngrok.com/use-endpoints"
)

// hasUseEndpointsAnnotation checks whether or not a set of annotations has the correct annotation for configuring an
// ingress/gateway to use endpoints instead of edges
func HasUseEndpointsAnnotation(annotations map[string]string) bool {
if val, exists := annotations[AnnotationUseEndpoints]; exists && strings.ToLower(val) == "true" {
return true
}
return false
}
71 changes: 71 additions & 0 deletions api/common/v1alpha1/common_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package common

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestHasUseEndpointsAnnotation(t *testing.T) {
tests := []struct {
name string
annotations map[string]string
expected bool
}{
{
name: "annotation missing",
annotations: map[string]string{},
expected: false,
},
{
name: "annotation exists but value is false",
annotations: map[string]string{
AnnotationUseEndpoints: "false",
},
expected: false,
},
{
name: "annotation exists with value TRUE (case-insensitive)",
annotations: map[string]string{
AnnotationUseEndpoints: "TRUE",
},
expected: true,
},
{
name: "annotation exists with value true (lowercase)",
annotations: map[string]string{
AnnotationUseEndpoints: "true",
},
expected: true,
},
{
name: "annotation exists but with an unrelated value",
annotations: map[string]string{
AnnotationUseEndpoints: "some-random-value",
},
expected: false,
},
{
name: "multiple annotations, correct one is true",
annotations: map[string]string{
"some-other-annotation": "value",
AnnotationUseEndpoints: "true",
},
expected: true,
},
{
name: "multiple annotations, correct one is missing",
annotations: map[string]string{
"some-other-annotation": "value",
},
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := HasUseEndpointsAnnotation(tt.annotations)
assert.Equal(t, tt.expected, result, "unexpected result for test case: %s", tt.name)
})
}
}
9 changes: 7 additions & 2 deletions internal/controller/ingress/moduleset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ func (r *ModuleSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
// being watched (in our case, NgrokModuleSets). If you tail the controller
// logs and delete, update, edit ngrokmoduleset objects, you see the events come in.
func (r *ModuleSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
err := r.Driver.SyncEdges(ctx, r.Client)
return ctrl.Result{}, err
if err := r.Driver.SyncEdges(ctx, r.Client); err != nil {
return ctrl.Result{}, err
}
if err := r.Driver.SyncEndpoints(ctx, r.Client); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
9 changes: 7 additions & 2 deletions internal/controller/ngrok/ngroktrafficpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,13 @@ func (r *NgrokTrafficPolicyReconciler) Reconcile(ctx context.Context, req ctrl.R
r.Recorder.Eventf(policy, v1.EventTypeWarning, events.PolicyDeprecation, "Traffic Policy has 'enabled' set. This is a legacy option that will stop being supported soon.")
}

err = r.Driver.SyncEdges(ctx, r.Client)
return ctrl.Result{}, err
if err := r.Driver.SyncEdges(ctx, r.Client); err != nil {
return ctrl.Result{}, err
}
if err := r.Driver.SyncEndpoints(ctx, r.Client); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

// SetupWithManager sets up the controller with the Manager.
Expand Down
108 changes: 108 additions & 0 deletions internal/ir/ir.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package ir

import (
"sort"

"github.com/ngrok/ngrok-operator/internal/trafficpolicy"
netv1 "k8s.io/api/networking/v1"
)

// The IR (Intermediate Representation) package serves as a midway point when translating configuration resources into other types of config/resources

// OwningResource is a reference to a resource + kind to keep track of IR that was generated from other resources (like Ingress)
type OwningResource struct {
Kind string
Name string
Namespace string
}

// Type alias to make it more apparent when using maps what the key represents
type IRHostname string

// IRVirtualHost represents a unique hostname and all of the rotues under that hostname
type IRVirtualHost struct {
// The names of any resources (such as Ingress) that were used in the construction of this IRVirtualHost
// Currently only used for debug/error logs, but can be added to generated resource statuses
OwningResources []OwningResource
Hostname string

// Keeps track of the namespace for this hostname. Since we do not allow multiple endpoints with the same hostname, we cannot support multiple ingresses
// using the same hostname in different namespaces.
Namespace string

// This traffic policy will apply to all routes under this hostname
TrafficPolicy *trafficpolicy.TrafficPolicy
Routes []*IRRoute

// The following is used to support ingress default backends (currently only supported for endpoints and not edges)
DefaultDestination *IRDestination
}

// IRRoute is a path match paired with a destination for requests with a matching path
type IRRoute struct {
Path string
PathType netv1.PathType
Destination *IRDestination
}

// IRDestination determines what should be done with a request. One of upstream service or a traffic policy can be supplied
type IRDestination struct {
Upstream *IRUpstream
TrafficPolicy *trafficpolicy.TrafficPolicy
}

// IRUpstream is a service upstream along with a list of the ingresses that route to that service
type IRUpstream struct {
// The names of any resources (such as Ingress) that were used in the construction of this IRUpstream
// Currently only used for debug/error logs, but can be added to generated resource statuses
OwningResources []OwningResource
Service IRService
}

// IRService is an upstream service that we can route requests to
type IRService struct {
UID string // UID of the service so that we don't generate the exact same endpoints for the same service running in two different clusters
Namespace string
Name string
Port int32
}

// sortRoutes sorts the routes for an IRVirtualHost.
// 1. We always generate the same ordering of route rules for any given set of ingresses
// 2. Since traffic policy rules are executed in-order, we need to order them in a way that results in best-match routing
func (h *IRVirtualHost) SortRoutes() {
sort.SliceStable(h.Routes, func(i, j int) bool {
// Exact matches before prefix matches
if h.Routes[i].PathType != h.Routes[j].PathType {
return h.Routes[i].PathType == netv1.PathTypeExact
}

// Then, longer paths before shorter paths
if len(h.Routes[i].Path) != len(h.Routes[j].Path) {
return len(h.Routes[i].Path) > len(h.Routes[j].Path)
}

// Finally, lexicographical order
return h.Routes[i].Path < h.Routes[j].Path
})
}

// addOwningIngress will append a new namespaced name to the list of owning ingresses for a virtual host
func (h *IRVirtualHost) AddOwningResource(new OwningResource) {
for _, current := range h.OwningResources {
if current == new {
return
}
}
h.OwningResources = append(h.OwningResources, new)
}

// addOwningIngress will append a new namespaced name to the list of owning ingresses for an upstream
func (h *IRUpstream) AddOwningResource(new OwningResource) {
for _, current := range h.OwningResources {
if current == new {
return
}
}
h.OwningResources = append(h.OwningResources, new)
}
Loading

0 comments on commit 0162b41

Please sign in to comment.