Skip to content

Commit c0c98fd

Browse files
jwierzbojwierzbo
authored
[Bugfix] Add ArangoSync TLS based rotation (#996)
1 parent 98964cb commit c0c98fd

File tree

5 files changed

+103
-37
lines changed

5 files changed

+103
-37
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Change Log
22

33
## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A)
4+
- (Feature) Add ArangoSync TLS based rotation
45

56
## [1.2.13](https://github.com/arangodb/kube-arangodb/tree/1.2.13) (2022-06-07)
67
- (Bugfix) Fix arangosync members state inspection

‎pkg/apis/shared/constants.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ const (
3333
ArangoExporterInternalEndpointV2 = "/_admin/metrics/v2"
3434
ArangoExporterDefaultEndpoint = "/metrics"
3535

36+
ArangoSyncStatusEndpoint = "/_api/version"
37+
3638
// K8s constants
3739
ClusterIPNone = "None"
3840
TopologyKeyHostname = "kubernetes.io/hostname"

‎pkg/deployment/reconcile/plan_builder_tls.go‎

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,20 @@ import (
3030
"reflect"
3131
"time"
3232

33-
memberTls "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tls"
34-
35-
"github.com/arangodb/kube-arangodb/pkg/deployment/features"
36-
37-
"github.com/arangodb/kube-arangodb/pkg/deployment/client"
38-
"github.com/arangodb/kube-arangodb/pkg/util/constants"
39-
inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
40-
4133
"github.com/arangodb/go-driver"
34+
4235
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
4336
"github.com/arangodb/kube-arangodb/pkg/apis/shared"
4437
"github.com/arangodb/kube-arangodb/pkg/deployment/actions"
38+
"github.com/arangodb/kube-arangodb/pkg/deployment/client"
39+
"github.com/arangodb/kube-arangodb/pkg/deployment/features"
4540
"github.com/arangodb/kube-arangodb/pkg/deployment/resources"
4641
"github.com/arangodb/kube-arangodb/pkg/util"
42+
"github.com/arangodb/kube-arangodb/pkg/util/constants"
4743
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
44+
inspectorInterface "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector"
45+
memberTls "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/tls"
46+
4847
"github.com/rs/zerolog"
4948
)
5049

@@ -286,6 +285,42 @@ func createCACleanPlan(ctx context.Context,
286285
return nil
287286
}
288287

288+
func createKeyfileRenewalPlanSynced(ctx context.Context,
289+
log zerolog.Logger, apiObject k8sutil.APIObject,
290+
spec api.DeploymentSpec, status api.DeploymentStatus,
291+
planCtx PlanBuilderContext) api.Plan {
292+
293+
if !spec.Sync.IsEnabled() || !spec.Sync.TLS.IsSecure() {
294+
return nil
295+
}
296+
297+
var plan api.Plan
298+
group := api.ServerGroupSyncMasters
299+
300+
for _, statusMember := range status.Members.AsListInGroup(group) {
301+
member := statusMember.Member
302+
303+
if !plan.IsEmpty() {
304+
return nil
305+
}
306+
307+
cache, ok := planCtx.ACS().ClusterCache(member.ClusterID)
308+
if !ok {
309+
continue
310+
}
311+
312+
lCtx, c := context.WithTimeout(ctx, 500*time.Millisecond)
313+
defer c()
314+
315+
if renew, _ := keyfileRenewalRequired(lCtx, log, apiObject, spec.Sync.TLS, spec, cache, planCtx, group, member, api.TLSRotateModeRecreate); renew {
316+
log.Info().Msg("Renewal of keyfile required - Recreate (sync master)")
317+
plan = append(plan, tlsRotateConditionAction(group, member.ID, "Restart sync master after keyfile removal"))
318+
}
319+
}
320+
321+
return plan
322+
}
323+
289324
func createKeyfileRenewalPlanDefault(ctx context.Context,
290325
log zerolog.Logger, apiObject k8sutil.APIObject,
291326
spec api.DeploymentSpec, status api.DeploymentStatus,
@@ -314,8 +349,8 @@ func createKeyfileRenewalPlanDefault(ctx context.Context,
314349
lCtx, c := context.WithTimeout(ctx, 500*time.Millisecond)
315350
defer c()
316351

317-
if renew, _ := keyfileRenewalRequired(lCtx, log, apiObject, spec, cache, planCtx, group, member, api.TLSRotateModeRecreate); renew {
318-
log.Info().Msg("Renewal of keyfile required - Recreate")
352+
if renew, _ := keyfileRenewalRequired(lCtx, log, apiObject, spec.TLS, spec, cache, planCtx, group, member, api.TLSRotateModeRecreate); renew {
353+
log.Info().Msg("Renewal of keyfile required - Recreate (server)")
319354
plan = append(plan, tlsRotateConditionAction(group, member.ID, "Restart server after keyfile removal"))
320355
}
321356
}
@@ -350,8 +385,8 @@ func createKeyfileRenewalPlanInPlace(ctx context.Context,
350385
lCtx, c := context.WithTimeout(ctx, 500*time.Millisecond)
351386
defer c()
352387

353-
if renew, recreate := keyfileRenewalRequired(lCtx, log, apiObject, spec, cache, planCtx, group, member, api.TLSRotateModeInPlace); renew {
354-
log.Info().Msg("Renewal of keyfile required - InPlace")
388+
if renew, recreate := keyfileRenewalRequired(lCtx, log, apiObject, spec.TLS, spec, cache, planCtx, group, member, api.TLSRotateModeInPlace); renew {
389+
log.Info().Msg("Renewal of keyfile required - InPlace (server)")
355390
if recreate {
356391
plan = append(plan, actions.NewAction(api.ActionTypeCleanTLSKeyfileCertificate, group, member, "Remove server keyfile and enforce renewal"))
357392
}
@@ -376,12 +411,16 @@ func createKeyfileRenewalPlan(ctx context.Context,
376411
gCtx, c := context.WithTimeout(ctx, 2*time.Second)
377412
defer c()
378413

414+
plan := createKeyfileRenewalPlanSynced(gCtx, log, apiObject, spec, status, planCtx)
415+
379416
switch createKeyfileRenewalPlanMode(spec, status) {
380417
case api.TLSRotateModeInPlace:
381-
return createKeyfileRenewalPlanInPlace(gCtx, log, apiObject, spec, status, planCtx)
418+
plan = append(plan, createKeyfileRenewalPlanInPlace(gCtx, log, apiObject, spec, status, planCtx)...)
382419
default:
383-
return createKeyfileRenewalPlanDefault(gCtx, log, apiObject, spec, status, planCtx)
420+
plan = append(plan, createKeyfileRenewalPlanDefault(gCtx, log, apiObject, spec, status, planCtx)...)
384421
}
422+
423+
return plan
385424
}
386425

387426
func createKeyfileRenewalPlanMode(
@@ -419,6 +458,9 @@ func createKeyfileRenewalPlanMode(
419458

420459
func checkServerValidCertRequest(ctx context.Context, context PlanBuilderContext, apiObject k8sutil.APIObject, group api.ServerGroup, member api.MemberStatus, ca resources.Certificates) (*tls.ConnectionState, error) {
421460
endpoint := fmt.Sprintf("https://%s:%d", k8sutil.CreatePodDNSNameWithDomain(apiObject, context.GetSpec().ClusterDomain, group.AsRole(), member.ID), shared.ArangoPort)
461+
if group == api.ServerGroupSyncMasters {
462+
endpoint = fmt.Sprintf("https://%s:%d%s", k8sutil.CreatePodDNSNameWithDomain(apiObject, context.GetSpec().ClusterDomain, group.AsRole(), member.ID), shared.ArangoSyncMasterPort, shared.ArangoSyncStatusEndpoint)
463+
}
422464

423465
tlsConfig := &tls.Config{
424466
RootCAs: ca.AsCertPool(),
@@ -452,12 +494,13 @@ func checkServerValidCertRequest(ctx context.Context, context PlanBuilderContext
452494
return resp.TLS, nil
453495
}
454496

497+
// keyfileRenewalRequired checks if a keyfile renewal is required and if recreation should be made
455498
func keyfileRenewalRequired(ctx context.Context,
456-
log zerolog.Logger, apiObject k8sutil.APIObject,
499+
log zerolog.Logger, apiObject k8sutil.APIObject, tls api.TLSSpec,
457500
spec api.DeploymentSpec, cachedStatus inspectorInterface.Inspector,
458501
context PlanBuilderContext,
459502
group api.ServerGroup, member api.MemberStatus, mode api.TLSRotateMode) (bool, bool) {
460-
if !spec.TLS.IsSecure() {
503+
if !tls.IsSecure() {
461504
return false, false
462505
}
463506

@@ -469,15 +512,15 @@ func keyfileRenewalRequired(ctx context.Context,
469512
return false, false
470513
}
471514

472-
caSecret, exists := cachedStatus.Secret().V1().GetSimple(spec.TLS.GetCASecretName())
515+
caSecret, exists := cachedStatus.Secret().V1().GetSimple(tls.GetCASecretName())
473516
if !exists {
474-
log.Warn().Str("secret", spec.TLS.GetCASecretName()).Msg("CA Secret does not exists")
517+
log.Warn().Str("secret", tls.GetCASecretName()).Msg("CA Secret does not exists")
475518
return false, false
476519
}
477520

478521
ca, _, err := resources.GetKeyCertFromSecret(log, caSecret, resources.CACertName, resources.CAKeyName)
479522
if err != nil {
480-
log.Warn().Err(err).Str("secret", spec.TLS.GetCASecretName()).Msg("CA Secret does not contains Cert")
523+
log.Warn().Err(err).Str("secret", tls.GetCASecretName()).Msg("CA Secret does not contains Cert")
481524
return false, false
482525
}
483526

@@ -487,13 +530,13 @@ func keyfileRenewalRequired(ctx context.Context,
487530
case *url.Error:
488531
switch v.Err.(type) {
489532
case x509.UnknownAuthorityError, x509.CertificateInvalidError:
490-
log.Debug().Err(v.Err).Str("type", reflect.TypeOf(v.Err).String()).Msg("Validation of server cert failed")
533+
log.Debug().Err(v.Err).Str("type", reflect.TypeOf(v.Err).String()).Msgf("Validation of cert for %s failed, renewal is required", memberName)
491534
return true, true
492535
default:
493-
log.Debug().Err(v.Err).Str("type", reflect.TypeOf(v.Err).String()).Msg("Validation of server cert failed")
536+
log.Debug().Err(v.Err).Str("type", reflect.TypeOf(v.Err).String()).Msgf("Validation of cert for %s failed, but cert looks fine - continuing", memberName)
494537
}
495538
default:
496-
log.Debug().Err(err).Str("type", reflect.TypeOf(err).String()).Msg("Validation of server cert failed")
539+
log.Debug().Err(err).Str("type", reflect.TypeOf(err).String()).Msgf("Validation of cert for %s failed, will try again next time", memberName)
497540
}
498541
return false, false
499542
}
@@ -514,7 +557,13 @@ func keyfileRenewalRequired(ctx context.Context,
514557
}
515558

516559
// Verify AltNames
517-
altNames, err := memberTls.GetServerAltNames(apiObject, spec, spec.TLS, service, group, member)
560+
var altNames memberTls.KeyfileInput
561+
if group.IsArangosync() {
562+
altNames, err = memberTls.GetSyncAltNames(apiObject, spec, tls, group, member)
563+
} else {
564+
altNames, err = memberTls.GetServerAltNames(apiObject, spec, tls, service, group, member)
565+
}
566+
518567
if err != nil {
519568
log.Warn().Msg("Unable to render alt names")
520569
return false, false
@@ -535,7 +584,7 @@ func keyfileRenewalRequired(ctx context.Context,
535584
}
536585

537586
// Ensure secret is propagated only on 3.7.0+ enterprise and inplace mode
538-
if mode == api.TLSRotateModeInPlace {
587+
if mode == api.TLSRotateModeInPlace && group.IsArangod() {
539588
conn, err := context.GetServerClient(ctx, group, member.ID)
540589
if err != nil {
541590
log.Warn().Err(err).Msg("Unable to get client")

‎pkg/deployment/resources/pod_creator.go‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"encoding/json"
2727
"fmt"
2828
"net"
29-
"net/url"
3029
"path/filepath"
3130
"strconv"
3231
"sync"
@@ -530,22 +529,11 @@ func (r *Resources) createPodForMember(ctx context.Context, cachedStatus inspect
530529
// Create TLS secret
531530
tlsKeyfileSecretName := k8sutil.CreateTLSKeyfileSecretName(apiObject.GetName(), role, m.ID)
532531

533-
names, err := tls.GetAltNames(spec.Sync.TLS)
532+
names, err := tls.GetSyncAltNames(apiObject, spec, spec.Sync.TLS, group, m)
534533
if err != nil {
535534
return errors.WithStack(errors.Wrapf(err, "Failed to render alt names"))
536535
}
537536

538-
names.AltNames = append(names.AltNames,
539-
k8sutil.CreateSyncMasterClientServiceName(apiObject.GetName()),
540-
k8sutil.CreateSyncMasterClientServiceDNSNameWithDomain(apiObject, spec.ClusterDomain),
541-
k8sutil.CreatePodDNSNameWithDomain(apiObject, spec.ClusterDomain, role, m.ID),
542-
)
543-
masterEndpoint := spec.Sync.ExternalAccess.ResolveMasterEndpoint(k8sutil.CreateSyncMasterClientServiceDNSNameWithDomain(apiObject, spec.ClusterDomain), shared.ArangoSyncMasterPort)
544-
for _, ep := range masterEndpoint {
545-
if u, err := url.Parse(ep); err == nil {
546-
names.AltNames = append(names.AltNames, u.Hostname())
547-
}
548-
}
549537
owner := apiObject.AsOwner()
550538
_, err = createTLSServerCertificate(ctx, log, cachedStatus, cachedStatus.SecretsModInterface().V1(), names, spec.Sync.TLS, tlsKeyfileSecretName, &owner)
551539
if err != nil && !k8sutil.IsAlreadyExists(err) {

‎pkg/util/k8sutil/tls/tls.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,13 @@
2121
package tls
2222

2323
import (
24+
"net/url"
25+
2426
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1"
27+
"github.com/arangodb/kube-arangodb/pkg/apis/shared"
2528
"github.com/arangodb/kube-arangodb/pkg/util/errors"
2629
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
30+
2731
core "k8s.io/api/core/v1"
2832
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
2933
)
@@ -86,3 +90,25 @@ func GetServerAltNames(deployment meta.Object, spec api.DeploymentSpec, tls api.
8690

8791
return k, nil
8892
}
93+
94+
func GetSyncAltNames(deployment meta.Object, spec api.DeploymentSpec, tls api.TLSSpec, group api.ServerGroup, member api.MemberStatus) (KeyfileInput, error) {
95+
k, err := GetAltNames(tls)
96+
if err != nil {
97+
return k, errors.WithStack(err)
98+
}
99+
100+
k.AltNames = append(k.AltNames,
101+
k8sutil.CreateSyncMasterClientServiceName(deployment.GetName()),
102+
k8sutil.CreateSyncMasterClientServiceDNSNameWithDomain(deployment, spec.ClusterDomain),
103+
k8sutil.CreatePodDNSNameWithDomain(deployment, spec.ClusterDomain, group.AsRole(), member.ID),
104+
)
105+
106+
masterEndpoint := spec.Sync.ExternalAccess.ResolveMasterEndpoint(k8sutil.CreateSyncMasterClientServiceDNSNameWithDomain(deployment, spec.ClusterDomain), shared.ArangoSyncMasterPort)
107+
for _, ep := range masterEndpoint {
108+
if u, err := url.Parse(ep); err == nil {
109+
k.AltNames = append(k.AltNames, u.Hostname())
110+
}
111+
}
112+
113+
return k, nil
114+
}

0 commit comments

Comments
 (0)