Skip to content

Commit 20b7e0e

Browse files
mnenciamnencia
authored
fix(azure): require storage account for explicit credentials (#177)
This change enforces that storage account must be specified when using explicit credentials (storage key or SAS token) and ensures connection string is mutually exclusive with other authentication parameters.
1 parent 1030369 commit 20b7e0e

File tree

2 files changed

+121
-40
lines changed

2 files changed

+121
-40
lines changed

‎pkg/api/config.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,12 @@ type S3Credentials struct {
113113

114114
// AzureCredentials is the type for the credentials to be used to upload
115115
// files to Azure Blob Storage. The connection string contains every needed
116-
// information. If the connection string is not specified, we'll need the
117-
// storage account name and also one (and only one) of:
116+
// information. If the connection string is not specified, one (and only one)
117+
// of the following authentication methods must be specified:
118118
//
119-
// - storageKey
120-
// - storageSasToken
121-
//
122-
// - inheriting the credentials from the pod environment by setting inheritFromAzureAD to true
119+
// - storageKey (requires storageAccount)
120+
// - storageSasToken (requires storageAccount)
121+
// - inheritFromAzureAD (inheriting credentials from the pod environment)
123122
type AzureCredentials struct {
124123
// The connection string to be used
125124
// +optional
@@ -347,10 +346,10 @@ func (azure *AzureCredentials) ValidateAzureCredentials(path *field.Path) field.
347346
path,
348347
azure,
349348
"when connection string is not specified, one and only one of "+
350-
"storage key and storage SAS token is allowed"))
349+
"storage key, storage SAS token, or Azure AD authentication is required"))
351350
}
352351

353-
if secrets != 0 && azure.ConnectionString != nil {
352+
if (secrets != 0 || azure.StorageAccount != nil) && azure.ConnectionString != nil {
354353
allErrors = append(
355354
allErrors,
356355
field.Invalid(
@@ -360,6 +359,16 @@ func (azure *AzureCredentials) ValidateAzureCredentials(path *field.Path) field.
360359
"must be empty"))
361360
}
362361

362+
// StorageAccount is required when using explicit credentials (StorageKey or StorageSasToken)
363+
if (azure.StorageKey != nil || azure.StorageSasToken != nil) && azure.StorageAccount == nil {
364+
allErrors = append(
365+
allErrors,
366+
field.Invalid(
367+
path,
368+
azure,
369+
"storage account must be specified when using storage key or SAS token"))
370+
}
371+
363372
return allErrors
364373
}
365374

‎pkg/api/config_test.go‎

Lines changed: 104 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,46 @@ var _ = Describe("DataBackupConfiguration.AppendAdditionalCommandArgs", func() {
4646
})
4747
})
4848

49-
var _ = Describe("WalBackupConfiguration.AppendAdditionalCommandArgs", func() {
49+
var _ = Describe("WalBackupConfiguration.AppendArchiveAdditionalCommandArgs", func() {
5050
var options []string
51-
var config DataBackupConfiguration
51+
var config WalBackupConfiguration
5252
BeforeEach(func() {
5353
options = []string{"--option1", "--option2"}
54-
config = DataBackupConfiguration{
55-
AdditionalCommandArgs: []string{"--option3", "--option4"},
54+
config = WalBackupConfiguration{
55+
ArchiveAdditionalCommandArgs: []string{"--option3", "--option4"},
5656
}
5757
})
5858

5959
It("should append additional command args to the options", func() {
60-
updatedOptions := config.AppendAdditionalCommandArgs(options)
60+
updatedOptions := config.AppendArchiveAdditionalCommandArgs(options)
6161
Expect(updatedOptions).To(Equal([]string{"--option1", "--option2", "--option3", "--option4"}))
6262
})
6363

6464
It("should return the original options if there are no additional command args", func() {
65-
config.AdditionalCommandArgs = nil
66-
updatedOptions := config.AppendAdditionalCommandArgs(options)
65+
config.ArchiveAdditionalCommandArgs = nil
66+
updatedOptions := config.AppendArchiveAdditionalCommandArgs(options)
67+
Expect(updatedOptions).To(Equal(options))
68+
})
69+
})
70+
71+
var _ = Describe("WalBackupConfiguration.AppendRestoreAdditionalCommandArgs", func() {
72+
var options []string
73+
var config WalBackupConfiguration
74+
BeforeEach(func() {
75+
options = []string{"--option1", "--option2"}
76+
config = WalBackupConfiguration{
77+
RestoreAdditionalCommandArgs: []string{"--option3", "--option4"},
78+
}
79+
})
80+
81+
It("should append additional command args to the options", func() {
82+
updatedOptions := config.AppendRestoreAdditionalCommandArgs(options)
83+
Expect(updatedOptions).To(Equal([]string{"--option1", "--option2", "--option3", "--option4"}))
84+
})
85+
86+
It("should return the original options if there are no additional command args", func() {
87+
config.RestoreAdditionalCommandArgs = nil
88+
updatedOptions := config.AppendRestoreAdditionalCommandArgs(options)
6789
Expect(updatedOptions).To(Equal(options))
6890
})
6991
})
@@ -120,7 +142,7 @@ var _ = Describe("Barman credentials", func() {
120142
var _ = Describe("azure credentials", func() {
121143
path := field.NewPath("spec", "backupConfiguration", "azureCredentials")
122144

123-
It("contain only one of storage account key and SAS token", func() {
145+
It("it is correct when the storage key is specified", func() {
124146
azureCredentials := AzureCredentials{
125147
StorageAccount: &machineryapi.SecretKeySelector{
126148
LocalObjectReference: machineryapi.LocalObjectReference{
@@ -134,93 +156,143 @@ var _ = Describe("azure credentials", func() {
134156
},
135157
Key: "storageKey",
136158
},
159+
StorageSasToken: nil,
160+
}
161+
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
162+
})
163+
164+
It("it is correct when the SAS token is specified", func() {
165+
azureCredentials := AzureCredentials{
166+
StorageAccount: &machineryapi.SecretKeySelector{
167+
LocalObjectReference: machineryapi.LocalObjectReference{
168+
Name: "azure-config",
169+
},
170+
Key: "storageAccount",
171+
},
172+
StorageKey: nil,
137173
StorageSasToken: &machineryapi.SecretKeySelector{
138174
LocalObjectReference: machineryapi.LocalObjectReference{
139175
Name: "azure-config",
140176
},
141177
Key: "sasToken",
142178
},
143179
}
144-
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
180+
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
181+
})
145182

146-
azureCredentials = AzureCredentials{
147-
StorageAccount: &machineryapi.SecretKeySelector{
183+
It("it is correct if only the connection string is specified", func() {
184+
azureCredentials := AzureCredentials{
185+
ConnectionString: &machineryapi.SecretKeySelector{
148186
LocalObjectReference: machineryapi.LocalObjectReference{
149187
Name: "azure-config",
150188
},
151-
Key: "storageAccount",
189+
Key: "connectionString",
152190
},
153-
StorageKey: nil,
154-
StorageSasToken: nil,
155191
}
156-
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
192+
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
157193
})
158194

159-
It("is correct when the storage key is used", func() {
195+
It("it is not correct when the connection string is specified with other parameters", func() {
160196
azureCredentials := AzureCredentials{
197+
ConnectionString: &machineryapi.SecretKeySelector{
198+
LocalObjectReference: machineryapi.LocalObjectReference{
199+
Name: "azure-config",
200+
},
201+
Key: "connectionString",
202+
},
161203
StorageAccount: &machineryapi.SecretKeySelector{
162204
LocalObjectReference: machineryapi.LocalObjectReference{
163205
Name: "azure-config",
164206
},
165207
Key: "storageAccount",
166208
},
209+
}
210+
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
211+
})
212+
213+
It("is not correct when storage key is specified without storage account", func() {
214+
azureCredentials := AzureCredentials{
167215
StorageKey: &machineryapi.SecretKeySelector{
168216
LocalObjectReference: machineryapi.LocalObjectReference{
169217
Name: "azure-config",
170218
},
171219
Key: "storageKey",
172220
},
173-
StorageSasToken: nil,
174221
}
175-
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
222+
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
176223
})
177224

178-
It("is correct when the sas token is used", func() {
225+
It("is not correct when SAS token is specified without storage account", func() {
226+
azureCredentials := AzureCredentials{
227+
StorageSasToken: &machineryapi.SecretKeySelector{
228+
LocalObjectReference: machineryapi.LocalObjectReference{
229+
Name: "azure-config",
230+
},
231+
Key: "sasToken",
232+
},
233+
}
234+
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
235+
})
236+
237+
It("is not correct when both storage key and SAS token are specified", func() {
179238
azureCredentials := AzureCredentials{
180239
StorageAccount: &machineryapi.SecretKeySelector{
181240
LocalObjectReference: machineryapi.LocalObjectReference{
182241
Name: "azure-config",
183242
},
184243
Key: "storageAccount",
185244
},
186-
StorageKey: nil,
245+
StorageKey: &machineryapi.SecretKeySelector{
246+
LocalObjectReference: machineryapi.LocalObjectReference{
247+
Name: "azure-config",
248+
},
249+
Key: "storageKey",
250+
},
187251
StorageSasToken: &machineryapi.SecretKeySelector{
188252
LocalObjectReference: machineryapi.LocalObjectReference{
189253
Name: "azure-config",
190254
},
191255
Key: "sasToken",
192256
},
193257
}
194-
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
195-
})
258+
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
196259

197-
It("is correct even if only the connection string is specified", func() {
198-
azureCredentials := AzureCredentials{
199-
ConnectionString: &machineryapi.SecretKeySelector{
260+
azureCredentials = AzureCredentials{
261+
StorageAccount: &machineryapi.SecretKeySelector{
200262
LocalObjectReference: machineryapi.LocalObjectReference{
201263
Name: "azure-config",
202264
},
203-
Key: "connectionString",
265+
Key: "storageAccount",
204266
},
267+
StorageKey: nil,
268+
StorageSasToken: nil,
269+
}
270+
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
271+
})
272+
273+
It("it is correct when Azure AD authentication is used", func() {
274+
azureCredentials := AzureCredentials{
275+
InheritFromAzureAD: true,
205276
}
206277
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
207278
})
208279

209-
It("it is not correct when the connection string is specified with other parameters", func() {
280+
It("is not correct when Azure AD authentication is combined with other credentials", func() {
210281
azureCredentials := AzureCredentials{
211-
ConnectionString: &machineryapi.SecretKeySelector{
282+
InheritFromAzureAD: true,
283+
StorageAccount: &machineryapi.SecretKeySelector{
212284
LocalObjectReference: machineryapi.LocalObjectReference{
213285
Name: "azure-config",
214286
},
215-
Key: "connectionString",
287+
Key: "storageAccount",
216288
},
217-
StorageAccount: &machineryapi.SecretKeySelector{
289+
StorageKey: &machineryapi.SecretKeySelector{
218290
LocalObjectReference: machineryapi.LocalObjectReference{
219291
Name: "azure-config",
220292
},
221-
Key: "storageAccount",
293+
Key: "storageKey",
222294
},
223295
}
224-
Expect(azureCredentials.ValidateAzureCredentials(path)).To(BeEmpty())
296+
Expect(azureCredentials.ValidateAzureCredentials(path)).ToNot(BeEmpty())
225297
})
226298
})

0 commit comments

Comments
 (0)