Skip to content

Commit de45014

Browse files
rajdeepc2792rajdeepc2792
committed
ARO-9391 update unit tests for azblob clients
1 parent 5bf4361 commit de45014

File tree

4 files changed

+152
-97
lines changed

4 files changed

+152
-97
lines changed

‎pkg/storage/azure/azure.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/Azure/azure-pipeline-go/pipeline"
1515
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
1616
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
17+
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1718
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
1819
"github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage"
1920
"github.com/Azure/azure-storage-blob-go/azblob"
@@ -308,6 +309,10 @@ type driver struct {
308309
// httpSender is for Azure Pipeline.
309310
// Added as a member to the struct to allow injection for testing.
310311
httpSender pipeline.Factory
312+
313+
// policies is for new Azure Client Pipeline execution.
314+
// Added as a member to the struct to allow injection for testing.
315+
policies []policy.Policy
311316
}
312317

313318
// NewDriver creates a new storage driver for Azure Blob Storage.
@@ -328,6 +333,7 @@ func (d *driver) newAzClient(cfg *Azure, environment autorestazure.Environment,
328333
FederatedTokenFile: cfg.FederatedTokenFile,
329334
SubscriptionID: cfg.SubscriptionID,
330335
TagSet: tagset,
336+
Policies: d.policies,
331337
})
332338
if err != nil {
333339
return nil, err

‎pkg/storage/azure/azure_test.go

Lines changed: 77 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"testing"
1515

1616
"github.com/Azure/azure-pipeline-go/pipeline"
17+
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1718
"github.com/Azure/go-autorest/autorest"
1819
"github.com/Azure/go-autorest/autorest/mocks"
1920
"github.com/Azure/go-autorest/autorest/to"
@@ -35,6 +36,32 @@ import (
3536
"github.com/openshift/cluster-image-registry-operator/pkg/envvar"
3637
)
3738

39+
const mockTenantID = "00000000-0000-0000-0000-000000000000"
40+
41+
type testDoer struct {
42+
response *http.Response
43+
body string
44+
statusCode int
45+
header http.Header
46+
}
47+
48+
// Do implements the Doer interface for mocking.
49+
// Do accepts the passed policy request and body, then appends the response and emits it.
50+
func (td *testDoer) Do(r *policy.Request) (resp *http.Response, err error) {
51+
// Helps in emitting sequential Responses for the same client
52+
if td.response != nil {
53+
return r.Next()
54+
}
55+
resp = &http.Response{
56+
StatusCode: td.statusCode,
57+
Request: r.Raw(),
58+
Body: io.NopCloser(bytes.NewBufferString(td.body)),
59+
Header: td.header,
60+
}
61+
td.response = resp
62+
return resp, nil
63+
}
64+
3865
func TestGetConfig(t *testing.T) {
3966
for _, tt := range []struct {
4067
name string
@@ -261,7 +288,7 @@ func findEnvVar(envvars envvar.List, name string) *envvar.EnvVar {
261288
return nil
262289
}
263290

264-
func TestConfigEnv(t *testing.T) {
291+
func TestConfigEnvNonAzureStackHub(t *testing.T) {
265292
ctx := context.Background()
266293

267294
cr := &imageregistryv1.Config{}
@@ -290,6 +317,7 @@ func TestConfigEnv(t *testing.T) {
290317
Data: map[string][]byte{
291318
"azure_subscription_id": []byte("subscription_id"),
292319
"azure_client_id": []byte("client_id"),
320+
"azure_tenant_id": []byte(mockTenantID),
293321
"azure_client_secret": []byte("client_secret"),
294322
"azure_resourcegroup": []byte("resourcegroup"),
295323
},
@@ -305,16 +333,12 @@ func TestConfigEnv(t *testing.T) {
305333
sender.AppendResponse(mocks.NewResponseWithContent(`{"keys":[{"value":"firstKey"}]}`))
306334
sender.AppendResponse(mocks.NewResponseWithContent(`{"keys":[{"value":"firstKey"}]}`))
307335

308-
httpSender := pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc {
309-
return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) {
310-
return pipeline.NewHTTPResponse(mocks.NewResponseWithContent(`{}`)), nil
311-
}
312-
})
313-
314336
d := NewDriver(ctx, config, &listers.StorageListers)
315337
d.authorizer = authorizer
316338
d.sender = sender
317-
d.httpSender = httpSender
339+
d.policies = []policy.Policy{
340+
&testDoer{statusCode: http.StatusCreated},
341+
}
318342
err := d.CreateStorage(cr)
319343
if err != nil {
320344
t.Fatal(err)
@@ -340,7 +364,7 @@ func TestConfigEnv(t *testing.T) {
340364
}
341365
}
342366

343-
func TestConfigEnvWorkloadIdentity(t *testing.T) {
367+
func TestConfigEnvWorkloadIdentityNonAzureStackHub(t *testing.T) {
344368
ctx := context.Background()
345369

346370
config := &imageregistryv1.ImageRegistryConfigStorageAzure{}
@@ -383,16 +407,15 @@ func TestConfigEnvWorkloadIdentity(t *testing.T) {
383407
sender.AppendResponse(mocks.NewResponseWithContent(`{"name":"account"}`))
384408
sender.AppendResponse(mocks.NewResponseWithContent(`{"keys":[{"value":"firstKey"}]}`))
385409
sender.AppendResponse(mocks.NewResponseWithContent(`{"keys":[{"value":"firstKey"}]}`))
386-
httpSender := pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc {
387-
return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) {
388-
return pipeline.NewHTTPResponse(mocks.NewResponseWithContent(`{}`)), nil
389-
}
390-
})
391410

392411
d := NewDriver(ctx, config, &listers.StorageListers)
393412
d.authorizer = authorizer
394413
d.sender = sender
395-
d.httpSender = httpSender
414+
d.policies = []policy.Policy{
415+
&testDoer{
416+
statusCode: http.StatusAccepted,
417+
},
418+
}
396419

397420
envvars, err := d.ConfigEnv()
398421
if err != nil {
@@ -1157,7 +1180,7 @@ func Test_containerExists(t *testing.T) {
11571180
}
11581181
}
11591182

1160-
func Test_storageManagementState(t *testing.T) {
1183+
func Test_storageManagementStateNonAzureStackHub(t *testing.T) {
11611184
builder := cirofake.NewFixturesBuilder()
11621185
builder.AddInfraConfig(&configv1.Infrastructure{
11631186
ObjectMeta: metav1.ObjectMeta{
@@ -1181,17 +1204,20 @@ func Test_storageManagementState(t *testing.T) {
11811204
Data: map[string][]byte{
11821205
"azure_subscription_id": []byte("subscription_id"),
11831206
"azure_client_id": []byte("client_id"),
1207+
"azure_tenant_id": []byte(mockTenantID),
11841208
"azure_client_secret": []byte("client_secret"),
11851209
"azure_resourcegroup": []byte("resourcegroup"),
11861210
},
11871211
})
11881212
listers := builder.BuildListers()
1213+
containerNotFoundHeader := http.Header{}
1214+
containerNotFoundHeader.Add("x-ms-error-code", "ContainerNotFound")
11891215

11901216
for _, tt := range []struct {
11911217
name string
11921218
registryConfig *imageregistryv1.Config
11931219
mockResponses []*http.Response
1194-
httpSender func(int) func(_ context.Context, _ pipeline.Request) (pipeline.Response, error)
1220+
policies []policy.Policy
11951221
err string
11961222
checkFn func(*imageregistryv1.Config)
11971223
}{
@@ -1209,6 +1235,11 @@ func Test_storageManagementState(t *testing.T) {
12091235
t.Error("unexpected empty container")
12101236
}
12111237
},
1238+
policies: []policy.Policy{
1239+
&testDoer{
1240+
statusCode: http.StatusCreated,
1241+
},
1242+
},
12121243
},
12131244
{
12141245
name: "user providing container and account name (both already exist)",
@@ -1237,6 +1268,11 @@ func Test_storageManagementState(t *testing.T) {
12371268
t.Errorf("container has changed to %s", cr.Spec.Storage.Azure.Container)
12381269
}
12391270
},
1271+
policies: []policy.Policy{
1272+
&testDoer{
1273+
statusCode: http.StatusOK,
1274+
},
1275+
},
12401276
},
12411277
{
12421278
name: "user providing container and account name (both don't exist)",
@@ -1250,19 +1286,6 @@ func Test_storageManagementState(t *testing.T) {
12501286
},
12511287
},
12521288
},
1253-
httpSender: func(req int) func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1254-
if req == 0 {
1255-
return func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1256-
r := mocks.NewResponseWithStatus("", http.StatusNotFound)
1257-
r.Header = map[string][]string{}
1258-
r.Header.Add("x-ms-error-code", "ContainerNotFound")
1259-
return pipeline.NewHTTPResponse(r), nil
1260-
}
1261-
}
1262-
return func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1263-
return pipeline.NewHTTPResponse(mocks.NewResponseWithContent(`{}`)), nil
1264-
}
1265-
},
12661289
checkFn: func(cr *imageregistryv1.Config) {
12671290
if cr.Spec.Storage.ManagementState != imageregistryv1.StorageManagementStateManaged {
12681291
t.Errorf("expected to be managed, %q instead", cr.Spec.Storage.ManagementState)
@@ -1274,6 +1297,15 @@ func Test_storageManagementState(t *testing.T) {
12741297
t.Errorf("container has changed to %s", cr.Spec.Storage.Azure.Container)
12751298
}
12761299
},
1300+
policies: []policy.Policy{
1301+
&testDoer{
1302+
statusCode: http.StatusNotFound,
1303+
header: containerNotFoundHeader,
1304+
},
1305+
&testDoer{
1306+
statusCode: http.StatusCreated,
1307+
},
1308+
},
12771309
},
12781310
{
12791311
name: "user providing container and account name (only account name exists)",
@@ -1298,23 +1330,19 @@ func Test_storageManagementState(t *testing.T) {
12981330
t.Errorf("container has changed to %s", cr.Spec.Storage.Azure.Container)
12991331
}
13001332
},
1301-
httpSender: func(req int) func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1302-
if req == 0 {
1303-
return func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1304-
r := mocks.NewResponseWithStatus("", http.StatusNotFound)
1305-
r.Header = map[string][]string{}
1306-
r.Header.Add("x-ms-error-code", "ContainerNotFound")
1307-
return pipeline.NewHTTPResponse(r), nil
1308-
}
1309-
}
1310-
return func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1311-
return pipeline.NewHTTPResponse(mocks.NewResponseWithContent(`{}`)), nil
1312-
}
1313-
},
13141333
mockResponses: []*http.Response{
13151334
mocks.NewResponseWithContent(`{"nameAvailable":false}`),
13161335
mocks.NewResponseWithContent(`{"keys":[{"value":"firstKey"}]}`),
13171336
},
1337+
policies: []policy.Policy{
1338+
&testDoer{
1339+
statusCode: http.StatusNotFound,
1340+
header: containerNotFoundHeader,
1341+
},
1342+
&testDoer{
1343+
statusCode: http.StatusCreated,
1344+
},
1345+
},
13181346
},
13191347
{
13201348
name: "do not overwrite management state already set by user",
@@ -1336,6 +1364,9 @@ func Test_storageManagementState(t *testing.T) {
13361364
t.Error("unexpected empty container")
13371365
}
13381366
},
1367+
policies: []policy.Policy{
1368+
&testDoer{statusCode: http.StatusCreated},
1369+
},
13391370
},
13401371
} {
13411372
t.Run(tt.name, func(t *testing.T) {
@@ -1363,23 +1394,9 @@ func Test_storageManagementState(t *testing.T) {
13631394
)
13641395
drv.authorizer = autorest.NullAuthorizer{}
13651396
drv.sender = sender
1366-
1367-
var requestCounter int
1368-
drv.httpSender = pipeline.FactoryFunc(
1369-
func(_ pipeline.Policy, _ *pipeline.PolicyOptions) pipeline.PolicyFunc {
1370-
defer func() {
1371-
requestCounter++
1372-
}()
1373-
1374-
if tt.httpSender != nil {
1375-
return tt.httpSender(requestCounter)
1376-
}
1377-
1378-
return func(_ context.Context, _ pipeline.Request) (pipeline.Response, error) {
1379-
return pipeline.NewHTTPResponse(mocks.NewResponseWithContent(`{}`)), nil
1380-
}
1381-
},
1382-
)
1397+
if tt.policies != nil {
1398+
drv.policies = tt.policies
1399+
}
13831400

13841401
if err := drv.CreateStorage(tt.registryConfig); err != nil {
13851402
if len(tt.err) == 0 {

0 commit comments

Comments
 (0)