From 0a339b413c492159a9c222cda64aa2293d76fcc9 Mon Sep 17 00:00:00 2001 From: Adam Janikowski <12255597+ajanikow@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:18:05 +0100 Subject: [PATCH] [Bugfix] Fix Gateway Options (#1753) --- CHANGELOG.md | 1 + .../templates/deployment-operator/role.yaml | 3 - .../templates/networking-operator/role.yaml | 57 ++----------- pkg/apis/networking/v1alpha1/route_spec.go | 2 +- .../resources/gateway/gateway_config.go | 8 +- .../gateway/gateway_config_destination.go | 26 ++++++ .../resources/gateway/gateway_config_sni.go | 4 +- .../resources/gateway/gateway_config_test.go | 83 ++++++++++++++++++- pkg/deployment/resources/gateway/marshal.go | 10 +++ pkg/scheduler/profiles.go | 10 ++- 10 files changed, 143 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8028ddec7..9b288f17a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - (Feature) (Integration) SchedulerV2 - (Feature) (Integration) Basic Envs - (Maintenance) Inspector Generics +- (Bugfix) Fix Gateway Options ## [1.2.43](https://github.com/arangodb/kube-arangodb/tree/1.2.43) (2024-10-14) - (Feature) ArangoRoute CRD diff --git a/chart/kube-arangodb/templates/deployment-operator/role.yaml b/chart/kube-arangodb/templates/deployment-operator/role.yaml index 18d5e8582..ad6fd3796 100644 --- a/chart/kube-arangodb/templates/deployment-operator/role.yaml +++ b/chart/kube-arangodb/templates/deployment-operator/role.yaml @@ -16,9 +16,6 @@ rules: - apiGroups: ["database.arangodb.com"] resources: ["arangodeployments", "arangodeployments/status","arangomembers", "arangomembers/status"] verbs: ["*"] - - apiGroups: ["networking.arangodb.com"] - resources: ["arangoroutes", "arangoroutes/status"] - verbs: ["*"] {{- if .Values.rbac.extensions.acs }} - apiGroups: ["database.arangodb.com"] resources: ["arangoclustersynchronizations", "arangoclustersynchronizations/status"] diff --git a/chart/kube-arangodb/templates/networking-operator/role.yaml b/chart/kube-arangodb/templates/networking-operator/role.yaml index 38339a312..3da6b9dff 100644 --- a/chart/kube-arangodb/templates/networking-operator/role.yaml +++ b/chart/kube-arangodb/templates/networking-operator/role.yaml @@ -13,56 +13,11 @@ metadata: app.kubernetes.io/instance: {{ .Release.Name }} release: {{ .Release.Name }} rules: - - apiGroups: - - "ml.arangodb.com" - resources: - - "arangomlextensions" - - "arangomlextensions/status" - - "arangomlbatchjobs" - - "arangomlbatchjobs/status" - - "arangomlcronjobs" - - "arangomlcronjobs/status" - - "arangomlstorages" - - "arangomlstorages/status" - verbs: - - "*" - - apiGroups: - - "scheduler.arangodb.com" - resources: - - "arangoprofiles" - - "arangoprofiles/status" - verbs: - - "*" - - apiGroups: - - "database.arangodb.com" - resources: - - "arangodeployments" - verbs: - - "get" - - "list" - - "watch" - - apiGroups: - - "rbac.authorization.k8s.io" - resources: - - "roles" - - "rolebindings" - verbs: ["*"] - - apiGroups: - - "batch" - resources: - - "cronjobs" - - "jobs" - verbs: ["*"] - - apiGroups: ["apps"] - resources: - - "statefulsets" - verbs: ["*"] - - apiGroups: [""] - resources: - - "pods" - - "secrets" - - "services" - - "serviceaccounts" - verbs: ["*"] + - apiGroups: ["networking.arangodb.com"] + resources: ["arangoroutes", "arangoroutes/status"] + verbs: ["*"] + - apiGroups: [""] + resources: ["pods", "services", "endpoints"] + verbs: ["get", "list", "watch"] {{- end }} {{- end }} \ No newline at end of file diff --git a/pkg/apis/networking/v1alpha1/route_spec.go b/pkg/apis/networking/v1alpha1/route_spec.go index 6f4fc16e5..85bb6348f 100644 --- a/pkg/apis/networking/v1alpha1/route_spec.go +++ b/pkg/apis/networking/v1alpha1/route_spec.go @@ -34,7 +34,7 @@ type ArangoRouteSpec struct { } func (s *ArangoRouteSpec) GetDeployment() string { - if s == nil || s.Destination == nil { + if s == nil || s.Deployment == nil { return "" } diff --git a/pkg/deployment/resources/gateway/gateway_config.go b/pkg/deployment/resources/gateway/gateway_config.go index e7c360e61..eb61cd463 100644 --- a/pkg/deployment/resources/gateway/gateway_config.go +++ b/pkg/deployment/resources/gateway/gateway_config.go @@ -170,7 +170,13 @@ func (c Config) RenderClusters() ([]*clusterAPI.Cluster, error) { UpstreamProtocolOptions: &upstreamHttpApi.HttpProtocolOptions_ExplicitHttpConfig_{ ExplicitHttpConfig: &upstreamHttpApi.HttpProtocolOptions_ExplicitHttpConfig{ ProtocolConfig: &upstreamHttpApi.HttpProtocolOptions_ExplicitHttpConfig_Http2ProtocolOptions{ - Http2ProtocolOptions: &coreAPI.Http2ProtocolOptions{}, + Http2ProtocolOptions: &coreAPI.Http2ProtocolOptions{ + ConnectionKeepalive: &coreAPI.KeepaliveSettings{ + Interval: durationpb.New(15 * time.Second), + Timeout: durationpb.New(30 * time.Second), + ConnectionIdleInterval: durationpb.New(60 * time.Second), + }, + }, }, }, }, diff --git a/pkg/deployment/resources/gateway/gateway_config_destination.go b/pkg/deployment/resources/gateway/gateway_config_destination.go index 3139b3f31..83c8b709e 100644 --- a/pkg/deployment/resources/gateway/gateway_config_destination.go +++ b/pkg/deployment/resources/gateway/gateway_config_destination.go @@ -24,8 +24,12 @@ import ( "time" clusterAPI "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" + coreAPI "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" endpointAPI "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" routeAPI "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + upstreamHttpApi "github.com/envoyproxy/go-control-plane/envoy/extensions/upstreams/http/v3" + "github.com/golang/protobuf/ptypes/any" + "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" shared "github.com/arangodb/kube-arangodb/pkg/apis/shared" @@ -115,6 +119,25 @@ func (c *ConfigDestination) RenderRoute(name, prefix string) (*routeAPI.Route, e } func (c *ConfigDestination) RenderCluster(name string) (*clusterAPI.Cluster, error) { + hpo, err := anypb.New(&upstreamHttpApi.HttpProtocolOptions{ + UpstreamProtocolOptions: &upstreamHttpApi.HttpProtocolOptions_ExplicitHttpConfig_{ + ExplicitHttpConfig: &upstreamHttpApi.HttpProtocolOptions_ExplicitHttpConfig{ + ProtocolConfig: &upstreamHttpApi.HttpProtocolOptions_ExplicitHttpConfig_Http2ProtocolOptions{ + Http2ProtocolOptions: &coreAPI.Http2ProtocolOptions{ + ConnectionKeepalive: &coreAPI.KeepaliveSettings{ + Interval: durationpb.New(15 * time.Second), + Timeout: durationpb.New(30 * time.Second), + ConnectionIdleInterval: durationpb.New(60 * time.Second), + }, + }, + }, + }, + }, + }) + if err != nil { + return nil, err + } + cluster := &clusterAPI.Cluster{ Name: name, ConnectTimeout: durationpb.New(time.Second), @@ -127,6 +150,9 @@ func (c *ConfigDestination) RenderCluster(name string) (*clusterAPI.Cluster, err }, }, }, + TypedExtensionProtocolOptions: map[string]*any.Any{ + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": hpo, + }, } if t, err := c.Type.RenderUpstreamTransportSocket(); err != nil { diff --git a/pkg/deployment/resources/gateway/gateway_config_sni.go b/pkg/deployment/resources/gateway/gateway_config_sni.go index 6eae003ab..678fc6163 100644 --- a/pkg/deployment/resources/gateway/gateway_config_sni.go +++ b/pkg/deployment/resources/gateway/gateway_config_sni.go @@ -32,8 +32,8 @@ import ( type ConfigSNIList []ConfigSNI func (c ConfigSNIList) RenderFilterChain(filters []*listenerAPI.Filter) ([]*listenerAPI.FilterChain, error) { - var r = make([]*listenerAPI.FilterChain, len(filters)) - for id := range filters { + var r = make([]*listenerAPI.FilterChain, len(c)) + for id := range c { if f, err := c[id].RenderFilterChain(filters); err != nil { return nil, err } else { diff --git a/pkg/deployment/resources/gateway/gateway_config_test.go b/pkg/deployment/resources/gateway/gateway_config_test.go index 0d0a86599..99ba60384 100644 --- a/pkg/deployment/resources/gateway/gateway_config_test.go +++ b/pkg/deployment/resources/gateway/gateway_config_test.go @@ -21,6 +21,7 @@ package gateway import ( + "fmt" "testing" bootstrapAPI "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" @@ -29,14 +30,18 @@ import ( "github.com/arangodb/kube-arangodb/pkg/util" ) -func renderAndPrintGatewayConfig(t *testing.T, cfg Config) *bootstrapAPI.Bootstrap { +func renderAndPrintGatewayConfig(t *testing.T, cfg Config, validates ...func(t *testing.T, b *bootstrapAPI.Bootstrap)) { data, checksum, obj, err := cfg.RenderYAML() require.NoError(t, err) t.Logf("Checksum: %s", checksum) t.Log(string(data)) - return obj + for id := range validates { + t.Run(fmt.Sprintf("Validation%d", id), func(t *testing.T) { + validates[id](t, obj) + }) + } } func Test_GatewayConfig(t *testing.T) { @@ -50,8 +55,27 @@ func Test_GatewayConfig(t *testing.T) { }, }, }, + }, func(t *testing.T, b *bootstrapAPI.Bootstrap) { + require.NotNil(t, b) + require.NotNil(t, b.StaticResources) + require.NotNil(t, b.StaticResources.Clusters) + require.Len(t, b.StaticResources.Clusters, 1) + require.NotNil(t, b.StaticResources.Clusters[0]) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints) + require.Len(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints, 1) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0]) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints) + require.Len(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints, 1) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints[0]) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints[0].GetEndpoint()) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints[0].GetEndpoint().Address) + require.NotNil(t, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints[0].GetEndpoint().Address.GetSocketAddress()) + require.EqualValues(t, "127.0.0.1", b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints[0].GetEndpoint().Address.GetSocketAddress().Address) + require.EqualValues(t, 12345, b.StaticResources.Clusters[0].LoadAssignment.Endpoints[0].LbEndpoints[0].GetEndpoint().Address.GetSocketAddress().GetPortValue()) }) }) + t.Run("Default", func(t *testing.T) { renderAndPrintGatewayConfig(t, Config{ DefaultDestination: ConfigDestination{ @@ -69,6 +93,7 @@ func Test_GatewayConfig(t *testing.T) { }, }) }) + t.Run("Default", func(t *testing.T) { renderAndPrintGatewayConfig(t, Config{ DefaultDestination: ConfigDestination{ @@ -87,6 +112,7 @@ func Test_GatewayConfig(t *testing.T) { }, }) }) + t.Run("Default", func(t *testing.T) { renderAndPrintGatewayConfig(t, Config{ DefaultDestination: ConfigDestination{ @@ -117,6 +143,7 @@ func Test_GatewayConfig(t *testing.T) { }, }) }) + t.Run("Default", func(t *testing.T) { renderAndPrintGatewayConfig(t, Config{ DefaultDestination: ConfigDestination{ @@ -147,6 +174,7 @@ func Test_GatewayConfig(t *testing.T) { }, }) }) + t.Run("Default", func(t *testing.T) { renderAndPrintGatewayConfig(t, Config{ DefaultDestination: ConfigDestination{ @@ -188,4 +216,55 @@ func Test_GatewayConfig(t *testing.T) { }, }) }) + + t.Run("Default", func(t *testing.T) { + renderAndPrintGatewayConfig(t, Config{ + DefaultDestination: ConfigDestination{ + Targets: []ConfigDestinationTarget{ + { + Host: "127.0.0.1", + Port: 12345, + }, + }, + Path: util.NewType("/test/path/"), + Type: util.NewType(ConfigDestinationTypeHTTPS), + }, + DefaultTLS: &ConfigTLS{ + CertificatePath: "/test", + PrivateKeyPath: "/test12", + }, + SNI: []ConfigSNI{ + { + ConfigTLS: ConfigTLS{ + CertificatePath: "/cp", + PrivateKeyPath: "/pp", + }, + ServerNames: []string{ + "example.com", + }, + }, + { + ConfigTLS: ConfigTLS{ + CertificatePath: "/c2", + PrivateKeyPath: "/p2", + }, + ServerNames: []string{ + "2.example.com", + }, + }, + }, + Destinations: ConfigDestinations{ + "/_test/": { + Targets: []ConfigDestinationTarget{ + { + Host: "127.0.0.1", + Port: 12346, + }, + }, + Path: util.NewType("/test/path/"), + Type: util.NewType(ConfigDestinationTypeHTTP), + }, + }, + }) + }) } diff --git a/pkg/deployment/resources/gateway/marshal.go b/pkg/deployment/resources/gateway/marshal.go index 69bb5b95a..16f3d3d5b 100644 --- a/pkg/deployment/resources/gateway/marshal.go +++ b/pkg/deployment/resources/gateway/marshal.go @@ -39,3 +39,13 @@ func Marshal[T proto.Message](in T) ([]byte, string, T, error) { data, err = yaml.JSONToYAML(data) return data, util.SHA256(data), in, err } + +func Unmarshal[T proto.Message](data []byte) (T, error) { + var v T + + if err := (protojson.UnmarshalOptions{}).Unmarshal(data, v); err != nil { + return util.Default[T](), err + } + + return v, nil +} diff --git a/pkg/scheduler/profiles.go b/pkg/scheduler/profiles.go index c6adef1bb..68230f2e1 100644 --- a/pkg/scheduler/profiles.go +++ b/pkg/scheduler/profiles.go @@ -81,7 +81,15 @@ func Profiles(ctx context.Context, client generic.ListInterface[*schedulerApi.Ar }) extractedProfiles = extractedProfiles.Sort(func(a, b *schedulerApi.ArangoProfile) bool { - return a.Spec.Template.GetPriority() > b.Spec.Template.GetPriority() + if ca, cb := a.Spec.Template.GetPriority(), b.Spec.Template.GetPriority(); ca != cb { + return ca > cb + } + + if ca, cb := a.GetName(), b.GetName(); ca != cb { + return ca > cb + } + + return a.GetCreationTimestamp().After(b.GetCreationTimestamp().Time) }) // Check if everything is valid