Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: r/vsphere_file provider crash #1866

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

stoyan-hristov
Copy link
Contributor

@stoyan-hristov stoyan-hristov commented Mar 24, 2023

Description

  • Updates resource_vsphere_file.createDirectory method to check whether the provided file path has any parent folder(s). If there are no folders to be created we return early without invoking FileManager.MakeDirectory. This fixes the “slice bounds out of range” runtime error which was crashing the provider.
  • Fixes the resource_vsphere_file_test acceptance tests as they were failing due to incorrect test vmdk files.
  • Adds a new TestAccResourceVSphereFile_uploadWithCreateDirectories test to cover the r/vsphere_file.create_directories configuration argument.

Testing done:

Performed terraform apply using the following configuration:

resource "vsphere_file" "iso" {
  datacenter       = data.vsphere_datacenter.datacenter.name
  datastore        = data.vsphere_datastore.nfs-tf.name
  source_file      = "${path.cwd}/ISO/test.iso"
  destination_file = "test.iso"
  create_directories = true
}

And verified that the file was uploaded correctly without any runtime errors.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?

Added TestAccResourceVSphereFile_uploadWithCreateDirectories

  • Have you run the acceptance tests on this branch?
Output from acceptance testing

make testacc TESTARGS="-run='TestAccResourceVSphereFile'"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run='TestAccResourceVSphereFile' -timeout 240m
?   	github.com/hashicorp/terraform-provider-vsphere	[no test files]
=== RUN   TestAccResourceVSphereFile_basic
--- PASS: TestAccResourceVSphereFile_basic (62.99s)
=== RUN   TestAccResourceVSphereFile_uploadWithCreateDirectories
--- PASS: TestAccResourceVSphereFile_uploadWithCreateDirectories (91.53s)
=== RUN   TestAccResourceVSphereFile_basicUploadAndCopy
--- PASS: TestAccResourceVSphereFile_basicUploadAndCopy (73.37s)
=== RUN   TestAccResourceVSphereFile_renamePostCreation
--- PASS: TestAccResourceVSphereFile_renamePostCreation (93.87s)
=== RUN   TestAccResourceVSphereFile_uploadAndCopyAndUpdate
--- PASS: TestAccResourceVSphereFile_uploadAndCopyAndUpdate (109.50s)
PASS

Release Note

Release note for CHANGELOG:

Bug Fix:

r/vsphere_file: Fixes a provider crash by updating the createDirectory method to check if the provided file path has any parent folder(s). If no folders need to be created FileManager.MakeDirectory is not invoked.

References

Closes #1779

…rp#1779

Update resource_vsphere_file.createDirectory method to check whether the provided file path has any parent folder(s). If there are no folders to be created we return early without invoking FileManager.MakeDirectory. This fixes the “slice bounds out of range” runtime error which was crashing the provider.

Fix the resource_vsphere_file_test acceptance tests as they were failing due to incorrect test vmdk files. Also, adding new TestAccResourceVSphereFile_uploadWithCreateDirectories test to cover the r/vsphere_file.create_directories configuration field.
@stoyan-hristov stoyan-hristov requested a review from a team as a code owner March 24, 2023 10:34
@github-actions github-actions bot added provider Type: Provider size/m Relative Sizing: Medium labels Mar 24, 2023
@tenthirtyam tenthirtyam added this to the v2.4.0 milestone Mar 24, 2023
@tenthirtyam tenthirtyam added bug Type: Bug area/storage Area: Storage labels Mar 24, 2023
@tenthirtyam tenthirtyam requested a review from appilon March 24, 2023 12:52
@tenthirtyam tenthirtyam changed the title fix: r/vsphere_file causes crash with incorrect configuration #1779 fix: r/vsphere_file causes crash with incorrect configuration Mar 28, 2023
@tenthirtyam tenthirtyam changed the title fix: r/vsphere_file causes crash with incorrect configuration fix: r/vsphere_file causes provider crash Mar 28, 2023
@tenthirtyam tenthirtyam changed the title fix: r/vsphere_file causes provider crash fix: r/vsphere_file provider crash Mar 28, 2023
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.

Thank you, @stoyan-hristov.

Reviewed, successfully tested, and approved. 🚀

@appilon - contingent on your review of the update and additonal acceptance test.

@appilon appilon self-assigned this Apr 20, 2023
Copy link
Contributor

@appilon appilon left a comment

Choose a reason for hiding this comment

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

LGTM

@appilon appilon merged commit d216b59 into hashicorp:main Apr 21, 2023
@github-actions
Copy link

github-actions bot commented May 5, 2023

This functionality has been released in v2.4.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

github-actions bot commented Jun 5, 2023

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 5, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area/storage Area: Storage bug Type: Bug provider Type: Provider size/m Relative Sizing: Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

r/vsphere_file causes crash with incorrect configuration
3 participants