Skip to content

Commit

Permalink
common: validate capacity reservation options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lbajolet-hashicorp committed May 24, 2024
1 parent c585c59 commit a89fa76
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 2 deletions.
17 changes: 15 additions & 2 deletions builder/common/run_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`))
}
Expand Down
121 changes: 121 additions & 0 deletions builder/common/run_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}

0 comments on commit a89fa76

Please sign in to comment.