letsencrypt · aarongable · Nov 22, 2024
@@ -15,6 +15,9 @@ import (
 // then call features.Set(parsedConfig) to load the parsed struct into this
 // package's global Config.
 type Config struct {
+	// Deprecated features. Delete once removed from all production configs.
+	MultipleCertificateProfiles bool
+
 	// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
 	// GET requests. WARNING: This feature is a draft and highly unstable.
 	ServeRenewalInfo bool
@@ -57,14 +60,6 @@ type Config struct {
 	// Only used when EnforceMultiCAA is true.
 	MultiCAAFullResults 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

@@ -13,7 +13,6 @@ import (
 	"github.com/letsencrypt/boulder/cmd"
 	"github.com/letsencrypt/boulder/core"
 	boulderDB "github.com/letsencrypt/boulder/db"
-	"github.com/letsencrypt/boulder/features"
 	blog "github.com/letsencrypt/boulder/log"
 )
 
@@ -273,11 +272,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")
-	if features.Get().MultipleCertificateProfiles {
-		dbMap.AddTableWithName(orderModelv2{}, "orders").SetKeys(true, "ID")
-	} else {
-		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")

@@ -0,0 +1 @@
+../../db/boulder_sa/20240304000000_CertificateProfiles.sql
@@ -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`;
@@ -381,18 +381,7 @@ 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
-}
-
-type orderModelv2 struct {
+type orderModel struct {
 	ID                     int64
 	RegistrationID         int64
 	Expires                time.Time
@@ -408,56 +397,8 @@ 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) {
-	om := &orderModelv2{
+func orderToModel(order *corepb.Order) (*orderModel, error) {
+	om := &orderModel{
 		ID:                     order.Id,
 		RegistrationID:         order.RegistrationID,
 		Expires:                order.Expires.AsTime(),
@@ -480,7 +421,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) {
 	order := &corepb.Order{
 		Id:                     om.ID,
 		RegistrationID:         om.RegistrationID,

@@ -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

@@ -644,27 +644,13 @@ 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{
-				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
+		om := orderModel{
+			RegistrationID:         req.NewOrder.RegistrationID,
+			Expires:                req.NewOrder.Expires.AsTime(),
+			Created:                ssa.clk.Now(),
+			CertificateProfileName: req.NewOrder.CertificateProfileName,
 		}
+		err := tx.Insert(ctx, &om)
 		if err != nil {
 			return nil, err
 		}
@@ -677,7 +663,7 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 			return nil, err
 		}
 		for _, id := range allAuthzIds {
-			err := inserter.Add([]interface{}{orderID, id})
+			err := inserter.Add([]interface{}{om.ID, id})
 			if err != nil {
 				return nil, err
 			}
@@ -688,15 +674,15 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 		}
 
 		// Fourth, insert the FQDNSet entry for the order.
-		err = addOrderFQDNSet(ctx, tx, req.NewOrder.DnsNames, orderID, req.NewOrder.RegistrationID, req.NewOrder.Expires.AsTime())
+		err = addOrderFQDNSet(ctx, tx, req.NewOrder.DnsNames, om.ID, req.NewOrder.RegistrationID, req.NewOrder.Expires.AsTime())
 		if err != nil {
 			return nil, err
 		}
 
 		if req.NewOrder.ReplacesSerial != "" {
 			// Update the replacementOrders table to indicate that this order
 			// replaces the provided certificate serial.
-			err := addReplacementOrder(ctx, tx, req.NewOrder.ReplacesSerial, orderID, req.NewOrder.Expires.AsTime())
+			err := addReplacementOrder(ctx, tx, req.NewOrder.ReplacesSerial, om.ID, req.NewOrder.Expires.AsTime())
 			if err != nil {
 				return nil, err
 			}
@@ -712,8 +698,8 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 		// Finally, build the overall Order PB.
 		res := &corepb.Order{
 			// ID and Created were auto-populated on the order model when it was inserted.
-			Id:      orderID,
-			Created: timestamppb.New(created),
+			Id:      om.ID,
+			Created: timestamppb.New(om.Created),
 			// These are carried over from the original request unchanged.
 			RegistrationID: req.NewOrder.RegistrationID,
 			Expires:        req.NewOrder.Expires,
@@ -797,7 +783,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,
 		})

@@ -1359,72 +1359,7 @@ func TestFinalizeOrder(t *testing.T) {
 	test.AssertEquals(t, updatedOrder.Status, string(core.StatusValid))
 }
 
-func TestOrderWithOrderModelv1(t *testing.T) {
-	sa, fc, cleanup := initSA(t)
-	defer cleanup()
-
-	reg := createWorkingRegistration(t, sa)
-	authzExpires := fc.Now().Add(time.Hour)
-	authzID := createPendingAuthorization(t, sa, "example.com", authzExpires)
-
-	// Set the order to expire in two hours
-	expires := fc.Now().Add(2 * time.Hour)
-
-	inputOrder := &corepb.Order{
-		RegistrationID:   reg.Id,
-		Expires:          timestamppb.New(expires),
-		DnsNames:         []string{"example.com"},
-		V2Authorizations: []int64{authzID},
-	}
-
-	// Create the order
-	order, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
-		NewOrder: &sapb.NewOrderRequest{
-			RegistrationID:   inputOrder.RegistrationID,
-			Expires:          inputOrder.Expires,
-			DnsNames:         inputOrder.DnsNames,
-			V2Authorizations: inputOrder.V2Authorizations,
-		},
-	})
-	test.AssertNotError(t, err, "sa.NewOrderAndAuthzs failed")
-
-	// The Order from GetOrder should match the following expected order
-	created := sa.clk.Now()
-	expectedOrder := &corepb.Order{
-		// The registration ID, authorizations, expiry, and names should match the
-		// input to NewOrderAndAuthzs
-		RegistrationID:   inputOrder.RegistrationID,
-		V2Authorizations: inputOrder.V2Authorizations,
-		DnsNames:         inputOrder.DnsNames,
-		Expires:          inputOrder.Expires,
-		// The ID should have been set to 1 by the SA
-		Id: 1,
-		// 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
-	storedOrder, err := sa.GetOrder(context.Background(), &sapb.OrderRequest{Id: order.Id})
-	test.AssertNotError(t, err, "sa.GetOrder failed")
-	test.AssertDeepEquals(t, storedOrder, expectedOrder)
-}
-
-func TestOrderWithOrderModelv2(t *testing.T) {
-	if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
-		t.Skip()
-	}
-
-	// The feature must be set before the SA is constructed because of a
-	// conditional on this feature in //sa/database.go.
-	features.Set(features.Config{MultipleCertificateProfiles: true})
-	defer features.Reset()
-
+func TestOrder(t *testing.T) {
 	fc := clock.NewFake()
 	fc.Set(time.Date(2015, 3, 4, 5, 0, 0, 0, time.UTC))
 
@@ -1493,11 +1428,7 @@ func TestOrderWithOrderModelv2(t *testing.T) {
 	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.
-	//
-
+	// Test that an order without a certificate profile name works as expected.
 	inputOrderNoName := &corepb.Order{
 		RegistrationID:   reg.Id,
 		Expires:          timestamppb.New(expires),