Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove obsolete path check #353

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

cyberops7
Copy link
Contributor

Apparently this check is obsolete (see hashicorp/packer#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 (#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

Apparently this check is obsolete (see hashicorp/packer#6975) and still has an open issue (hashicorp#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
@cyberops7 cyberops7 requested a review from a team as a code owner January 1, 2024 08:15
@hashicorp-cla
Copy link

hashicorp-cla commented Jan 1, 2024

CLA assistant check
All committers have signed the CLA.

@tenthirtyam
Copy link
Collaborator

Error: post-processor/vsphere-template/post-processor.go:13:2: "strings" imported and not used

Copy link
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tenthirtyam tenthirtyam changed the title Remove obsolete check that breaks the vsphere-template post-processor fix: remove obsolete path check Jan 5, 2024
@tenthirtyam tenthirtyam added the bug label Jan 5, 2024
@cyberops7
Copy link
Contributor Author

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?

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @cyberops7,

Thanks for the PR and the ping.

The code looks good to me, we can probably merge this soon.

@tenthirtyam did you have anything else in mind that should change here? If this looks good to you as well we can merge this one today I'd think

@tenthirtyam
Copy link
Collaborator

No other changes from my side, Lucas.

@lbajolet-hashicorp
Copy link
Contributor

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

@lbajolet-hashicorp lbajolet-hashicorp merged commit bb397ad into hashicorp:main Jan 9, 2024
12 checks passed
@hashicorp hashicorp locked and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vsphere-template post-processor maybe should not enforce folder parameter bound to root
4 participants