Skip to content

Commit ee52f80

Browse files
ewoutpewoutp
authored
Merge pull request #186 from arangodb/bugfix/tls-and-sync-fixes
Various TLS & Sync related fixes
2 parents 0d08a69 + 39a4479 commit ee52f80

File tree

9 files changed

+71
-68
lines changed

9 files changed

+71
-68
lines changed

‎docs/Manual/Deployment/Kubernetes/DeploymentReplicationResource.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ metadata:
3939
name: "replication-from-a-to-b"
4040
spec:
4141
source:
42-
endpoint: ["https://163.172.149.229:31888", "https://51.15.225.110:31888", "https://51.15.229.133:31888"]
42+
masterEndpoint: ["https://163.172.149.229:31888", "https://51.15.225.110:31888", "https://51.15.229.133:31888"]
4343
auth:
4444
keyfileSecretName: cluster-a-sync-auth
4545
tls:
@@ -67,7 +67,7 @@ with sync enabled.
6767

6868
This cluster configured as the replication source.
6969

70-
### `spec.source.endpoint: []string`
70+
### `spec.source.masterEndpoint: []string`
7171

7272
This setting specifies zero or more master endpoint URL's of the source cluster.
7373

@@ -108,7 +108,7 @@ with sync enabled.
108108

109109
This cluster configured as the replication destination.
110110

111-
### `spec.destination.endpoint: []string`
111+
### `spec.destination.masterEndpoint: []string`
112112

113113
This setting specifies zero or more master endpoint URL's of the destination cluster.
114114

‎pkg/apis/deployment/v1alpha/sync_external_access_spec.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import (
3535
type SyncExternalAccessSpec struct {
3636
ExternalAccessSpec
3737
MasterEndpoint []string `json:"masterEndpoint,omitempty"`
38-
AccessPackageSecretNames []string `json:accessPackageSecretNames,omitempty"`
38+
AccessPackageSecretNames []string `json:"accessPackageSecretNames,omitempty"`
3939
}
4040

4141
// GetMasterEndpoint returns the value of masterEndpoint.

‎pkg/apis/replication/v1alpha/endpoint_spec.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ type EndpointSpec struct {
3636
// DeploymentName holds the name of an ArangoDeployment resource.
3737
// If set this provides default values for masterEndpoint, auth & tls.
3838
DeploymentName *string `json:"deploymentName,omitempty"`
39-
// Endpoint holds a list of URLs used to reach the syncmaster(s).
40-
Endpoint []string `json:"endpoint,omitempty"`
39+
// MasterEndpoint holds a list of URLs used to reach the syncmaster(s).
40+
MasterEndpoint []string `json:"masterEndpoint,omitempty"`
4141
// Authentication holds settings needed to authentication at the syncmaster.
4242
Authentication EndpointAuthenticationSpec `json:"auth"`
4343
// TLS holds settings needed to verify the TLS connection to the syncmaster.
@@ -60,13 +60,13 @@ func (s EndpointSpec) Validate(isSourceEndpoint bool) error {
6060
if err := k8sutil.ValidateOptionalResourceName(s.GetDeploymentName()); err != nil {
6161
return maskAny(err)
6262
}
63-
for _, ep := range s.Endpoint {
63+
for _, ep := range s.MasterEndpoint {
6464
if _, err := url.Parse(ep); err != nil {
6565
return maskAny(errors.Wrapf(ValidationError, "Invalid master endpoint '%s': %s", ep, err))
6666
}
6767
}
6868
hasDeploymentName := s.HasDeploymentName()
69-
if !hasDeploymentName && len(s.Endpoint) == 0 {
69+
if !hasDeploymentName && len(s.MasterEndpoint) == 0 {
7070
return maskAny(errors.Wrapf(ValidationError, "Provide a deploy name or at least one master endpoint"))
7171
}
7272
if err := s.Authentication.Validate(isSourceEndpoint || !hasDeploymentName); err != nil {

‎pkg/apis/replication/v1alpha/zz_generated.deepcopy.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ func (in *EndpointSpec) DeepCopyInto(out *EndpointSpec) {
240240
**out = **in
241241
}
242242
}
243-
if in.Endpoint != nil {
244-
in, out := &in.Endpoint, &out.Endpoint
243+
if in.MasterEndpoint != nil {
244+
in, out := &in.MasterEndpoint, &out.MasterEndpoint
245245
*out = make([]string, len(*in))
246246
copy(*out, *in)
247247
}

‎pkg/deployment/context_impl.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ package deployment
2525
import (
2626
"context"
2727
"fmt"
28+
"net"
29+
"strconv"
2830

2931
"github.com/arangodb/arangosync/client"
3032
"github.com/arangodb/arangosync/tasks"
@@ -175,7 +177,11 @@ func (d *Deployment) GetSyncServerClient(ctx context.Context, group api.ServerGr
175177
dnsName := k8sutil.CreatePodDNSName(d.apiObject, group.AsRole(), id)
176178

177179
// Build client
178-
source := client.Endpoint{dnsName}
180+
port := k8sutil.ArangoSyncMasterPort
181+
if group == api.ServerGroupSyncWorkers {
182+
port = k8sutil.ArangoSyncWorkerPort
183+
}
184+
source := client.Endpoint{"https://" + net.JoinHostPort(dnsName, strconv.Itoa(port))}
179185
tlsAuth := tasks.TLSAuthentication{
180186
TLSClientAuthentication: tasks.TLSClientAuthentication{
181187
ClientToken: monitoringToken,

‎pkg/deployment/reconcile/action_wait_for_member_up.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func (a *actionWaitForMemberUp) checkProgressCluster(ctx context.Context) (bool,
174174
// of a sync master / worker.
175175
func (a *actionWaitForMemberUp) checkProgressArangoSync(ctx context.Context) (bool, bool, error) {
176176
log := a.log
177-
c, err := a.actionCtx.GetSyncServerClient(ctx, a.action.Group, a.action.ID)
177+
c, err := a.actionCtx.GetSyncServerClient(ctx, a.action.Group, a.action.MemberID)
178178
if err != nil {
179179
log.Debug().Err(err).Msg("Failed to create arangosync client")
180180
return false, false, maskAny(err)

‎pkg/deployment/reconcile/plan_builder_tls.go

+47-43
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,14 @@ func createRotateTLSServerCertificatePlan(log zerolog.Logger, spec api.Deploymen
6363
Msg("Failed to get TLS secret")
6464
continue
6565
}
66-
renewalNeeded := tlsKeyfileNeedsRenewal(log, keyfile)
66+
tlsSpec := spec.TLS
67+
if group.IsArangosync() {
68+
tlsSpec = spec.Sync.TLS
69+
}
70+
renewalNeeded, reason := tlsKeyfileNeedsRenewal(log, keyfile, tlsSpec)
6771
if renewalNeeded {
6872
plan = append(append(plan,
69-
api.NewAction(api.ActionTypeRenewTLSCertificate, group, m.ID)),
73+
api.NewAction(api.ActionTypeRenewTLSCertificate, group, m.ID, reason)),
7074
createRotateMemberPlan(log, m, group, "TLS certificate renewal")...,
7175
)
7276
}
@@ -133,8 +137,32 @@ func createRotateTLSCAPlan(log zerolog.Logger, apiObject k8sutil.APIObject,
133137

134138
// tlsKeyfileNeedsRenewal decides if the certificate in the given keyfile
135139
// should be renewed.
136-
func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string) bool {
140+
func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string, spec api.TLSSpec) (bool, string) {
137141
raw := []byte(keyfile)
142+
// containsAll returns true when all elements in the expected list
143+
// are in the actual list.
144+
containsAll := func(actual []string, expected []string) bool {
145+
for _, x := range expected {
146+
found := false
147+
for _, y := range actual {
148+
if x == y {
149+
found = true
150+
break
151+
}
152+
}
153+
if !found {
154+
return false
155+
}
156+
}
157+
return true
158+
}
159+
ipsToStringSlice := func(list []net.IP) []string {
160+
result := make([]string, len(list))
161+
for i, x := range list {
162+
result[i] = x.String()
163+
}
164+
return result
165+
}
138166
for {
139167
var derBlock *pem.Block
140168
derBlock, raw = pem.Decode(raw)
@@ -146,7 +174,7 @@ func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string) bool {
146174
if err != nil {
147175
// We do not understand the certificate, let's renew it
148176
log.Warn().Err(err).Msg("Failed to parse x509 certificate. Renewing it")
149-
return true
177+
return true, "Cannot parse x509 certificate: " + err.Error()
150178
}
151179
if cert.IsCA {
152180
// Only look at the server certificate, not CA or intermediate
@@ -162,42 +190,31 @@ func tlsKeyfileNeedsRenewal(log zerolog.Logger, keyfile string) bool {
162190
Str("not-after", cert.NotAfter.String()).
163191
Str("expiration-date", expirationDate.String()).
164192
Msg("TLS certificate renewal needed")
165-
return true
193+
return true, "Server certificate about to expire"
194+
}
195+
// Check alternate names against spec
196+
dnsNames, ipAddresses, emailAddress, err := spec.GetParsedAltNames()
197+
if err == nil {
198+
if !containsAll(cert.DNSNames, dnsNames) {
199+
return true, "Some alternate DNS names are missing"
200+
}
201+
if !containsAll(ipsToStringSlice(cert.IPAddresses), ipAddresses) {
202+
return true, "Some alternate IP addresses are missing"
203+
}
204+
if !containsAll(cert.EmailAddresses, emailAddress) {
205+
return true, "Some alternate email addresses are missing"
206+
}
166207
}
167208
}
168209
}
169-
return false
210+
return false, ""
170211
}
171212

172213
// tlsCANeedsRenewal decides if the given CA certificate
173214
// should be renewed.
174215
// Returns: shouldRenew, reason
175216
func tlsCANeedsRenewal(log zerolog.Logger, cert string, spec api.TLSSpec) (bool, string) {
176217
raw := []byte(cert)
177-
// containsAll returns true when all elements in the expected list
178-
// are in the actual list.
179-
containsAll := func(actual []string, expected []string) bool {
180-
for _, x := range expected {
181-
found := false
182-
for _, y := range actual {
183-
if x == y {
184-
found = true
185-
break
186-
}
187-
}
188-
if !found {
189-
return false
190-
}
191-
}
192-
return true
193-
}
194-
ipsToStringSlice := func(list []net.IP) []string {
195-
result := make([]string, len(list))
196-
for i, x := range list {
197-
result[i] = x.String()
198-
}
199-
return result
200-
}
201218
for {
202219
var derBlock *pem.Block
203220
derBlock, raw = pem.Decode(raw)
@@ -227,19 +244,6 @@ func tlsCANeedsRenewal(log zerolog.Logger, cert string, spec api.TLSSpec) (bool,
227244
Msg("TLS CA certificate renewal needed")
228245
return true, "CA Certificate about to expire"
229246
}
230-
// Check alternate names against spec
231-
dnsNames, ipAddresses, emailAddress, err := spec.GetParsedAltNames()
232-
if err == nil {
233-
if !containsAll(cert.DNSNames, dnsNames) {
234-
return true, "Some alternate DNS names are missing"
235-
}
236-
if !containsAll(ipsToStringSlice(cert.IPAddresses), ipAddresses) {
237-
return true, "Some alternate IP addresses are missing"
238-
}
239-
if !containsAll(cert.EmailAddresses, emailAddress) {
240-
return true, "Some alternate email addresses are missing"
241-
}
242-
}
243247
}
244248
}
245249
return false, ""

‎pkg/deployment/resources/certificates_tls.go

+5-12
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,13 @@ const (
4545
// specified in the given spec.
4646
func createTLSCACertificate(log zerolog.Logger, cli v1.CoreV1Interface, spec api.TLSSpec, deploymentName, namespace string, ownerRef *metav1.OwnerReference) error {
4747
log = log.With().Str("secret", spec.GetCASecretName()).Logger()
48-
dnsNames, ipAddresses, emailAddress, err := spec.GetParsedAltNames()
49-
if err != nil {
50-
log.Debug().Err(err).Msg("Failed to get alternate names")
51-
return maskAny(err)
52-
}
5348

5449
options := certificates.CreateCertificateOptions{
55-
CommonName: fmt.Sprintf("%s Root Certificate", deploymentName),
56-
Hosts: append(dnsNames, ipAddresses...),
57-
EmailAddresses: emailAddress,
58-
ValidFrom: time.Now(),
59-
ValidFor: caTTL,
60-
IsCA: true,
61-
ECDSACurve: tlsECDSACurve,
50+
CommonName: fmt.Sprintf("%s Root Certificate", deploymentName),
51+
ValidFrom: time.Now(),
52+
ValidFor: caTTL,
53+
IsCA: true,
54+
ECDSACurve: tlsECDSACurve,
6255
}
6356
cert, priv, err := certificates.CreateCertificate(options, nil)
6457
if err != nil {

‎pkg/replication/sync_client.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func (dr *DeploymentReplication) createArangoSyncEndpoint(epSpec api.EndpointSpe
113113
dnsName := k8sutil.CreateSyncMasterClientServiceDNSName(depl)
114114
return client.Endpoint{"https://" + net.JoinHostPort(dnsName, strconv.Itoa(k8sutil.ArangoSyncMasterPort))}, nil
115115
}
116-
return client.Endpoint(epSpec.Endpoint), nil
116+
return client.Endpoint(epSpec.MasterEndpoint), nil
117117
}
118118

119119
// createArangoSyncTLSAuthentication creates the authentication needed to authenticate

0 commit comments

Comments
 (0)