Skip to content

Commit 69262e2

Browse files
JorresJorres
committed
wip: refactor api
1 parent 62d1ca7 commit 69262e2

File tree

12 files changed

+134
-69
lines changed

12 files changed

+134
-69
lines changed

‎api/v1alpha1/const.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ const (
99
DefaultDomainName = "cluster.local"
1010
DNSDomainAnnotation = "dns.domain"
1111

12-
GRPCPort = 2135
13-
GRPCServicePortName = "grpc"
14-
GRPCServiceAdditionalPortName = "additional-grpc"
15-
GRPCProto = "grpc://"
16-
GRPCSProto = "grpcs://"
17-
GRPCServiceFQDNFormat = "%s-grpc.%s.svc.%s"
12+
GRPCPort = 2135
13+
GRPCServicePortName = "grpc"
14+
GRPCServiceInsecurePortName = "insecure-grpc"
15+
GRPCProto = "grpc://"
16+
GRPCSProto = "grpcs://"
17+
GRPCServiceFQDNFormat = "%s-grpc.%s.svc.%s"
1818

1919
InterconnectPort = 19001
2020
InterconnectServicePortName = "interconnect"

‎api/v1alpha1/service_types.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ type TLSConfiguration struct {
2020
type GRPCService struct {
2121
Service `json:""`
2222

23-
AdditionalPort int32 `json:"additionalPort,omitempty"`
24-
Port int32 `json:"port,omitempty"`
23+
InsecurePort int32 `json:"insecurePort,omitempty"`
24+
Port int32 `json:"port,omitempty"`
2525

2626
TLSConfiguration *TLSConfiguration `json:"tls,omitempty"`
2727
ExternalHost string `json:"externalHost,omitempty"`

‎api/v1alpha1/storage_webhook.go‎

Lines changed: 72 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import (
44
"context"
55
"fmt"
66
"math/rand"
7-
"reflect"
8-
"sort"
97

108
"github.com/golang-jwt/jwt/v4"
119
"github.com/google/go-cmp/cmp"
@@ -453,39 +451,90 @@ func (r *Storage) ValidateUpdate(old runtime.Object) error {
453451
}
454452

455453
func (r *Storage) validateGrpcPorts() error {
456-
servicePorts := []int32{}
454+
// There are three possible ways to configure grpc ports:
455+
456+
// service:
457+
// grpc: == this means one insecure port, tls is disabled
458+
// port: 2135
459+
460+
// service:
461+
// grpc:
462+
// port: 2136 == this means one secure port, tls is enabled
463+
// tls:
464+
// enabled: true
465+
466+
// service:
467+
// grpc:
468+
// insecurePort: 2135 == this means two ports, one secure \ one insecure
469+
// port: 2136
470+
// tls:
471+
// enabled: true
457472

458-
firstPort := int32(GRPCPort)
459-
if r.Spec.Service.GRPC.Port != 0 {
460-
firstPort = r.Spec.Service.GRPC.Port
461-
}
462-
servicePorts = append(servicePorts, firstPort)
463-
if r.Spec.Service.GRPC.AdditionalPort != 0 {
464-
servicePorts = append(servicePorts, r.Spec.Service.GRPC.AdditionalPort)
465-
}
466473
configuration, err := ParseConfiguration(r.Spec.Configuration)
467474
if err != nil {
468475
return fmt.Errorf("failed to parse configuration immediately after building it, should not happen, %w", err)
469476
}
470-
471-
configurationPorts := []int32{}
477+
configurationPort := int32(GRPCPort)
472478
if configuration.GrpcConfig.Port != 0 {
473-
configurationPorts = append(configurationPorts, configuration.GrpcConfig.Port)
479+
configurationPort = configuration.GrpcConfig.Port
474480
}
481+
configurationSslPort := int32(0)
475482
if configuration.GrpcConfig.SslPort != 0 {
476-
configurationPorts = append(configurationPorts, configuration.GrpcConfig.SslPort)
483+
configurationSslPort = configuration.GrpcConfig.SslPort
477484
}
478485

479-
sort.Slice(servicePorts, func(i, j int) bool {
480-
return servicePorts[i] < servicePorts[j]
481-
})
486+
if !r.Spec.Service.GRPC.TLSConfiguration.Enabled {
487+
// there should be only 1 port, both in service and in config, insecure
488+
servicePort := int32(GRPCPort)
489+
if r.Spec.Service.GRPC.Port != 0 {
490+
servicePort = r.Spec.Service.GRPC.Port
491+
}
492+
if configurationPort != servicePort {
493+
return fmt.Errorf(
494+
"inconsistent grpc ports: spec.service.grpc.port (%v) != configuration.grpc_config.port (%v)",
495+
servicePort,
496+
configurationPort,
497+
)
498+
}
482499

483-
sort.Slice(configurationPorts, func(i, j int) bool {
484-
return configurationPorts[i] < configurationPorts[j]
485-
})
500+
if r.Spec.Service.GRPC.InsecurePort != 0 {
501+
return fmt.Errorf(
502+
"spec.service.grpc.tls.enabled is false, use `port` instead of `insecurePort` field to assign non-tls grpc port",
503+
)
504+
}
505+
return nil
506+
}
507+
508+
// otherwise, there might be 1 (secure only) port...
509+
servicePort := int32(GRPCPort)
510+
if r.Spec.Service.GRPC.Port != 0 {
511+
servicePort = r.Spec.Service.GRPC.Port
512+
}
513+
if configurationSslPort == 0 {
514+
return fmt.Errorf(
515+
"configuration.grpc_config.ssl_port is absent in cluster configuration, but spec.service.grpc has tls enabled and port %v",
516+
servicePort,
517+
)
518+
}
519+
if configurationSslPort != servicePort {
520+
return fmt.Errorf(
521+
"inconsistent grpc ports: spec.service.grpc.port (%v) != configuration.grpc_config.ssl_port (%v)",
522+
servicePort,
523+
configurationSslPort,
524+
)
525+
}
526+
527+
// or, optionally, one more: insecure port
528+
if r.Spec.Service.GRPC.InsecurePort != 0 {
529+
serviceInsecurePort := r.Spec.Service.GRPC.InsecurePort
486530

487-
if !reflect.DeepEqual(servicePorts, configurationPorts) {
488-
return fmt.Errorf("grpc port mismatch: %v in spec.service.grpc, %v in YDB configuration", servicePorts, configurationPorts)
531+
if configurationPort != serviceInsecurePort {
532+
return fmt.Errorf(
533+
"inconsistent grpc insecure ports: spec.service.grpc.insecure_port (%v) != configuration.grpc_config.port (%v)",
534+
serviceInsecurePort,
535+
configurationPort,
536+
)
537+
}
489538
}
490539

491540
return nil

‎deploy/ydb-operator/crds/database.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4073,14 +4073,14 @@ spec:
40734073
additionalProperties:
40744074
type: string
40754075
type: object
4076-
additionalPort:
4077-
format: int32
4078-
type: integer
40794076
externalHost:
40804077
type: string
40814078
externalPort:
40824079
format: int32
40834080
type: integer
4081+
insecurePort:
4082+
format: int32
4083+
type: integer
40844084
ipDiscovery:
40854085
properties:
40864086
enabled:

‎deploy/ydb-operator/crds/databasenodeset.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2685,14 +2685,14 @@ spec:
26852685
additionalProperties:
26862686
type: string
26872687
type: object
2688-
additionalPort:
2689-
format: int32
2690-
type: integer
26912688
externalHost:
26922689
type: string
26932690
externalPort:
26942691
format: int32
26952692
type: integer
2693+
insecurePort:
2694+
format: int32
2695+
type: integer
26962696
ipDiscovery:
26972697
properties:
26982698
enabled:

‎deploy/ydb-operator/crds/remotedatabasenodeset.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2686,14 +2686,14 @@ spec:
26862686
additionalProperties:
26872687
type: string
26882688
type: object
2689-
additionalPort:
2690-
format: int32
2691-
type: integer
26922689
externalHost:
26932690
type: string
26942691
externalPort:
26952692
format: int32
26962693
type: integer
2694+
insecurePort:
2695+
format: int32
2696+
type: integer
26972697
ipDiscovery:
26982698
properties:
26992699
enabled:

‎deploy/ydb-operator/crds/remotestoragenodeset.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2713,14 +2713,14 @@ spec:
27132713
additionalProperties:
27142714
type: string
27152715
type: object
2716-
additionalPort:
2717-
format: int32
2718-
type: integer
27192716
externalHost:
27202717
type: string
27212718
externalPort:
27222719
format: int32
27232720
type: integer
2721+
insecurePort:
2722+
format: int32
2723+
type: integer
27242724
ipDiscovery:
27252725
properties:
27262726
enabled:

‎deploy/ydb-operator/crds/storage.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5214,14 +5214,14 @@ spec:
52145214
additionalProperties:
52155215
type: string
52165216
type: object
5217-
additionalPort:
5218-
format: int32
5219-
type: integer
52205217
externalHost:
52215218
type: string
52225219
externalPort:
52235220
format: int32
52245221
type: integer
5222+
insecurePort:
5223+
format: int32
5224+
type: integer
52255225
ipDiscovery:
52265226
properties:
52275227
enabled:

‎deploy/ydb-operator/crds/storagenodeset.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2712,14 +2712,14 @@ spec:
27122712
additionalProperties:
27132713
type: string
27142714
type: object
2715-
additionalPort:
2716-
format: int32
2717-
type: integer
27182715
externalHost:
27192716
type: string
27202717
externalPort:
27212718
format: int32
27222719
type: integer
2720+
insecurePort:
2721+
format: int32
2722+
type: integer
27232723
ipDiscovery:
27242724
properties:
27252725
enabled:

‎internal/controllers/storage/controller_test.go‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ var _ = Describe("Storage controller medium tests", func() {
317317

318318
storage.Spec.Service.GRPC.Port = 2137
319319

320-
configWithNewPorts, err := patchGRPCPortsInConfiguration(storage.Spec.Configuration, 2137, -1)
320+
configWithNewPorts, err := patchGRPCPortsInConfiguration(storage.Spec.Configuration, -1, 2137)
321321
Expect(err).To(BeNil())
322322
storage.Spec.Configuration = configWithNewPorts
323323

@@ -348,17 +348,23 @@ var _ = Describe("Storage controller medium tests", func() {
348348
)
349349
})
350350

351-
By("Checking additionalPort propagation in GRPC Service...", func() {
351+
By("Checking insecurePort propagation in GRPC Service...", func() {
352352
storage := v1alpha1.Storage{}
353353
Expect(k8sClient.Get(ctx, types.NamespacedName{
354354
Name: testobjects.StorageName,
355355
Namespace: testobjects.YdbNamespace,
356356
}, &storage)).Should(Succeed())
357357

358358
storage.Spec.Service.GRPC.Port = v1alpha1.GRPCPort
359-
storage.Spec.Service.GRPC.AdditionalPort = 2136
359+
storage.Spec.Service.GRPC.InsecurePort = 2136
360+
storage.Spec.Service.GRPC.TLSConfiguration.Enabled = true
361+
362+
configWithNewPorts, err := patchGRPCPortsInConfiguration(
363+
storage.Spec.Configuration,
364+
storage.Spec.Service.GRPC.Port,
365+
storage.Spec.Service.GRPC.InsecurePort,
366+
)
360367

361-
configWithNewPorts, err := patchGRPCPortsInConfiguration(storage.Spec.Configuration, 2135, 2136)
362368
Expect(err).To(BeNil())
363369
storage.Spec.Configuration = configWithNewPorts
364370

@@ -382,9 +388,9 @@ var _ = Describe("Storage controller medium tests", func() {
382388
g.Expect(len(ports)).To(Equal(2), "expected 2 ports but got %d", len(ports))
383389
g.Expect(ports[0].Port).To(Equal(int32(v1alpha1.GRPCPort)))
384390
g.Expect(ports[0].Name).To(Equal(v1alpha1.GRPCServicePortName))
385-
g.Expect(ports[1].Port).To(Equal(storage.Spec.Service.GRPC.AdditionalPort))
386-
g.Expect(ports[1].Name).To(Equal(v1alpha1.GRPCServiceAdditionalPortName))
387-
g.Expect(ports[1].TargetPort.IntVal).To(Equal(storage.Spec.Service.GRPC.AdditionalPort))
391+
g.Expect(ports[1].Port).To(Equal(storage.Spec.Service.GRPC.InsecurePort))
392+
g.Expect(ports[1].Name).To(Equal(v1alpha1.GRPCServiceInsecurePortName))
393+
g.Expect(ports[1].TargetPort.IntVal).To(Equal(storage.Spec.Service.GRPC.InsecurePort))
388394
return nil
389395
}, test.Timeout, test.Interval).Should(Succeed(),
390396
"Service %s/%s should eventually have proper ports", testobjects.YdbNamespace, serviceName,
@@ -398,38 +404,48 @@ var _ = Describe("Storage controller medium tests", func() {
398404
Namespace: testobjects.YdbNamespace,
399405
}, &storage)).Should(Succeed())
400406

407+
storage.Spec.Service.GRPC.TLSConfiguration.Enabled = true
408+
401409
storage.Spec.Service.GRPC.Port = v1alpha1.GRPCPort
402410
By("Specify 2136 in manifest spec...")
403-
storage.Spec.Service.GRPC.AdditionalPort = 2136
411+
storage.Spec.Service.GRPC.InsecurePort = 2136
404412

405413
By("And then specify 2137 in manifest spec...")
406414
configWithNewPorts, err := patchGRPCPortsInConfiguration(storage.Spec.Configuration, v1alpha1.GRPCPort, 2137)
407415
Expect(err).To(BeNil())
408416
storage.Spec.Configuration = configWithNewPorts
409417

410418
err = k8sClient.Update(ctx, &storage)
411-
Expect(err).To(MatchError(ContainSubstring("grpc port mismatch")))
419+
Expect(err).To(MatchError(ContainSubstring(
420+
"inconsistent grpc insecure ports: spec.service.grpc.insecure_port (2136) != configuration.grpc_config.port (2137)",
421+
)))
412422
})
413423
})
414424
})
415425

416-
func patchGRPCPortsInConfiguration(in string, port, sslPort int) (string, error) {
417-
m := make(map[string]interface{})
426+
func patchGRPCPortsInConfiguration(in string, sslPort, port int32) (string, error) {
427+
m := make(map[string]any)
418428
if err := yaml.Unmarshal([]byte(in), &m); err != nil {
419429
return "", err
420430
}
421431

422-
cfg, _ := m["grpc_config"].(map[string]interface{})
432+
cfg, _ := m["grpc_config"].(map[string]any)
423433
if cfg == nil {
424-
cfg = make(map[string]interface{})
434+
cfg = make(map[string]any)
425435
}
426436

427437
if sslPort != -1 {
428438
cfg["ssl_port"] = sslPort
439+
} else {
440+
delete(cfg, "ssl_port")
429441
}
442+
430443
if port != -1 {
431444
cfg["port"] = port
445+
} else {
446+
delete(cfg, "port")
432447
}
448+
433449
m["grpc_config"] = cfg
434450

435451
res, err := yaml.Marshal(m)

0 commit comments

Comments
 (0)