Skip to content

Commit b3ed62a

Browse files
linglingye001linglingye001
authored
Fix nil pointer when empty secret refresh (#79)
* fix nil pointer when not loaded KVR * rename secretReference
1 parent 52ccc90 commit b3ed62a

8 files changed

+196
-198
lines changed

‎internal/controller/appconfigurationprovider_controller.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ type ReconciliationState struct {
5454
SentinelETags map[acpv1.Sentinel]*azcore.ETag
5555
KeyValueETags map[acpv1.Selector][]*azcore.ETag
5656
FeatureFlagETags map[acpv1.Selector][]*azcore.ETag
57-
ExistingSecretReferences map[string]*loader.TargetSecretReference
57+
ExistingK8sSecrets map[string]*loader.TargetK8sSecretMetadata
5858
NextKeyValueRefreshReconcileTime metav1.Time
5959
NextSecretReferenceRefreshReconcileTime metav1.Time
6060
NextFeatureFlagRefreshReconcileTime metav1.Time
@@ -158,7 +158,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
158158
}
159159

160160
if reconciler.ProvidersReconcileState[req.NamespacedName] != nil {
161-
for name := range reconciler.ProvidersReconcileState[req.NamespacedName].ExistingSecretReferences {
161+
for name := range reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets {
162162
if _, ok := existingSecrets[name]; !ok {
163163
existingSecret = corev1.Secret{
164164
ObjectMeta: metav1.ObjectMeta{
@@ -183,7 +183,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
183183
SentinelETags: make(map[acpv1.Sentinel]*azcore.ETag),
184184
KeyValueETags: make(map[acpv1.Selector][]*azcore.ETag),
185185
FeatureFlagETags: make(map[acpv1.Selector][]*azcore.ETag),
186-
ExistingSecretReferences: make(map[string]*loader.TargetSecretReference),
186+
ExistingK8sSecrets: make(map[string]*loader.TargetK8sSecretMetadata),
187187
ClientManager: nil,
188188
}
189189
}
@@ -194,11 +194,11 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
194194
}
195195

196196
if provider.Spec.Secret == nil {
197-
reconciler.ProvidersReconcileState[req.NamespacedName].ExistingSecretReferences = make(map[string]*loader.TargetSecretReference)
197+
reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets = make(map[string]*loader.TargetK8sSecretMetadata)
198198
} else {
199-
for name := range reconciler.ProvidersReconcileState[req.NamespacedName].ExistingSecretReferences {
199+
for name := range reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets {
200200
if _, ok := existingSecrets[name]; !ok {
201-
reconciler.ProvidersReconcileState[req.NamespacedName].ExistingSecretReferences[name].SecretResourceVersion = ""
201+
reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets[name].SecretResourceVersion = ""
202202
}
203203
}
204204
}
@@ -281,7 +281,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
281281

282282
// Expel the secrets which are no longer selected by the provider.
283283
if provider.Spec.Secret == nil || processor.RefreshOptions.SecretSettingPopulated {
284-
result, err := reconciler.expelRemovedSecrets(ctx, provider, existingSecrets, processor.Settings.SecretReferences)
284+
result, err := reconciler.expelRemovedSecrets(ctx, provider, existingSecrets, processor.Settings.K8sSecrets)
285285
if err != nil {
286286
return result, nil
287287
}
@@ -332,7 +332,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) logAndSetFailStatus(
332332
} else if reconcileState != nil &&
333333
reconcileState.ConfigMapResourceVersion != nil &&
334334
(provider.Spec.Secret == nil ||
335-
len(reconcileState.ExistingSecretReferences) == 0) {
335+
len(reconcileState.ExistingK8sSecrets) == 0) {
336336
// If the target ConfigMap or Secret does exists, just show error as warning.
337337
showErrorAsWarning = true
338338
}
@@ -431,8 +431,8 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateSecrets
431431

432432
for secretName, secret := range processor.Settings.SecretSettings {
433433
if !shouldCreateOrUpdate(processor, secretName) {
434-
if _, ok := reconciler.ProvidersReconcileState[namespacedName].ExistingSecretReferences[secretName]; ok {
435-
processor.Settings.SecretReferences[secretName].SecretResourceVersion = reconciler.ProvidersReconcileState[namespacedName].ExistingSecretReferences[secretName].SecretResourceVersion
434+
if _, ok := reconciler.ProvidersReconcileState[namespacedName].ExistingK8sSecrets[secretName]; ok {
435+
processor.Settings.K8sSecrets[secretName].SecretResourceVersion = reconciler.ProvidersReconcileState[namespacedName].ExistingK8sSecrets[secretName].SecretResourceVersion
436436
}
437437
klog.V(5).Infof("Skip updating the secret %q in %q namespace since data is not changed", secretName, provider.Namespace)
438438
continue
@@ -465,7 +465,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateSecrets
465465
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, err
466466
}
467467

468-
processor.Settings.SecretReferences[secretName].SecretResourceVersion = secretObj.ResourceVersion
468+
processor.Settings.K8sSecrets[secretName].SecretResourceVersion = secretObj.ResourceVersion
469469
klog.V(5).Infof("Secret %q in %q namespace is %s", secretObj.Name, secretObj.Namespace, string(operationResult))
470470
}
471471

@@ -476,7 +476,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) expelRemovedSecrets(
476476
ctx context.Context,
477477
provider *acpv1.AzureAppConfigurationProvider,
478478
existingSecrets map[string]corev1.Secret,
479-
secretReferences map[string]*loader.TargetSecretReference) (reconcile.Result, error) {
479+
secretReferences map[string]*loader.TargetK8sSecretMetadata) (reconcile.Result, error) {
480480
for name := range existingSecrets {
481481
if _, ok := secretReferences[name]; !ok {
482482
err := reconciler.Client.Delete(ctx, &corev1.Secret{

‎internal/controller/appconfigurationprovider_controller_test.go‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,14 @@ var _ = Describe("AppConfiguationProvider controller", func() {
176176
Type: corev1.SecretTypeOpaque,
177177
},
178178
},
179-
SecretReferences: map[string]*loader.TargetSecretReference{
179+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
180180
secretName: {
181-
Type: corev1.SecretTypeTLS,
182-
SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata),
181+
Type: corev1.SecretTypeTLS,
182+
SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
183183
},
184184
secretName2: {
185-
Type: corev1.SecretTypeOpaque,
186-
SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata),
185+
Type: corev1.SecretTypeOpaque,
186+
SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
187187
},
188188
},
189189
}
@@ -262,10 +262,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
262262
Type: corev1.SecretTypeOpaque,
263263
},
264264
},
265-
SecretReferences: map[string]*loader.TargetSecretReference{
265+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
266266
secretName: {
267-
Type: corev1.SecretTypeOpaque,
268-
SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata),
267+
Type: corev1.SecretTypeOpaque,
268+
SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
269269
},
270270
},
271271
}
@@ -336,10 +336,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
336336
},
337337
},
338338
ConfigMapSettings: configMapResult,
339-
SecretReferences: map[string]*loader.TargetSecretReference{
339+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
340340
secretName: {
341-
Type: corev1.SecretType("Opaque"),
342-
SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata),
341+
Type: corev1.SecretType("Opaque"),
342+
SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
343343
},
344344
},
345345
}
@@ -1121,10 +1121,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
11211121
},
11221122
},
11231123
ConfigMapSettings: configMapResult,
1124-
SecretReferences: map[string]*loader.TargetSecretReference{
1124+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
11251125
secretName: {
1126-
Type: corev1.SecretType("Opaque"),
1127-
SecretsMetadata: secretMetadata,
1126+
Type: corev1.SecretType("Opaque"),
1127+
SecretsKeyVaultMetadata: secretMetadata,
11281128
},
11291129
},
11301130
}
@@ -1244,10 +1244,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
12441244
},
12451245
},
12461246
ConfigMapSettings: configMapResult,
1247-
SecretReferences: map[string]*loader.TargetSecretReference{
1247+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
12481248
secretName: {
1249-
Type: corev1.SecretType("Opaque"),
1250-
SecretsMetadata: secretMetadata,
1249+
Type: corev1.SecretType("Opaque"),
1250+
SecretsKeyVaultMetadata: secretMetadata,
12511251
},
12521252
},
12531253
}
@@ -1370,10 +1370,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
13701370
},
13711371
},
13721372
ConfigMapSettings: configMapResult,
1373-
SecretReferences: map[string]*loader.TargetSecretReference{
1373+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
13741374
secretName: {
1375-
Type: corev1.SecretType("Opaque"),
1376-
SecretsMetadata: secretMetadata,
1375+
Type: corev1.SecretType("Opaque"),
1376+
SecretsKeyVaultMetadata: secretMetadata,
13771377
},
13781378
},
13791379
}
@@ -1454,15 +1454,15 @@ var _ = Describe("AppConfiguationProvider controller", func() {
14541454
newSecretMetadata["testSecretKey"] = loader.KeyVaultSecretMetadata{
14551455
SecretId: &newFakeId,
14561456
}
1457-
mockedSecretReference := make(map[string]*loader.TargetSecretReference)
1458-
mockedSecretReference[secretName] = &loader.TargetSecretReference{
1459-
Type: corev1.SecretType("Opaque"),
1460-
SecretsMetadata: newSecretMetadata,
1457+
mockedSecretReference := make(map[string]*loader.TargetK8sSecretMetadata)
1458+
mockedSecretReference[secretName] = &loader.TargetK8sSecretMetadata{
1459+
Type: corev1.SecretType("Opaque"),
1460+
SecretsKeyVaultMetadata: newSecretMetadata,
14611461
}
14621462

14631463
newTargetSettings := &loader.TargetKeyValueSettings{
1464-
SecretSettings: newResolvedSecret,
1465-
SecretReferences: mockedSecretReference,
1464+
SecretSettings: newResolvedSecret,
1465+
K8sSecrets: mockedSecretReference,
14661466
}
14671467

14681468
mockConfigurationSettings.EXPECT().ResolveSecretReferences(gomock.Any(), gomock.Any(), gomock.Any()).Return(newTargetSettings, nil)
@@ -1480,8 +1480,8 @@ var _ = Describe("AppConfiguationProvider controller", func() {
14801480

14811481
// Mocked secret refresh scenario when secretMetadata is not changed
14821482
newTargetSettings2 := &loader.TargetKeyValueSettings{
1483-
SecretSettings: make(map[string]corev1.Secret),
1484-
SecretReferences: mockedSecretReference,
1483+
SecretSettings: make(map[string]corev1.Secret),
1484+
K8sSecrets: mockedSecretReference,
14851485
}
14861486

14871487
mockConfigurationSettings.EXPECT().ResolveSecretReferences(gomock.Any(), gomock.Any(), gomock.Any()).Return(newTargetSettings2, nil)
@@ -1810,10 +1810,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
18101810
},
18111811
},
18121812
ConfigMapSettings: mapResult,
1813-
SecretReferences: map[string]*loader.TargetSecretReference{
1813+
K8sSecrets: map[string]*loader.TargetK8sSecretMetadata{
18141814
secretName: {
1815-
Type: corev1.SecretType("Opaque"),
1816-
SecretsMetadata: secretMetadata,
1815+
Type: corev1.SecretType("Opaque"),
1816+
SecretsKeyVaultMetadata: secretMetadata,
18171817
},
18181818
},
18191819
}
@@ -1939,15 +1939,15 @@ var _ = Describe("AppConfiguationProvider controller", func() {
19391939
newSecretMetadata["testSecretKey"] = loader.KeyVaultSecretMetadata{
19401940
SecretId: &newFakeId,
19411941
}
1942-
mockedSecretReference := make(map[string]*loader.TargetSecretReference)
1943-
mockedSecretReference[secretName] = &loader.TargetSecretReference{
1944-
Type: corev1.SecretType("Opaque"),
1945-
SecretsMetadata: newSecretMetadata,
1942+
mockedSecretReference := make(map[string]*loader.TargetK8sSecretMetadata)
1943+
mockedSecretReference[secretName] = &loader.TargetK8sSecretMetadata{
1944+
Type: corev1.SecretType("Opaque"),
1945+
SecretsKeyVaultMetadata: newSecretMetadata,
19461946
}
19471947

19481948
newTargetSettings := &loader.TargetKeyValueSettings{
1949-
SecretSettings: newResolvedSecret,
1950-
SecretReferences: mockedSecretReference,
1949+
SecretSettings: newResolvedSecret,
1950+
K8sSecrets: mockedSecretReference,
19511951
}
19521952

19531953
mockConfigurationSettings.EXPECT().ResolveSecretReferences(gomock.Any(), gomock.Any(), gomock.Any()).Return(newTargetSettings, nil)

‎internal/controller/processor.go‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -226,27 +226,25 @@ func (processor *AppConfigurationProviderProcessor) processSecretReferenceRefres
226226
}
227227

228228
// Only resolve the secret references that not specified the secret version
229-
secretReferencesToSolve := make(map[string]*loader.TargetSecretReference)
230-
copiedSecretReferences := make(map[string]*loader.TargetSecretReference)
231-
for secretName, reference := range reconcileState.ExistingSecretReferences {
232-
for key, secretMetadata := range reference.SecretsMetadata {
229+
secretReferencesToSolve := make(map[string]*loader.TargetK8sSecretMetadata)
230+
copiedSecretReferences := make(map[string]*loader.TargetK8sSecretMetadata)
231+
for secretName, k8sSecret := range reconcileState.ExistingK8sSecrets {
232+
copiedSecretReferences[secretName] = &loader.TargetK8sSecretMetadata{
233+
Type: k8sSecret.Type,
234+
SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
235+
}
236+
237+
for key, secretMetadata := range k8sSecret.SecretsKeyVaultMetadata {
233238
if secretMetadata.SecretVersion == "" {
234239
if secretReferencesToSolve[secretName] == nil {
235-
secretReferencesToSolve[secretName] = &loader.TargetSecretReference{
236-
Type: reference.Type,
237-
SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata),
240+
secretReferencesToSolve[secretName] = &loader.TargetK8sSecretMetadata{
241+
Type: k8sSecret.Type,
242+
SecretsKeyVaultMetadata: make(map[string]loader.KeyVaultSecretMetadata),
238243
}
239244
}
240-
secretReferencesToSolve[secretName].SecretsMetadata[key] = secretMetadata
241-
}
242-
243-
if copiedSecretReferences[secretName] == nil {
244-
copiedSecretReferences[secretName] = &loader.TargetSecretReference{
245-
Type: reference.Type,
246-
SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata),
247-
}
245+
secretReferencesToSolve[secretName].SecretsKeyVaultMetadata[key] = secretMetadata
248246
}
249-
copiedSecretReferences[secretName].SecretsMetadata[key] = secretMetadata
247+
copiedSecretReferences[secretName].SecretsKeyVaultMetadata[key] = secretMetadata
250248
}
251249
}
252250

@@ -262,12 +260,12 @@ func (processor *AppConfigurationProviderProcessor) processSecretReferenceRefres
262260
}
263261
}
264262

265-
for secretName, reference := range resolvedSecrets.SecretReferences {
266-
maps.Copy(copiedSecretReferences[secretName].SecretsMetadata, reference.SecretsMetadata)
263+
for secretName, k8sSecret := range resolvedSecrets.K8sSecrets {
264+
maps.Copy(copiedSecretReferences[secretName].SecretsKeyVaultMetadata, k8sSecret.SecretsKeyVaultMetadata)
267265
}
268266

269267
processor.Settings.SecretSettings = existingSecrets
270-
processor.Settings.SecretReferences = copiedSecretReferences
268+
processor.Settings.K8sSecrets = copiedSecretReferences
271269
processor.RefreshOptions.SecretSettingPopulated = true
272270

273271
// Update next refresh time only if settings updated successfully
@@ -295,13 +293,13 @@ func (processor *AppConfigurationProviderProcessor) shouldReconcile(
295293
return false
296294
}
297295

298-
if len(processor.ReconciliationState.ExistingSecretReferences) == 0 ||
299-
len(processor.ReconciliationState.ExistingSecretReferences) != len(existingSecrets) {
296+
if len(processor.ReconciliationState.ExistingK8sSecrets) == 0 ||
297+
len(processor.ReconciliationState.ExistingK8sSecrets) != len(existingSecrets) {
300298
return true
301299
}
302300

303301
for name, secret := range existingSecrets {
304-
if processor.ReconciliationState.ExistingSecretReferences[name].SecretResourceVersion != secret.ResourceVersion {
302+
if processor.ReconciliationState.ExistingK8sSecrets[name].SecretResourceVersion != secret.ResourceVersion {
305303
return true
306304
}
307305
}
@@ -313,7 +311,7 @@ func (processor *AppConfigurationProviderProcessor) Finish() (ctrl.Result, error
313311
processor.ReconciliationState.Generation = processor.Provider.Generation
314312

315313
if processor.RefreshOptions.SecretSettingPopulated {
316-
processor.ReconciliationState.ExistingSecretReferences = processor.Settings.SecretReferences
314+
processor.ReconciliationState.ExistingK8sSecrets = processor.Settings.K8sSecrets
317315
}
318316

319317
if processor.RefreshOptions.updatedKeyValueETags != nil {

0 commit comments

Comments
 (0)