Skip to content

Commit 472515d

Browse files
committed
ARO-9391 Remove the Access Key dependency when using Federated Token
1 parent 27c842b commit 472515d

File tree

3 files changed

+298
-31
lines changed

3 files changed

+298
-31
lines changed

pkg/storage/azure/azure.go

Lines changed: 226 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ func GetConfig(secLister kcorelisters.SecretNamespaceLister, infraLister configl
138138
}, nil
139139
}
140140

141+
func isAzureStackCloud(name string) bool {
142+
return strings.EqualFold(name, "AZURESTACKCLOUD")
143+
}
144+
141145
func getEnvironmentByName(name string) (autorestazure.Environment, error) {
142146
if name == "" {
143147
return autorestazure.PublicCloud, nil
@@ -179,13 +183,14 @@ func (d *driver) createStorageAccount(storageAccountsClient storage.AccountsClie
179183
// disabling public network access at all... either we update the storage
180184
// account after creation using the new sdk, or we use the new sdk to create it
181185
// outside of azure stack hub, and the old sdk for azure stack hub.
186+
// Also doesn't support AccessKey usage disabling
182187
params := &storage.AccountPropertiesCreateParameters{
183188
EnableHTTPSTrafficOnly: to.BoolPtr(true),
184189
AllowBlobPublicAccess: to.BoolPtr(false),
185190
MinimumTLSVersion: storage.TLS12,
186191
}
187192

188-
if strings.EqualFold(cloudName, "AZURESTACKCLOUD") {
193+
if isAzureStackCloud(cloudName) {
189194
// It seems Azure Stack Hub does not support new API.
190195
kind = storage.Storage
191196
params = &storage.AccountPropertiesCreateParameters{}
@@ -314,6 +319,22 @@ func NewDriver(ctx context.Context, c *imageregistryv1.ImageRegistryConfigStorag
314319
}
315320
}
316321

322+
func (d *driver) newAzClient(cfg *Azure, environment autorestazure.Environment, tagset map[string]*string) (*azureclient.Client, error) {
323+
client, err := azureclient.New(&azureclient.Options{
324+
Environment: environment,
325+
TenantID: cfg.TenantID,
326+
ClientID: cfg.ClientID,
327+
ClientSecret: cfg.ClientSecret,
328+
FederatedTokenFile: cfg.FederatedTokenFile,
329+
SubscriptionID: cfg.SubscriptionID,
330+
TagSet: tagset,
331+
})
332+
if err != nil {
333+
return nil, err
334+
}
335+
return client, nil
336+
}
337+
317338
func (d *driver) storageAccountsClient(cfg *Azure, environment autorestazure.Environment) (storage.AccountsClient, error) {
318339
storageAccountsClient := storage.NewAccountsClientWithBaseURI(environment.ResourceManagerEndpoint, cfg.SubscriptionID)
319340
storageAccountsClient.PollingDelay = 10 * time.Second
@@ -516,6 +537,48 @@ func (d *driver) containerExists(ctx context.Context, environment autorestazure.
516537
return true, nil
517538
}
518539

540+
func (d *driver) storageExistsViaTrack2SDK(cr *imageregistryv1.Config, cfg *Azure, environment autorestazure.Environment) (exists bool, err error) {
541+
key := cfg.AccountKey
542+
federated_token := cfg.FederatedTokenFile
543+
azClient, err := d.newAzClient(cfg, environment, nil)
544+
if err != nil {
545+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to create azure client: %s", err))
546+
return false, err
547+
}
548+
if key == "" && federated_token == "" {
549+
key, err = d.getKey(cfg, environment)
550+
if err != nil {
551+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to get storage account key: %s", err))
552+
return false, err
553+
}
554+
555+
}
556+
557+
u, err := getBlobServiceURL(environment, d.Config.AccountName)
558+
if err != nil {
559+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonConfigError, fmt.Sprintf("Unable to parse blob url: %s", err))
560+
return false, err
561+
}
562+
blobClient, err := azClient.NewBlobClient(environment, d.Config.AccountName, key, fmt.Sprintf("%s://%s/", u.Scheme, u.Host))
563+
if err != nil {
564+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to create blob client: %s", err))
565+
return false, err
566+
}
567+
568+
exists, err = blobClient.ContainerExists(d.Context, d.Config.AccountName, d.Config.Container)
569+
if err != nil {
570+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("%s", err))
571+
return false, err
572+
}
573+
if !exists {
574+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerNotFound, fmt.Sprintf("Could not find storage container %s", d.Config.Container))
575+
return false, nil
576+
}
577+
578+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionTrue, storageExistsReasonContainerExists, "Storage container exists")
579+
return true, nil
580+
}
581+
519582
// StorageExists checks if the storage container exists and is accessible.
520583
func (d *driver) StorageExists(cr *imageregistryv1.Config) (bool, error) {
521584
if d.Config.AccountName == "" || d.Config.Container == "" {
@@ -535,6 +598,10 @@ func (d *driver) StorageExists(cr *imageregistryv1.Config) (bool, error) {
535598
return false, err
536599
}
537600

601+
if !isAzureStackCloud(d.Config.CloudName) {
602+
return d.storageExistsViaTrack2SDK(cr, cfg, environment)
603+
}
604+
538605
key, err := d.getKey(cfg, environment)
539606
if err != nil {
540607
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to get storage account key: %s", err))
@@ -570,15 +637,7 @@ func (d *driver) assurePrivateAccount(cfg *Azure, infra *configv1.Infrastructure
570637
if err != nil {
571638
return "", err
572639
}
573-
azclient, err := azureclient.New(&azureclient.Options{
574-
Environment: environment,
575-
TenantID: cfg.TenantID,
576-
ClientID: cfg.ClientID,
577-
ClientSecret: cfg.ClientSecret,
578-
FederatedTokenFile: cfg.FederatedTokenFile,
579-
SubscriptionID: cfg.SubscriptionID,
580-
TagSet: tagset,
581-
})
640+
azclient, err := d.newAzClient(cfg, environment, tagset)
582641
if err != nil {
583642
return "", err
584643
}
@@ -708,9 +767,83 @@ func (d *driver) assureStorageAccount(cfg *Azure, infra *configv1.Infrastructure
708767
}
709768
}
710769

770+
if !isAzureStackCloud(d.Config.CloudName) && cfg.FederatedTokenFile != "" {
771+
azClient, err := d.newAzClient(cfg, environment, tagset)
772+
if err != nil {
773+
return "", false, err
774+
}
775+
err = azClient.DisableStorageAccountAccessKeyAccess(d.Context, cfg.ResourceGroup, accountName)
776+
if err != nil {
777+
return "", false, err
778+
}
779+
}
780+
711781
return accountName, storageAccountCreated, nil
712782
}
713783

784+
func (d *driver) assureContainerViaTrack2SDK(cfg *Azure) (string, bool, error) {
785+
environment, err := getEnvironmentByName(d.Config.CloudName)
786+
if err != nil {
787+
return "", false, err
788+
}
789+
key := cfg.AccountKey
790+
federated_token := cfg.FederatedTokenFile
791+
azClient, err := d.newAzClient(cfg, environment, nil)
792+
if err != nil {
793+
return "", false, err
794+
}
795+
if key == "" && federated_token == "" {
796+
storageAccountsClient, err := d.storageAccountsClient(cfg, environment)
797+
if err != nil {
798+
return "", false, err
799+
}
800+
key, err = d.getAccountPrimaryKey(
801+
storageAccountsClient, cfg.ResourceGroup, d.Config.AccountName,
802+
)
803+
if err != nil {
804+
return "", false, err
805+
}
806+
}
807+
808+
u, err := getBlobServiceURL(environment, d.Config.AccountName)
809+
if err != nil {
810+
return "", false, err
811+
}
812+
blobClient, err := azClient.NewBlobClient(environment, d.Config.AccountName, key, fmt.Sprintf("%s://%s/", u.Scheme, u.Host))
813+
if err != nil {
814+
return "", false, err
815+
}
816+
if d.Config.Container == "" {
817+
containerName, err := util.GenerateStorageName(d.Listers, "")
818+
if err != nil {
819+
return "", false, err
820+
}
821+
822+
if err = blobClient.CreateStorageContainer(
823+
d.Context, containerName,
824+
); err != nil {
825+
return "", false, err
826+
}
827+
828+
return containerName, true, nil
829+
}
830+
831+
if exists, err := blobClient.ContainerExists(
832+
d.Context, d.Config.AccountName, d.Config.Container,
833+
); err != nil {
834+
return "", false, err
835+
} else if exists {
836+
return d.Config.Container, false, nil
837+
}
838+
839+
if err = blobClient.CreateStorageContainer(
840+
d.Context, d.Config.Container,
841+
); err != nil {
842+
return "", false, err
843+
}
844+
return d.Config.Container, true, nil
845+
}
846+
714847
// assureContainer makes sure we have a container in place. Container name may be provided or
715848
// generated automatically. Returns the container name (the provided one or the automatically
716849
// generated), if the container was created or was already there and an error.
@@ -880,7 +1013,13 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
8801013
}
8811014
d.Config.AccountName = storageAccountName
8821015

883-
containerName, containerCreated, err := d.assureContainer(cfg)
1016+
var containerName string
1017+
var containerCreated bool
1018+
if isAzureStackCloud(d.Config.CloudName) {
1019+
containerName, containerCreated, err = d.assureContainer(cfg)
1020+
} else {
1021+
containerName, containerCreated, err = d.assureContainerViaTrack2SDK(cfg)
1022+
}
8841023
if err != nil {
8851024
util.UpdateCondition(
8861025
cr,
@@ -937,6 +1076,75 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
9371076
return nil
9381077
}
9391078

1079+
func (d *driver) removeStorageContainerViaTrack2SDK(cr *imageregistryv1.Config, cfg *Azure, environment autorestazure.Environment, storageAccountsClient storage.AccountsClient) (exists bool, err error) {
1080+
key := cfg.AccountKey
1081+
federated_token := cfg.FederatedTokenFile
1082+
azClient, err := d.newAzClient(cfg, environment, nil)
1083+
if err != nil {
1084+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to create azure client: %s", err))
1085+
return false, err
1086+
}
1087+
if key == "" && federated_token == "" {
1088+
key, err = d.getAccountPrimaryKey(storageAccountsClient, cfg.ResourceGroup, d.Config.AccountName)
1089+
if _, ok := err.(*errDoesNotExist); ok {
1090+
d.Config.AccountName = ""
1091+
cr.Spec.Storage.Azure.AccountName = "" // TODO
1092+
cr.Status.Storage.Azure.AccountName = ""
1093+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerNotFound, fmt.Sprintf("Container has been already deleted: %s", err))
1094+
return false, nil
1095+
}
1096+
}
1097+
1098+
u, err := getBlobServiceURL(environment, d.Config.AccountName)
1099+
if err != nil {
1100+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonConfigError, fmt.Sprintf("Unable to parse blob url: %s", err))
1101+
return false, err
1102+
}
1103+
blobClient, err := azClient.NewBlobClient(environment, d.Config.AccountName, key, fmt.Sprintf("%s://%s/", u.Scheme, u.Host))
1104+
if err != nil {
1105+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to create blob client: %s", err))
1106+
return false, err
1107+
}
1108+
err = blobClient.DeleteStorageContainer(d.Context, d.Config.Container)
1109+
if err != nil {
1110+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to delete storage container: %s", err))
1111+
return false, err // TODO: is it retryable?
1112+
}
1113+
1114+
d.Config.Container = ""
1115+
cr.Spec.Storage.Azure.Container = "" // TODO: what if it was provided by a user?
1116+
cr.Status.Storage.Azure.Container = ""
1117+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerDeleted, "Storage container has been deleted")
1118+
return true, nil
1119+
}
1120+
1121+
func (d *driver) removeStorageContainer(cr *imageregistryv1.Config, cfg *Azure, environment autorestazure.Environment, storageAccountsClient storage.AccountsClient) (exists bool, err error) {
1122+
key, err := d.getAccountPrimaryKey(storageAccountsClient, cfg.ResourceGroup, d.Config.AccountName)
1123+
if _, ok := err.(*errDoesNotExist); ok {
1124+
d.Config.AccountName = ""
1125+
cr.Spec.Storage.Azure.AccountName = "" // TODO
1126+
cr.Status.Storage.Azure.AccountName = ""
1127+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerNotFound, fmt.Sprintf("Container has been already deleted: %s", err))
1128+
return false, nil
1129+
}
1130+
if err != nil {
1131+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to get account primary keys: %s", err))
1132+
return false, err
1133+
}
1134+
1135+
err = d.deleteStorageContainer(environment, d.Config.AccountName, key, d.Config.Container)
1136+
if err != nil {
1137+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to delete storage container: %s", err))
1138+
return false, err // TODO: is it retryable?
1139+
}
1140+
1141+
d.Config.Container = ""
1142+
cr.Spec.Storage.Azure.Container = "" // TODO: what if it was provided by a user?
1143+
cr.Status.Storage.Azure.Container = ""
1144+
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerDeleted, "Storage container has been deleted")
1145+
return true, nil
1146+
}
1147+
9401148
// RemoveStorage deletes the storage medium that was created.
9411149
func (d *driver) RemoveStorage(cr *imageregistryv1.Config) (retry bool, err error) {
9421150
if cr.Spec.Storage.ManagementState != imageregistryv1.StorageManagementStateManaged {
@@ -1016,29 +1224,18 @@ func (d *driver) RemoveStorage(cr *imageregistryv1.Config) (retry bool, err erro
10161224
}
10171225

10181226
if d.Config.Container != "" {
1019-
key, err := d.getAccountPrimaryKey(storageAccountsClient, cfg.ResourceGroup, d.Config.AccountName)
1020-
if _, ok := err.(*errDoesNotExist); ok {
1021-
d.Config.AccountName = ""
1022-
cr.Spec.Storage.Azure.AccountName = "" // TODO
1023-
cr.Status.Storage.Azure.AccountName = ""
1024-
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerNotFound, fmt.Sprintf("Container has been already deleted: %s", err))
1025-
return false, nil
1227+
var exists bool
1228+
if isAzureStackCloud(d.Config.CloudName) {
1229+
exists, err = d.removeStorageContainer(cr, cfg, environment, storageAccountsClient)
1230+
} else {
1231+
exists, err = d.removeStorageContainerViaTrack2SDK(cr, cfg, environment, storageAccountsClient)
10261232
}
10271233
if err != nil {
1028-
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to get account primary keys: %s", err))
10291234
return false, err
10301235
}
1031-
1032-
err = d.deleteStorageContainer(environment, d.Config.AccountName, key, d.Config.Container)
1033-
if err != nil {
1034-
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionUnknown, storageExistsReasonAzureError, fmt.Sprintf("Unable to delete storage container: %s", err))
1035-
return false, err // TODO: is it retryable?
1236+
if !exists {
1237+
return false, nil
10361238
}
1037-
1038-
d.Config.Container = ""
1039-
cr.Spec.Storage.Azure.Container = "" // TODO: what if it was provided by a user?
1040-
cr.Status.Storage.Azure.Container = ""
1041-
util.UpdateCondition(cr, defaults.StorageExists, operatorapiv1.ConditionFalse, storageExistsReasonContainerDeleted, "Storage container has been deleted")
10421239
}
10431240

10441241
_, err = storageAccountsClient.Delete(d.Context, cfg.ResourceGroup, d.Config.AccountName)

0 commit comments

Comments
 (0)