Skip to content

Commit 5c81c66

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #829 from chiragkyal/cfe-678
[CFE-678] Add UserTags while creating Azure Storage Account
2 parents 901305b + 32963fd commit 5c81c66

File tree

62 files changed

+1905
-250
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+1905
-250
lines changed

‎go.mod

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ require (
2727
github.com/gophercloud/utils v0.0.0-20221124081324-7bac6f5cdf99
2828
github.com/goware/urlx v0.3.2
2929
github.com/jongio/azidext/go/azidext v0.4.0
30-
github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2
30+
github.com/openshift/api v0.0.0-20230210191657-68f4ffcda083
3131
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d
3232
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084
3333
github.com/openshift/library-go v0.0.0-20230120214501-9bc305884fcb
@@ -36,15 +36,15 @@ require (
3636
github.com/prometheus/common v0.37.0
3737
github.com/spf13/cobra v1.6.1
3838
github.com/stretchr/testify v1.8.1
39-
golang.org/x/net v0.5.0
39+
golang.org/x/net v0.6.0
4040
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5
4141
google.golang.org/api v0.57.0
4242
gopkg.in/yaml.v2 v2.4.0
4343
k8s.io/api v0.26.1
4444
k8s.io/apimachinery v0.26.1
4545
k8s.io/client-go v0.26.1
4646
k8s.io/klog/v2 v2.90.0
47-
k8s.io/utils v0.0.0-20230115233650-391b47cb4029
47+
k8s.io/utils v0.0.0-20230209194617-a36077c30491
4848
)
4949

5050
require (
@@ -143,9 +143,9 @@ require (
143143
go.uber.org/zap v1.19.0 // indirect
144144
golang.org/x/crypto v0.1.0 // indirect
145145
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect
146-
golang.org/x/sys v0.4.0 // indirect
147-
golang.org/x/term v0.4.0 // indirect
148-
golang.org/x/text v0.6.0 // indirect
146+
golang.org/x/sys v0.5.0 // indirect
147+
golang.org/x/term v0.5.0 // indirect
148+
golang.org/x/text v0.7.0 // indirect
149149
golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect
150150
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
151151
google.golang.org/appengine v1.6.7 // indirect

‎go.sum

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
589589
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
590590
github.com/onsi/gomega v1.10.3/go.mod h1:V9xEwhxec5O8UDM77eCW8vLymOMltsqPVYWrpDsH8xc=
591591
github.com/onsi/gomega v1.23.0 h1:/oxKu9c2HVap+F3PfKort2Hw5DEU+HGlW8n+tguWsys=
592-
github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2 h1:+nw0/d4spq880W7S74Twi5YU2ulsl3/a9o4OEZptYp0=
593-
github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
592+
github.com/openshift/api v0.0.0-20230210191657-68f4ffcda083 h1:Wc3z06B8JaEOnPlNovVB3ZR92CoSLWFupOeYO3khzCE=
593+
github.com/openshift/api v0.0.0-20230210191657-68f4ffcda083/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
594594
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d h1:RR4ah7FfaPR1WePizm0jlrsbmPu91xQZnAsVVreQV1k=
595595
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
596596
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE=
@@ -890,8 +890,8 @@ golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su
890890
golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
891891
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
892892
golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco=
893-
golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw=
894-
golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws=
893+
golang.org/x/net v0.6.0 h1:L4ZwwTvKW9gr0ZMS1yrHD9GZhIuVjOBBnaKH+SPQK0Q=
894+
golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
895895
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
896896
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
897897
golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -1005,13 +1005,13 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc
10051005
golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
10061006
golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
10071007
golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
1008-
golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18=
1009-
golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
1008+
golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
1009+
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
10101010
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
10111011
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
10121012
golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
1013-
golang.org/x/term v0.4.0 h1:O7UWfv5+A2qiuulQk30kVinPoMtoIPeVaKLEgLpVkvg=
1014-
golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ=
1013+
golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY=
1014+
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
10151015
golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
10161016
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
10171017
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -1023,8 +1023,8 @@ golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
10231023
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
10241024
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
10251025
golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
1026-
golang.org/x/text v0.6.0 h1:3XmdazWV+ubf7QgHSTWeykHOci5oeekaGJBLkrkaw4k=
1027-
golang.org/x/text v0.6.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
1026+
golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo=
1027+
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
10281028
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
10291029
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
10301030
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
@@ -1314,8 +1314,8 @@ k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 h1:+70TFaan3hfJzs+7VK2o+O
13141314
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280/go.mod h1:+Axhij7bCpeqhklhUTe3xmOn6bWxolyZEeyaFpjGtl4=
13151315
k8s.io/utils v0.0.0-20191114184206-e782cd3c129f/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
13161316
k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
1317-
k8s.io/utils v0.0.0-20230115233650-391b47cb4029 h1:L8zDtT4jrxj+TaQYD0k8KNlr556WaVQylDXswKmX+dE=
1318-
k8s.io/utils v0.0.0-20230115233650-391b47cb4029/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
1317+
k8s.io/utils v0.0.0-20230209194617-a36077c30491 h1:r0BAOLElQnnFhE/ApUsg3iHdVYYPBjNSSOMowRZxxsY=
1318+
k8s.io/utils v0.0.0-20230209194617-a36077c30491/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
13191319
modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=
13201320
modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
13211321
modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=

‎pkg/storage/azure/azure.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (d *driver) accountExists(storageAccountsClient storage.AccountsClient, acc
154154
)
155155
}
156156

157-
func (d *driver) createStorageAccount(storageAccountsClient storage.AccountsClient, resourceGroupName, accountName, location, cloudName string) error {
157+
func (d *driver) createStorageAccount(storageAccountsClient storage.AccountsClient, resourceGroupName, accountName, location, cloudName string, tagset map[string]*string) error {
158158
klog.Infof("attempt to create azure storage account %s (resourceGroup=%q, location=%q)...", accountName, resourceGroupName, location)
159159

160160
kind := storage.StorageV2
@@ -181,6 +181,7 @@ func (d *driver) createStorageAccount(storageAccountsClient storage.AccountsClie
181181
Name: storage.StandardLRS,
182182
},
183183
AccountPropertiesCreateParameters: params,
184+
Tags: tagset,
184185
},
185186
)
186187
if err != nil {
@@ -485,9 +486,9 @@ func (d *driver) StorageChanged(cr *imageregistryv1.Config) bool {
485486
return !reflect.DeepEqual(cr.Status.Storage.Azure, cr.Spec.Storage.Azure)
486487
}
487488

488-
// assureStorageAccount makes sure there is a storage account in place. If no storage account name
489-
// is provided it attempts to generate one. Returns the account name (either the one provided or
490-
// the one generated), if the account was created or was already there and an error.
489+
// assureStorageAccount makes sure there is a storage account in place and apply any provided tags.
490+
// If no storage account name is provided it attempts to generate one. Returns the account name
491+
// (either the one provided or the one generated), if the account was created or was already there and an error.
491492
func (d *driver) assureStorageAccount(cfg *Azure, infra *configv1.Infrastructure) (string, bool, error) {
492493
environment, err := getEnvironmentByName(d.Config.CloudName)
493494
if err != nil {
@@ -516,13 +517,33 @@ func (d *driver) assureStorageAccount(cfg *Azure, infra *configv1.Infrastructure
516517
return "", false, fmt.Errorf("create storage account failed, name not available")
517518
}
518519

520+
// Tag the storage account with the openshiftClusterID
521+
// along with any user defined tags from the cluster configuration
522+
klog.V(2).Info("setting azure storage account tags")
523+
524+
tagset := map[string]*string{
525+
fmt.Sprintf("kubernetes.io_cluster.%s", infra.Status.InfrastructureName): to.StringPtr("owned"),
526+
}
527+
528+
// at this stage we are not keeping user tags in sync. as per enhancement proposal
529+
// we only set user provided tags when we created the bucket.
530+
hasAzureStatus := infra.Status.PlatformStatus != nil && infra.Status.PlatformStatus.Azure != nil && infra.Status.PlatformStatus.Azure.ResourceTags != nil
531+
if hasAzureStatus {
532+
klog.V(5).Infof("user has provided %d tags", len(infra.Status.PlatformStatus.Azure.ResourceTags))
533+
for _, tag := range infra.Status.PlatformStatus.Azure.ResourceTags {
534+
klog.V(5).Infof("user has provided storage account tag: %s: %s", tag.Key, tag.Value)
535+
tagset[tag.Key] = to.StringPtr(tag.Value)
536+
}
537+
}
538+
klog.V(5).Infof("tagging storage account with tags: %+v", tagset)
539+
519540
// regardless if the storage account name was provided by the user or we generated it,
520541
// if it is available, we do attempt to create it.
521542
var storageAccountCreated bool
522543
if *result.NameAvailable {
523544
storageAccountCreated = true
524545
if err := d.createStorageAccount(
525-
storageAccountsClient, cfg.ResourceGroup, accountName, cfg.Region, d.Config.CloudName,
546+
storageAccountsClient, cfg.ResourceGroup, accountName, cfg.Region, d.Config.CloudName, tagset,
526547
); err != nil {
527548
return "", false, err
528549
}

‎pkg/storage/azure/azure_test.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package azure
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/base64"
7+
"encoding/json"
8+
"fmt"
9+
"io"
610
"net/http"
711
"reflect"
812
"regexp"
@@ -12,6 +16,7 @@ import (
1216
"github.com/Azure/azure-pipeline-go/pipeline"
1317
"github.com/Azure/go-autorest/autorest"
1418
"github.com/Azure/go-autorest/autorest/mocks"
19+
"github.com/google/go-cmp/cmp"
1520

1621
corev1 "k8s.io/api/core/v1"
1722
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -329,6 +334,138 @@ func TestConfigEnvWithUserKey(t *testing.T) {
329334
}
330335
}
331336

337+
// custom sender for mocking
338+
type sender struct {
339+
response []*http.Response
340+
body string
341+
}
342+
343+
// Do implements the Sender interface for mocking
344+
// Do accepts the passed request and body, then appends the response and emits it.
345+
func (s *sender) Do(r *http.Request) (*http.Response, error) {
346+
resp := &http.Response{
347+
StatusCode: http.StatusOK,
348+
Request: r,
349+
Body: io.NopCloser(bytes.NewBufferString(s.body)),
350+
}
351+
s.response = append(s.response, resp)
352+
return resp, nil
353+
}
354+
355+
func TestUserProvidedTags(t *testing.T) {
356+
for _, tt := range []struct {
357+
name string
358+
userTags []configv1.AzureResourceTag
359+
expectedTags map[string]string
360+
infraName string
361+
responseBody string
362+
}{
363+
{
364+
name: "no-user-tags",
365+
infraName: "some-infra",
366+
// only default tags
367+
expectedTags: map[string]string{
368+
"kubernetes.io_cluster.some-infra": "owned",
369+
},
370+
responseBody: `{"nameAvailable":true}`,
371+
},
372+
{
373+
name: "with-user-tags",
374+
infraName: "test-infra",
375+
userTags: []configv1.AzureResourceTag{
376+
{
377+
Key: "tag1",
378+
Value: "value1",
379+
},
380+
{
381+
Key: "tag2",
382+
Value: "value2",
383+
},
384+
},
385+
// default tags and user tags
386+
expectedTags: map[string]string{
387+
"kubernetes.io_cluster.test-infra": "owned",
388+
"tag1": "value1",
389+
"tag2": "value2",
390+
},
391+
responseBody: `{"nameAvailable":true}`,
392+
},
393+
} {
394+
t.Run(tt.name, func(t *testing.T) {
395+
sender := &sender{
396+
body: tt.responseBody,
397+
}
398+
399+
storageConfig := &imageregistryv1.ImageRegistryConfigStorageAzure{}
400+
401+
drv := NewDriver(context.Background(), storageConfig, nil)
402+
drv.authorizer = autorest.NullAuthorizer{}
403+
drv.sender = sender
404+
405+
_, _, err := drv.assureStorageAccount(
406+
&Azure{
407+
SubscriptionID: "subscription-id",
408+
ResourceGroup: "resource-group",
409+
},
410+
&configv1.Infrastructure{
411+
Status: configv1.InfrastructureStatus{
412+
InfrastructureName: tt.infraName,
413+
Platform: configv1.AzurePlatformType,
414+
PlatformStatus: &configv1.PlatformStatus{
415+
Type: configv1.AzurePlatformType,
416+
Azure: &configv1.AzurePlatformStatus{
417+
ResourceTags: tt.userTags,
418+
},
419+
},
420+
},
421+
},
422+
)
423+
if err != nil {
424+
t.Errorf("unexpected error %q", err)
425+
}
426+
427+
// flag to confirm presence of tags
428+
foundTags := false
429+
430+
for _, resp := range sender.response {
431+
if resp != nil && resp.Request != nil && resp.Request.Body != nil {
432+
433+
reqBody := make(map[string]interface{})
434+
if err := json.NewDecoder(resp.Request.Body).Decode(&reqBody); err != nil {
435+
t.Fatalf("error decoding request: %q", err)
436+
}
437+
438+
// ignore request without tags
439+
if _, ok := reqBody["tags"]; ok {
440+
foundTags = true
441+
442+
tags, ok := reqBody["tags"].(map[string]interface{})
443+
if !ok {
444+
t.Fatal("unable to type assert tags field")
445+
}
446+
// convert into correct type
447+
receivedTags := make(map[string]string)
448+
for k, v := range tags {
449+
receivedTags[k] = fmt.Sprintf("%+v", v)
450+
}
451+
452+
// compare the tags
453+
if !reflect.DeepEqual(tt.expectedTags, receivedTags) {
454+
t.Fatalf(
455+
"unexpected tags: %s",
456+
cmp.Diff(tt.expectedTags, receivedTags),
457+
)
458+
}
459+
}
460+
}
461+
}
462+
if !foundTags {
463+
t.Fatal("no tags present in the request")
464+
}
465+
})
466+
}
467+
}
468+
332469
func Test_assureStorageAccount(t *testing.T) {
333470
for _, tt := range []struct {
334471
name string

0 commit comments

Comments
 (0)