feat: add a datasource for vm_host

[seb54000]

This is intended to resolve : #122

First, all the credits should go to @skatsaounis who opened a PR #147 that needs some fix as reported by @sparkiegeek
I apologize but I don't know how to push on @skatsaounis PR so I did a new one here with the fixes and one improvement + some questions

improvement : key is now a sensitive value (as it is the private key for the LXD connection)

questions :

• Should we really get and put in the tfstate those sensitive values ? (power_pass and key) We can change them through code and don't really need to know the actual value in the tfstate as it is VERY sensitive and not so useful in my opinion (what do you think ?)
• Should we prefix some parameters dedicated to lxd or virsh ? LIke : virsh_power_pass or lxd_certificate
• Should we state in the docs .md file parameters that are sensitive and maybe the fact that some relies only to LXD and some other to virsh ? (I looked into the machine docs datasource but there is no such thing so I don't know what is the usual practice her)

I tested this code on our maas install with a lxd host and a virsh one. Everything seems to work well.

Thanks a lot for your review, I will update the PR accordingly to the answers you may provide to my question (if needed before merging)

[skatsaounis]

Hi @seb54000. Thank you for the PR and the contribution spirit. I want to diff it with my branch to better review all the changes on top. I have no problem to merge yours and actually I will support this 🙂 .

Some comments:

• I abandoned mine since I found that in MAAS itself, there is an issue with the name uniqueness. In theory you can have more than one LXD VM host with the same name so that makes the data source problematic. If we expect to find the VM host by name and the code base of MAAS allows more than one to exist, we need to think of another way to guarantee uniqueness. This is not the case for Virsh hosts.
• Terraform state is a sensitive file and special care should be made by the practitioner to save it in a safe place and allow access only to personas with appropriate permissions. I find no issue with putting secrets there. Ref: https://developer.hashicorp.com/terraform/language/state/sensitive-data
• Regarding the prefix: technically is not wrong but my original intention was to use the same parameter naming with MAAS itself. We will need to think about it.
• With a closer look to autogenerated docs you can see a reference to sensitivity of a field , e.g.: power_pass (String, Sensitive) User password to use for power control of the VM host.. So it will be mentioned if a field is declared as Sensitive: true in the Schema definition. By the way, please try to run make generate_docs since the CI is complaining that the .md is not in sync with the Schema definition.

[skatsaounis]

Some extra comments:

• We should aim for acceptance tests.
• We will need you to sign Canonical CLA before merging. I will share more on this in a short time

[seb54000]

Hi @skatsaounis and thanks for your positive feedback :

• I did update the docs (make generate_docs) so the tests should pass now
• I understand about the double LXD host. We aim mainly to use virsh and we can take care of this from our side but is can be problematic. For virsh, I didn't fully test but are you sure you cannot create a second KVM host with the same name ? Of course the power address need to be different
• OK for state file, I know you have to take care of it, let's put everything in it, anyway these attribute are stored when using the resource (instead of datasource)
• OK for prefix, you're right to align with MaaS naming
• Acceptance test, yes but how ? I can just say that I did manually some
• CANONICAL CLA : let me know

I'm really happy that this provider exists, thus I'm not sure about the remaining effort and time to have it covering several use case and reach good maturity level (and a broad community of users)
We're actually struggling a bit to use it extensively, it would be great to have the opportunity of a meeting to discuss that

[seb54000]

Hi @skatsaounis , how can I help to get this merged ?

[skatsaounis]

Hi @seb54000 . Sorry for the long delay in my review. Among other things, I am also hunting some CI issues that are causing integration test failures irrelevant with changes in PRs. I was able to solve some of them but not everything. Overall the PR looks good and thank you for reviving it.

Some things to get this PR merged:

• Please take care of the project field as requested.
• Please sign the Canonical CLA so that our CI check (yes I added one) is happy. You can start from this page: https://ubuntu.com/legal/contributors
• Please add acceptance tests. We will need two tests. One that is adding a virsh VM host with a terraform resource and another one for a LXD host. Both tests will use terraform data source to fetch the VM host from MAAS and also confirm that the fields are populated with expected values. maas/data_source_maas_resource_pool_test.go is a good source for inspiration. If you face any problem, I can always add a skeleton in your PR so that you can finalize it. Please let me know 🙂

[github-actions]

Hey! seb54000 has not signed the Canonical CLA which is required to get this contribution merged on this project.

Please head over to https://ubuntu.com/legal/contributors to read more about it.