SA: Clean up pre-profile order schema and feature flag (#7953)

Deprecate the MultipleCertificateProfiles feature flag, which has been
enabled in both Staging and Prod. Delete all code protected by that flag
being false, namely the orderModelv1 type and its support code. Update
the config schema to match the config-next schema.

Fixes https://github.com/letsencrypt/boulder/issues/7324
Fixes https://github.com/letsencrypt/boulder/issues/7408
This commit is contained in:
Aaron Gable 2025-01-17 17:15:01 -08:00 committed by GitHub
parent dbe2fe24a4
commit 6b1e7f04e8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 65 additions and 325 deletions

View File

@ -19,6 +19,7 @@ type Config struct {
IncrementRateLimits bool
UseKvLimitsForNewOrder bool
DisableLegacyLimitWrites bool
MultipleCertificateProfiles bool
// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
// GET requests. WARNING: This feature is a draft and highly unstable.
@ -55,14 +56,6 @@ type Config struct {
// make a valid/invalid decision with the results.
EnforceMultiCAA bool
// MultipleCertificateProfiles, when enabled, triggers the following
// behavior:
// - SA.NewOrderAndAuthzs: upon receiving a NewOrderRequest with a
// `certificateProfileName` value, will add that value to the database's
// `orders.certificateProfileName` column. Values in this column are
// allowed to be empty.
MultipleCertificateProfiles bool
// CheckIdentifiersPaused checks if any of the identifiers in the order are
// currently paused at NewOrder time. If any are paused, an error is
// returned to the Subscriber indicating that the order cannot be processed

View File

@ -15,7 +15,6 @@ import (
"fmt"
"math/big"
mrand "math/rand/v2"
"os"
"regexp"
"strconv"
"strings"
@ -1391,6 +1390,7 @@ func TestNewOrder_OrderReusex(t *testing.T) {
orderReq := &rapb.NewOrderRequest{
RegistrationID: Registration.Id,
DnsNames: names,
CertificateProfileName: "test",
}
firstOrder, err := ra.NewOrder(context.Background(), orderReq)
test.AssertNotError(t, err, "Adding an initial order for regA failed")
@ -1406,12 +1406,14 @@ func TestNewOrder_OrderReusex(t *testing.T) {
Name string
RegistrationID int64
DnsNames []string
Profile string
ExpectReuse bool
}{
{
Name: "Duplicate order, same regID",
RegistrationID: Registration.Id,
DnsNames: names,
Profile: "test",
// We expect reuse since the order matches firstOrder
ExpectReuse: true,
},
@ -1419,6 +1421,7 @@ func TestNewOrder_OrderReusex(t *testing.T) {
Name: "Subset of order names, same regID",
RegistrationID: Registration.Id,
DnsNames: names[:1],
Profile: "test",
// We do not expect reuse because the order names don't match firstOrder
ExpectReuse: false,
},
@ -1426,13 +1429,30 @@ func TestNewOrder_OrderReusex(t *testing.T) {
Name: "Superset of order names, same regID",
RegistrationID: Registration.Id,
DnsNames: append(names, "blog.zombo.com"),
Profile: "test",
// We do not expect reuse because the order names don't match firstOrder
ExpectReuse: false,
},
{
Name: "Missing profile, same regID",
RegistrationID: Registration.Id,
DnsNames: append(names, "blog.zombo.com"),
// We do not expect reuse because the profile is missing
ExpectReuse: false,
},
{
Name: "Missing profile, same regID",
RegistrationID: Registration.Id,
DnsNames: append(names, "blog.zombo.com"),
Profile: "different",
// We do not expect reuse because a different profile is specified
ExpectReuse: false,
},
{
Name: "Duplicate order, different regID",
RegistrationID: secondReg.Id,
DnsNames: names,
Profile: "test",
// We do not expect reuse because the order regID differs from firstOrder
ExpectReuse: false,
},
@ -1445,6 +1465,7 @@ func TestNewOrder_OrderReusex(t *testing.T) {
order, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
RegistrationID: tc.RegistrationID,
DnsNames: tc.DnsNames,
CertificateProfileName: tc.Profile,
})
test.AssertNotError(t, err, "NewOrder returned an unexpected error")
test.AssertNotNil(t, order.Id, "NewOrder returned an order with a nil Id")
@ -1462,50 +1483,6 @@ func TestNewOrder_OrderReusex(t *testing.T) {
}
}
// TestNewOrder_OrderReuse_Profile tests that order reuse respects profiles.
// This is not simply a test case in TestNewOrder_OrderReuse because it relies
// on feature-flag gated behavior. It should be unified with that function when
// the feature flag is removed.
func TestNewOrder_OrderReuse_Profile(t *testing.T) {
// TODO(#7324): Integrate these cases into TestNewOrder_OrderReuse.
if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
t.Skip("this test requires the db to have the certificateProfileName column in the orders table")
}
_, _, ra, _, _, cleanUp := initAuthorities(t)
defer cleanUp()
features.Set(features.Config{MultipleCertificateProfiles: true})
defer features.Reset()
// Create an initial order with a profile name.
extant, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
RegistrationID: Registration.Id,
CertificateProfileName: "test",
DnsNames: []string{"a.com", "b.com"},
})
test.AssertNotError(t, err, "creating test order")
// Creating an identical order should reuse the first one.
new, err := ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
RegistrationID: Registration.Id,
CertificateProfileName: "test",
DnsNames: []string{"a.com", "b.com"},
})
test.AssertNotError(t, err, "creating test order")
test.AssertEquals(t, new.Id, extant.Id)
// Creating a new order for the same names but a different profile should not
// reuse the first one.
new, err = ra.NewOrder(context.Background(), &rapb.NewOrderRequest{
RegistrationID: Registration.Id,
CertificateProfileName: "test2",
DnsNames: []string{"a.com", "b.com"},
})
test.AssertNotError(t, err, "creating test order")
test.AssertNotEquals(t, new.Id, extant.Id)
}
// TestNewOrder_OrderReuse_Expired tests that expired orders are not reused.
// This is not simply a test case in TestNewOrder_OrderReuse because it has
// side effects.

View File

@ -271,8 +271,7 @@ func initTables(dbMap *borp.DbMap) {
dbMap.AddTableWithName(core.Certificate{}, "certificates").SetKeys(true, "ID")
dbMap.AddTableWithName(core.CertificateStatus{}, "certificateStatus").SetKeys(true, "ID")
dbMap.AddTableWithName(core.FQDNSet{}, "fqdnSets").SetKeys(true, "ID")
dbMap.AddTableWithName(orderModelv2{}, "orders").SetKeys(true, "ID")
dbMap.AddTableWithName(orderModelv1{}, "orders").SetKeys(true, "ID")
dbMap.AddTableWithName(orderModel{}, "orders").SetKeys(true, "ID")
dbMap.AddTableWithName(orderToAuthzModel{}, "orderToAuthz").SetKeys(false, "OrderID", "AuthzID")
dbMap.AddTableWithName(orderFQDNSet{}, "orderFqdnSets").SetKeys(true, "ID")
dbMap.AddTableWithName(authzModel{}, "authz2").SetKeys(true, "ID")

View File

@ -1,9 +0,0 @@
-- +migrate Up
-- SQL in section 'Up' is executed when this migration is applied
ALTER TABLE `orders` ADD COLUMN `certificateProfileName` varchar(32) DEFAULT NULL;
-- +migrate Down
-- SQL section 'Down' is executed when this migration is rolled back
ALTER TABLE `orders` DROP COLUMN `certificateProfileName`;

View File

@ -0,0 +1 @@
../../db/boulder_sa/20240304000000_CertificateProfiles.sql

View File

@ -0,0 +1,9 @@
-- +migrate Up
-- SQL in section 'Up' is executed when this migration is applied
ALTER TABLE `orders` ADD COLUMN `certificateProfileName` varchar(32) DEFAULT NULL;
-- +migrate Down
-- SQL section 'Down' is executed when this migration is rolled back
ALTER TABLE `orders` DROP COLUMN `certificateProfileName`;

View File

@ -370,20 +370,9 @@ type lintingCertModel struct {
Expires time.Time
}
// TODO(#7324) orderModelv1 is deprecated, use orderModelv2 moving forward.
type orderModelv1 struct {
ID int64
RegistrationID int64
Expires time.Time
Created time.Time
Error []byte
CertificateSerial string
BeganProcessing bool
}
// orderModelv2 represents one row in the orders table. The
// CertificateProfileName column is a pointer because the column is NULL-able.
type orderModelv2 struct {
// orderModel represents one row in the orders table. The CertificateProfileName
// column is a pointer because the column is NULL-able.
type orderModel struct {
ID int64
RegistrationID int64
Expires time.Time
@ -399,59 +388,11 @@ type orderToAuthzModel struct {
AuthzID int64
}
// TODO(#7324) orderToModelv1 is deprecated, use orderModelv2 moving forward.
func orderToModelv1(order *corepb.Order) (*orderModelv1, error) {
om := &orderModelv1{
ID: order.Id,
RegistrationID: order.RegistrationID,
Expires: order.Expires.AsTime(),
Created: order.Created.AsTime(),
BeganProcessing: order.BeganProcessing,
CertificateSerial: order.CertificateSerial,
}
if order.Error != nil {
errJSON, err := json.Marshal(order.Error)
if err != nil {
return nil, err
}
if len(errJSON) > mediumBlobSize {
return nil, fmt.Errorf("Error object is too large to store in the database")
}
om.Error = errJSON
}
return om, nil
}
// TODO(#7324) modelToOrderv1 is deprecated, use orderModelv2 moving forward.
func modelToOrderv1(om *orderModelv1) (*corepb.Order, error) {
order := &corepb.Order{
Id: om.ID,
RegistrationID: om.RegistrationID,
Expires: timestamppb.New(om.Expires),
Created: timestamppb.New(om.Created),
CertificateSerial: om.CertificateSerial,
BeganProcessing: om.BeganProcessing,
}
if len(om.Error) > 0 {
var problem corepb.ProblemDetails
err := json.Unmarshal(om.Error, &problem)
if err != nil {
return &corepb.Order{}, badJSONError(
"failed to unmarshal order model's error",
om.Error,
err)
}
order.Error = &problem
}
return order, nil
}
func orderToModelv2(order *corepb.Order) (*orderModelv2, error) {
func orderToModel(order *corepb.Order) (*orderModel, error) {
// Make a local copy so we can take a reference to it below.
profile := order.CertificateProfileName
om := &orderModelv2{
om := &orderModel{
ID: order.Id,
RegistrationID: order.RegistrationID,
Expires: order.Expires.AsTime(),
@ -474,7 +415,7 @@ func orderToModelv2(order *corepb.Order) (*orderModelv2, error) {
return om, nil
}
func modelToOrderv2(om *orderModelv2) (*corepb.Order, error) {
func modelToOrder(om *orderModel) (*corepb.Order, error) {
profile := ""
if om.CertificateProfileName != nil {
profile = *om.CertificateProfileName

View File

@ -237,7 +237,7 @@ func TestAuthzModel(t *testing.T) {
// validation error JSON field to an Order produces the expected bad JSON error.
func TestModelToOrderBadJSON(t *testing.T) {
badJSON := []byte(`{`)
_, err := modelToOrderv2(&orderModelv2{
_, err := modelToOrder(&orderModel{
Error: badJSON,
})
test.AssertError(t, err, "expected error from modelToOrderv2")
@ -250,21 +250,6 @@ func TestOrderModelThereAndBackAgain(t *testing.T) {
clk := clock.New()
now := clk.Now()
order := &corepb.Order{
Id: 0,
RegistrationID: 2016,
Expires: timestamppb.New(now.Add(24 * time.Hour)),
Created: timestamppb.New(now),
Error: nil,
CertificateSerial: "1",
BeganProcessing: true,
}
model1, err := orderToModelv1(order)
test.AssertNotError(t, err, "orderToModelv1 should not have errored")
returnOrder, err := modelToOrderv1(model1)
test.AssertNotError(t, err, "modelToOrderv1 should not have errored")
test.AssertDeepEquals(t, order, returnOrder)
anotherOrder := &corepb.Order{
Id: 1,
RegistrationID: 2024,
Expires: timestamppb.New(now.Add(24 * time.Hour)),
@ -274,11 +259,11 @@ func TestOrderModelThereAndBackAgain(t *testing.T) {
BeganProcessing: true,
CertificateProfileName: "phljny",
}
model2, err := orderToModelv2(anotherOrder)
model, err := orderToModel(order)
test.AssertNotError(t, err, "orderToModelv2 should not have errored")
returnOrder, err = modelToOrderv2(model2)
returnOrder, err := modelToOrder(model)
test.AssertNotError(t, err, "modelToOrderv2 should not have errored")
test.AssertDeepEquals(t, anotherOrder, returnOrder)
test.AssertDeepEquals(t, order, returnOrder)
}
// TestPopulateAttemptedFieldsBadJSON tests that populating a challenge from an

View File

@ -568,30 +568,18 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
}
// Second, insert the new order.
var orderID int64
var err error
created := ssa.clk.Now()
if features.Get().MultipleCertificateProfiles {
omv2 := orderModelv2{
om := orderModel{
RegistrationID: req.NewOrder.RegistrationID,
Expires: req.NewOrder.Expires.AsTime(),
Created: created,
CertificateProfileName: &req.NewOrder.CertificateProfileName,
}
err = tx.Insert(ctx, &omv2)
orderID = omv2.ID
} else {
omv1 := orderModelv1{
RegistrationID: req.NewOrder.RegistrationID,
Expires: req.NewOrder.Expires.AsTime(),
Created: created,
}
err = tx.Insert(ctx, &omv1)
orderID = omv1.ID
}
err := tx.Insert(ctx, &om)
if err != nil {
return nil, err
}
orderID := om.ID
// Third, insert all of the orderToAuthz relations.
// Have to combine the already-associated and newly-created authzs.
@ -713,7 +701,7 @@ func (ssa *SQLStorageAuthority) SetOrderError(ctx context.Context, req *sapb.Set
return nil, errIncompleteRequest
}
_, overallError := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) {
om, err := orderToModelv2(&corepb.Order{
om, err := orderToModel(&corepb.Order{
Id: req.Id,
Error: req.Error,
})

View File

@ -18,7 +18,6 @@ import (
"math/bits"
mrand "math/rand/v2"
"net"
"os"
"reflect"
"slices"
"strings"
@ -1175,7 +1174,7 @@ func TestFinalizeOrder(t *testing.T) {
test.AssertEquals(t, updatedOrder.Status, string(core.StatusValid))
}
func TestOrderWithOrderModelv1(t *testing.T) {
func TestOrder(t *testing.T) {
sa, fc, cleanup := initSA(t)
defer cleanup()
@ -1231,17 +1230,10 @@ func TestOrderWithOrderModelv1(t *testing.T) {
test.AssertDeepEquals(t, storedOrder, expectedOrder)
}
func TestOrderWithOrderModelv2(t *testing.T) {
if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
t.Skip("Test requires 20240304000000_CertificateProfiles.sql migration to have run")
}
func TestOrderWithProfile(t *testing.T) {
sa, fc, cleanup := initSA(t)
defer cleanup()
features.Set(features.Config{MultipleCertificateProfiles: true})
defer features.Reset()
reg := createWorkingRegistration(t, sa)
authzExpires := fc.Now().Add(time.Hour)
authzID := createPendingAuthorization(t, sa, "example.com", authzExpires)
@ -1295,130 +1287,6 @@ func TestOrderWithOrderModelv2(t *testing.T) {
storedOrder, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: order.Id})
test.AssertNotError(t, err, "sa.GetOrder failed")
test.AssertDeepEquals(t, storedOrder, expectedOrder)
//
// Test that an order without a certificate profile name, but with the
// MultipleCertificateProfiles feature flag enabled works as expected.
//
inputOrderNoName := &corepb.Order{
RegistrationID: reg.Id,
Expires: timestamppb.New(expires),
DnsNames: []string{"example.com"},
V2Authorizations: []int64{authzID},
}
// Create the order
orderNoName, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
NewOrder: &sapb.NewOrderRequest{
RegistrationID: inputOrderNoName.RegistrationID,
Expires: inputOrderNoName.Expires,
DnsNames: inputOrderNoName.DnsNames,
V2Authorizations: inputOrderNoName.V2Authorizations,
CertificateProfileName: inputOrderNoName.CertificateProfileName,
},
})
test.AssertNotError(t, err, "sa.NewOrderAndAuthzs failed")
// The Order from GetOrder should match the following expected order
created = sa.clk.Now()
expectedOrderNoName := &corepb.Order{
// The registration ID, authorizations, expiry, and names should match the
// input to NewOrderAndAuthzs
RegistrationID: inputOrderNoName.RegistrationID,
V2Authorizations: inputOrderNoName.V2Authorizations,
DnsNames: inputOrderNoName.DnsNames,
Expires: inputOrderNoName.Expires,
// The ID should have been set to 2 by the SA
Id: 2,
// The status should be pending
Status: string(core.StatusPending),
// The serial should be empty since this is a pending order
CertificateSerial: "",
// We should not be processing it
BeganProcessing: false,
// The created timestamp should have been set to the current time
Created: timestamppb.New(created),
}
// Fetch the order by its ID and make sure it matches the expected
storedOrderNoName, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: orderNoName.Id})
test.AssertNotError(t, err, "sa.GetOrder failed")
test.AssertDeepEquals(t, storedOrderNoName, expectedOrderNoName)
}
func TestOrderModelMigration(t *testing.T) {
if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
t.Skip("Test requires 20240304000000_CertificateProfiles.sql migration to have run")
}
sa, fc, cleanup := initSA(t)
defer cleanup()
reg := createWorkingRegistration(t, sa)
// Create an order using the v1 model
authzID := createPendingAuthorization(t, sa, "example.com", fc.Now().Add(time.Hour))
order, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
NewOrder: &sapb.NewOrderRequest{
RegistrationID: reg.Id,
Expires: timestamppb.New(fc.Now().Add(2 * time.Hour)),
DnsNames: []string{"example.com"},
V2Authorizations: []int64{authzID},
},
})
if err != nil {
t.Fatalf("failed to insert order using orderModelv1: %s", err)
}
// Retrieve that order using the v2 model
features.Set(features.Config{MultipleCertificateProfiles: true})
defer features.Reset()
storedOrder, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: order.Id})
if err != nil {
t.Fatalf("failed to retrieve order using orderModelv2: %s", err)
}
if storedOrder.CertificateProfileName != "" {
t.Errorf("order inserted with v1 schema should have empty profilename, instead got %q", storedOrder.CertificateProfileName)
}
}
func TestOrderModelMigrationRollback(t *testing.T) {
if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
t.Skip("Test requires 20240304000000_CertificateProfiles.sql migration to have run")
}
sa, fc, cleanup := initSA(t)
defer cleanup()
reg := createWorkingRegistration(t, sa)
// Create an order using the v2 model
features.Set(features.Config{MultipleCertificateProfiles: true})
defer features.Reset()
authzID := createPendingAuthorization(t, sa, "example.com", fc.Now().Add(time.Hour))
order, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
NewOrder: &sapb.NewOrderRequest{
RegistrationID: reg.Id,
Expires: timestamppb.New(fc.Now().Add(2 * time.Hour)),
DnsNames: []string{"example.com"},
V2Authorizations: []int64{authzID},
CertificateProfileName: "asdf",
},
})
if err != nil {
t.Fatalf("failed to insert order using orderModelv2: %s", err)
}
// Retrieve that order using the v1 model
features.Reset()
storedOrder, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: order.Id})
if err != nil {
t.Fatalf("failed to retrieve order using orderModelv1: %s", err)
}
if storedOrder.CertificateProfileName != "" {
t.Errorf("order retrieved with v1 schema should have empty profilename, instead got %q", storedOrder.CertificateProfileName)
}
}
// TestGetAuthorization2NoRows ensures that the GetAuthorization2 function returns

View File

@ -22,7 +22,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"
"github.com/letsencrypt/boulder/identifier"
blog "github.com/letsencrypt/boulder/log"
@ -410,13 +409,7 @@ func (ssa *SQLStorageAuthorityRO) GetOrder(ctx context.Context, req *sapb.OrderR
}
txn := func(tx db.Executor) (interface{}, error) {
var omObj interface{}
var err error
if features.Get().MultipleCertificateProfiles {
omObj, err = tx.Get(ctx, orderModelv2{}, req.Id)
} else {
omObj, err = tx.Get(ctx, orderModelv1{}, req.Id)
}
omObj, err := tx.Get(ctx, orderModel{}, req.Id)
if err != nil {
if db.IsNoRows(err) {
return nil, berrors.NotFoundError("no order found for ID %d", req.Id)
@ -427,12 +420,7 @@ func (ssa *SQLStorageAuthorityRO) GetOrder(ctx context.Context, req *sapb.OrderR
return nil, berrors.NotFoundError("no order found for ID %d", req.Id)
}
var order *corepb.Order
if features.Get().MultipleCertificateProfiles {
order, err = modelToOrderv2(omObj.(*orderModelv2))
} else {
order, err = modelToOrderv1(omObj.(*orderModelv1))
}
order, err := modelToOrder(omObj.(*orderModel))
if err != nil {
return nil, err
}

View File

@ -49,7 +49,6 @@
},
"healthCheckInterval": "4s",
"features": {
"MultipleCertificateProfiles": true,
"InsertAuthzsIndividually": true
}
},

View File

@ -48,7 +48,8 @@
}
},
"features": {
"UseKvLimitsForNewOrder": true
"UseKvLimitsForNewOrder": true,
"MultipleCertificateProfiles": true
}
},
"syslog": {