Skip to content

Commit

Permalink
locate and delete app registrations by tag
Browse files Browse the repository at this point in the history
With cloud-cred-operator tagging Service Principals, we can now located the Service Principals by tag, and find their parent Application Registration and delete them as part of cluster destroy.
  • Loading branch information
Joel Diaz committed Aug 23, 2019
1 parent 9447003 commit 80ca614
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 21 deletions.
2 changes: 1 addition & 1 deletion pkg/asset/installconfig/azure/azure.go
Expand Up @@ -80,7 +80,7 @@ func getRegions() (map[string]string, error) {
return nil, err
}
client := azsub.NewClient()
client.Authorizer = session.Authorizer
client.Authorizer = session.ResourceManagerAuthorizer
ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second)
defer cancel()
locations, err := client.ListLocations(ctx, session.Credentials.SubscriptionID)
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/installconfig/azure/dns.go
Expand Up @@ -109,7 +109,7 @@ func NewDNSConfig() (*DNSConfig, error) {

func newZonesClient(session *Session) ZonesGetter {
azureClient := azdns.NewZonesClient(session.Credentials.SubscriptionID)
azureClient.Authorizer = session.Authorizer
azureClient.Authorizer = session.ResourceManagerAuthorizer
return &ZonesClient{azureClient: azureClient}
}

Expand Down
19 changes: 13 additions & 6 deletions pkg/asset/installconfig/azure/session.go
Expand Up @@ -20,8 +20,9 @@ var defaultAuthFilePath = filepath.Join(os.Getenv("HOME"), ".azure", "osServiceP

//Session is an object representing session for subscription
type Session struct {
Authorizer autorest.Authorizer
Credentials Credentials
GraphAuthorizer autorest.Authorizer
ResourceManagerAuthorizer autorest.Authorizer

This comment has been minimized.

Copy link
@abhinavdahiya

abhinavdahiya Aug 23, 2019

Contributor

Don't change the name for the default one.

Credentials Credentials
}

//Credentials is the data type for credentials as understood by the azure sdk
Expand All @@ -46,7 +47,7 @@ func newSessionFromFile(authFilePath string) (*Session, error) {
// NewAuthorizerFromFileWithResource uses `auth.GetSettingsFromFile`, which uses the `azureAuthEnv` to fetch the auth credentials.
// therefore setting the local env here to authFilePath allows NewAuthorizerFromFileWithResource to load credentials.
os.Setenv(azureAuthEnv, authFilePath)
authorizer, err := auth.NewAuthorizerFromFileWithResource(azureenv.PublicCloud.ResourceManagerEndpoint)
_, err := auth.NewAuthorizerFromFileWithResource(azureenv.PublicCloud.ResourceManagerEndpoint)
if err != nil {
logrus.Debug("Could not get an azure authorizer from file. Asking user to provide authentication info")
credentials, err := askForCredentials()
Expand All @@ -70,14 +71,20 @@ func newSessionFromFile(authFilePath string) (*Session, error) {
return nil, errors.Wrap(err, "failed to map authsettings to credentials")
}

authorizer, err = authSettings.ClientCredentialsAuthorizerWithResource(azureenv.PublicCloud.ResourceManagerEndpoint)
rmAuthorizer, err := authSettings.ClientCredentialsAuthorizerWithResource(azureenv.PublicCloud.ResourceManagerEndpoint)
if err != nil {
return nil, errors.Wrap(err, "failed to get client credentials authorizer from saved azure auth settings")
}

graphAuthorizer, err := authSettings.ClientCredentialsAuthorizerWithResource(azureenv.PublicCloud.GraphEndpoint)
if err != nil {
return nil, errors.Wrap(err, "failed to get GraphEndpoint authorizer from saved azure auth settings")
}

return &Session{
Authorizer: authorizer,
Credentials: *credentials,
GraphAuthorizer: graphAuthorizer,
ResourceManagerAuthorizer: rmAuthorizer,
Credentials: *credentials,
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/machines/azure/zones.go
Expand Up @@ -32,7 +32,7 @@ func skusClient() (client *compute.ResourceSkusClient, err error) {
}

skusClient := compute.NewResourceSkusClient(ssn.Credentials.SubscriptionID)
skusClient.Authorizer = ssn.Authorizer
skusClient.Authorizer = ssn.ResourceManagerAuthorizer
return &skusClient, nil
}

Expand Down
110 changes: 98 additions & 12 deletions pkg/destroy/azure/azure.go
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac"
"github.com/Azure/azure-sdk-for-go/services/preview/dns/mgmt/2018-03-01-preview/dns"
"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
"github.com/Azure/go-autorest/autorest"
Expand All @@ -24,27 +25,37 @@ import (

// ClusterUninstaller holds the various options for the cluster we want to delete.
type ClusterUninstaller struct {
SubscriptionID string
Authorizer autorest.Authorizer
SubscriptionID string
TenantID string
GraphAuthorizer autorest.Authorizer
ResourceManagerAuthorizer autorest.Authorizer

This comment has been minimized.

Copy link
@abhinavdahiya

abhinavdahiya Aug 23, 2019

Contributor

I don't think we should be changing the default pre-existing authorizer


InfraID string

Logger logrus.FieldLogger

resourceGroupsClient resources.GroupsGroupClient
zonesClient dns.ZonesClient
recordsClient dns.RecordSetsClient
resourceGroupsClient resources.GroupsGroupClient
zonesClient dns.ZonesClient
recordsClient dns.RecordSetsClient
serviceprincipalsClient graphrbac.ServicePrincipalsClient
applicationsClient graphrbac.ApplicationsClient
}

func (o *ClusterUninstaller) configureClients() {
o.resourceGroupsClient = resources.NewGroupsGroupClient(o.SubscriptionID)
o.resourceGroupsClient.Authorizer = o.Authorizer
o.resourceGroupsClient.Authorizer = o.ResourceManagerAuthorizer

o.zonesClient = dns.NewZonesClient(o.SubscriptionID)
o.zonesClient.Authorizer = o.Authorizer
o.zonesClient.Authorizer = o.ResourceManagerAuthorizer

o.recordsClient = dns.NewRecordSetsClient(o.SubscriptionID)
o.recordsClient.Authorizer = o.Authorizer
o.recordsClient.Authorizer = o.ResourceManagerAuthorizer

o.serviceprincipalsClient = graphrbac.NewServicePrincipalsClient(o.TenantID)
o.serviceprincipalsClient.Authorizer = o.GraphAuthorizer

o.applicationsClient = graphrbac.NewApplicationsClient(o.TenantID)
o.applicationsClient.Authorizer = o.GraphAuthorizer
}

// New returns an Azure destroyer from ClusterMetadata.
Expand All @@ -55,10 +66,12 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers.
}

return &ClusterUninstaller{
SubscriptionID: session.Credentials.SubscriptionID,
Authorizer: session.Authorizer,
InfraID: metadata.InfraID,
Logger: logger,
SubscriptionID: session.Credentials.SubscriptionID,
TenantID: session.Credentials.TenantID,
GraphAuthorizer: session.GraphAuthorizer,
ResourceManagerAuthorizer: session.ResourceManagerAuthorizer,
InfraID: metadata.InfraID,
Logger: logger,
}, nil
}

Expand All @@ -76,6 +89,11 @@ func (o *ClusterUninstaller) Run() error {
o.Logger.Debug(err)
return errors.Wrap(err, "failed to delete resource group")
}
o.Logger.Debug("deleting application registrations")
if err := deleteApplicationRegistrations(context.TODO(), o.applicationsClient, o.serviceprincipalsClient, o.Logger, o.InfraID); err != nil {
o.Logger.Debug(err)
return errors.Wrap(err, "failed to delete application registrations and their service principals")
}

return nil
}
Expand Down Expand Up @@ -217,3 +235,71 @@ func deleteResourceGroup(ctx context.Context, client resources.GroupsGroupClient
func wasNotFound(resp *http.Response) bool {
return resp != nil && resp.StatusCode == http.StatusNotFound
}

func deleteApplicationRegistrations(ctx context.Context, appClient graphrbac.ApplicationsClient, spClient graphrbac.ServicePrincipalsClient, logger logrus.FieldLogger, infraID string) error {
errorList := []error{}

tag := fmt.Sprintf("kubernetes.io_cluster.%s=owned", infraID)
servicePrincipals, err := getServicePrincipalsByTag(ctx, spClient, tag, infraID)
if err != nil {
return errors.Wrap(err, "failed to gather list of Service Principals by tag")
}

for _, sp := range servicePrincipals {
logger = logger.WithField("appID", *sp.AppID)
appFilter := fmt.Sprintf("appId eq '%s'", *sp.AppID)
appResults, err := appClient.List(ctx, appFilter)
if err != nil {
logger.WithError(err).Errorf("unable to get App")

This comment has been minimized.

Copy link
@abhinavdahiya

abhinavdahiya Aug 23, 2019

Contributor

Use pkg/errors for wrapping errors.

This comment has been minimized.

Copy link
@abhinavdahiya

abhinavdahiya Aug 23, 2019

Contributor

And don't log all transitive errors..

errorList = append(errorList, err)
continue
}

apps := appResults.Values()
if len(apps) != 1 {
msg := fmt.Sprintf("should have recieved only a single result matching AppID, received %d instead", len(apps))
logger.Error(msg)
errorList = append(errorList, errors.New(msg))
continue

This comment has been minimized.

Copy link
@abhinavdahiya

abhinavdahiya Aug 23, 2019

Contributor

Why leave them?

This comment has been minimized.

Copy link
@joelddiaz

joelddiaz Aug 23, 2019

Contributor

The thinking here was that if I'm trying to fetch a single App Registration that is associated with the Service Principal that we want to remove, and I get back more than one result in my query, then I'll give up rather than delete an App Registration that should be left alone.

}

_, err = appClient.Delete(ctx, *apps[0].ObjectID)
if err != nil {
logger.WithError(err).Error("error deleting Application Registration")
errorList = append(errorList, err)
continue
}
logger.Info("deleted")
}

return utilerrors.NewAggregate(errorList)
}

func getServicePrincipalsByTag(ctx context.Context, spClient graphrbac.ServicePrincipalsClient, matchTag, infraID string) ([]graphrbac.ServicePrincipal, error) {
matchedSPs := []graphrbac.ServicePrincipal{}

infraFilter := fmt.Sprintf("startswith(displayName,'%s')", infraID)
spResults, err := spClient.List(ctx, infraFilter)
if err != nil {
return matchedSPs, err
}

for {
for _, sp := range spResults.Values() {
for _, tag := range *sp.Tags {
if tag == matchTag {
matchedSPs = append(matchedSPs, sp)
break
}
}
}

if spResults.NotDone() {
spResults.NextWithContext(ctx)
} else {
break
}
}

return matchedSPs, nil
}

0 comments on commit 80ca614

Please sign in to comment.