Skip to content

Commit 7084df0

Browse files
authored
Precise lock lease renewal for storages that support it (#347)
* Implement precise lock lease renewal for storage backends that support lease renewal. - Added:`LockLeaseRenewer` interface to support renewing lock leases. `renewLockLease` function to extend lease duration based on retry attempts. - Updated certificate management functions to renew lock leases during long-running retry loops. - Added tests for lease renewal functionality in `config_test.go` to ensure correct behavior with and without lease support. * Log renew lock lease
1 parent 621b7e9 commit 7084df0

File tree

3 files changed

+127
-0
lines changed

3 files changed

+127
-0
lines changed

config.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,33 @@ func (cfg *Config) manageOne(ctx context.Context, domainName string, async bool)
490490
return renew()
491491
}
492492

493+
// renewLockLease extends the lease duration on an existing lock if the storage
494+
// backend supports it. The lease duration is calculated based on the retry attempt
495+
// number and includes the certificate obtain timeout. This prevents locks from
496+
// expiring during long-running certificate operations with retries.
497+
func (cfg *Config) renewLockLease(ctx context.Context, storage Storage, lockKey string, attempt int) error {
498+
l, ok := storage.(LockLeaseRenewer)
499+
if !ok {
500+
return nil
501+
}
502+
503+
leaseDuration := maxRetryDuration
504+
if attempt < len(retryIntervals) && attempt >= 0 {
505+
leaseDuration = retryIntervals[attempt]
506+
}
507+
leaseDuration = leaseDuration + DefaultACME.CertObtainTimeout
508+
log := cfg.Logger.Named("renewLockLease")
509+
log.Debug("renewing lock lease", zap.String("lockKey", lockKey), zap.Int("attempt", attempt))
510+
511+
err := l.RenewLockLease(ctx, lockKey, leaseDuration)
512+
if err == nil {
513+
locksMu.Lock()
514+
locks[lockKey] = storage
515+
locksMu.Unlock()
516+
}
517+
return err
518+
}
519+
493520
// ObtainCertSync generates a new private key and obtains a certificate for
494521
// name using cfg in the foreground; i.e. interactively and without retries.
495522
// It stows the renewed certificate and its assets in storage if successful.
@@ -546,6 +573,15 @@ func (cfg *Config) obtainCert(ctx context.Context, name string, interactive bool
546573
log.Info("lock acquired", zap.String("identifier", name))
547574

548575
f := func(ctx context.Context) error {
576+
// renew lease on the lock if the certificate store supports it
577+
attempt, ok := ctx.Value(AttemptsCtxKey).(*int)
578+
if ok {
579+
err = cfg.renewLockLease(ctx, cfg.Storage, lockKey, *attempt)
580+
if err != nil {
581+
return fmt.Errorf("unable to renew lock lease '%s': %v", lockKey, err)
582+
}
583+
}
584+
549585
// check if obtain is still needed -- might have been obtained during lock
550586
if cfg.storageHasCertResourcesAnyIssuer(ctx, name) {
551587
log.Info("certificate already exists in storage", zap.String("identifier", name))
@@ -805,6 +841,16 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
805841
log.Info("lock acquired", zap.String("identifier", name))
806842

807843
f := func(ctx context.Context) error {
844+
// renew lease on the certificate store lock if the store implementation supports it;
845+
// prevents the lock from being acquired by another process/instance while we're renewing
846+
attempt, ok := ctx.Value(AttemptsCtxKey).(*int)
847+
if ok {
848+
err = cfg.renewLockLease(ctx, cfg.Storage, lockKey, *attempt)
849+
if err != nil {
850+
return fmt.Errorf("unable to renew lock lease '%s': %v", lockKey, err)
851+
}
852+
}
853+
808854
// prepare for renewal (load PEM cert, key, and meta)
809855
certRes, err := cfg.loadCertResourceAnyIssuer(ctx, name)
810856
if err != nil {

config_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import (
2121
"os"
2222
"reflect"
2323
"testing"
24+
"time"
2425

26+
"github.com/caddyserver/certmagic/internal/testutil"
2527
"github.com/mholt/acmez/v3/acme"
2628
)
2729

@@ -76,6 +78,75 @@ func TestSaveCertResource(t *testing.T) {
7678
}
7779
}
7880

81+
type mockStorageWithLease struct {
82+
*FileStorage
83+
renewCalled bool
84+
renewError error
85+
lastLockKey string
86+
lastDuration time.Duration
87+
}
88+
89+
func (m *mockStorageWithLease) RenewLockLease(ctx context.Context, lockKey string, leaseDuration time.Duration) error {
90+
m.renewCalled = true
91+
m.lastLockKey = lockKey
92+
m.lastDuration = leaseDuration
93+
return m.renewError
94+
}
95+
96+
func TestRenewLockLeaseDuration(t *testing.T) {
97+
ctx := context.Background()
98+
tmpDir, err := os.MkdirTemp(os.TempDir(), "certmagic-test*")
99+
testutil.RequireNoError(t, err, "allocating tmp dir")
100+
defer os.RemoveAll(tmpDir)
101+
102+
mockStorage := &mockStorageWithLease{
103+
FileStorage: &FileStorage{Path: tmpDir},
104+
}
105+
106+
// Test attempt 0
107+
cfg := &Config{Logger: defaultTestLogger}
108+
cfg.renewLockLease(ctx, mockStorage, "test-lock", 0)
109+
expected := retryIntervals[0] + DefaultACME.CertObtainTimeout
110+
testutil.RequireEqual(t, expected, mockStorage.lastDuration)
111+
112+
// Test attempt beyond array bounds
113+
cfg.renewLockLease(ctx, mockStorage, "test-lock", 999)
114+
expected = maxRetryDuration + DefaultACME.CertObtainTimeout
115+
testutil.RequireEqual(t, expected, mockStorage.lastDuration)
116+
}
117+
118+
// Test that lease renewal works when storage supports it
119+
func TestRenewLockLeaseWithInterface(t *testing.T) {
120+
ctx := context.Background()
121+
tmpDir, err := os.MkdirTemp(os.TempDir(), "certmagic-test*")
122+
testutil.RequireNoError(t, err, "allocating tmp dir")
123+
defer os.RemoveAll(tmpDir)
124+
125+
mockStorage := &mockStorageWithLease{
126+
FileStorage: &FileStorage{Path: tmpDir},
127+
}
128+
129+
cfg := &Config{Logger: defaultTestLogger}
130+
err = cfg.renewLockLease(ctx, mockStorage, "test-lock", 0)
131+
testutil.RequireNoError(t, err)
132+
133+
testutil.RequireEqual(t, true, mockStorage.renewCalled)
134+
}
135+
136+
// Test that no error occurs when storage doesn't support lease renewal
137+
func TestRenewLockLeaseWithoutInterface(t *testing.T) {
138+
ctx := context.Background()
139+
tmpDir, err := os.MkdirTemp(os.TempDir(), "certmagic-test*")
140+
testutil.RequireNoError(t, err, "allocating tmp dir")
141+
defer os.RemoveAll(tmpDir)
142+
143+
storage := &FileStorage{Path: tmpDir}
144+
145+
cfg := &Config{Logger: defaultTestLogger}
146+
err = cfg.renewLockLease(ctx, storage, "test-lock", 0)
147+
testutil.RequireNoError(t, err)
148+
}
149+
79150
func mustJSON(val any) []byte {
80151
result, err := json.Marshal(val)
81152
if err != nil {

storage.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,16 @@ type Locker interface {
149149
Unlock(ctx context.Context, name string) error
150150
}
151151

152+
// LockLeaseRenewer is an optional interface that can be implemented by a Storage
153+
// implementation to support renewing the lease on a lock. This is useful for
154+
// long-running operations that need to be synchronized across a cluster.
155+
type LockLeaseRenewer interface {
156+
// RenewLockLease renews the lease on the lock for the given lockKey for the
157+
// given leaseDuration. This is used to prevent the lock from being acquired
158+
// by another process.
159+
RenewLockLease(ctx context.Context, lockKey string, leaseDuration time.Duration) error
160+
}
161+
152162
// KeyInfo holds information about a key in storage.
153163
// Key and IsTerminal are required; Modified and Size
154164
// are optional if the storage implementation is not

0 commit comments

Comments
 (0)