Skip to content

Commit 6c85b8d

Browse files
beautifulentropybeautifulentropy
authored
wfe/sa/features: Deprecate TrackReplacementCertificatesARI (#7766)
1 parent e5edb70 commit 6c85b8d

File tree

11 files changed

+27
-63
lines changed

11 files changed

+27
-63
lines changed

‎features/features.go‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type Config struct {
2929
CertCheckerRequiresCorrespondence bool
3030
ECDSAForAll bool
3131
CheckRenewalExemptionAtWFE bool
32+
TrackReplacementCertificatesARI bool
3233
UseKvLimitsForNewAccount bool
3334

3435
// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
@@ -73,16 +74,6 @@ type Config struct {
7374
// maxRemoteValidationFailures. Only used when EnforceMultiCAA is true.
7475
MultiCAAFullResults bool
7576

76-
// TrackReplacementCertificatesARI, when enabled, triggers the following
77-
// behavior:
78-
// - SA.NewOrderAndAuthzs: upon receiving a NewOrderRequest with a
79-
// 'replacesSerial' value, will create a new entry in the 'replacement
80-
// Orders' table. This will occur inside of the new order transaction.
81-
// - SA.FinalizeOrder will update the 'replaced' column of any row with
82-
// a 'orderID' matching the finalized order to true. This will occur
83-
// inside of the finalize (order) transaction.
84-
TrackReplacementCertificatesARI bool
85-
8677
// MultipleCertificateProfiles, when enabled, triggers the following
8778
// behavior:
8879
// - SA.NewOrderAndAuthzs: upon receiving a NewOrderRequest with a

‎sa/model_test.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"google.golang.org/protobuf/types/known/timestamppb"
2020

2121
"github.com/letsencrypt/boulder/db"
22-
"github.com/letsencrypt/boulder/features"
2322
"github.com/letsencrypt/boulder/grpc"
2423
"github.com/letsencrypt/boulder/probs"
2524
"github.com/letsencrypt/boulder/test/vars"
@@ -461,9 +460,6 @@ func TestAddReplacementOrder(t *testing.T) {
461460
sa, _, cleanUp := initSA(t)
462461
defer cleanUp()
463462

464-
features.Set(features.Config{TrackReplacementCertificatesARI: true})
465-
defer features.Reset()
466-
467463
oldCertSerial := "1234567890"
468464
orderId := int64(1337)
469465
orderExpires := time.Now().Add(24 * time.Hour).UTC().Truncate(time.Second)
@@ -513,9 +509,6 @@ func TestSetReplacementOrderFinalized(t *testing.T) {
513509
sa, _, cleanUp := initSA(t)
514510
defer cleanUp()
515511

516-
features.Set(features.Config{TrackReplacementCertificatesARI: true})
517-
defer features.Reset()
518-
519512
oldCertSerial := "1234567890"
520513
orderId := int64(1337)
521514
orderExpires := time.Now().Add(24 * time.Hour).UTC().Truncate(time.Second)

‎sa/sa.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -739,11 +739,9 @@ func (ssa *SQLStorageAuthority) FinalizeOrder(ctx context.Context, req *sapb.Fin
739739
return nil, err
740740
}
741741

742-
if features.Get().TrackReplacementCertificatesARI {
743-
err = setReplacementOrderFinalized(ctx, tx, req.Id)
744-
if err != nil {
745-
return nil, err
746-
}
742+
err = setReplacementOrderFinalized(ctx, tx, req.Id)
743+
if err != nil {
744+
return nil, err
747745
}
748746

749747
return nil, nil

‎sa/sa_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4059,9 +4059,6 @@ func TestReplacementOrderExists(t *testing.T) {
40594059
sa, fc, cleanUp := initSA(t)
40604060
defer cleanUp()
40614061

4062-
features.Set(features.Config{TrackReplacementCertificatesARI: true})
4063-
defer features.Reset()
4064-
40654062
oldCertSerial := "1234567890"
40664063

40674064
// Check that a non-existent replacement order does not exist.

‎test/config-next/sa.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
"healthCheckInterval": "4s",
5151
"features": {
5252
"MultipleCertificateProfiles": true,
53-
"TrackReplacementCertificatesARI": true,
5453
"DisableLegacyLimitWrites": true,
5554
"InsertAuthzsIndividually": true
5655
}

‎test/config-next/wfe2.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@
128128
},
129129
"features": {
130130
"ServeRenewalInfo": true,
131-
"TrackReplacementCertificatesARI": true,
132131
"CheckIdentifiersPaused": true,
133132
"UseKvLimitsForNewOrder": true
134133
},

‎test/config/sa.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@
4747
}
4848
}
4949
},
50-
"features": {}
50+
"features": {
51+
"TrackReplacementCertificatesARI": true
52+
}
5153
},
5254
"syslog": {
5355
"stdoutlevel": 6,

‎test/config/wfe2.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
"pendingAuthorizationLifetimeDays": 7,
107107
"features": {
108108
"ServeRenewalInfo": true,
109+
"TrackReplacementCertificatesARI": true,
109110
"CheckRenewalExemptionAtWFE": true,
110111
"UseKvLimitsForNewAccount": true
111112
}

‎test/integration/ari_test.go‎

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"crypto/rand"
99
"crypto/x509/pkix"
1010
"math/big"
11-
"os"
1211
"testing"
1312
"time"
1413

@@ -50,27 +49,17 @@ func TestARI(t *testing.T) {
5049
test.AssertEquals(t, ari.SuggestedWindow.End.Sub(time.Now()).Round(time.Hour), 1463*time.Hour)
5150
test.AssertEquals(t, ari.RetryAfter.Sub(time.Now()).Round(time.Hour), 6*time.Hour)
5251

53-
// TODO(@pgporada): Clean this up when 'test/config/{sa,wfe2}.json' sets
54-
// TrackReplacementCertificatesARI=true.
55-
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config-next" {
56-
// Make a new order which indicates that it replaces the cert issued above.
57-
_, order, err := makeClientAndOrder(client, key, []string{name}, true, cert)
58-
test.AssertNotError(t, err, "failed to issue test cert")
59-
replaceID, err := acme.GenerateARICertID(cert)
60-
test.AssertNotError(t, err, "failed to generate ARI certID")
61-
test.AssertEquals(t, order.Replaces, replaceID)
62-
test.AssertNotEquals(t, order.Replaces, "")
63-
64-
// Try it again and verify it fails
65-
_, order, err = makeClientAndOrder(client, key, []string{name}, true, cert)
66-
test.AssertError(t, err, "subsequent ARI replacements for a replaced cert should fail, but didn't")
67-
} else {
68-
// ARI is disabled so we only use the client to POST the replacement
69-
// order, but we never finalize it.
70-
replacementOrder, err := client.ReplacementOrder(client.Account, cert, []acme.Identifier{{Type: "dns", Value: name}})
71-
test.AssertNotError(t, err, "ARI replacement request should have succeeded")
72-
test.AssertNotEquals(t, replacementOrder.Replaces, "")
73-
}
52+
// Make a new order which indicates that it replaces the cert issued above.
53+
_, order, err := makeClientAndOrder(client, key, []string{name}, true, cert)
54+
test.AssertNotError(t, err, "failed to issue test cert")
55+
replaceID, err := acme.GenerateARICertID(cert)
56+
test.AssertNotError(t, err, "failed to generate ARI certID")
57+
test.AssertEquals(t, order.Replaces, replaceID)
58+
test.AssertNotEquals(t, order.Replaces, "")
59+
60+
// Try it again and verify it fails
61+
_, order, err = makeClientAndOrder(client, key, []string{name}, true, cert)
62+
test.AssertError(t, err, "subsequent ARI replacements for a replaced cert should fail, but didn't")
7463

7564
// Revoke the cert and re-request ARI. The renewal window should now be in
7665
// the past indicating to the client that a renewal should happen

‎wfe2/wfe.go‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,12 +2348,10 @@ func (wfe *WebFrontEndImpl) NewOrder(
23482348

23492349
var replaces string
23502350
var isARIRenewal bool
2351-
if features.Get().TrackReplacementCertificatesARI {
2352-
replaces, isARIRenewal, err = wfe.validateReplacementOrder(ctx, acct, names, newOrderRequest.Replaces)
2353-
if err != nil {
2354-
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "While validating order as a replacement an error occurred"), err)
2355-
return
2356-
}
2351+
replaces, isARIRenewal, err = wfe.validateReplacementOrder(ctx, acct, names, newOrderRequest.Replaces)
2352+
if err != nil {
2353+
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "While validating order as a replacement an error occurred"), err)
2354+
return
23572355
}
23582356

23592357
var isRenewal bool
@@ -2391,12 +2389,10 @@ func (wfe *WebFrontEndImpl) NewOrder(
23912389
var newOrderSuccessful bool
23922390
var errIsRateLimit bool
23932391
defer func() {
2394-
if features.Get().TrackReplacementCertificatesARI {
2395-
wfe.stats.ariReplacementOrders.With(prometheus.Labels{
2396-
"isReplacement": fmt.Sprintf("%t", replaces != ""),
2397-
"limitsExempt": fmt.Sprintf("%t", isARIRenewal),
2398-
}).Inc()
2399-
}
2392+
wfe.stats.ariReplacementOrders.With(prometheus.Labels{
2393+
"isReplacement": fmt.Sprintf("%t", replaces != ""),
2394+
"limitsExempt": fmt.Sprintf("%t", isARIRenewal),
2395+
}).Inc()
24002396

24012397
if !newOrderSuccessful && !errIsRateLimit && refundLimits != nil {
24022398
go refundLimits()

0 commit comments

Comments
 (0)