Skip to content

Commit

Permalink
Admin action to delete a cluster managed resource (#3286)
Browse files Browse the repository at this point in the history
* add ResourceDeleteAndWait to azureactions

* add delete resource admin action and frontend routing

* add helper functions for lb config manipulation

* refactor azure actions
- moves resource delete code to seperate file
- adds loadbalancer client to handle deleting FrontendIPConfiguration
- updates ResourceDeleteAndWait to handle deleting FrontendIPConfigurations
- adds DeleteByIDAndWait to features/resources client

* add e2e tests

* fix imports and add license headers

* cleanup / fix lint

* add command example to doc

* rename to "managed" resource id

* change query param to camel case

* use var group instead

* return error as adminReply already wraps in CloudError

* fix missed camelCase of query param

* use regex to match frontend ip configurations

* remove focus

* add deny list to prevent deleting PLS and Storage

* fix mixed import

* use fake pls name to prevent accidently deleting e2e cluster pls

* fix test

* add PE to deny list
  • Loading branch information
tony-schndr authored Nov 29, 2023
1 parent a017435 commit 9b92b4f
Show file tree
Hide file tree
Showing 14 changed files with 819 additions and 2 deletions.
6 changes: 6 additions & 0 deletions docs/deploy-development-rp.md
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,12 @@ After that, when you [create](https://github.com/Azure/ARO-RP/blob/master/docs/d
curl -X PATCH -k "https://localhost:8443/admin/subscriptions/$AZURE_SUBSCRIPTION_ID/resourceGroups/$RESOURCEGROUP/providers/Microsoft.RedHatOpenShift/openShiftClusters/$CLUSTER/etcdrecovery"
```
* Delete a managed resource
```bash
MANAGED_RESOURCEID=<id of managed resource to delete>
curl -X POST -k "https://localhost:8443/admin/subscriptions/$AZURE_SUBSCRIPTION_ID/resourceGroups/$RESOURCEGROUP/providers/Microsoft.RedHatOpenShift/openShiftClusters/$CLUSTER/deletemanagedresource?managedResourceID=$MANAGED_RESOURCEID"
```
## OpenShift Version
* We have a cosmos container which contains supported installable OCP versions, more information on the definition in `pkg/api/openshiftversion.go`.
Expand Down
67 changes: 67 additions & 0 deletions pkg/frontend/admin_openshiftcluster_delete_managedresource.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package frontend

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"context"
"net/http"
"path/filepath"
"strings"

"github.com/Azure/go-autorest/autorest"
"github.com/go-chi/chi/v5"
"github.com/sirupsen/logrus"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/database/cosmosdb"
"github.com/Azure/ARO-RP/pkg/frontend/middleware"
)

func (f *frontend) postAdminOpenShiftDeleteManagedResource(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
log := ctx.Value(middleware.ContextKeyLog).(*logrus.Entry)
r.URL.Path = filepath.Dir(r.URL.Path)

err := f._postAdminOpenShiftClusterDeleteManagedResource(ctx, r, log)
adminReply(log, w, nil, nil, err)
}

func (f *frontend) _postAdminOpenShiftClusterDeleteManagedResource(ctx context.Context, r *http.Request, log *logrus.Entry) error {
resType, resName, resGroupName := chi.URLParam(r, "resourceType"), chi.URLParam(r, "resourceName"), chi.URLParam(r, "resourceGroupName")
managedResourceID := r.URL.Query().Get("managedResourceID")
resourceID := strings.TrimPrefix(r.URL.Path, "/admin")

doc, err := f.dbOpenShiftClusters.Get(ctx, resourceID)
switch {
case cosmosdb.IsErrorStatusCode(err, http.StatusNotFound):
return api.NewCloudError(http.StatusNotFound, api.CloudErrorCodeResourceNotFound, "", "The Resource '%s/%s' under resource group '%s' was not found.", resType, resName, resGroupName)
case err != nil:
return err
}

if !strings.HasPrefix(strings.ToLower(managedResourceID), strings.ToLower(doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID)) {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "The resource %s is not within the cluster's managed resource group %s.", managedResourceID, doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID)
}

subscriptionDoc, err := f.getSubscriptionDocument(ctx, doc.Key)
if err != nil {
return err
}

a, err := f.azureActionsFactory(log, f.env, doc.OpenShiftCluster, subscriptionDoc)
if err != nil {
return err
}

err = a.ResourceDeleteAndWait(ctx, managedResourceID)
if err != nil {
if detailedErr, ok := err.(autorest.DetailedError); ok &&
detailedErr.StatusCode == http.StatusNotFound {
return api.NewCloudError(http.StatusNotFound, api.CloudErrorCodeNotFound, "", "The resource '%s' could not be found.", managedResourceID)
}
return err
}

return nil
}
138 changes: 138 additions & 0 deletions pkg/frontend/admin_openshiftcluster_delete_managedresource_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package frontend

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"context"
"fmt"
"net/http"
"strings"
"testing"

"github.com/Azure/go-autorest/autorest"
"github.com/golang/mock/gomock"
"github.com/sirupsen/logrus"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/env"
"github.com/Azure/ARO-RP/pkg/frontend/adminactions"
"github.com/Azure/ARO-RP/pkg/metrics/noop"
mock_adminactions "github.com/Azure/ARO-RP/pkg/util/mocks/adminactions"
testdatabase "github.com/Azure/ARO-RP/test/database"
)

func TestAdminDeleteManagedResource(t *testing.T) {
mockSubID := "00000000-0000-0000-0000-000000000000"
mockTenantID := "00000000-0000-0000-0000-000000000000"

ctx := context.Background()

type test struct {
name string
resourceID string
managedResourceID string
mocks func(*test, *mock_adminactions.MockAzureActions)
wantStatusCode int
wantResponse []byte
wantError string
}

for _, tt := range []*test{
{
name: "delete managed resource within cluster managed resourcegroup",
resourceID: testdatabase.GetResourcePath(mockSubID, "resourceName"),
managedResourceID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster/providers/Microsoft.Network/publicIPAddresses/infraID-adce98f85c7dd47c5a21263a5e39c083", mockSubID),
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
a.EXPECT().ResourceDeleteAndWait(gomock.Any(), tt.managedResourceID).Return(nil)
},
wantStatusCode: http.StatusOK,
},
{
name: "delete managed resource not within cluster managed resourcegroup fails",
resourceID: testdatabase.GetResourcePath(mockSubID, "resourceName"),
managedResourceID: fmt.Sprintf("/subscriptions/%s/resourceGroups/notmanagedresourcegroup/providers/Microsoft.Network/publicIPAddresses/infraID-adce98f85c7dd47c5a21263a5e39c083", mockSubID),
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
},
wantStatusCode: http.StatusBadRequest,
wantError: "400: InvalidParameter: : The resource /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/notmanagedresourcegroup/providers/Microsoft.Network/publicIPAddresses/infraID-adce98f85c7dd47c5a21263a5e39c083 is not within the cluster's managed resource group /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-cluster.",
},
{
name: "delete a resource that doesn't exist fails",
resourceID: testdatabase.GetResourcePath(mockSubID, "resourceName"),
managedResourceID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster/providers/Microsoft.Network/publicIPAddresses/infraID-adce98f85c7dd47c5a21263a5e39c083", mockSubID),
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
a.EXPECT().ResourceDeleteAndWait(gomock.Any(), tt.managedResourceID).Return(autorest.DetailedError{StatusCode: 404})
},
wantStatusCode: http.StatusNotFound,
wantError: fmt.Sprintf("404: NotFound: : The resource '%s' could not be found.", fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster/providers/Microsoft.Network/publicIPAddresses/infraID-adce98f85c7dd47c5a21263a5e39c083", mockSubID)),
},
{
name: "cannot delete resources in the deny list",
resourceID: testdatabase.GetResourcePath(mockSubID, "resourceName"),
managedResourceID: fmt.Sprintf("/subscriptions/%s/resourcegroups/test-cluster/providers/Microsoft.Network/privateLinkServices/infraID", mockSubID),
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
a.EXPECT().ResourceDeleteAndWait(gomock.Any(), tt.managedResourceID).Return(api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "",
fmt.Sprintf("deletion of resource /subscriptions/%s/resourcegroups/test-cluster/providers/Microsoft.Network/privateLinkServices/infraID is forbidden", mockSubID)),
)
},
wantStatusCode: http.StatusBadRequest,
wantError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "",
fmt.Sprintf("deletion of resource /subscriptions/%s/resourcegroups/test-cluster/providers/Microsoft.Network/privateLinkServices/infraID is forbidden", mockSubID)).Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
ti := newTestInfra(t).WithOpenShiftClusters().WithSubscriptions()
defer ti.done()
ti.fixture.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{
Key: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")),
OpenShiftCluster: &api.OpenShiftCluster{
ID: testdatabase.GetResourcePath(mockSubID, "resourceName"),
Properties: api.OpenShiftClusterProperties{
ClusterProfile: api.ClusterProfile{
ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", mockSubID),
},
},
},
})
ti.fixture.AddSubscriptionDocuments(&api.SubscriptionDocument{
ID: mockSubID,
Subscription: &api.Subscription{
State: api.SubscriptionStateRegistered,
Properties: &api.SubscriptionProperties{
TenantID: mockTenantID,
},
},
})

err := ti.buildFixtures(nil)
if err != nil {
t.Fatal(err)
}

a := mock_adminactions.NewMockAzureActions(ti.controller)
tt.mocks(tt, a)

f, err := NewFrontend(ctx, ti.audit, ti.log, ti.env, ti.asyncOperationsDatabase, ti.clusterManagerDatabase, ti.openShiftClustersDatabase, ti.subscriptionsDatabase, nil, api.APIs, &noop.Noop{}, &noop.Noop{}, nil, nil, nil, func(*logrus.Entry, env.Interface, *api.OpenShiftCluster, *api.SubscriptionDocument) (adminactions.AzureActions, error) {
return a, nil
}, nil)

if err != nil {
t.Fatal(err)
}

go f.Run(ctx, nil, nil)
resp, b, err := ti.request(http.MethodPost,
fmt.Sprintf("https://server/admin%s/deletemanagedresource?managedResourceID=%s", tt.resourceID, tt.managedResourceID),
nil, nil)
if err != nil {
t.Error(err)
}

err = validateResponse(resp, b, tt.wantStatusCode, tt.wantError, tt.wantResponse)
if err != nil {
t.Error(err)
}
})
}
}
3 changes: 3 additions & 0 deletions pkg/frontend/adminactions/azureactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type AzureActions interface {
VMSerialConsole(ctx context.Context, w http.ResponseWriter, log *logrus.Entry, vmName string) error
AppLensGetDetector(ctx context.Context, detectorId string) ([]byte, error)
AppLensListDetectors(ctx context.Context) ([]byte, error)
ResourceDeleteAndWait(ctx context.Context, resourceID string) error
}

type azureActions struct {
Expand All @@ -54,6 +55,7 @@ type azureActions struct {
routeTables network.RouteTablesClient
storageAccounts storage.AccountsClient
networkInterfaces network.InterfacesClient
loadBalancers network.LoadBalancersClient
appLens applens.AppLensClient
}

Expand Down Expand Up @@ -89,6 +91,7 @@ func NewAzureActions(log *logrus.Entry, env env.Interface, oc *api.OpenShiftClus
routeTables: network.NewRouteTablesClient(env.Environment(), subscriptionDoc.ID, fpAuth),
storageAccounts: storage.NewAccountsClient(env.Environment(), subscriptionDoc.ID, fpAuth),
networkInterfaces: network.NewInterfacesClient(env.Environment(), subscriptionDoc.ID, fpAuth),
loadBalancers: network.NewLoadBalancersClient(env.Environment(), subscriptionDoc.ID, fpAuth),
appLens: appLensClient,
}, nil
}
Expand Down
73 changes: 73 additions & 0 deletions pkg/frontend/adminactions/delete_managedresource.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package adminactions

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"context"
"net/http"
"regexp"
"strings"

"github.com/Azure/go-autorest/autorest/azure"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/util/azureclient"
"github.com/Azure/ARO-RP/pkg/util/loadbalancer"
)

var (
frontendIPConfigurationPattern = `(?i)^/subscriptions/(.+)/resourceGroups/(.+)/providers/Microsoft\.Network/loadBalancers/(.+)/frontendIPConfigurations/([^/]+)$`
denyList = []string{
`(?i)^/subscriptions/(.+)/resourceGroups/(.+)/providers/Microsoft\.Network/privateLinkServices/([^/]+)$`,
`(?i)^/subscriptions/(.+)/resourceGroups/(.+)/providers/Microsoft\.Network/privateEndpoints/([^/]+)$`,
`(?i)^/subscriptions/(.+)/resourceGroups/(.+)/providers/Microsoft\.Storage/(.+)$`,
}
)

func (a *azureActions) ResourceDeleteAndWait(ctx context.Context, resourceID string) error {
idParts, err := azure.ParseResourceID(resourceID)
if err != nil {
return err
}

for _, regex := range denyList {
re := regexp.MustCompile(regex)
if re.MatchString(resourceID) {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "deletion of resource %s is forbidden", resourceID)
}
}

apiVersion := azureclient.APIVersion(strings.ToLower(idParts.Provider + "/" + idParts.ResourceType))

_, err = a.resources.GetByID(ctx, resourceID, apiVersion)
if err != nil {
return err
}

re := regexp.MustCompile(frontendIPConfigurationPattern)
// FrontendIPConfiguration cannot be deleted with DeleteByIDAndWait (DELETE method is invalid on frontendIPConfiguration resourceID)
if re.MatchString(resourceID) {
return a.deleteFrontendIPConfiguration(ctx, resourceID)
}

return a.resources.DeleteByIDAndWait(ctx, resourceID, apiVersion)
}

func (a *azureActions) deleteFrontendIPConfiguration(ctx context.Context, resourceID string) error {
idParts := strings.Split(resourceID, "/")
rg := idParts[4]
lbName := idParts[8]

lb, err := a.loadBalancers.Get(ctx, rg, lbName, "")
if err != nil {
return err
}

err = loadbalancer.RemoveFrontendIPConfiguration(&lb, resourceID)
if err != nil {
return err
}

return a.loadBalancers.CreateOrUpdateAndWait(ctx, rg, lbName, lb)
}
Loading

0 comments on commit 9b92b4f

Please sign in to comment.