Skip to content

Commit 1ac90a4

Browse files
authored
Implement OAuth in the modern test framework (#1410)
* Configure Azure Pipelines for OAuth on E2E * Handle separate source/destination request lists * Reduce surface area of oauth testing * Update function signatures * Fix panic * Set OAuth info * Fix auth for remove * Fix resume support * Fix objectTarget * Fix CopyToWrongBlobType * Fix StripTopDir * Reduce scale of TestResume_LargeGeneric * Add CPK vars * Add HNS key * Fix test definitions * Add E2E classic account key * Resolve Mohit's comments
1 parent 6e2c8e7 commit 1ac90a4

21 files changed

+436
-182
lines changed

azure-pipelines.yml

+10-1
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,15 @@ jobs:
101101
set -e
102102
GOARCH=amd64 GOOS=linux go build -o azcopy_linux_amd64
103103
export AZCOPY_E2E_EXECUTABLE_PATH=$(pwd)/azcopy_linux_amd64
104-
go test -timeout 20m -race -short -cover ./e2etest
104+
go test -timeout 30m -race -short -cover ./e2etest
105105
env:
106106
AZCOPY_E2E_ACCOUNT_KEY: $(AZCOPY_E2E_ACCOUNT_KEY)
107107
AZCOPY_E2E_ACCOUNT_NAME: $(AZCOPY_E2E_ACCOUNT_NAME)
108108
AZCOPY_E2E_ACCOUNT_KEY_HNS: $(AZCOPY_E2E_ACCOUNT_KEY_HNS)
109109
AZCOPY_E2E_ACCOUNT_NAME_HNS: $(AZCOPY_E2E_ACCOUNT_NAME_HNS)
110+
AZCOPY_E2E_TENANT_ID: $(OAUTH_TENANT_ID)
111+
AZCOPY_E2E_APPLICATION_ID: $(ACTIVE_DIRECTORY_APPLICATION_ID)
112+
AZCOPY_E2E_CLIENT_SECRET: $(AZCOPY_SPA_CLIENT_SECRET)
110113
AZCOPY_E2E_CLASSIC_ACCOUNT_NAME: $(AZCOPY_E2E_CLASSIC_ACCOUNT_NAME)
111114
AZCOPY_E2E_CLASSIC_ACCOUNT_KEY: $(AZCOPY_E2E_CLASSIC_ACCOUNT_KEY)
112115
CPK_ENCRYPTION_KEY: $(CPK_ENCRYPTION_KEY)
@@ -125,6 +128,9 @@ jobs:
125128
AZCOPY_E2E_ACCOUNT_NAME: $(AZCOPY_E2E_ACCOUNT_NAME)
126129
AZCOPY_E2E_ACCOUNT_KEY_HNS: $(AZCOPY_E2E_ACCOUNT_KEY_HNS)
127130
AZCOPY_E2E_ACCOUNT_NAME_HNS: $(AZCOPY_E2E_ACCOUNT_NAME_HNS)
131+
AZCOPY_E2E_TENANT_ID: $(OAUTH_TENANT_ID)
132+
AZCOPY_E2E_APPLICATION_ID: $(ACTIVE_DIRECTORY_APPLICATION_ID)
133+
AZCOPY_E2E_CLIENT_SECRET: $(AZCOPY_SPA_CLIENT_SECRET)
128134
AZCOPY_E2E_CLASSIC_ACCOUNT_NAME: $(AZCOPY_E2E_CLASSIC_ACCOUNT_NAME)
129135
AZCOPY_E2E_CLASSIC_ACCOUNT_KEY: $(AZCOPY_E2E_CLASSIC_ACCOUNT_KEY)
130136
CPK_ENCRYPTION_KEY: $(CPK_ENCRYPTION_KEY)
@@ -145,6 +151,9 @@ jobs:
145151
AZCOPY_E2E_ACCOUNT_NAME: $(AZCOPY_E2E_ACCOUNT_NAME)
146152
AZCOPY_E2E_ACCOUNT_KEY_HNS: $(AZCOPY_E2E_ACCOUNT_KEY_HNS)
147153
AZCOPY_E2E_ACCOUNT_NAME_HNS: $(AZCOPY_E2E_ACCOUNT_NAME_HNS)
154+
AZCOPY_E2E_TENANT_ID: $(OAUTH_TENANT_ID)
155+
AZCOPY_E2E_APPLICATION_ID: $(ACTIVE_DIRECTORY_APPLICATION_ID)
156+
AZCOPY_E2E_CLIENT_SECRET: $(AZCOPY_SPA_CLIENT_SECRET)
148157
AZCOPY_E2E_CLASSIC_ACCOUNT_NAME: $(AZCOPY_E2E_CLASSIC_ACCOUNT_NAME)
149158
AZCOPY_E2E_CLASSIC_ACCOUNT_KEY: $(AZCOPY_E2E_CLASSIC_ACCOUNT_KEY)
150159
CPK_ENCRYPTION_KEY: $(CPK_ENCRYPTION_KEY)

e2etest/config.go

+12
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ import (
3434
// the general guidance is to take in as few parameters as possible
3535
type GlobalInputManager struct{}
3636

37+
func (GlobalInputManager) GetServicePrincipalAuth() (tenantID string, applicationID string, clientSecret string) {
38+
tenantID = os.Getenv("AZCOPY_E2E_TENANT_ID")
39+
applicationID = os.Getenv("AZCOPY_E2E_APPLICATION_ID")
40+
clientSecret = os.Getenv("AZCOPY_E2E_CLIENT_SECRET")
41+
42+
if applicationID == "" || clientSecret == "" {
43+
panic("Insufficient information was supplied for service principal authentication")
44+
}
45+
46+
return
47+
}
48+
3749
func (GlobalInputManager) GetAccountAndKey(accountType AccountType) (string, string) {
3850
var name, key string
3951

e2etest/declarativeRunner.go

+129-52
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,84 @@ import (
3333
// In particular, it lets one test cover a range of different source/dest types, and even cover both sync and copy.
3434
// See first test in zt_enumeration for an annotated example.
3535

36+
var validCredTypesPerLocation = map[common.Location][]common.CredentialType{
37+
common.ELocation.Unknown(): {common.ECredentialType.Unknown(), common.ECredentialType.Anonymous(), common.ECredentialType.OAuthToken()}, // Delete!
38+
common.ELocation.File(): {common.ECredentialType.Anonymous()},
39+
common.ELocation.Blob(): {common.ECredentialType.Anonymous(), common.ECredentialType.OAuthToken()},
40+
common.ELocation.BlobFS(): {common.ECredentialType.Anonymous(), common.ECredentialType.OAuthToken()}, // todo: currently, account key auth isn't even supported in e2e tests.
41+
common.ELocation.Local(): {common.ECredentialType.Anonymous()},
42+
common.ELocation.Pipe(): {common.ECredentialType.Anonymous()},
43+
common.ELocation.S3(): {common.ECredentialType.S3AccessKey()},
44+
common.ELocation.GCP(): {common.ECredentialType.GoogleAppCredentials()},
45+
}
46+
47+
var allCredentialTypes []common.CredentialType = nil
48+
var anonymousAuthOnly = []common.CredentialType{common.ECredentialType.Anonymous()}
49+
50+
func getValidCredCombinationsForFromTo(fromTo common.FromTo, requestedCredentialTypesSrc, requestedCredentialTypesDst []common.CredentialType) [][2]common.CredentialType {
51+
output := make([][2]common.CredentialType, 0)
52+
53+
credIsRequested := func(cType common.CredentialType, dst bool) bool {
54+
if (dst && requestedCredentialTypesDst == nil) || (!dst && requestedCredentialTypesSrc == nil) {
55+
return true
56+
}
57+
58+
toSearch := requestedCredentialTypesSrc
59+
if dst {
60+
toSearch = requestedCredentialTypesDst
61+
}
62+
63+
for _, v := range toSearch {
64+
if v == cType {
65+
return true
66+
}
67+
}
68+
69+
return false
70+
}
71+
72+
// determine source types
73+
var sourceTypes []common.CredentialType
74+
if fromTo.IsS2S() {
75+
// source must always be anonymous-- no exceptions until OAuth over S2S is introduced.
76+
sourceTypes = []common.CredentialType{common.ECredentialType.Anonymous()}
77+
} else {
78+
sourceTypes = validCredTypesPerLocation[fromTo.From()]
79+
}
80+
81+
for _, srcCredType := range sourceTypes {
82+
for _, dstCredType := range validCredTypesPerLocation[fromTo.To()] {
83+
// make sure the user asked for this.
84+
if !(credIsRequested(srcCredType, false) && credIsRequested(dstCredType, true)) {
85+
continue
86+
}
87+
88+
output = append(output, [2]common.CredentialType{srcCredType, dstCredType})
89+
}
90+
}
91+
92+
return output
93+
}
94+
3695
// RunScenarios is the key entry point for declarative testing.
3796
// It constructs and executes scenarios (subtest in Go-speak), according to its parameters, and checks their results
3897
func RunScenarios(
3998
t *testing.T,
4099
operations Operation,
41100
testFromTo TestFromTo,
42101
validate Validate, // TODO: do we really want the test author to have to nominate which validation should happen? Pros: better perf of tests. Cons: they have to tell us, and if they tell us wrong test may not test what they think it tests
43-
/*_ interface{}, // TODO if we want it??, blockBLobsOnly or specifc/all blob types
44-
_ interface{}, // TODO if we want it??, default auth type only, or specific/all auth types*/
102+
// _ interface{}, // TODO if we want it??, blockBLobsOnly or specifc/all blob types
103+
104+
// It would be a pain to list out every combo by hand,
105+
// In addition to the fact that not every credential type is sensible.
106+
// Thus, the E2E framework takes in a requested set of credential types, and applies them where sensible.
107+
// This allows you to make tests use OAuth only, SAS only, etc.
108+
requestedCredentialTypesSrc []common.CredentialType,
109+
requestedCredentialTypesDst []common.CredentialType,
45110
p params,
46111
hs *hooks,
47112
fs testFiles,
48-
// TODO: do we need something here to explicitly say that we expect success or failure? For now, we are just inferring that from the elements of sourceFiles
113+
// TODO: do we need something here to explicitly say that we expect success or failure? For now, we are just inferring that from the elements of sourceFiles
49114
destAccountType AccountType,
50115
accountType AccountType,
51116
scenarioSuffix string) {
@@ -58,7 +123,7 @@ func RunScenarios(
58123
}
59124

60125
// construct all the scenarios
61-
scenarios := make([]scenario, 0, 16)
126+
scenarios := make([]scenario, 0)
62127
for _, op := range operations.getValues() {
63128
if op == eOperation.Resume() {
64129
continue
@@ -73,44 +138,50 @@ func RunScenarios(
73138
}
74139
seenFromTos[fromTo] = true
75140

76-
// Create unique name for generating container names
77-
compactScenarioName := fmt.Sprintf("%.4s-%s-%c-%c%c", suiteName, testName, op.String()[0], fromTo.From().String()[0], fromTo.To().String()[0])
78-
fullScenarioName := fmt.Sprintf("%s.%s.%s-%s", suiteName, testName, op.String(), fromTo.String())
141+
credentialTypes := getValidCredCombinationsForFromTo(fromTo, requestedCredentialTypesSrc, requestedCredentialTypesDst)
79142

80-
// Sub-test name is not globally unique (it doesn't need to be) but it is more human-readable
81-
subtestName := fmt.Sprintf("%s-%s", op, fromTo)
82-
if scenarioSuffix != "" {
83-
subtestName += "-" + scenarioSuffix
84-
}
143+
for _, credTypes := range credentialTypes {
144+
// Create unique name for generating container names
145+
compactScenarioName := fmt.Sprintf("%.4s-%s-%c-%c%c", suiteName, testName, op.String()[0], fromTo.From().String()[0], fromTo.To().String()[0])
146+
fullScenarioName := fmt.Sprintf("%s.%s.%s-%s", suiteName, testName, op.String(), fromTo.String())
147+
// Sub-test name is not globally unique (it doesn't need to be) but it is more human-readable
148+
subtestName := fmt.Sprintf("%s-%s", op, fromTo)
85149

86-
hsToUse := hooks{}
87-
if hs != nil {
88-
hsToUse = *hs
89-
}
150+
hsToUse := hooks{}
151+
if hs != nil {
152+
hsToUse = *hs
153+
}
90154

91-
// Handles destination being different account type
92-
isSourceAcc := true
93-
if destAccountType != accountType {
94-
isSourceAcc = false
95-
}
96-
s := scenario{
97-
srcAccountType: accountType,
98-
destAccountType: destAccountType,
99-
subtestName: subtestName,
100-
compactScenarioName: compactScenarioName,
101-
fullScenarioName: fullScenarioName,
102-
operation: op,
103-
fromTo: fromTo,
104-
validate: validate,
105-
p: p, // copies them, because they are a struct. This is what we need, since they may be morphed while running
106-
hs: hsToUse,
107-
fs: fs.DeepCopy(),
108-
needResume: operations&eOperation.Resume() != 0,
109-
stripTopDir: p.stripTopDir,
110-
isSourceAcc: isSourceAcc,
111-
}
155+
if scenarioSuffix != "" {
156+
subtestName += "-" + scenarioSuffix
157+
}
112158

113-
scenarios = append(scenarios, s)
159+
// Handles destination being different account type
160+
isSourceAcc := true
161+
if destAccountType != accountType {
162+
isSourceAcc = false
163+
}
164+
165+
s := scenario{
166+
srcAccountType: accountType,
167+
destAccountType: destAccountType,
168+
subtestName: subtestName,
169+
compactScenarioName: compactScenarioName,
170+
fullScenarioName: fullScenarioName,
171+
operation: op,
172+
fromTo: fromTo,
173+
credTypes: credTypes,
174+
validate: validate,
175+
p: p, // copies them, because they are a struct. This is what we need, since they may be morphed while running
176+
hs: hsToUse,
177+
fs: fs.DeepCopy(),
178+
needResume: operations&eOperation.Resume() != 0,
179+
stripTopDir: p.stripTopDir,
180+
isSourceAcc: isSourceAcc,
181+
}
182+
183+
scenarios = append(scenarios, s)
184+
}
114185
}
115186
}
116187

@@ -122,22 +193,28 @@ func RunScenarios(
122193
// run them in parallel if not debugging, but sequentially (for easier debugging) if a debugger is attached
123194
parallel := !isLaunchedByDebugger && !p.disableParallelTesting // this only works if gops.exe is on your path. See azcopyDebugHelper.go for instructions.
124195
for _, s := range scenarios {
125-
sen := s // capture to separate var inside the loop, for the parallel case
126196
// use t.Run to get proper sub-test support
127197
t.Run(s.subtestName, func(t *testing.T) {
128-
if parallel {
129-
t.Parallel() // tell testing that it can run stuff in parallel with us
130-
}
131-
// set asserter now (and only now), since before this point we don't have the right "t"
132-
sen.a = &testingAsserter{
133-
t: t,
134-
fullScenarioName: sen.fullScenarioName,
135-
compactScenarioName: sen.compactScenarioName,
136-
}
137-
if hs != nil {
138-
sen.runHook(hs.beforeTestRun)
139-
}
140-
sen.Run()
198+
sen := s // capture to separate var inside the loop, for the parallel case
199+
credNames := fmt.Sprintf("%s-%s", s.credTypes[0].String(), s.credTypes[1].String())
200+
201+
t.Run(credNames, func(t *testing.T) {
202+
if parallel {
203+
t.Parallel() // tell testing that it can run stuff in parallel with us
204+
}
205+
// set asserter now (and only now), since before this point we don't have the right "t"
206+
sen.a = &testingAsserter{
207+
t: t,
208+
fullScenarioName: sen.fullScenarioName,
209+
compactScenarioName: sen.compactScenarioName,
210+
}
211+
212+
if hs != nil {
213+
sen.runHook(hs.beforeTestRun)
214+
}
215+
216+
sen.Run()
217+
})
141218
})
142219
}
143220
}

e2etest/declarativeScenario.go

+19-10
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ type scenario struct {
4646
operation Operation
4747
validate Validate
4848
fromTo common.FromTo
49+
credTypes [2]common.CredentialType
4950
p params
5051
hs hooks
5152
fs testFiles
@@ -125,7 +126,7 @@ func (s *scenario) Run() {
125126

126127
func (s *scenario) runHook(h hookFunc) bool {
127128
if h == nil {
128-
return true //nothing to do. So "successful"
129+
return true // nothing to do. So "successful"
129130
}
130131

131132
// run the hook, passing ourself in as the implementation of hookHelper interface
@@ -186,21 +187,28 @@ func (s *scenario) runAzCopy() {
186187
}
187188
}
188189

189-
// run AzCopy
190-
const useSas = true // TODO: support other auth options (see params of RunTest)
191190
tf := s.GetTestFiles()
192-
var srcUseSas = tf.sourcePublic == azblob.PublicAccessNone
191+
// run AzCopy
193192
result, wasClean, err := r.ExecuteAzCopyCommand(
194193
s.operation,
195-
s.state.source.getParam(s.stripTopDir, srcUseSas, tf.objectTarget),
196-
// Prefer the destination target over the object target itself.
197-
s.state.dest.getParam(false, useSas, common.IffString(tf.destTarget != "", tf.destTarget, tf.objectTarget)),
194+
s.state.source.getParam(s.stripTopDir, s.credTypes[0] == common.ECredentialType.Anonymous(), tf.objectTarget),
195+
s.state.dest.getParam(false, s.credTypes[1] == common.ECredentialType.Anonymous(), common.IffString(tf.destTarget != "", tf.destTarget, tf.objectTarget)),
196+
s.credTypes[0] == common.ECredentialType.OAuthToken() || s.credTypes[1] == common.ECredentialType.OAuthToken(), // needsOAuth
198197
afterStart, s.chToStdin)
199198

200199
if !wasClean {
201200
s.a.AssertNoErr(err, "running AzCopy")
202201
}
203202

203+
// Generally, a cancellation is done when auth fails.
204+
if result.finalStatus.JobStatus == common.EJobStatus.Cancelled() {
205+
for _, v := range result.finalStatus.FailedTransfers {
206+
if v.ErrorCode == 403 {
207+
s.a.Error("authorization failed, perhaps SPN auth or the SAS token is bad?")
208+
}
209+
}
210+
}
211+
204212
s.state.result = &result
205213
}
206214

@@ -232,6 +240,7 @@ func (s *scenario) resumeAzCopy() {
232240
eOperation.Resume(),
233241
s.state.result.jobID.String(),
234242
"",
243+
false,
235244
afterStart,
236245
s.chToStdin,
237246
)
@@ -429,7 +438,7 @@ func (s *scenario) validateContent() {
429438
}
430439
}
431440

432-
//// Individual property validation routines
441+
// // Individual property validation routines
433442

434443
func (s *scenario) validateMetadata(expected, actual map[string]string, isFolder bool) {
435444
if isFolder { // hdi_isfolder is service-relevant metadata, not something we'd be testing for. This can pop up when specifying a folder() on blob.
@@ -551,7 +560,7 @@ func (s *scenario) validateLastWriteTime(expected, actual *time.Time) {
551560
expected, actual))
552561
}
553562

554-
//nolint
563+
// nolint
555564
func (s *scenario) validateSMBAttrs(expected, actual *uint32) {
556565
if expected == nil {
557566
// These properties were not explicitly stated for verification
@@ -575,7 +584,7 @@ func (s *scenario) cleanup() {
575584
}
576585
}
577586

578-
/// support the hookHelper functions. These are use by our hooks to modify the state, or resources, of the running test
587+
// / support the hookHelper functions. These are use by our hooks to modify the state, or resources, of the running test
579588

580589
func (s *scenario) FromTo() common.FromTo {
581590
return s.fromTo

e2etest/factory.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ func (TestResourceFactory) CreateNewContainer(c asserter, publicAccess azblob.Pu
159159
name = TestResourceNameGenerator{}.GenerateContainerName(c)
160160
container = TestResourceFactory{}.GetBlobServiceURL(accountType).NewContainerURL(name)
161161

162+
162163
cResp, err := container.Create(context.Background(), nil, publicAccess)
163164
c.AssertNoErr(err)
164165
c.Assert(cResp.StatusCode(), equals(), 201)
@@ -252,18 +253,18 @@ func generateName(c asserter, prefix string, maxLen int) string {
252253
name := c.CompactScenarioName() // don't want to just use test name here, because each test contains multiple scenarios with the declarative runner
253254

254255
textualPortion := fmt.Sprintf("%s-%s", prefix, strings.ToLower(name))
255-
currentTime := time.Now()
256-
numericSuffix := fmt.Sprintf("%02d%02d%d", currentTime.Minute(), currentTime.Second(), currentTime.Nanosecond())
256+
// GUIDs are less prone to overlap than times.
257+
guidSuffix := uuid.New().String()
257258
if maxLen > 0 {
258-
maxTextLen := maxLen - len(numericSuffix)
259+
maxTextLen := maxLen - len(guidSuffix)
259260
if maxTextLen < 1 {
260261
panic("max len too short")
261262
}
262263
if len(textualPortion) > maxTextLen {
263264
textualPortion = textualPortion[:maxTextLen]
264265
}
265266
}
266-
name = textualPortion + numericSuffix
267+
name = textualPortion + guidSuffix
267268
return name
268269
}
269270

0 commit comments

Comments
 (0)