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.

Sign up for GitHub

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

Jump to bottom

feat: add support for vmrest #161

Closed
wants to merge 24 commits into from
Closed

feat: add support for vmrest #161

wants to merge 24 commits into from

Conversation

niwamo
Copy link

@niwamo niwamo commented Nov 16, 2023

As described in #157:

Creates a new driver for vmrest, the API that ships with VMWare Workstation Pro. Due to limitations in the API, only the vmware-vmx builder can be implemented.

Checks have been added to the vmware-iso builder to throw an explanatory error if a user mistakenly tries to use the vmrest remote_type. Additional checks have been added to throw errors if incompatible configurations are set while using the vmrest remote_type.

The vmware-vmx documentation was also updated.

A test for the new driver has been provided in #157.

Closes #157

niwamo and others added 19 commits November 9, 2023 11:38
@niwamo
+driver_vmrest.go (template) +driver.go (updates)
1141841
@niwamo
3 driver funcs left to implement
e9f35c2
@niwamo
+HostIP
1bfc8e4
@niwamo
+HostIP v2
f978756
@niwamo
+ updates to remote config validation
3d1547c
@niwamo
+VNCAddress()
94cf148
@niwamo
+UpdateVMX() and +UpdateVMConfig()
d975d09
@niwamo
+Register()
18d18e1
@niwamo
+upload()
ed1c3ed
@niwamo
minor fixes
5048a4f
@niwamo
+addtl comments in driver_vmrest.go
f4d121f
@niwamo
updated docs and ran .web-docs/scripts/compile-to-webdocs.sh
28cfed6
@niwamo
addtl detail in documentation
8bd303d
@niwamo
+.web-docs updates
31c595b
@tenthirtyam @niwamo
chore(deps): golang.org/x/net
a4df5a5 
Address the following CVEs:

- CVE-2023-39325
- CVE-2023-44487
- CVE-2023-3978

Signed-off-by: Ryan Johnson <johnsonryan@vmware.com>
@tenthirtyam @niwamo
chore(deps): google.golang.org/grpc
e684d3c 
Address the following CVE:

- CVE-2023-44487

Signed-off-by: Ryan Johnson <johnsonryan@vmware.com>
@tenthirtyam @niwamo
chore(deps): bump github.com/vmware/govmomi from 0.26.0 to 0.33.1
92d5260 
- Bumps `github.com/vmware/govmomi` from 0.29.0 to 0.33.1.
- Added github.com/vmware/govmomi to dependabot dependency checks.

Signed-off-by: Ryan Johnson <johnsonryan@vmware.com>
@niwamo
Merge branch 'hashicorp:main' into main
8e4a9a0
@niwamo
fix web-docs
e7972b4
@niwamo niwamo requested a review from a team as a code owner November 16, 2023 18:56
@hashicorp-cla
Copy link

hashicorp-cla commented Nov 16, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@niwamo
Fix/Update driver_config_test.go
8230923 
Updated expected error value in "Invalid remote type" and updated
expected default in "Set default values". Note: driver_config.go was
updated to only set exsi-relevant defaults is esx5 is selected
@niwamo
Fixes linter-identified issues
381957d 
Fixes all issues in driver_vmrest.go identified by golangci-lint
@niwamo
Fix minor discrepancies between docs and .web-docs
e9480a2 
Fixes the two-line discrepancy identified by CI
@niwamo
Copy link
Author

niwamo commented Nov 17, 2023

All issues identified by the first round of checks should be addressed. There were three categories of issues:

  1. Tests for driver_config.go that I failed to update, including an error message that was intentionally changed, and the adjustment to only set esxi-relevant defaults if esxi is selected; Both have been fixed
  2. A small number of linter-identified issues in driver_vmrest.go. All good catches; All now addressed.
  3. Exactly two lines with discrepancies between docs and web-docs; Now fixed

I've also re-run my live test of the vmrest builder to ensure changes did not affect functionality

@niwamo
Copy link
Author

niwamo commented Nov 28, 2023

@tenthirtyam Since you were kind enough to review - do you know if there are additional steps I should be taking and/or reviewers I should be talking to? This has been slow to take off, and I want to make sure I'm doing my due diligence. The members of my team that need this functionality are already using a fork, but it would be nice to have this in the official plugin.

@nywilken
Copy link
Contributor

nywilken commented Dec 5, 2023

@tenthirtyam we will throw some time on your calendar to sync up on this change before starting to review.

@tenthirtyam tenthirtyam marked this pull request as draft May 9, 2024 13:03
@tenthirtyam tenthirtyam modified the milestones: Backlog, v2.0.0 Jul 5, 2024
@tenthirtyam tenthirtyam changed the title Adds support for building with the VMWare Workstation Pro API Adds support for building with the VMware Workstation Pro API Aug 8, 2024
@tenthirtyam tenthirtyam changed the title Adds support for building with the VMware Workstation Pro API 🚧 feat: add support for vmrest Aug 9, 2024
@niwamo
Copy link
Author

niwamo commented Aug 23, 2024

@tenthirtyam Thanks for the recent follow-up(s). Going to close this merge request, re-factor/clean-up the code, and re-open a request in the next couple of weeks. Will leave the same issue open. Thanks.

@niwamo niwamo closed this Aug 23, 2024
@tenthirtyam
Copy link
Collaborator

tenthirtyam commented Aug 23, 2024 •
edited
Loading

HI @niwamo 👋 - sorry. I've been meaning to get back with you on this one.

I'm in the midst of doing some consolidation of the drivers based on the recent changes the desktop hypervisors (personal license, too). Those are moving along well and we'll plan to pull out Player around the next major release for Workstation/Fusion and Player EOL.

With regards to vmrest, I'd suggest pausing on this one for the time being. There are some product enhancements planned (can't share publically, sorry!) where there will be some changes that may make these changes moot but lead to a more streamlined approach. I plan to address these in a major (e.g. v2.0.0) release at that time.

Ryan Johnson
Distinguished Engineer, VMware by Broadcom

@tenthirtyam tenthirtyam changed the title 🚧 feat: add support for vmrest feat: add support for vmrest Aug 23, 2024
@tenthirtyam tenthirtyam removed this from the v2.0.0 milestone Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tenthirtyam tenthirtyam tenthirtyam left review comments

Assignees

@niwamo niwamo

Labels
builder/vmware-vmx hypervisor/workstation VMware Workstation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for the VMware Workstation Pro API with the vmware-vmx builder
4 participants
@niwamo @hashicorp-cla @nywilken @tenthirtyam