Skip to content

Commit

Permalink
sort wasm 'policies' within the wasm plugin config by hostname from m…
Browse files Browse the repository at this point in the history
…ost specific to least specific

Signed-off-by: Guilherme Cassolato <guicassolato@gmail.com>
  • Loading branch information
guicassolato committed Oct 12, 2024
1 parent 780365f commit 6e04e34
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 39 deletions.
37 changes: 27 additions & 10 deletions controllers/istio_extension_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"sort"
"sync"

"github.com/samber/lo"
Expand All @@ -23,6 +24,7 @@ import (
kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1"
kuadrantv1beta3 "github.com/kuadrant/kuadrant-operator/api/v1beta3"
kuadrantistio "github.com/kuadrant/kuadrant-operator/pkg/istio"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
"github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm"
)
Expand Down Expand Up @@ -135,15 +137,15 @@ func (r *istioExtensionReconciler) Reconcile(ctx context.Context, _ []controller
func (r *istioExtensionReconciler) buildWasmPoliciesPerGateway(ctx context.Context, state *sync.Map) (map[string][]wasm.Policy, error) {
logger := controller.LoggerFromContext(ctx).WithName("istioExtensionReconciler").WithName("buildWasmPolicies")

Check warning on line 138 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L137-L138

Added lines #L137 - L138 were not covered by tests

wasmPolicies := make(map[string][]wasm.Policy)

effectivePolicies, ok := state.Load(StateEffectiveRateLimitPolicies)
if !ok {
return wasmPolicies, ErrMissingStateEffectiveRateLimitPolicies
return nil, ErrMissingStateEffectiveRateLimitPolicies

Check warning on line 142 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L140-L142

Added lines #L140 - L142 were not covered by tests
}

logger.V(1).Info("building wasm policies for istio extension", "effectivePolicies", len(effectivePolicies.(EffectiveRateLimitPolicies)))

Check warning on line 145 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L145

Added line #L145 was not covered by tests

wasmPolicies := make(map[string]kuadrantgatewayapi.SortableHTTPRouteRuleConfigs)

Check warning on line 147 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L147

Added line #L147 was not covered by tests

// build wasm config for effective rate limit policies
for pathID, effectivePolicy := range effectivePolicies.(EffectiveRateLimitPolicies) {

Check warning on line 150 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L150

Added line #L150 was not covered by tests
// assumes the path is always [gatewayclass, gateway, listener, httproute, httprouterule]
Expand All @@ -159,7 +161,6 @@ func (r *istioExtensionReconciler) buildWasmPoliciesPerGateway(ctx context.Conte
}

limitsNamespace := wasm.LimitsNamespaceFromRoute(httpRoute.HTTPRoute)

Check warning on line 163 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L163

Added line #L163 was not covered by tests
hostnames := hostnamesFromListenerAndHTTPRoute(listener, httpRoute)

var wasmRules []wasm.Rule
for limitKey, mergeableLimit := range effectivePolicy.Spec.Rules() {
Expand All @@ -176,14 +177,30 @@ func (r *istioExtensionReconciler) buildWasmPoliciesPerGateway(ctx context.Conte
wasmRules = append(wasmRules, wasmRule)

Check warning on line 177 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L174-L177

Added lines #L174 - L177 were not covered by tests
}

wasmPolicies[gateway.GetLocator()] = append(wasmPolicies[gateway.GetLocator()], wasm.Policy{
Name: pathID,
Hostnames: utils.HostnamesToStrings(hostnames),
Rules: wasmRules, // we may need to sort the rule from the most specific to the least specific
})
hostnames := hostnamesFromListenerAndHTTPRoute(listener, httpRoute)

Check warning on line 180 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L180

Added line #L180 was not covered by tests

wasmPolicies[gateway.GetLocator()] = append(wasmPolicies[gateway.GetLocator()], lo.Map(hostnames, func(hostname gatewayapiv1.Hostname, i int) kuadrantgatewayapi.HTTPRouteRuleConfig {
return kuadrantgatewayapi.HTTPRouteRuleConfig{
HTTPRouteRule: *httpRouteRule.HTTPRouteRule,
Hostname: string(hostname),
Config: wasm.Policy{
Name: fmt.Sprintf("%s-%d", pathID, i),
Hostnames: []string{string(hostname)},
Rules: wasmRules,
},

Check warning on line 190 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L182-L190

Added lines #L182 - L190 were not covered by tests
}
})...)

Check warning on line 192 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L192

Added line #L192 was not covered by tests
}

return wasmPolicies, nil
return lo.MapValues(wasmPolicies, func(configs kuadrantgatewayapi.SortableHTTPRouteRuleConfigs, _ string) []wasm.Policy {
sortedConfigs := make(kuadrantgatewayapi.SortableHTTPRouteRuleConfigs, len(configs))
copy(sortedConfigs, configs)
sort.Sort(sortedConfigs)
return lo.Map(sortedConfigs, func(c kuadrantgatewayapi.HTTPRouteRuleConfig, _ int) wasm.Policy {
wasmPolicy, _ := c.Config.(wasm.Policy)
return wasmPolicy
})

Check warning on line 202 in controllers/istio_extension_reconciler.go

View check run for this annotation

Codecov / codecov/patch

controllers/istio_extension_reconciler.go#L195-L202

Added lines #L195 - L202 were not covered by tests
}), nil
}

// buildWasmPluginForGateway reconciles the WasmPlugin custom resource for a given gateway and slice of wasm policies
Expand Down
17 changes: 17 additions & 0 deletions pkg/library/gatewayapi/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,20 @@ func GetGatewayParentKeys(route *gatewayapiv1.HTTPRoute) []client.ObjectKey {
}
})
}

// HTTPRouteRuleConfig stores any config associated to an HTTPRouteRule
type HTTPRouteRuleConfig struct {
Hostname string
HTTPRouteRule gatewayapiv1.HTTPRouteRule
Config any
}

// SortableHTTPRouteRuleConfigs is a slice of HTTPRouteRuleConfig that implements sort.Interface
type SortableHTTPRouteRuleConfigs []HTTPRouteRuleConfig

func (c SortableHTTPRouteRuleConfigs) Len() int { return len(c) }
func (c SortableHTTPRouteRuleConfigs) Swap(i, j int) { c[i], c[j] = c[j], c[i] }
func (c SortableHTTPRouteRuleConfigs) Less(i, j int) bool {
return utils.CompareHostnamesSpecificity(c[i].Hostname, c[j].Hostname)

Check warning on line 211 in pkg/library/gatewayapi/utils.go

View check run for this annotation

Codecov / codecov/patch

pkg/library/gatewayapi/utils.go#L208-L211

Added lines #L208 - L211 were not covered by tests
// TODO: implement the rest of the comparison – problem: HTTPRouteRules have multiple HTTPRouteMatches, we can sort HTTPRouteMatches but not HTTPRouteRules (https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteRule)
}
22 changes: 22 additions & 0 deletions pkg/library/utils/hostname.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,25 @@ func HostnamesToStrings(hostnames []gatewayapiv1.Hostname) []string {
return string(hostname)
})
}

// SortableHostnames is a slice of hostnames that can be sorted from the most specific to the least specific
type SortableHostnames []string

func (h SortableHostnames) Len() int { return len(h) }
func (h SortableHostnames) Swap(i, j int) { h[i], h[j] = h[j], h[i] }
func (h SortableHostnames) Less(i, j int) bool { return CompareHostnamesSpecificity(h[i], h[j]) }

// CompareHostnamesSpecificity returns true if hostname1 is more specific than hostname2
func CompareHostnamesSpecificity(hostname1, hostname2 string) bool {
labels1 := len(strings.Split(hostname1, "."))
labels2 := len(strings.Split(hostname2, "."))
if labels1 != labels2 {
return labels1 > labels2
}
hasWildcard1 := strings.HasPrefix(hostname1, "*")
hasWildcard2 := strings.HasPrefix(hostname2, "*")
if hasWildcard1 != hasWildcard2 {
return !hasWildcard1
}
return hostname1 < hostname2
}
45 changes: 45 additions & 0 deletions pkg/library/utils/hostname_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package utils

import (
"reflect"
"sort"
"testing"

gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand Down Expand Up @@ -128,3 +129,47 @@ func TestHostnamesToStrings(t *testing.T) {
})
}
}

func TestSortableHostnames(t *testing.T) {
testCases := []struct {
name string
inputHostnames []string
expectedOutput []string
}{
{
name: "when input is empty then return empty output",
inputHostnames: []string{},
expectedOutput: []string{},
},
{
name: "when input has a single precise hostname then return the hostname",
inputHostnames: []string{"example.com"},
expectedOutput: []string{"example.com"},
},
{
name: "when input has multiple precise hostnames then return the hostnames ordered lexicographically",
inputHostnames: []string{"example.com", "test.com", "localhost"},
expectedOutput: []string{"example.com", "test.com", "localhost"},
},
{
name: "when input has precise and wildcard hostnames then return the hostnames ordered from most specific to least specific",
inputHostnames: []string{"*.com", "*.example.com", "*", "other.example.com"},
expectedOutput: []string{"other.example.com", "*.example.com", "*.com", "*"},
},
{
name: "when input contains repeated hostnames then return the equal hostnames adjacent to each other",
inputHostnames: []string{"*.com", "other.example.com", "*.example.com", "*", "*.com", "*.example.com", "other.example.com"},
expectedOutput: []string{"other.example.com", "other.example.com", "*.example.com", "*.example.com", "*.com", "*.com", "*"},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
hostnames := SortableHostnames(tc.inputHostnames)
sort.Sort(hostnames)
if !reflect.DeepEqual(tc.expectedOutput, tc.expectedOutput) {
t.Errorf("Unexpected output. Expected %v but got %v", tc.expectedOutput, tc.expectedOutput)
}
})
}
}
92 changes: 63 additions & 29 deletions pkg/rlptools/wasm/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"errors"

"github.com/samber/lo"
_struct "google.golang.org/protobuf/types/known/structpb"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand Down Expand Up @@ -127,10 +126,17 @@ type Condition struct {
}

func (c *Condition) EqualTo(other Condition) bool {
return len(c.AllOf) == len(other.AllOf) &&
lo.EveryBy(c.AllOf, func(expression PatternExpression) bool {
return lo.ContainsBy(other.AllOf, expression.EqualTo)
})
if len(c.AllOf) != len(other.AllOf) {
return false

Check warning on line 130 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L128-L130

Added lines #L128 - L130 were not covered by tests
}

for i := range c.AllOf {
if !c.AllOf[i].EqualTo(other.AllOf[i]) {
return false

Check warning on line 135 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L133-L135

Added lines #L133 - L135 were not covered by tests
}
}

return true

Check warning on line 139 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L139

Added line #L139 was not covered by tests
}

type Rule struct {
Expand All @@ -144,14 +150,23 @@ type Rule struct {
}

func (r *Rule) EqualTo(other Rule) bool {
return len(r.Conditions) == len(other.Conditions) &&
len(r.Actions) == len(other.Actions) &&
lo.EveryBy(r.Conditions, func(condition Condition) bool {
return lo.ContainsBy(other.Conditions, condition.EqualTo)
}) &&
lo.EveryBy(r.Actions, func(action Action) bool {
return lo.ContainsBy(other.Actions, action.EqualTo)
})
if len(r.Conditions) != len(other.Conditions) || len(r.Actions) != len(other.Actions) {
return false

Check warning on line 154 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L152-L154

Added lines #L152 - L154 were not covered by tests
}

for i := range r.Conditions {
if !r.Conditions[i].EqualTo(other.Conditions[i]) {
return false

Check warning on line 159 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L157-L159

Added lines #L157 - L159 were not covered by tests
}
}

for i := range r.Actions {
if !r.Actions[i].EqualTo(other.Actions[i]) {
return false

Check warning on line 165 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L163-L165

Added lines #L163 - L165 were not covered by tests
}
}

return true

Check warning on line 169 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L169

Added line #L169 was not covered by tests
}

type Policy struct {
Expand All @@ -164,13 +179,23 @@ type Policy struct {
}

func (p *Policy) EqualTo(other Policy) bool {
return p.Name == other.Name &&
len(p.Hostnames) == len(other.Hostnames) &&
len(p.Rules) == len(other.Rules) &&
lo.Every(p.Hostnames, other.Hostnames) &&
lo.EveryBy(p.Rules, func(rule Rule) bool {
return lo.ContainsBy(other.Rules, rule.EqualTo)
})
if p.Name != other.Name || len(p.Hostnames) != len(other.Hostnames) || len(p.Rules) != len(other.Rules) {
return false

Check warning on line 183 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L181-L183

Added lines #L181 - L183 were not covered by tests
}

for i := range p.Hostnames {
if p.Hostnames[i] != other.Hostnames[i] {
return false

Check warning on line 188 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L186-L188

Added lines #L186 - L188 were not covered by tests
}
}

for i := range p.Rules {
if !p.Rules[i].EqualTo(other.Rules[i]) {
return false

Check warning on line 194 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L192-L194

Added lines #L192 - L194 were not covered by tests
}
}

return true

Check warning on line 198 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L198

Added line #L198 was not covered by tests
}

type Action struct {
Expand All @@ -182,12 +207,17 @@ type Action struct {
}

func (a *Action) EqualTo(other Action) bool {
return a.Scope == other.Scope &&
a.ExtensionName == other.ExtensionName &&
len(a.Data) == len(other.Data) &&
lo.EveryBy(a.Data, func(data DataType) bool {
return lo.ContainsBy(other.Data, data.EqualTo)
})
if a.Scope != other.Scope || a.ExtensionName != other.ExtensionName || len(a.Data) != len(other.Data) {
return false

Check warning on line 211 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L209-L211

Added lines #L209 - L211 were not covered by tests
}

for i := range a.Data {
if !a.Data[i].EqualTo(other.Data[i]) {
return false

Check warning on line 216 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L214-L216

Added lines #L214 - L216 were not covered by tests
}
}

return true

Check warning on line 220 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L220

Added line #L220 was not covered by tests
}

// +kubebuilder:validation:Enum:=ratelimit;auth
Expand Down Expand Up @@ -254,9 +284,13 @@ func (c *Config) EqualTo(other *Config) bool {
}
}

return lo.EveryBy(c.Policies, func(policy Policy) bool {
return lo.ContainsBy(other.Policies, policy.EqualTo)
})
for i := range c.Policies {
if !c.Policies[i].EqualTo(other.Policies[i]) {
return false

Check warning on line 289 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L287-L289

Added lines #L287 - L289 were not covered by tests
}
}

return true

Check warning on line 293 in pkg/rlptools/wasm/types.go

View check run for this annotation

Codecov / codecov/patch

pkg/rlptools/wasm/types.go#L293

Added line #L293 was not covered by tests
}

func ConfigFromStruct(structure *_struct.Struct) (*Config, error) {
Expand Down

0 comments on commit 6e04e34

Please sign in to comment.