Deprecate StoreKeyHashes flag (letsencrypt#4927)

aarongable · web-flow · commit 35c19c2e08d0 · 2020-07-06T10:02:39.000-07:00
The StoreKeyHashes feature flag controls whether rows are added to the keyHashToSerial table. This feature is now enabled everywhere, so the flag-protected code can be turned on unconditionally and the flag removed from configs. Related to letsencrypt#4895
diff --git a/features/featureflag_string.go b/features/featureflag_string.go
diff --git a/features/features.go b/features/features.go
@@ -19,6 +19,7 @@ const (
 	ParallelCheckFailedValidation
 	DeleteUnusedChallenges
 	BlockedKeyTable
+	StoreKeyHashes
 
 	//   Currently in-use features
 	// Check CAA and respect validationmethods parameter.
@@ -48,8 +49,6 @@ const (
 	// StoreIssuerInfo enables storage of information identifying the issuer of
 	// a certificate in the certificateStatus table.
 	StoreIssuerInfo
-	// StoreKeyHashes enables storage of SPKI hashes associated with certificates.
-	StoreKeyHashes
 	// StoreRevokerInfo enables storage of the revoker and a bool indicating if the row
 	// was checked for extant unrevoked certificates in the blockedKeys table.
 	StoreRevokerInfo
diff --git a/sa/precertificates.go b/sa/precertificates.go
@@ -12,7 +12,6 @@ import (
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	"github.com/letsencrypt/boulder/db"
 	berrors "github.com/letsencrypt/boulder/errors"
-	"github.com/letsencrypt/boulder/features"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 )
@@ -111,10 +110,8 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb
 		if err := addIssuedNames(txWithCtx, parsed, isRenewal); err != nil {
 			return nil, err
 		}
-		if features.Enabled(features.StoreKeyHashes) {
-			if err := addKeyHash(txWithCtx, parsed); err != nil {
-				return nil, err
-			}
+		if err := addKeyHash(txWithCtx, parsed); err != nil {
+			return nil, err
 		}
 
 		return nil, nil
diff --git a/sa/precertificates_test.go b/sa/precertificates_test.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/letsencrypt/boulder/db"
 	berrors "github.com/letsencrypt/boulder/errors"
-	"github.com/letsencrypt/boulder/features"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	"github.com/letsencrypt/boulder/sa/satest"
 	"github.com/letsencrypt/boulder/test"
@@ -108,13 +107,10 @@ func TestAddPrecertificateKeyHash(t *testing.T) {
 	sa, _, cleanUp := initSA(t)
 	defer cleanUp()
 	reg := satest.CreateWorkingRegistration(t, sa)
-	err := features.Set(map[string]bool{"StoreKeyHashes": true})
-	test.AssertNotError(t, err, "failed to set features")
-	defer features.Reset()
 
 	serial, testCert := test.ThrowAwayCert(t, 1)
 	issued := testCert.NotBefore.UnixNano()
-	_, err = sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
+	_, err := sa.AddPrecertificate(ctx, &sapb.AddCertificateRequest{
 		Der:    testCert.Raw,
 		RegID:  &reg.ID,
 		Ocsp:   []byte{1, 2, 3},
diff --git a/test/config-next/sa.json b/test/config-next/sa.json
@@ -25,7 +25,6 @@
     },
     "features": {
       "StoreIssuerInfo": true,
-      "StoreKeyHashes": true,
       "StoreRevokerInfo": true,
       "FasterNewOrdersRateLimit": true
     }
diff --git a/test/config/sa.json b/test/config/sa.json
@@ -25,7 +25,6 @@
     },
     "features": {
       "StoreIssuerInfo": true,
-      "StoreKeyHashes": true,
       "StoreRevokerInfo": true
     }
   },

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,6 @@ import (`
`12`	`12`	`corepb "github.com/letsencrypt/boulder/core/proto"`
`13`	`13`	`"github.com/letsencrypt/boulder/db"`
`14`	`14`	`berrors "github.com/letsencrypt/boulder/errors"`
`15`		`- "github.com/letsencrypt/boulder/features"`
`16`	`15`	`bgrpc "github.com/letsencrypt/boulder/grpc"`
`17`	`16`	`sapb "github.com/letsencrypt/boulder/sa/proto"`
`18`	`17`	`)`
`@@ -111,10 +110,8 @@ func (ssa SQLStorageAuthority) AddPrecertificate(ctx context.Context, req sapb`
`111`	`110`	`if err := addIssuedNames(txWithCtx, parsed, isRenewal); err != nil {`
`112`	`111`	`return nil, err`
`113`	`112`	`}`
`114`		`- if features.Enabled(features.StoreKeyHashes) {`
`115`		`- if err := addKeyHash(txWithCtx, parsed); err != nil {`
`116`		`- return nil, err`
`117`		`- }`
	`113`	`+ if err := addKeyHash(txWithCtx, parsed); err != nil {`
	`114`	`+ return nil, err`
`118`	`115`	`}`
`119`	`116`
`120`	`117`	`return nil, nil`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,6 @@`
`25`	`25`	`},`
`26`	`26`	`"features": {`
`27`	`27`	`"StoreIssuerInfo": true,`
`28`		`- "StoreKeyHashes": true,`
`29`	`28`	`"StoreRevokerInfo": true,`
`30`	`29`	`"FasterNewOrdersRateLimit": true`
`31`	`30`	`}`