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

Allow cluster creation by a normal ACS user #357

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -452,15 +452,22 @@ 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)
volIDs, err := c.listVMInstanceDatadiskVolumeIDs(*csMachine.Spec.InstanceID)
if err != nil {
return err
}
p.SetExpunge(true)
setArrayIfNotEmpty(volIDs, p.SetVolumeids)
if _, err := c.csAsync.VirtualMachine.DestroyVirtualMachine(p); err != nil &&
p2.SetExpunge(expunge)
setArrayIfNotEmpty(volIDs, p2.SetVolumeids)
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 @@ -46,19 +46,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 @@ -152,7 +152,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
Loading