Deprecate LeaseCRLShards feature (#7009)

This feature flag is enabled in both staging and prod.

crl/updater/updater.go

@@ -22,7 +22,6 @@ import (
 	"github.com/letsencrypt/boulder/core/proto"
 	"github.com/letsencrypt/boulder/crl"
 	cspb "github.com/letsencrypt/boulder/crl/storer/proto"
-	"github.com/letsencrypt/boulder/features"
 	"github.com/letsencrypt/boulder/issuance"
 	blog "github.com/letsencrypt/boulder/log"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
@@ -360,22 +359,19 @@ func (cu *crlUpdater) tickShard(ctx context.Context, atTime time.Time, issuerNam
 	cu.log.Infof(
 		"Generating CRL shard: id=[%s] numChunks=[%d]", crlID, len(chunks))
 
-	if features.Enabled(features.LeaseCRLShards) {
-		// Notify the database that we're working on this shard.
-		deadline, ok := ctx.Deadline()
-		if !ok {
-			return fmt.Errorf("context has no deadline")
-		}
-
-		_, err = cu.sa.LeaseCRLShard(ctx, &sapb.LeaseCRLShardRequest{
-			IssuerNameID: int64(issuerNameID),
-			MinShardIdx:  int64(shardIdx),
-			MaxShardIdx:  int64(shardIdx),
-			Until:        timestamppb.New(deadline.Add(-time.Second)),
-		})
-		if err != nil {
-			return fmt.Errorf("leasing shard: %w", err)
-		}
+	// Notify the database that we're working on this shard.
+	deadline, ok := ctx.Deadline()
+	if !ok {
+		return fmt.Errorf("context has no deadline")
+	}
+	_, err = cu.sa.LeaseCRLShard(ctx, &sapb.LeaseCRLShardRequest{
+		IssuerNameID: int64(issuerNameID),
+		MinShardIdx:  int64(shardIdx),
+		MaxShardIdx:  int64(shardIdx),
+		Until:        timestamppb.New(deadline.Add(-time.Second)),
+	})
+	if err != nil {
+		return fmt.Errorf("leasing shard: %w", err)
 	}
 
 	// Get the full list of CRL Entries for this shard from the SA.
@@ -495,14 +491,12 @@ func (cu *crlUpdater) tickShard(ctx context.Context, atTime time.Time, issuerNam
 		return fmt.Errorf("closing CRLStorer upload stream: %w", err)
 	}
 
-	if features.Enabled(features.LeaseCRLShards) {
-		// Notify the database that that we're done.
-		_, err = cu.sa.UpdateCRLShard(ctx, &sapb.UpdateCRLShardRequest{
-			IssuerNameID: int64(issuerNameID),
-			ShardIdx:     int64(shardIdx),
-			ThisUpdate:   timestamppb.New(atTime),
-		})
-	}
+	// Notify the database that that we're done.
+	_, err = cu.sa.UpdateCRLShard(ctx, &sapb.UpdateCRLShardRequest{
+		IssuerNameID: int64(issuerNameID),
+		ShardIdx:     int64(shardIdx),
+		ThisUpdate:   timestamppb.New(atTime),
+	})
 
 	cu.log.Infof(
 		"Generated CRL shard: id=[%s] size=[%d] hash=[%x]",

crl/updater/updater_test.go

@@ -15,7 +15,6 @@ import (
 	capb "github.com/letsencrypt/boulder/ca/proto"
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	cspb "github.com/letsencrypt/boulder/crl/storer/proto"
-	"github.com/letsencrypt/boulder/features"
 	"github.com/letsencrypt/boulder/issuance"
 	blog "github.com/letsencrypt/boulder/log"
 	"github.com/letsencrypt/boulder/metrics"
@@ -177,8 +176,7 @@ func TestTickShard(t *testing.T) {
 	}, 1)
 	cu.updatedCounter.Reset()
 
-	// With leasing enabled, errors while leasing should bubble up early.
-	_ = features.Set(map[string]bool{"LeaseCRLShards": true})
+	// Errors while leasing should bubble up early.
 	cu.sa.(*fakeSAC).leaseError = sentinelErr
 	err = cu.tickShard(ctx, cu.clk.Now(), e1.NameID(), 0, testChunks)
 	test.AssertError(t, err, "leasing error")
@@ -188,7 +186,7 @@ func TestTickShard(t *testing.T) {
 		"issuer": "(TEST) Elegant Elephant E1", "result": "failed",
 	}, 1)
 	cu.updatedCounter.Reset()
-	features.Reset()
+	cu.sa.(*fakeSAC).leaseError = nil
 
 	// Errors closing the Storer upload stream should bubble up.
 	cu.cs = &fakeCSC{ucc: fakeUCC{recvErr: sentinelErr}}
@@ -253,6 +251,8 @@ func TestTickShardWithRetry(t *testing.T) {
 	test.AssertNotError(t, err, "loading test issuer")
 
 	sentinelErr := errors.New("oops")
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
 
 	clk := clock.NewFake()
 	clk.Set(time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
@@ -276,7 +276,7 @@ func TestTickShardWithRetry(t *testing.T) {
 	// Ensure that having MaxAttempts set to 1 results in the clock not moving
 	// forward at all.
 	startTime := cu.clk.Now()
-	err = cu.tickShardWithRetry(context.Background(), cu.clk.Now(), e1.NameID(), 0, testChunks)
+	err = cu.tickShardWithRetry(ctx, cu.clk.Now(), e1.NameID(), 0, testChunks)
 	test.AssertError(t, err, "database error")
 	test.AssertErrorIs(t, err, sentinelErr)
 	test.AssertEquals(t, cu.clk.Now(), startTime)
@@ -286,7 +286,7 @@ func TestTickShardWithRetry(t *testing.T) {
 	// in, so we have to be approximate.
 	cu.maxAttempts = 5
 	startTime = cu.clk.Now()
-	err = cu.tickShardWithRetry(context.Background(), cu.clk.Now(), e1.NameID(), 0, testChunks)
+	err = cu.tickShardWithRetry(ctx, cu.clk.Now(), e1.NameID(), 0, testChunks)
 	test.AssertError(t, err, "database error")
 	test.AssertErrorIs(t, err, sentinelErr)
 	t.Logf("start: %v", startTime)

features/featureflag_string.go

@@ -15,23 +15,23 @@ func _() {
 	_ = x[StoreLintingCertificateInsteadOfPrecertificate-4]
 	_ = x[CAAValidationMethods-5]
 	_ = x[CAAAccountURI-6]
-	_ = x[EnforceMultiVA-7]
-	_ = x[MultiVAFullResults-8]
-	_ = x[ECDSAForAll-9]
-	_ = x[ServeRenewalInfo-10]
-	_ = x[AllowUnrecognizedFeatures-11]
-	_ = x[ExpirationMailerUsesJoin-12]
-	_ = x[CertCheckerChecksValidations-13]
-	_ = x[CertCheckerRequiresValidations-14]
-	_ = x[CertCheckerRequiresCorrespondence-15]
-	_ = x[AsyncFinalize-16]
-	_ = x[RequireCommonName-17]
-	_ = x[LeaseCRLShards-18]
+	_ = x[LeaseCRLShards-7]
+	_ = x[EnforceMultiVA-8]
+	_ = x[MultiVAFullResults-9]
+	_ = x[ECDSAForAll-10]
+	_ = x[ServeRenewalInfo-11]
+	_ = x[AllowUnrecognizedFeatures-12]
+	_ = x[ExpirationMailerUsesJoin-13]
+	_ = x[CertCheckerChecksValidations-14]
+	_ = x[CertCheckerRequiresValidations-15]
+	_ = x[CertCheckerRequiresCorrespondence-16]
+	_ = x[AsyncFinalize-17]
+	_ = x[RequireCommonName-18]
 }
 
-const _FeatureFlag_name = "unusedStoreRevokerInfoROCSPStage6ROCSPStage7StoreLintingCertificateInsteadOfPrecertificateCAAValidationMethodsCAAAccountURIEnforceMultiVAMultiVAFullResultsECDSAForAllServeRenewalInfoAllowUnrecognizedFeaturesExpirationMailerUsesJoinCertCheckerChecksValidationsCertCheckerRequiresValidationsCertCheckerRequiresCorrespondenceAsyncFinalizeRequireCommonNameLeaseCRLShards"
+const _FeatureFlag_name = "unusedStoreRevokerInfoROCSPStage6ROCSPStage7StoreLintingCertificateInsteadOfPrecertificateCAAValidationMethodsCAAAccountURILeaseCRLShardsEnforceMultiVAMultiVAFullResultsECDSAForAllServeRenewalInfoAllowUnrecognizedFeaturesExpirationMailerUsesJoinCertCheckerChecksValidationsCertCheckerRequiresValidationsCertCheckerRequiresCorrespondenceAsyncFinalizeRequireCommonName"
 
-var _FeatureFlag_index = [...]uint16{0, 6, 22, 33, 44, 90, 110, 123, 137, 155, 166, 182, 207, 231, 259, 289, 322, 335, 352, 366}
+var _FeatureFlag_index = [...]uint16{0, 6, 22, 33, 44, 90, 110, 123, 137, 151, 169, 180, 196, 221, 245, 273, 303, 336, 349, 366}
 
 func (i FeatureFlag) String() string {
 	if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

features/features.go

@@ -19,6 +19,7 @@ const (
 	StoreLintingCertificateInsteadOfPrecertificate
 	CAAValidationMethods
 	CAAAccountURI
+	LeaseCRLShards
 
 	//   Currently in-use features
 	// EnforceMultiVA causes the VA to block on remote VA PerformValidation
@@ -73,11 +74,6 @@ const (
 	// According to the BRs Section 7.1.4.2.2(a), the commonName field is
 	// Deprecated, and its inclusion is discouraged but not (yet) prohibited.
 	RequireCommonName
-
-	// LeaseCRLShards causes the crl-updater to use the database to control which
-	// instance of crl-updater is responsible for updating each shard. This flag
-	// should only be enabled if the `crlShards` table exists in the database.
-	LeaseCRLShards
 )
 
 // List of features and their default value, protected by fMu

sa/db-next/boulder_sa/20230519000000_CrlShards.sql

@@ -1,18 +0,0 @@
--- +migrate Up
--- SQL in section 'Up' is executed when this migration is applied
-
-CREATE TABLE `crlShards` (
-  `id` bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT,
-  `issuerID` bigint(20) NOT NULL,
-  `idx` int UNSIGNED NOT NULL,
-  `thisUpdate` datetime,
-  `nextUpdate` datetime,
-  `leasedUntil` datetime NOT NULL,
-  PRIMARY KEY (`id`),
-  UNIQUE KEY `shardID` (`issuerID`, `idx`)
-) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
-
--- +migrate Down
--- SQL section 'Down' is executed when this migration is rolled back
-
-DROP TABLE `crlShards`;

sa/db-next/boulder_sa/20230519000000_CrlShards.sql

@@ -0,0 +1 @@
+../../db/boulder_sa/20230519000000_CrlShards.sql

sa/db/boulder_sa/20230519000000_CrlShards.sql

@@ -0,0 +1,18 @@
+-- +migrate Up
+-- SQL in section 'Up' is executed when this migration is applied
+
+CREATE TABLE `crlShards` (
+  `id` bigint(20) UNSIGNED NOT NULL AUTO_INCREMENT,
+  `issuerID` bigint(20) NOT NULL,
+  `idx` int UNSIGNED NOT NULL,
+  `thisUpdate` datetime,
+  `nextUpdate` datetime,
+  `leasedUntil` datetime NOT NULL,
+  PRIMARY KEY (`id`),
+  UNIQUE KEY `shardID` (`issuerID`, `idx`)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
+
+-- +migrate Down
+-- SQL section 'Down' is executed when this migration is rolled back
+
+DROP TABLE `crlShards`;

test/config-next/crl-updater.json

@@ -49,9 +49,7 @@
 		"updateTimeout": "3600s",
 		"maxParallelism": 10,
 		"maxAttempts": 5,
-		"features": {
-			"LeaseCRLShards": true
-		}
+		"features": {}
 	},
 	"syslog": {
 		"stdoutlevel": 6,

test/config/crl-updater.json

@@ -30,7 +30,9 @@
 		"certificateLifetime": "2160h",
 		"updatePeriod": "6h",
 		"updateOffset": "9120s",
-		"maxParallelism": 10
+		"maxParallelism": 10,
+		"maxAttempts": 5,
+		"features": {}
 	},
 	"syslog": {
 		"stdoutlevel": 6,

test/integration/crl_test.go

@@ -73,12 +73,10 @@ func TestCRLPipeline(t *testing.T) {
 	test.AssertEquals(t, resp.StatusCode, 200)
 
 	// Reset the "leasedUntil" column to prepare for another round of CRLs.
-	if strings.Contains(configDir, "config-next") {
-		db, err := sql.Open("mysql", vars.DBConnSAIntegrationFullPerms)
-		test.AssertNotError(t, err, "opening database connection")
-		_, err = db.Exec(`UPDATE crlShards SET leasedUntil = ?`, fc.Now().Add(-time.Minute))
-		test.AssertNotError(t, err, "resetting leasedUntil column")
-	}
+	db, err := sql.Open("mysql", vars.DBConnSAIntegrationFullPerms)
+	test.AssertNotError(t, err, "opening database connection")
+	_, err = db.Exec(`UPDATE crlShards SET leasedUntil = ?`, fc.Now().Add(-time.Minute))
+	test.AssertNotError(t, err, "resetting leasedUntil column")
 
 	// Confirm that the cert now *does* show up in the CRLs.
 	runUpdater(t, configFile)