Allow vic-machine configure to set appropriate roles for ops user (#7777

)

cmd/vic-machine/common/ops_credentials.go

@@ -63,7 +63,7 @@ func (o *OpsCredentials) Flags(hidden bool) []cli.Flag {
 // operation, adminUser and adminPassword are not needed.
 func (o *OpsCredentials) ProcessOpsCredentials(op trace.Operation, isCreateOp bool, adminUser string, adminPassword *string) error {
 	if o.OpsUser == nil && o.OpsPassword != nil {
-		return errors.New("Password for operations user specified without user having been specified")
+		return errors.New("Password for operations user specified without operations username")
 	}
 
 	if isCreateOp {

cmd/vic-machine/configure/configure.go

@@ -158,7 +158,7 @@ func (c *Configure) processParams(op trace.Operation) error {
 // copyChangedConf takes the mostly-empty new config and copies it to the old one. NOTE: o gets installed on the VCH, not n
 // Currently we cannot automatically override old configuration with any difference in the new configuration, because some options are set during the VCH
 // Creation process, for example, image store path, volume store path, network slot id, etc. So we'll copy changes based on user input
-func (c *Configure) copyChangedConf(o *config.VirtualContainerHostConfigSpec, n *config.VirtualContainerHostConfigSpec) {
+func (c *Configure) copyChangedConf(o *config.VirtualContainerHostConfigSpec, n *config.VirtualContainerHostConfigSpec, clic *cli.Context) {
 	//TODO: copy changed data
 	personaSession := o.ExecutorConfig.Sessions[config.PersonaService]
 	vicAdminSession := o.ExecutorConfig.Sessions[config.VicAdminService]
@@ -197,6 +197,11 @@ func (c *Configure) copyChangedConf(o *config.VirtualContainerHostConfigSpec, n
 	if c.OpsCredentials.IsSet {
 		o.Username = n.Username
 		o.Token = n.Token
+
+		// if the user explicitly set the `ops-grant-user` option, update the permissions level
+		if clic.IsSet("ops-grant-perms") {
+			o.GrantPermsLevel = n.GrantPermsLevel
+		}
 	}
 
 	// Copy the thumbprint directly since it has already been validated.
@@ -314,7 +319,7 @@ func (c *Configure) Run(clic *cli.Context) (err error) {
 
 	validator, err := validate.NewValidator(op, c.Data)
 	if err != nil {
-		op.Errorf("Configuring cannot continue - failed to create validator: %s", err)
+		op.Errorf("Configure cannot continue - failed to create validator: %s", err)
 		return errors.New("configure failed")
 	}
 	defer validator.Session.Logout(parentOp) // parentOp is used here to ensure the logout occurs, even in the event of timeout
@@ -417,7 +422,7 @@ func (c *Configure) Run(clic *cli.Context) (err error) {
 	c.Data.ResourceLimits = mergedResources
 
 	// TODO: copy changed configuration here. https://github.com/vmware/vic/issues/2911
-	c.copyChangedConf(vchConfig, newConfig)
+	c.copyChangedConf(vchConfig, newConfig, clic)
 
 	vConfig := validator.AddDeprecatedFields(op, vchConfig, c.Data)
 	vConfig.Timeout = c.Timeout

lib/install/management/configure.go

@@ -111,8 +111,8 @@ func (d *Dispatcher) Configure(conf *config.VirtualContainerHostConfigSpec, sett
 		return err
 	}
 
-	// rollback function
-	rollback := func() {
+	err = d.update(conf, settings)
+	if err != nil {
 		// Roll back
 		d.op.Errorf("Failed to %s: %s", d.Action.String(), err)
 		d.op.Infof("Rolling back %s", d.Action.String())
@@ -124,34 +124,9 @@ func (d *Dispatcher) Configure(conf *config.VirtualContainerHostConfigSpec, sett
 		d.op.Infof("Appliance is rolled back to previous version")
 		d.deleteISOs(ds, settings)
 		d.deleteSnapshot(newSnapshotRef, snapshotName, conf.Name)
-	}
-
-	err = d.update(conf, settings)
-	if err != nil {
-		rollback()
 		return err
 	}
 
-	// if we are upgrading evaluate need for inventory upgrade
-	// vApp support planned: https://github.com/vmware/vic/issues/7670
-	if d.Action == UpgradeAction && d.session.IsVC() && d.vchPool.Reference().Type != "VirtualApp" {
-		err := d.inventoryUpdate(conf.Name)
-		if err != nil {
-			rollback()
-			return err
-		}
-	}
-
-	// If successful try to grant permissions to the ops-user
-	if conf.ShouldGrantPerms() {
-		err := opsuser.GrantOpsUserPerms(d.op, d.session, conf)
-		if err != nil {
-			err = errors.Errorf("Failed to grant permissions to ops-user, failure: %s", err)
-			rollback()
-			return err
-		}
-	}
-
 	// compatible with old version's upgrade snapshot name
 	if oldSnapshot != nil && (vm.IsConfigureSnapshot(oldSnapshot, ConfigurePrefix) || vm.IsConfigureSnapshot(oldSnapshot, UpgradePrefix)) {
 		d.deleteSnapshot(&oldSnapshot.Snapshot, oldSnapshot.Name, conf.Name)
@@ -309,6 +284,32 @@ func (d *Dispatcher) update(conf *config.VirtualContainerHostConfigSpec, setting
 		return err
 	}
 
+	// if we are upgrading evaluate need for inventory upgrade
+	// vApp support planned: https://github.com/vmware/vic/issues/7670
+	if d.Action == UpgradeAction && d.session.IsVC() && d.vchPool.Reference().Type != "VirtualApp" {
+		err = d.inventoryUpdate(conf.Name)
+		if err != nil {
+			return errors.Errorf("Failed to perform inventory update: %s", err)
+		}
+	}
+
+	// if we're on VC, update the VCH folder now that we've updated the inventory
+	if d.appliance.IsVC() {
+		vchFolder, err := d.appliance.Folder(d.op)
+		if err != nil {
+			return err
+		}
+		d.session.VCHFolder = vchFolder
+	}
+
+	// try to grant permissions to the ops-user
+	if conf.ShouldGrantPerms() {
+		err = opsuser.GrantOpsUserPerms(d.op, d.session, conf)
+		if err != nil {
+			return errors.Errorf("Failed to grant permissions to ops-user, failure: %s", err)
+		}
+	}
+
 	if err = d.appliance.PowerOn(d.op); err != nil {
 		return err
 	}

lib/migration/feature/feature.go

@@ -28,6 +28,10 @@ const (
 	// create time is stored in nanoseconds (previously seconds) in the portlayer.
 	ContainerCreateTimestampVersion
 
+	// VCHFolderSupportVersion represents the VCH version that first introduced
+	// VM folder support for the VCH.
+	VCHFolderSupportVersion
+
 	// Add new feature flag here
 
 	// MaxPluginVersion must be the last

pkg/vsphere/session/session.go

@@ -382,7 +382,6 @@ func (s *Session) Populate(ctx context.Context) (*Session, error) {
 		// This will provide standalone ESXi and backwards
 		// compatibility to non-folder versions.
 		s.VCHFolder = folders.VmFolder
-
 	}
 
 	if len(errs) > 0 {

tests/manual-test-cases/Group5-Functional-Tests/5-25-OPS-User-Grant.md

@@ -13,16 +13,52 @@ This test requires access to VMware Nimbus cluster for dynamic ESXi and vCenter
 3. Install the VIC appliance into the cluster with the --ops-grant-perms option
 4. With the ops-user, use govc to attempt to change the DRS settings on the cluster
 5. Run a variety of docker operations on the VCH
-6. Create a container
-7. Use govc to attempt to out-of-band destroy the container from Step 6
-8. Clean up the VCH
+6. Run privilege-dependent docker operations against the VCH
+7. Create a container
+8. Use govc to attempt to out-of-band destroy the container from Step 7
+9. Clean up the VCH
+10. Install version v1.3.0 of the VIC appliance into the cluster with the --ops-grant-perms option
+11. Perform a VCH upgrade to the current version
+12. With the ops-user, use govc to attempt to create a resource pool
+13. Run a variety of docker operations on the VCH
+14. Run privilege-dependent docker operations against the VCH
+15. Create a container
+16. Use govc to attempt to out-of-band destroy the container from Step 15
+17. Clean up the VCH
+18. Install the VIC appliance into the cluster with the --ops-grant-perms and --affinity-vm-group options
+19. With the ops-user, use govc to attempt to create a resource pool
+20. Run a variety of docker operations on the VCH
+21. Run privilege-dependent docker operations against the VCH
+22. Create a container
+23. Use govc to attempt to out-of-band destroy the container from Step 6
+24. Clean up the VCH
+25. Install the VIC appliance into the cluster without any ops user options
+26. Reconfigure the VCH with the --ops-user, --ops-password, --ops-grant-perms options
+27. With the ops-user, use govc to attempt to change the DRS settings on the cluster
+28. Run a variety of docker operations on the VCH
+29. Create a container
+30. Use govc to attempt to out-of-band destroy the container from Step 6
+31. Clean up the VCH
 
 # Expected Outcome:
 * Steps 1-3 should succeed
-* Step 4 should fail since the ops-user does not have enough permissions for the operation
-* Step 5 and 6 should succeed
-* Step 7 should fail since the destroy method should be disabled by VIC
-* Step 8 should succeed
+* Step 4 should fail since the ops-user does not have required permissions to execute the operation
+* Steps 5-7 should succeed
+* Step 8 should fail since the destroy method should be disabled by VIC
+* Steps 9-11 should succeed
+* Step 12 should fail since the ops-user does not have required permission to execute the operation
+* steps 13-15 should succeed
+* Step 16 should fail since the destroy method should be disabled by VIC
+* Steps 17 and 18 should succeed
+* Step 19 should fail since the ops-user does not have required permission to execute the operation
+* Step 20-22 should succeed
+* Step 23 should fail since the destroy method should be disabled by VIC
+* Step 24-26 should succeed
+* Step 27 should fail since the ops-user does not have required permission to execute the operation
+* Step 28 and 29 should succeed
+* Step 30 should fail since the destroy method should be disabled by VIC
+* Step 31 should succeed
+
 
 # Possible Problems:
 None

tests/manual-test-cases/Group5-Functional-Tests/5-25-OPS-User-Grant.robot

@@ -15,9 +15,10 @@
 *** Settings ***
 Documentation  Test 5-25 - OPS-User-Grant
 Resource  ../../resources/Util.robot
-Suite Setup  Wait Until Keyword Succeeds  10x  10m  Ops User Create
-Suite Teardown  Run Keyword And Ignore Error  Nimbus Cleanup  ${list}
-Test Teardown  Run Keyword If Test Failed  Gather vSphere Logs
+Suite Setup  Ops User Create
+#Suite Setup  Wait Until Keyword Succeeds  10x  10m  Ops User Create
+#Suite Teardown  Run Keyword And Ignore Error  Nimbus Cleanup  ${list}
+#Test Teardown  Run Keyword If Test Failed  Gather vSphere Logs
 
 *** Keywords ***
 Ops User Create
@@ -82,17 +83,32 @@ Run privilege-dependent docker operations
     ${rc}  ${output}=  Run And Return Rc And Output  docker %{VCH-PARAMS} rm -f ${c5}
     Should Be Equal As Integers  ${rc}  0
 
-*** Test Cases ***
-vic-machine create grants ops-user perms
-    Install VIC Appliance To Test Server  additional-args=--ops-user ${ops_user_name} --ops-password ${ops_user_password} --ops-grant-perms
+Reconfigure VCH With Ops User
+    ${rc}  ${output}=  Run And Return Rc And Output  bin/vic-machine-linux configure --target %{TEST_URL} --user %{TEST_USERNAME} --password=%{TEST_PASSWORD} --compute-resource=%{TEST_RESOURCE} --name %{VCH-NAME} --ops-user=${ops_user_name} --ops-password=${ops_user_password} --ops-grant-perms --thumbprint=%{TEST_THUMBPRINT} --debug=1
+    Should Be Equal As Integers  ${rc}  0
+    Should Contain  ${output}  Completed successfully
 
-    # Run a govc test to check that access is denied on some resources
+Attempt To Disable DRS
     Log To Console  Running govc to set drs-enabled, it should fail
     ${rc}  ${output}=  Run And Return Rc And Output  GOVC_USERNAME=${ops_user_name} GOVC_PASSWORD=${ops_user_password} govc cluster.change -drs-enabled /${datacenter}/host/${cluster}
     Log  Govc output: ${output}
     Should Be Equal As Integers  ${rc}  1
     Should Contain  ${output}  Permission to perform this operation was denied
 
+Attempt To Create Resource Pool
+    Log To Console  Running govc to create a resource pool named "5-25-OPS-User-Grant-%{DRONE_BUILD_NUMBER}", it should fail
+    ${rc}  ${output}=  Run And Return Rc And Output  GOVC_USERNAME=${ops_user_name} GOVC_PASSWORD=${ops_user_password} govc pool.create */Resources/5-25-OPS-User-Grant-%{DRONE_BUILD_NUMBER}
+    Log  Govc output: ${output}
+    Should Be Equal As Integers  ${rc}  1
+    Should Contain  ${output}  Permission to perform this operation was denied
+
+*** Test Cases ***
+vic-machine create grants ops-user perms
+    Install VIC Appliance To Test Server  additional-args=--ops-user ${ops_user_name} --ops-password ${ops_user_password} --ops-grant-perms
+
+    # Run a govc test to check that access is denied on some resources
+    Attempt To Disable DRS
+
     Run Regression Tests
 
     Run privilege-dependent docker operations
@@ -107,11 +123,7 @@ granted ops-user perms work after upgrade
     Check Upgraded Version
 
     # Run a govc test to check that access is denied on some resources
-    Log To Console  Running govc to set drs-enabled, it should fail
-    ${rc}  ${output}=  Run And Return Rc And Output  GOVC_USERNAME=${ops_user_name} GOVC_PASSWORD=${ops_user_password} govc cluster.change -drs-enabled /${datacenter}/host/${cluster}
-    Log  Govc output: ${output}
-    Should Be Equal As Integers  ${rc}  1
-    Should Contain  ${output}  Permission to perform this operation was denied
+    Attempt To Create Resource Pool
 
     Run Regression Tests
 
@@ -124,14 +136,22 @@ Test with VM-Host Affinity
     Install VIC Appliance To Test Server  additional-args=--ops-user ${ops_user_name} --ops-password ${ops_user_password} --ops-grant-perms --affinity-vm-group
 
     # Run a govc test to check that access is denied on some resources
-    Log To Console  Running govc to create a resource pool named "5-25-OPS-User-Grant-%{DRONE_BUILD_NUMBER}", it should fail
-    ${rc}  ${output}=  Run And Return Rc And Output  GOVC_USERNAME=${ops_user_name} GOVC_PASSWORD=${ops_user_password} govc pool.create */Resources/5-25-OPS-User-Grant-%{DRONE_BUILD_NUMBER}
-    Log  Govc output: ${output}
-    Should Be Equal As Integers  ${rc}  1
-    Should Contain  ${output}  Permission to perform this operation was denied
+    Attempt To Create Resource Pool
 
     Run Regression Tests
 
     Run privilege-dependent docker operations
 
     Cleanup VIC Appliance On Test Server
+
+vic-machine configure grants ops-user perms
+    Install VIC Appliance To Test Server
+
+    Reconfigure VCH With Ops User
+
+    # Run a govc test to check that access is denied on some resources
+    Attempt To Disable DRS
+
+    Run privilege-dependent docker operations
+
+    Cleanup VIC Appliance On Test Server