hashicorp / packer-plugin-vsphere

Packer plugin for VMware vSphere Builder
https://www.packer.io/docs/builders/vsphere
Mozilla Public License 2.0
93 stars 91 forks source link

fix: remove obsolete path check #353

Closed cyberops7 closed 6 months ago

cyberops7 commented 6 months ago

Apparently this check is obsolete (see https://github.com/hashicorp/packer/issues/6975, specifically "You're right, the folder bound to the root doesn't do any effect at code level, that was because the first implementation used to use that, sadly I forgot to remove it when I changed it.") and still has an open issue (https://github.com/hashicorp/packer-plugin-vsphere/issues/3). With this check in place, any attempt to use the vsphere-template post-processor seems to fail with errors like:

Error: Failed preparing post-processor-block "vsphere-template" ""

  on ubuntu-2204.pkr.hcl line 303:
  (source code not available)

1 error(s) occurred:

* Folder must be bound to the root

This was tested against the latest version.

gist with DEBUG log output: https://gist.github.com/cyberops7/751cd2f23189579f487174c7580f594a

Test case: include the vsphere-template post-processor block:

post-processors {
    post-processor "vsphere-template"{
      host             = var.vcenter_server
      insecure         = var.vcenter_insecure_connection
      username         = var.vcenter_username
      password         = var.vcenter_password
      datacenter       = var.vcenter_datacenter
      folder           = var.vcenter_template_folder
    }
  }

Closes #3

hashicorp-cla commented 6 months ago

CLA assistant check
All committers have signed the CLA.

tenthirtyam commented 6 months ago
Error: post-processor/vsphere-template/post-processor.go:13:2: "strings" imported and not used
cyberops7 commented 6 months ago

I see this is assigned to me, but I do not see any action that I can take. It seems like we are waiting on one more code reviewer?

tenthirtyam commented 6 months ago

No other changes from my side, Lucas.

lbajolet-hashicorp commented 6 months ago

It's a merge then, thanks for the confirmation @tenthirtyam !