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.

Sign up for GitHub

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

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

Merged
merged 2 commits into from
Apr 25, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a97b834
Add VCHFolderSupport plugin version
jzt Apr 18, 2018
8325d30
Allow configure to grant permissions to ops user
jzt Apr 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/vic-machine/common/ops_credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 8 additions & 3 deletions cmd/vic-machine/configure/configure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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
Expand Down Expand Up @@ -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
Expand Down
55 changes: 28 additions & 27 deletions lib/install/management/configure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions lib/migration/feature/feature.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/vsphere/session/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
50 changes: 43 additions & 7 deletions tests/manual-test-cases/Group5-Functional-Tests/5-25-OPS-User-Grant.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
54 changes: 37 additions & 17 deletions tests/manual-test-cases/Group5-Functional-Tests/5-25-OPS-User-Grant.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be Attempt To Disable DRS, can't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be, but it was basically a merge of your additions and @anchal-agrawal's additions, so I left both Attempt To Create Resource Pool and Attempt To Disable DRS in there for variety. Still trying to get granted ops-user perms work after upgrade to pass, so I'll clean it up as I go forward.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, disabling DRS requires less privilege than creating a resource pool. Usually disabling DRS makes a good sanity check that we're not granting the operations user privileges they don't need.

In the specific case that we're using --affinity-vm-group, we currently have to grant the operations user the privilege that lets them disable DRS (as that's the same privilege that lets them manage affinity constructs), so we check creating a resource pool instead.


Run Regression Tests

Expand All @@ -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