Introduce access control management #543

dataclouder · 2020-08-16T18:10:32Z

Add resource vcd_vapp_access_control, running in tenant context. (issue vcd_vapp: Add access control api #509)
Add data source for Org User
Skip check conditionally in test TestAccVcdVAppVmDhcpWait

Note: there is no automated test for groups in access control yet.

go.mod

vbauzys

Nice work! First review.

vcd/datasource_vcd_org_user.go

vcd/provider.go

vcd/resource_vcd_access_control_vapp.go

vcd/resource_vcd_access_control_vapp_test.go

jpbuecken · 2020-08-18T12:18:30Z

The "read/refresh" is not yet ok: (VCD 10.1.1)

I created a vapp with

resource "vcd_vapp_access_control" "vappacl" {
  lifecycle {
    prevent_destroy = "true"
  }

  vapp_id               = vcd_vapp.vapp.id
  shared_to_everyone    = true
  everyone_access_level = "ReadOnly"
}

Then I changed everyone_access_level to Read/Write via GUI.

Then I run terraform plan again

It does not update everyone_access_level, it should mention a change
It says shared_to_everyone is false, which is wrong

  # vcd_vapp_access_control.vappacl will be updated in-place
  ~ resource "vcd_vapp_access_control" "vappacl" {
        everyone_access_level = "ReadOnly"
        id                    = "urn:vcloud:vapp:932c15d1-3335-4290-a0d0-3f642db470c4"
      ~ shared_to_everyone    = false -> true
        vapp_id               = "urn:vcloud:vapp:932c15d1-3335-4290-a0d0-3f642db470c4"
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to apply these changes?
Then type 'y'. Default is 'N'.

Apply changes?: y

dataclouder · 2020-08-18T13:00:04Z

@jpbuecken Thanks for testing.
I was unable to reproduce the problem using the procedure you mentioned.

resource "vcd_vapp" "Vapp-AC-0" {
  name = "Vapp-AC-0"
}

resource "vcd_vapp_access_control" "AC-Vapp0" {
  lifecycle {
    prevent_destroy = "true"
  }

  vapp_id  = vcd_vapp.Vapp-AC-0.id

  shared_to_everyone    = true
  everyone_access_level = "ReadOnly"
}

After modifying the access control in the GUI, I ran terraform apply again and I got:

$ terraform  apply
vcd_vapp.Vapp-AC-0: Refreshing state... [id=urn:vcloud:vapp:cc59cf96-878f-4ec2-add5-d4424545f76c]
vcd_vapp_access_control.AC-Vapp0: Refreshing state... [id=urn:vcloud:vapp:cc59cf96-878f-4ec2-add5-d4424545f76c]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # vcd_vapp_access_control.AC-Vapp0 will be updated in-place
  ~ resource "vcd_vapp_access_control" "AC-Vapp0" {
      ~ everyone_access_level = "Change" -> "ReadOnly"
        id                    = "urn:vcloud:vapp:cc59cf96-878f-4ec2-add5-d4424545f76c"
        org                   = "datacloud"
        shared_to_everyone    = true
        vapp_id               = "urn:vcloud:vapp:cc59cf96-878f-4ec2-add5-d4424545f76c"
        vdc                   = "vdc-datacloud"
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Just to be clear, the steps to reproduce are:

run terraform apply (creates a vApp and its access control resource)
modify the access control in the GUI
run terraform plan or terraform apply

I tried the above method in 10.1 and 10.0, and I always get the same outcome

jpbuecken · 2020-08-19T10:56:57Z

@dataclouder

I now used exactly your hcl to test this on our
VCD 10.0.0.16081830 and VCD 10.1.1.16282995

I can confirm it is working in 10.0.0.16081830 but 10.1.1.16282995 is broken with the result above.
So I assume a change between 10.1.0 and 10.1.1.

dataclouder · 2020-08-19T12:03:56Z

@jpbuecken
Thanks for trying again. Unfortunately, from our side we are unable to reproduce the issue. Could you join the slack channel #vcd-terraform-dev at vmwarecode.slack.com, so we can look at it with more detail?

jpbuecken · 2020-08-19T14:20:10Z

@jpbuecken
Thanks for trying again. Unfortunately, from our side we are unable to reproduce the issue. Could you join the slack channel #vcd-terraform-dev at vmwarecode.slack.com, so we can look at it with more detail?

Sorry for the noise, found it. Our VCD 10.1.1 has a custom role with more restrict permissions for our terraform user. We need to fix this role

EDIT: Unfortunately, our role is correct. If you want to read the ACL of a ORG as a system user, that has a role with "View vApp ACL" permissions, you need to add X-VMWARE-VCLOUD-TENANT-CONTEXT: <ORGID> to your http header. With this the manually api call works with this header, and without this header it just give the wrong default as reponse

Also removed deprecated vcd_network resource

Explicitly change the version label in HCL configuration file

Didainius · 2020-08-27T07:23:47Z

Thanks for eda7d01. In my short test for one item it worked perfectly.

website/docs/r/vapp_access_control.html.markdown

lvirbalas · 2020-08-27T14:23:58Z

website/docs/r/vapp_access_control.html.markdown

+definition for the same vApp ID will result in a previous instance to be overwritten.
+
+!> **Note:** access control operations run in tenant context, meaning that, even if the user is a system administrator,
+in every request it uses headers items that define the tenant context as restricted to the organization to which the vApp belongs.


This is a nice write-up, but after reading it one question rises "why is this important to know?". If we can describe that in a short sentence, would be great.

Also, please change this note to yellow color. No need for it to be red and scary :)
Maybe even the first warning could be yellow?

It's useful to know for the ones who need this feature. The others can easily ignore it.

lvirbalas · 2020-08-27T14:51:43Z

vcd/resource_vcd_vapp_access_control.go

+		accessControl.IsSharedToEveryone = true
+		accessControl.EveryoneAccessLevel = &everyoneAccessLevel
+		if len(sharedList) > 0 {
+			return fmt.Errorf("[resourceAccessControlVappUpdate] when 'shared_with_everyone' is true, 'shared_with' must not be filled")


I think I hit a bug. Reproduce:

Create resource with shared_with_everyone and everyone_access_level. Apply.

Modify resource by removing shared_with_everyone and adding shared_with. Apply.

This is what I got in this situation:

Resource actions are indicated with the following symbols: + create Terraform will perform the following actions: # vcd_vapp_access_control.demo_vapp_sharing will be created + resource "vcd_vapp_access_control" "demo_vapp_sharing" { + everyone_access_level = "Change" + id = (known after apply) + shared_with_everyone = true + vapp_id = "urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85" } Plan: 1 to add, 0 to change, 0 to destroy. Do you want to perform these actions? Terraform will perform the actions described above. Only 'yes' will be accepted to approve. Enter a value: yes vcd_vapp_access_control.demo_vapp_sharing: Creating... vcd_vapp_access_control.demo_vapp_sharing: Still creating... [10s elapsed] vcd_vapp_access_control.demo_vapp_sharing: Still creating... [20s elapsed] vcd_vapp_access_control.demo_vapp_sharing: Creation complete after 23s [id=urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85] Apply complete! Resources: 1 added, 0 changed, 0 destroyed. Outputs: demo_vm_wordpress_ip = 192.168.0.2 first_external_ip = 10.150.160.116 vapp_id = urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85 vm_id = urn:vcloud:vm:a60a0c5f-0498-4610-8f48-27c7c9de8f5b lvirbalas-a01:radio2020 lvirbalas$ terraform apply data.vcd_org_user.org_admin: Refreshing state... vcd_external_network.demo_ext_net: Refreshing state... [id=urn:vcloud:network:30759524-ff11-4236-9579-8b2b1f18c0f7] vcd_org.demo_org: Refreshing state... [id=urn:vcloud:org:992e69cd-fd5e-4815-862a-d0c010825ae7] vcd_org_vdc.demo_vdc: Refreshing state... [id=urn:vcloud:vdc:1e4a69c8-dff5-4ce7-9a23-c36605cc7743] vcd_edgegateway.demo_gw: Refreshing state... [id=urn:vcloud:gateway:9a050d14-2afb-4c0e-8b67-178aff85d5d6] vcd_network_routed.demo_routed_net_web: Refreshing state... [id=urn:vcloud:network:612d27d9-493d-479b-a8e9-ccdbebdc29b7] vcd_nsxv_firewall_rule.demo_nsxv_fw_rule: Refreshing state... [id=131078] vcd_vapp.demo_vapp: Refreshing state... [id=urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85] vcd_vapp_org_network.demo_vapp_net_org: Refreshing state... [id=urn:vcloud:network:57eff952-7363-4d4c-b246-3fa0edf70fb4] vcd_vapp_access_control.demo_vapp_sharing: Refreshing state... [id=urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85] vcd_vapp_network.demo_vapp_net_w_org: Refreshing state... [id=urn:vcloud:network:1e6a60ad-3195-42e8-b9e7-58d39330a8ad] vcd_vapp_firewall_rules.vapp_org_fw: Refreshing state... [id=urn:vcloud:network:57eff952-7363-4d4c-b246-3fa0edf70fb4] vcd_vapp_vm.demo_vm_photon: Refreshing state... [id=urn:vcloud:vm:cdfccf69-ebbc-4918-b8c7-ad141776bedf] vcd_vapp_vm.demo_vm_wordpress: Refreshing state... [id=urn:vcloud:vm:a60a0c5f-0498-4610-8f48-27c7c9de8f5b] vcd_vapp_static_routing.vapp_demo_static_routing1: Refreshing state... [id=urn:vcloud:network:1e6a60ad-3195-42e8-b9e7-58d39330a8ad] vcd_vapp_firewall_rules.vapp_fw: Refreshing state... [id=urn:vcloud:network:1e6a60ad-3195-42e8-b9e7-58d39330a8ad] vcd_nsxv_dnat.demo_dnat_wordpress_https1: Refreshing state... [id=196609] vcd_vapp_nat_rules.vapp_nat: Refreshing state... [id=urn:vcloud:network:1e6a60ad-3195-42e8-b9e7-58d39330a8ad] An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: ~ update in-place Terraform will perform the following actions: # vcd_vapp_access_control.demo_vapp_sharing will be updated in-place ~ resource "vcd_vapp_access_control" "demo_vapp_sharing" { everyone_access_level = "Change" id = "urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85" shared_with_everyone = true vapp_id = "urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85" + shared_with { + access_level = "FullControl" + subject_name = (known after apply) + user_id = "urn:vcloud:user:ee04f71a-27a5-4519-865a-fb3ba35538e3" } } Plan: 0 to add, 1 to change, 0 to destroy. Do you want to perform these actions? Terraform will perform the actions described above. Only 'yes' will be accepted to approve. Enter a value: yes vcd_vapp_access_control.demo_vapp_sharing: Modifying... [id=urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85] Error: [resourceAccessControlVappUpdate] when 'shared_with_everyone' is true, 'shared_with' must not be filled on radio2020.tf line 188, in resource "vcd_vapp_access_control" "demo_vapp_sharing": 188: resource "vcd_vapp_access_control" "demo_vapp_sharing" { lvirbalas-a01:radio2020 lvirbalas$ terraform apply Error: Missing required argument on radio2020.tf line 188, in resource "vcd_vapp_access_control" "demo_vapp_sharing": 188: resource "vcd_vapp_access_control" "demo_vapp_sharing" { The argument "shared_with_everyone" is required, but no definition was found.

This is how my last HCL looks like:

# 3.0.0 resource "vcd_vapp_access_control" "demo_vapp_sharing" { vapp_id = vcd_vapp.demo_vapp.id /*shared_with_everyone = true everyone_access_level = "Change"*/ shared_with { user_id = data.vcd_org_user.org_admin.id access_level = "FullControl" } }

It doesn't seem to have noticed the commenting out of the shared_with_everyone fields.

Can't reproduce. My terraform client recognizes the comments as such, and in fact complains that a required property shared_with_everyone is missing.
Try with single line comments.

OK, so that was the issue. I thought I can remove shared_with_everyone and add shared_with instead, but apparently I need both. Interesting approach to semantics. A bit verbal, but maybe clearer. I mean we could make shared_with_everyone optional with default false.

We are mimicking the UI, where you have to declare your intention about sharing with everyone always.

lvirbalas · 2020-08-27T15:11:33Z

vcd/resource_vcd_vapp_access_control_test.go

+	}
+}
+
+func testAccCheckVappAccessControlDestroy(orgName, vdcName string, vappNames []string) resource.TestCheckFunc {


I hit an issue with destroy. Removed the resource:

An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: - destroy Terraform will perform the following actions: # vcd_vapp_access_control.demo_vapp_sharing will be destroyed - resource "vcd_vapp_access_control" "demo_vapp_sharing" { - everyone_access_level = "Change" -> null - id = "urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85" -> null - shared_with_everyone = true -> null - vapp_id = "urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85" -> null - shared_with { - access_level = "FullControl" -> null - user_id = "urn:vcloud:user:ee04f71a-27a5-4519-865a-fb3ba35538e3" -> null } } Plan: 0 to add, 0 to change, 1 to destroy. Do you want to perform these actions? Terraform will perform the actions described above. Only 'yes' will be accepted to approve. Enter a value: yes vcd_vapp_access_control.demo_vapp_sharing: Destroying... [id=urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85] vcd_vapp_access_control.demo_vapp_sharing: Still destroying... [id=urn:vcloud:vapp:0983cdbb-61b2-460d-8e63-cdfeec8dbb85, 10s elapsed] vcd_vapp_access_control.demo_vapp_sharing: Destruction complete after 13s Apply complete! Resources: 0 added, 0 changed, 1 destroyed.

But in GUI this is what I got:

It is not a bug.
The GUI doesn't have a way of saying "not sharing", so it shows that is "shared with", and no user or group is ticked.
It shows "Users (0)" and "Groups (0)"

lvirbalas

LGTM!

vbauzys

LGTM. Just a simple fixes needed

vcd/resource_vcd_vapp_access_control.go

Giuseppe Maxia added 7 commits August 16, 2020 19:48

update vendor

eb24592

Add data source Org User

446040f

Skip faulty check in test TestAccVcdVAppVmDhcpWait

2bbe0b1

Fix minor spelling issues in org_user docs

a96b295

Add resource vcd_access_control_vapp

f40af79

Move common test functions to config_test.go

401d570

Merge branch 'master' into access-control

40a356e

vmwclabot added the cla-not-required label Aug 16, 2020

Giuseppe Maxia added 2 commits August 16, 2020 20:12

Update vendor after govcd merge

0c81ad9

Add Changelog entries

43dd98e

dataclouder requested review from vbauzys, Didainius and lvirbalas August 16, 2020 18:37

dataclouder marked this pull request as ready for review August 16, 2020 18:38

jpbuecken reviewed Aug 17, 2020

View reviewed changes

go.mod Outdated Show resolved Hide resolved

vbauzys reviewed Aug 17, 2020

View reviewed changes

Giuseppe Maxia added 4 commits August 17, 2020 22:12

Fix comments

8be8b78

Merge branch 'master' into access-control

410bcbf

update vendor

ff3fe2e

Rename access_control_vapp to vapp_access_control

15b05cf

Giuseppe Maxia added 2 commits August 19, 2020 12:20

update vendor

22d6b14

merge master

c1e09da

Giuseppe Maxia added 2 commits August 19, 2020 13:49

Remove leftover debug statements

866bc2a

update go.mod

ba34d36

Merge branch 'master' into access-control

753c7de

Giuseppe Maxia added 10 commits August 26, 2020 13:15

Update v2.10.0 to v3.0.0 in docs

58d7ed3

Update test running timeout to 500 minutes

4288750

update vendor

5de2ca0

Update resource versions in provider.go

eef2f17

Also removed deprecated vcd_network resource

rename 'shared*' properties to 'shared_with*'

cc19b0c

Import updated govcd files

79ccf06

Update v2 to v3 in project root

3d15b6a

Merge branch 'master' into access-control

5d969b8

Update vendor

14d2358

[skip ci] Fix property name in example

5743115

jpbuecken mentioned this pull request Aug 26, 2020

vcd_vapp_vm: network: Update of is_primary from false to true does not work for first interface #549

Closed

Didainius approved these changes Aug 27, 2020

View reviewed changes

Improve upgrade test

eda7d01

Explicitly change the version label in HCL configuration file

lvirbalas reviewed Aug 27, 2020

View reviewed changes

website/docs/r/vapp_access_control.html.markdown Outdated Show resolved Hide resolved

[skip ci] Fix docs formatting

94956fa

This was referenced Aug 27, 2020

vcd_vapp_vm: network: Update of network name does not show update for IP #551

Open

vcd_vapp_vm: network: Update of ip_allocation_mode does not work from POOL to MANUAL if you keep the same ip #552

Closed

lvirbalas reviewed Aug 27, 2020

View reviewed changes

Change notes colors in access control docs

69e62e0

lvirbalas approved these changes Aug 28, 2020

View reviewed changes

vbauzys approved these changes Aug 28, 2020

View reviewed changes

vcd/resource_vcd_vapp_access_control.go Outdated Show resolved Hide resolved

vcd/resource_vcd_vapp_access_control.go Outdated Show resolved Hide resolved

Giuseppe Maxia added 3 commits August 28, 2020 14:24

Add check for everyone_access_level correctness

658712a

Remove unnecessary "Required:false" from resources

722c820

update vendor

56702f5

dataclouder merged commit 85100ed into vmware:master Aug 28, 2020

dataclouder mentioned this pull request Aug 28, 2020

vcd_vapp: Add access control api #509

Closed

dataclouder deleted the access-control branch November 20, 2023 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce access control management #543

Introduce access control management #543

dataclouder commented Aug 16, 2020 •

edited

Loading

vbauzys left a comment

jpbuecken commented Aug 18, 2020

dataclouder commented Aug 18, 2020

jpbuecken commented Aug 19, 2020

dataclouder commented Aug 19, 2020 •

edited

Loading

jpbuecken commented Aug 19, 2020 •

edited

Loading

Didainius commented Aug 27, 2020

lvirbalas Aug 27, 2020

dataclouder Aug 27, 2020

lvirbalas Aug 27, 2020

dataclouder Aug 27, 2020

lvirbalas Aug 28, 2020

dataclouder Aug 28, 2020

lvirbalas Aug 27, 2020

dataclouder Aug 27, 2020

lvirbalas left a comment

vbauzys left a comment

Introduce access control management #543

Introduce access control management #543

Conversation

dataclouder commented Aug 16, 2020 • edited Loading

vbauzys left a comment

Choose a reason for hiding this comment

jpbuecken commented Aug 18, 2020

dataclouder commented Aug 18, 2020

jpbuecken commented Aug 19, 2020

dataclouder commented Aug 19, 2020 • edited Loading

jpbuecken commented Aug 19, 2020 • edited Loading

Didainius commented Aug 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lvirbalas left a comment

Choose a reason for hiding this comment

vbauzys left a comment

Choose a reason for hiding this comment

dataclouder commented Aug 16, 2020 •

edited

Loading

dataclouder commented Aug 19, 2020 •

edited

Loading

jpbuecken commented Aug 19, 2020 •

edited

Loading