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

Create vApp VM Security Tag Tests #1046

Merged
merged 25 commits into from
Apr 19, 2023
Merged

Conversation

adezxc
Copy link

@adezxc adezxc commented Apr 15, 2023

This PR introduces security_tags field tests of vcd_vapp_vm and vcd_vm resources that got introduced in #1006

Adam Jasinski added 5 commits April 15, 2023 04:37
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
@adezxc adezxc marked this pull request as ready for review April 15, 2023 18:32
@adezxc adezxc removed the request for review from lvirbalas April 15, 2023 18:32
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

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

A few initial comments.

  • The vcd_security_tag is not necessary to create tag directly on vcd_vapp_vm or vcd_vm. We need to test at least such cases:

    • Create a security tag only by security_tags field on vcd_vapp_vm and vcd_vm (to check if it worked - we could either plug in a govcd function, or add data sources of these resources and check their values as we need to validate data sources anyway)
    • Improve TestAccVcdSecurityTag by checking if vcd_security_tag field gets populated on a vcd_vapp_vm resource that is being used in the test
    • Check a scenario when you create a security tag on in vcd_vapp_vm and what happens when you delete that VM (is the tag removed completely or left as orphan without any VMs)
    • Check that once you create a tag using security_tags on a VM and then remove that field from vm definition - there is still a tag on that VM (this will require using govcd function, probably testAccCheckSecurityTagOnVMCreated can be reused)
  • We also need to add vcd.ResourceSchema-vcd_vapp_vm.tf and vcd.ResourceSchema-vcd_vm.tf to skip-upgrade-tests.txt because their schema has changed (security_tags field added)

vcd/resource_vcd_security_tag_test.go Outdated Show resolved Hide resolved
vcd/resource_vcd_security_tag_test.go Outdated Show resolved Hide resolved
vcd/resource_vcd_security_tag_test.go Outdated Show resolved Hide resolved
Adam Jasinski added 2 commits April 17, 2023 10:48
@adezxc adezxc marked this pull request as draft April 17, 2023 17:32
Adam Jasinski and others added 11 commits April 18, 2023 01:28
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
#
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: abarreiro <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
@adezxc adezxc marked this pull request as ready for review April 18, 2023 16:04
Adam Jasinski added 2 commits April 19, 2023 09:19
Signed-off-by: Adam Jasinski <[email protected]>
Copy link
Collaborator

@Didainius Didainius 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

website/docs/r/vapp_vm.html.markdown Outdated Show resolved Hide resolved
website/docs/r/vapp_vm.html.markdown Outdated Show resolved Hide resolved
website/docs/r/vapp_vm.html.markdown Outdated Show resolved Hide resolved
website/docs/r/vapp_vm.html.markdown Outdated Show resolved Hide resolved
website/docs/r/security_tag.html.markdown Outdated Show resolved Hide resolved
Adam Jasinski added 5 commits April 19, 2023 11:03
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
Signed-off-by: Adam Jasinski <[email protected]>
@adezxc adezxc merged commit ce34919 into vmware:main Apr 19, 2023
@adezxc adezxc deleted the security_tags_tests branch November 14, 2023 14:18
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.

4 participants