Skip to content

Commit

Permalink
fix: Allow normal ACS user to create a cluster
Browse files Browse the repository at this point in the history
  • Loading branch information
hrak committed Sep 12, 2024
1 parent 1e6a2dc commit 0c71016
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 15 deletions.
5 changes: 3 additions & 2 deletions docs/book/src/topics/cloudstack-permissions.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# CloudStack Permissions for CAPC

The account that CAPC runs under must minimally be a Domain Admin type account with a role offering the following permissions
The account that CAPC runs under must minimally be a User type account with a role offering the following permissions

* assignToLoadBalancerRule
* associateIpAddress
Expand All @@ -19,7 +19,6 @@ The account that CAPC runs under must minimally be a Domain Admin type account w
* listAccounts
* listAffinityGroups
* listDiskOfferings
* listDomains
* listLoadBalancerRuleInstances
* listLoadBalancerRules
* listNetworkOfferings
Expand All @@ -39,4 +38,6 @@ The account that CAPC runs under must minimally be a Domain Admin type account w
* stopVirtualMachine
* updateVMAffinityGroup

> Note: If the user doesn't have permissions to expunge the VM, it will be left in a destroyed state. The user will need to manually expunge the VM.
This permission set has been verified to successfully run the CAPC E2E test suite (Oct 11, 2022).
15 changes: 11 additions & 4 deletions pkg/cloud/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,18 +502,25 @@ func findVirtualMachine(

// DestroyVMInstance Destroys a VM instance. Assumes machine has been fetched prior and has an instance ID.
func (c *client) DestroyVMInstance(csMachine *infrav1.CloudStackMachine) error {
p := c.cs.Configuration.NewListCapabilitiesParams()
capabilities, err := c.cs.Configuration.ListCapabilities(p)
expunge := true
if err == nil {
expunge = capabilities.Capabilities.Allowuserexpungerecovervm
}

// Attempt deletion regardless of machine state.
p := c.csAsync.VirtualMachine.NewDestroyVirtualMachineParams(*csMachine.Spec.InstanceID)
p2 := c.csAsync.VirtualMachine.NewDestroyVirtualMachineParams(*csMachine.Spec.InstanceID)
// If an additional data disk was requested on creation of this machine, find it and expunge it as well.
if csMachine.Spec.DiskOffering != nil {
volIDs, err := c.listVMInstanceDatadiskVolumeIDs(*csMachine.Spec.InstanceID)
if err != nil {
return err
}
setArrayIfNotEmpty(volIDs, p.SetVolumeids)
setArrayIfNotEmpty(volIDs, p2.SetVolumeids)
}
p.SetExpunge(true)
if _, err := c.csAsync.VirtualMachine.DestroyVirtualMachine(p); err != nil &&
p2.SetExpunge(expunge)
if _, err := c.csAsync.VirtualMachine.DestroyVirtualMachine(p2); err != nil &&
strings.Contains(strings.ToLower(err.Error()), "unable to find uuid for id") {
// VM doesn't exist. Success...
return nil
Expand Down
27 changes: 19 additions & 8 deletions pkg/cloud/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,21 @@ var _ = Describe("Instance", func() {
unknownError := errors.New(unknownErrorMessage)

var (
mockCtrl *gomock.Controller
mockClient *cloudstack.CloudStackClient
vms *cloudstack.MockVirtualMachineServiceIface
sos *cloudstack.MockServiceOfferingServiceIface
dos *cloudstack.MockDiskOfferingServiceIface
ts *cloudstack.MockTemplateServiceIface
vs *cloudstack.MockVolumeServiceIface
client cloud.Client
mockCtrl *gomock.Controller
mockClient *cloudstack.CloudStackClient
configuration *cloudstack.MockConfigurationServiceIface
vms *cloudstack.MockVirtualMachineServiceIface
sos *cloudstack.MockServiceOfferingServiceIface
dos *cloudstack.MockDiskOfferingServiceIface
ts *cloudstack.MockTemplateServiceIface
vs *cloudstack.MockVolumeServiceIface
client cloud.Client
)

BeforeEach(func() {
mockCtrl = gomock.NewController(GinkgoT())
mockClient = cloudstack.NewMockClient(mockCtrl)
configuration = mockClient.Configuration.(*cloudstack.MockConfigurationServiceIface)
vms = mockClient.VirtualMachine.(*cloudstack.MockVirtualMachineServiceIface)
sos = mockClient.ServiceOffering.(*cloudstack.MockServiceOfferingServiceIface)
dos = mockClient.DiskOffering.(*cloudstack.MockDiskOfferingServiceIface)
Expand Down Expand Up @@ -872,8 +874,12 @@ var _ = Describe("Instance", func() {
})

Context("when destroying a VM instance", func() {
listCapabilitiesParams := &cloudstack.ListCapabilitiesParams{}
expungeDestroyParams := &cloudstack.DestroyVirtualMachineParams{}
expungeDestroyParams.SetExpunge(true)
listCapabilitiesResponse := &cloudstack.ListCapabilitiesResponse{
Capabilities: &cloudstack.Capability{Allowuserexpungerecovervm: true},
}
listVolumesParams := &cloudstack.ListVolumesParams{}
listVolumesResponse := &cloudstack.ListVolumesResponse{
Volumes: []*cloudstack.Volume{
Expand All @@ -886,6 +892,11 @@ var _ = Describe("Instance", func() {
},
}

BeforeEach(func() {
configuration.EXPECT().NewListCapabilitiesParams().Return(listCapabilitiesParams)
configuration.EXPECT().ListCapabilities(listCapabilitiesParams).Return(listCapabilitiesResponse, nil)
})

It("calls destroy and finds VM doesn't exist, then returns nil", func() {
listVolumesParams.SetVirtualmachineid(*dummies.CSMachine1.Spec.InstanceID)
listVolumesParams.SetType("DATADISK")
Expand Down
3 changes: 2 additions & 1 deletion pkg/cloud/user_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ func (c *client) ResolveDomain(domain *Domain) error {
// ResolveAccount resolves an account's information.
func (c *client) ResolveAccount(account *Account) error {
// Resolve domain prior to any account resolution activity.
if err := c.ResolveDomain(&account.Domain); err != nil {
if err := c.ResolveDomain(&account.Domain); err != nil &&
!strings.Contains(err.Error(), "The API [listDomains] does not exist or is not available for the account Account") {
return errors.Wrapf(err, "resolving domain %s details", account.Domain.Name)
}

Expand Down

0 comments on commit 0c71016

Please sign in to comment.