Skip to content

Commit

Permalink
fix: don't assume driver isn't nil in StepConnect (#437)
Browse files Browse the repository at this point in the history
* common/connect: don't assume driver isn't nil

When attempting to cleanup the existing connections to the VSphere
cluster, we get the driver from the state shared between all steps in
order to run the commands necessary for that.

This can however fail, if the step couldn't succeed for any reason.

Then, if the connection fails and the driver isn't set in the state,
when running Cleanup on the connect step, we get the driver from state
and cast it to a driver.Driver.
This will make the plugin crash, and so we change how we proceed here,
by getting the driver with a `GetOk`, exiting if it failed to be fetched
from state, then casting the result if it was created in state.

This should prevent crashes like those.

* Apply suggestions from code review

Co-authored-by: Ryan Johnson <[email protected]>

---------

Co-authored-by: Wilken Rivera <[email protected]>
Co-authored-by: Ryan Johnson <[email protected]>
  • Loading branch information
3 people authored Jun 12, 2024
1 parent 597a7c8 commit 6fe9830
Showing 1 changed file with 20 additions and 8 deletions.
28 changes: 20 additions & 8 deletions builder/vsphere/common/step_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"context"
"fmt"
"log"
"reflect"

"github.com/hashicorp/packer-plugin-sdk/multistep"
packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
Expand Down Expand Up @@ -76,14 +77,25 @@ func (s *StepConnect) Run(_ context.Context, state multistep.StateBag) multistep

func (s *StepConnect) Cleanup(state multistep.StateBag) {
ui := state.Get("ui").(packersdk.Ui)
d, ok := state.GetOk("driver")
if !ok {
log.Printf("[INFO] No driver in state; nothing to cleanup.")
return
}

driver, ok := d.(driver.Driver)
if !ok {
log.Printf("[ERROR] The object stored in the state under 'driver' key is of type '%s', not 'driver.Driver'. This could indicate a problem with the state initialization or management.", reflect.TypeOf(d))
return
}

ui.Message("Closing sessions ....")
if driver, ok := state.Get("driver").(driver.Driver); ok {
errorRestClient, errorSoapClient := driver.Cleanup()
if errorRestClient != nil {
log.Printf("[WARN] Failed to close REST client session. The session may already be closed: %s", errorRestClient.Error())
}
if errorSoapClient != nil {
log.Printf("[WARN] Failed to close SOAP client session. The session may already be closed: %s", errorSoapClient.Error())
}

errorRestClient, errorSoapClient := driver.Cleanup()
if errorRestClient != nil {
log.Printf("[WARN] Failed to close REST client session. The session may already be closed: %s", errorRestClient.Error())
}
if errorSoapClient != nil {
log.Printf("[WARN] Failed to close SOAP client session. The session may already be closed: %s", errorSoapClient.Error())
}
}

0 comments on commit 6fe9830

Please sign in to comment.