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 a new module vmware_guest_tpm #1075

Merged
merged 6 commits into from
Oct 28, 2021

Conversation

Tomorrow9
Copy link
Collaborator

SUMMARY

Add a new module for adding or removing vTPM device to VM.

Fixes #1053

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

vmware_guest_tpm

ADDITIONAL INFORMATION

@goneri
Copy link
Member

goneri commented Oct 14, 2021

recheck

1 similar comment
@Tomorrow9
Copy link
Collaborator Author

recheck

@Tomorrow9
Copy link
Collaborator Author

Hi @Akasurde @goneri @mariolenz @sky-joker would you please help reviewing this merge request? Thanks.

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

Thanks for this new module @Tomorrow9! I wasn't able to test this yet but the code LGTM so far.

However, I really like check mode. Could you please implement this? It's really great to test changes to a playbook in a production environment by running it in check mode.

It would be great to have a test case for this, but I don't think this is really possible at the moment since we would need a key provider. I think the current vCSA comes with an embedded KMIP server, maybe we can make use of this in the long run. That's just an idea, nothing to be implemented in this PR.

@goneri
Copy link
Member

goneri commented Oct 18, 2021

Hi @Tomorrow9, thank you for the PR. Is it possible to test this PR with our CI? Does this module require some special hardware?

@mariolenz
Copy link
Collaborator

@goneri

Is it possible to test this PR with our CI? Does this module require some special hardware?

Nope. But vTMP needs a key server afaik. vSphere 7.0U2 vCenter comes with a Native Key Provider that we could use for a test.

The question is: Should we set up the key provider in the test case or in the CI pipeline itself? I'd prefer to have this native key provider enabled in the test environment created by zuul. You see, there are several features that need a key server to work and it would be easier to write tests if the CI provided environment includes one.

@Tomorrow9
Copy link
Collaborator Author

Tomorrow9 commented Oct 19, 2021

@goneri @mariolenz yeah, there required a key provider on vCenter configured firstly, then users can add vTPM to the VM. So I'll create a new module to support creating new Standard key provider and Native key provider on vCenter later. With this module the whole process can be automated.
While Standard key provider requires user to provide an external KMIP server with IP address and port. Native key provider does not require external server. So if our CI environment is vSphere 7.0U2 or later, Native key provider can be enabled to support this testing. Or can deploy an extra KMIP server and add it to vCenter.

Thanks for reviewing this.

@sky-joker
Copy link
Collaborator

Thank @Tomorrow9 for making the new module.
My vSphere environment has a key provider for vTPM, so I’ll try to test the module.

Copy link
Collaborator

@sky-joker sky-joker left a comment

Choose a reason for hiding this comment

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

I confirmed the module works well in my environment.
All that is left is that could you please add check mode processing as @mariolenz says?
After that, I'll add LGTM to the comment :)

@mariolenz
Copy link
Collaborator

So if our CI environment is vSphere 7.0U2 or later, Native key provider can be enabled to support this testing.

I think we are running 7.0U2 in the CI: ansible-network/windmill-config#887

I'm not sure about the ESXi version we're using (maybe @goneri can tell us) but as far as I can see the vCSA version is important here. The ESXi hosts shouldn't mind if it's an external KMIP server or the native key provider as far as I understand.

ansible-zuul bot pushed a commit that referenced this pull request Oct 22, 2021
vmware: Add vTPM information to facts data

SUMMARY
This PR will add vTPM information to default gather information for a virtual machine.
ref: #1075
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/module_utils/vmware.py
changelogs/fragments/1082_vmware.yml
ADDITIONAL INFORMATION
tested on vCenter/ESXi 6.7 and 7.0
RETURN VALUE EXAMPLE
If vTPM is enabled
            "tpm_info": {
                "provider_id": "HyTrust KMS",
                "tpm_present": true
            },

If vTPM isn't enabled
            "tpm_info": {
                "provider_id": null,
                "tpm_present": false
            },

Reviewed-by: Mario Lenz <[email protected]>
Reviewed-by: None <None>
@Tomorrow9
Copy link
Collaborator Author

Tomorrow9 commented Oct 25, 2021

I confirmed the module works well in my environment. All that is left is that could you please add check mode processing as @mariolenz says? After that, I'll add LGTM to the comment :)

Thanks for testing and reviewing this. @sky-joker @mariolenz check mode is added, please review it. Thanks.

Signed-off-by: dianew <[email protected]>
Signed-off-by: dianew <[email protected]>
Signed-off-by: dianew <[email protected]>
Signed-off-by: dianew <[email protected]>
plugins/modules/vmware_guest_tpm.py Outdated Show resolved Hide resolved
plugins/modules/vmware_guest_tpm.py Show resolved Hide resolved
Signed-off-by: dianew <[email protected]>
@Tomorrow9
Copy link
Collaborator Author

In check mode:

ok: [localhost] => {
    "test": {
        "changed": false,
        "desired_operation": "remove vTPM",
        "failed": false,
        "msg": "No vTPM device found on VM"
    }
}

ok: [localhost] => {
    "test": {
        "changed": true,
        "desired_operation": "remove vTPM",
        "failed": false
    }
}

ok: [localhost] => {
    "test": {
        "changed": false,
        "desired_operation": "add vTPM",
        "failed": false,
        "msg": "vTPM device already exist on VM",
        "vtpm_info": {
            "key": 11000,
            "label": "Virtual TPM",
            "summary": "Virtual Trusted Platform Module"
        }
    }
}

ok: [localhost] => {
    "test": {
        "changed": true,
        "desired_operation": "add vTPM",
        "failed": false
    }
}

@Tomorrow9
Copy link
Collaborator Author

Tomorrow9 commented Oct 27, 2021

Not in check mode:

ok: [localhost] => {
    "test": {
        "changed": true,
        "failed": false,
        "vtpm_info": {},
        "vtpm_operation": "remove vTPM"
    }
}

ok: [localhost] => {
    "test": {
        "changed": false,
        "failed": false,
        "msg": "No vTPM device found on VM",
        "vtpm_operation": "remove vTPM"
    }
}

ok: [localhost] => {
    "test": {
        "changed": true,
        "failed": false,
        "vtpm_info": {
            "key": 11000,
            "label": "Virtual TPM",
            "summary": "Virtual Trusted Platform Module"
        },
        "vtpm_operation": "add vTPM"
    }
}

ok: [localhost] => {
    "test": {
        "changed": false,
        "failed": false,
        "msg": "vTPM device already exist on VM",
        "vtpm_info": {
            "key": 11000,
            "label": "Virtual TPM",
            "summary": "Virtual Trusted Platform Module"
        },
        "vtpm_operation": "add vTPM"
    }
}

If VM not in power off state:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "Please make sure VM is powered off before configuring vTPM device, current state is 'poweredOn'"}

If not connect to vCenter:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "Please connect to vCenter Server to configure vTPM device of virtual machine."}

@mariolenz
Copy link
Collaborator

recheck

Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

Thanks for implementing check mode, LGTM now!

@sky-joker Could you please review again, too?

Copy link
Collaborator

@sky-joker sky-joker left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @Tomorrow9 :)

@sky-joker sky-joker added the gate label Oct 28, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@ansible-zuul ansible-zuul bot merged commit dfb2f8a into ansible-collections:main Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a new module for adding vTPM to existing VM
4 participants