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

Add support for all Guest Customization options in resource and data source vcd_vapp_vm #462

Merged
merged 15 commits into from
Mar 5, 2020

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Mar 2, 2020

Closes #303, closes #52, closes #427

This PR adds support for all Guest customization options available in the UI and adds the following fields to customization block:

  • enabled
  • change_sid
  • allow_local_admin_password
  • must_change_password_on_first_login
  • auto_generate_password
  • admin_password
  • number_of_auto_logons
  • join_domain
  • join_org_domain
  • join_domain_name
  • join_domain_user
  • join_domain_password
  • join_domain_account_ou
  • initscript (the root component initscript is deprecated and moved to customization block to reflect UI behavior and API position)

All the above fields are computed and optional. This means that there are no breaking changes. User is free to set only some of the fields and the others will not be overridden as they may be coming from catalog item itself.

Note it is pretty much hard to cover behavior in tests because majority of these fields require a Windows machine, which is too big to include into testing routine.

Bugs covered in this PR:
  • resource/vcd_vapp_vm when a single value of customization.0.force=false was specified in the customization block - it would crash with interface {} is nil error (was previously raised as a separate PR https://github.com/terraform-providers/terraform-provider-vcd/pull/456, but because it was not merged and these PRs go over each other I have closed that one)
  • resource/vcd_vapp_vm customization.0.force=true could have skipped "Forced customization" on each apply
Extra features:
  • Adds make test-upgrade-prepare with similar behavior to make test-binary-prepare to only generate files and not trigger test run itself
Tests have passed on the following:
  • 9.5 and 10.0 (acceptance tests)
  • 9.5 (binary tests)
  • 9.5 (upgrade tests)

* `enabled` (Optional; *v2.7+*) `true` will enable guest customization which may occur on first boot or if the `force` flag is used.
This option should be selected for **Power on and Force re-customization to work**. For backwards compatibility it is
enabled by default when deprecated field `initscript` is used.
* `change_sid` (Optional; *v2.7+*) Allows to change SID (security identifier). Only applicable for Windows operating systems.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only applicable for Windows operating systems.

This is a useful comment. Are there more options below which should have it too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The customization is pretty much "mystified". Many of the fields may work or not work properly based on the guest operating system. What I did find is that change_sid is mentioned exclusively in vCD documentation - https://docs.vmware.com/en/vCloud-Director/9.7/com.vmware.vcloud.user.doc/GUID-BB682E4D-DCD7-4936-A665-0B0FBD6F0EB5.html

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

One small doc polish, nice otherwise!

* `join_domain_user` (Optional; *v2.7+*) User to be used for domain join.
* `join_domain_password` (Optional; *v2.7+*) Password to be used for domain join.
* `join_domain_account_ou` (Optional; *v2.7+*) Organizational unit to be used for domain join.
* `initscript` (Optional; *v2.7+*) Provide initscript to be executed when customization is applied.

## Example forced customization workflow
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
## Example forced customization workflow
## Example of a Forced Customization Workflow

Also, nit: the // ... in the examples below are out of indent and missing new line after them.

Screenshot 2020-03-03 at 16 19 20

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed from // to # (the real HCL comment symbol).

if len(customizationSlice) == 1 {
cust := customizationSlice[0]
if cust != nil {
// customBlock := cust.(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need this comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. removed.

Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

Overall looks straight forward

// Only react to "enabled" field when legacy `initscript` is not specified. Legacy behavior is such that when `initscript`
// is specified - guest customization is enabled by default therefore we ignore "enabled" field
if _, isSetDeprecatedInitScript := d.GetOk("initscript"); !isSetDeprecatedInitScript {
// if enabled, isEnabled := customBlock["enabled"]; isEnabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need this comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. removed.

@ghost ghost added size/XXL and removed size/XL labels Mar 5, 2020
@ghost ghost added size/XL and removed size/XXL labels Mar 5, 2020
Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

Run on 9.5 looks good. LGTM

Copy link
Contributor

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

LGTM

@Didainius Didainius merged commit f954e09 into vmware:master Mar 5, 2020
@Didainius Didainius deleted the change-sid branch March 5, 2020 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants