From a89fa76af06d6736f31a3c35ce06086f2c523245 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 23 May 2024 16:14:25 -0400 Subject: [PATCH] common: validate capacity reservation options The capacity reservation options (ID, ARN, or Preference) are all mutually exclusive, only one of the three should be set in a given configuration. The code wasn't enforcing this, leading to cases in which conflicts could be specified in the configuration, and the call to start the instance would then fail during build instead of before it starts. This commit adds some logic to ensure that only valid options are accepted, and some tests are added to make sure these options cannot conflict. --- builder/common/run_config.go | 17 ++++- builder/common/run_config_test.go | 121 ++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/builder/common/run_config.go b/builder/common/run_config.go index 80dc86112..84f9b2a2f 100644 --- a/builder/common/run_config.go +++ b/builder/common/run_config.go @@ -887,11 +887,24 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { } - if c.CapacityReservationPreference == "" { + capacityReservationTargetSet := false + if c.CapacityReservationId != "" || c.CapacityReservationGroupArn != "" { + capacityReservationTargetSet = true + } + + if c.CapacityReservationGroupArn != "" && c.CapacityReservationId != "" { + errs = append(errs, fmt.Errorf("capacity_reservation_id and capacity_reservation_group_arn are mutually exclusive, only one should be used")) + } + + if capacityReservationTargetSet && c.CapacityReservationPreference != "" { + errs = append(errs, fmt.Errorf("capacity_reservation_id, capacity_reservation_group_arn and capacity_reservation_preference are mutually exclusive, only one should be set")) + } + + if c.CapacityReservationPreference == "" && c.CapacityReservationId == "" && c.CapacityReservationGroupArn == "" { c.CapacityReservationPreference = "none" } switch c.CapacityReservationPreference { - case "none", "open": + case "", "none", "open": default: errs = append(errs, fmt.Errorf(`capacity_reservation_preference only accepts 'none' or 'open' values`)) } diff --git a/builder/common/run_config_test.go b/builder/common/run_config_test.go index f29f35e3c..b7f793e71 100644 --- a/builder/common/run_config_test.go +++ b/builder/common/run_config_test.go @@ -461,3 +461,124 @@ func TestRunConfigPrepare_InvalidTenantForHost(t *testing.T) { }) } } + +func TestRunConfigPrepare_WithCapacityReservations(t *testing.T) { + tests := []struct { + name string + reservationID string + reservationPreference string + reservationARN string + expectedReservationID string + expectedReservationPreference string + expectedReservationARN string + expectError bool + }{ + { + name: "None set, preference should be set to none, no error", + reservationID: "", + reservationPreference: "", + reservationARN: "", + expectedReservationID: "", + expectedReservationPreference: "none", + expectedReservationARN: "", + expectError: false, + }, + { + name: "Preference set to none, preference should be set to none, no error", + reservationID: "", + reservationPreference: "none", + reservationARN: "", + expectedReservationID: "", + expectedReservationPreference: "none", + expectedReservationARN: "", + expectError: false, + }, + { + name: "Preference set to open, preference should be set to open, no error", + reservationID: "", + reservationPreference: "open", + reservationARN: "", + expectedReservationID: "", + expectedReservationPreference: "open", + expectedReservationARN: "", + expectError: false, + }, + { + name: "Preference set to and invalid value, expect an error", + reservationID: "", + reservationPreference: "invalid", + reservationARN: "", + expectedReservationID: "", + expectedReservationPreference: "invalid", + expectedReservationARN: "", + expectError: true, + }, + { + name: "ID set to something, no preference, no error, no changes to config", + reservationID: "cr-123456", + reservationPreference: "", + reservationARN: "", + expectedReservationID: "cr-123456", + expectedReservationPreference: "", + expectedReservationARN: "", + expectError: false, + }, + { + name: "ARN set to something, no preference, no error, no changes to config", + reservationID: "", + reservationPreference: "", + reservationARN: "arn-asduilovgf", + expectedReservationID: "", + expectedReservationPreference: "", + expectedReservationARN: "arn-asduilovgf", + expectError: false, + }, + { + name: "Preference set to none, ID not empty, should error as both are incompatible", + reservationID: "cr-123456", + reservationPreference: "none", + reservationARN: "", + expectedReservationID: "cr-123456", + expectedReservationPreference: "none", + expectedReservationARN: "", + expectError: true, + }, + { + name: "ID and ARN not empty, should error as both are incompatible", + reservationID: "cr-123456", + reservationPreference: "", + reservationARN: "arn-aseldigubh", + expectedReservationID: "cr-123456", + expectedReservationPreference: "", + expectedReservationARN: "arn-aseldigubh", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := testConfig() + c.CapacityReservationGroupArn = tt.reservationARN + c.CapacityReservationId = tt.reservationID + c.CapacityReservationPreference = tt.reservationPreference + + errs := c.Prepare(nil) + if (len(errs) != 0) != tt.expectError { + t.Errorf("expected %t errors, got %t", tt.expectError, len(errs) != 0) + t.Logf("errors: %v", errs) + } + + if c.CapacityReservationGroupArn != tt.expectedReservationARN { + t.Errorf("expected Reservation ARN %q, got %q", tt.expectedReservationARN, c.CapacityReservationGroupArn) + } + + if c.CapacityReservationId != tt.expectedReservationID { + t.Errorf("expected Reservation ID %q, got %q", tt.expectedReservationARN, c.CapacityReservationId) + } + + if c.CapacityReservationPreference != tt.expectedReservationPreference { + t.Errorf("expected Reservation Preference %q, got %q", tt.expectedReservationPreference, c.CapacityReservationPreference) + } + }) + } +}