diff --git a/internal/controller/appconfigurationprovider_controller.go b/internal/controller/appconfigurationprovider_controller.go index 832c7cc..6a00ddf 100644 --- a/internal/controller/appconfigurationprovider_controller.go +++ b/internal/controller/appconfigurationprovider_controller.go @@ -269,7 +269,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context } } - result, err := reconciler.createOrUpdateSecrets(ctx, provider, processor.Settings) + result, err := reconciler.createOrUpdateSecrets(ctx, provider, processor) if err != nil { return result, nil } @@ -277,7 +277,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context // Expel the secrets which are no longer selected by the provider. if provider.Spec.Secret == nil || processor.RefreshOptions.SecretSettingPopulated { - result, err := reconciler.expelRemovedSecrets(ctx, provider, existingSecrets, processor.ReconciliationState.ExistingSecretReferences) + result, err := reconciler.expelRemovedSecrets(ctx, provider, existingSecrets, processor.Settings.SecretReferences) if err != nil { return result, nil } @@ -411,8 +411,8 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateConfigM func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateSecrets( ctx context.Context, provider *acpv1.AzureAppConfigurationProvider, - settings *loader.TargetKeyValueSettings) (reconcile.Result, error) { - if len(settings.SecretSettings) == 0 { + processor *AppConfigurationProviderProcessor) (reconcile.Result, error) { + if len(processor.Settings.SecretSettings) == 0 { klog.V(3).Info("No secret settings are fetched from Azure AppConfiguration") } @@ -425,7 +425,15 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateSecrets Namespace: provider.Namespace, } - for secretName, secret := range settings.SecretSettings { + for secretName, secret := range processor.Settings.SecretSettings { + if !shouldCreateOrUpdate(processor, secretName) { + if _, ok := reconciler.ProvidersReconcileState[namespacedName].ExistingSecretReferences[secretName]; ok { + processor.Settings.SecretReferences[secretName].SecretResourceVersion = reconciler.ProvidersReconcileState[namespacedName].ExistingSecretReferences[secretName].SecretResourceVersion + } + klog.V(5).Infof("Skip updating the secret %q in %q namespace since data is not changed", secretName, provider.Namespace) + continue + } + secretObj := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: secretName, @@ -453,7 +461,7 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateSecrets return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, err } - reconciler.ProvidersReconcileState[namespacedName].ExistingSecretReferences[secretObj.Name].SecretResourceVersion = secretObj.ResourceVersion + processor.Settings.SecretReferences[secretName].SecretResourceVersion = secretObj.ResourceVersion klog.V(5).Infof("Secret %q in %q namespace is %s", secretObj.Name, secretObj.Namespace, string(operationResult)) } diff --git a/internal/controller/appconfigurationprovider_controller_test.go b/internal/controller/appconfigurationprovider_controller_test.go index bc25d01..d44c028 100644 --- a/internal/controller/appconfigurationprovider_controller_test.go +++ b/internal/controller/appconfigurationprovider_controller_test.go @@ -12,6 +12,7 @@ import ( acpv1 "azappconfig/provider/api/v1" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -172,12 +173,12 @@ var _ = Describe("AppConfiguationProvider controller", func() { }, SecretReferences: map[string]*loader.TargetSecretReference{ secretName: { - Type: corev1.SecretTypeTLS, - UriSegments: make(map[string]loader.KeyVaultSecretUriSegment), + Type: corev1.SecretTypeTLS, + SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata), }, secretName2: { - Type: corev1.SecretTypeOpaque, - UriSegments: make(map[string]loader.KeyVaultSecretUriSegment), + Type: corev1.SecretTypeOpaque, + SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata), }, }, } @@ -265,8 +266,8 @@ var _ = Describe("AppConfiguationProvider controller", func() { ConfigMapSettings: configMapResult, SecretReferences: map[string]*loader.TargetSecretReference{ secretName: { - Type: corev1.SecretType("Opaque"), - UriSegments: make(map[string]loader.KeyVaultSecretUriSegment), + Type: corev1.SecretType("Opaque"), + SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata), }, }, } @@ -609,7 +610,7 @@ var _ = Describe("AppConfiguationProvider controller", func() { _ = k8sClient.Delete(ctx, configProvider) }) - It("Should refresh secret", func() { + It("Should refresh secret when data change", func() { By("By enabling refresh on secret") configMapResult := make(map[string]string) configMapResult["testKey"] = "testValue" @@ -618,10 +619,14 @@ var _ = Describe("AppConfiguationProvider controller", func() { secretResult := make(map[string][]byte) secretResult["testSecretKey"] = []byte("testSecretValue") - secretResult["testSecretKey2"] = []byte("testSecretValue2") - secretResult["testSecretKey3"] = []byte("testSecretValue3") secretName := "secret-to-be-refreshed-3" + var fakeId azsecrets.ID = "fakeSecretId" + secretMetadata := make(map[string]loader.KeyVaultSecretMetadata) + secretMetadata["testSecretKey"] = loader.KeyVaultSecretMetadata{ + SecretId: &fakeId, + } + allSettings := &loader.TargetKeyValueSettings{ SecretSettings: map[string]corev1.Secret{ secretName: { @@ -632,8 +637,8 @@ var _ = Describe("AppConfiguationProvider controller", func() { ConfigMapSettings: configMapResult, SecretReferences: map[string]*loader.TargetSecretReference{ secretName: { - Type: corev1.SecretType("Opaque"), - UriSegments: make(map[string]loader.KeyVaultSecretUriSegment), + Type: corev1.SecretType("Opaque"), + SecretsMetadata: secretMetadata, }, }, } @@ -697,14 +702,10 @@ var _ = Describe("AppConfiguationProvider controller", func() { Expect(secret.Namespace).Should(Equal(ProviderNamespace)) Expect(string(secret.Data["testSecretKey"])).Should(Equal("testSecretValue")) - Expect(string(secret.Data["testSecretKey2"])).Should(Equal("testSecretValue2")) - Expect(string(secret.Data["testSecretKey3"])).Should(Equal("testSecretValue3")) Expect(secret.Type).Should(Equal(corev1.SecretType("Opaque"))) newSecretResult := make(map[string][]byte) newSecretResult["testSecretKey"] = []byte("newTestSecretValue") - newSecretResult["testSecretKey2"] = []byte("newTestSecretValue2") - newSecretResult["testSecretKey3"] = []byte("newTestSecretValue3") newResolvedSecret := map[string]corev1.Secret{ secretName: { @@ -713,7 +714,42 @@ var _ = Describe("AppConfiguationProvider controller", func() { }, } - mockConfigurationSettings.EXPECT().ResolveSecretReferences(gomock.Any(), gomock.Any(), gomock.Any()).Return(newResolvedSecret, nil) + var newFakeId azsecrets.ID = "newFakeSecretId" + newSecretMetadata := make(map[string]loader.KeyVaultSecretMetadata) + newSecretMetadata["testSecretKey"] = loader.KeyVaultSecretMetadata{ + SecretId: &newFakeId, + } + mockedSecretReference := make(map[string]*loader.TargetSecretReference) + mockedSecretReference[secretName] = &loader.TargetSecretReference{ + Type: corev1.SecretType("Opaque"), + SecretsMetadata: newSecretMetadata, + } + + newTargetSettings := &loader.TargetKeyValueSettings{ + SecretSettings: newResolvedSecret, + SecretReferences: mockedSecretReference, + } + + mockConfigurationSettings.EXPECT().ResolveSecretReferences(gomock.Any(), gomock.Any(), gomock.Any()).Return(newTargetSettings, nil) + // Refresh interval is 1 minute, wait for 65 seconds to make sure the refresh is triggered + time.Sleep(65 * time.Second) + + Eventually(func() bool { + err := k8sClient.Get(ctx, secretLookupKey, secret) + return err == nil + }, timeout, interval).Should(BeTrue()) + + Expect(secret.Namespace).Should(Equal(ProviderNamespace)) + Expect(string(secret.Data["testSecretKey"])).Should(Equal("newTestSecretValue")) + Expect(secret.Type).Should(Equal(corev1.SecretType("Opaque"))) + + // Mocked secret refresh scenario when secretMetadata is not changed + newTargetSettings2 := &loader.TargetKeyValueSettings{ + SecretSettings: make(map[string]corev1.Secret), + SecretReferences: mockedSecretReference, + } + + mockConfigurationSettings.EXPECT().ResolveSecretReferences(gomock.Any(), gomock.Any(), gomock.Any()).Return(newTargetSettings2, nil) // Refresh interval is 1 minute, wait for 65 seconds to make sure the refresh is triggered time.Sleep(65 * time.Second) @@ -724,8 +760,6 @@ var _ = Describe("AppConfiguationProvider controller", func() { Expect(secret.Namespace).Should(Equal(ProviderNamespace)) Expect(string(secret.Data["testSecretKey"])).Should(Equal("newTestSecretValue")) - Expect(string(secret.Data["testSecretKey2"])).Should(Equal("newTestSecretValue2")) - Expect(string(secret.Data["testSecretKey3"])).Should(Equal("newTestSecretValue3")) Expect(secret.Type).Should(Equal(corev1.SecretType("Opaque"))) }) }) diff --git a/internal/controller/processor.go b/internal/controller/processor.go index 3f6130d..ae77937 100644 --- a/internal/controller/processor.go +++ b/internal/controller/processor.go @@ -70,7 +70,6 @@ func (processor *AppConfigurationProviderProcessor) processFullReconciliation() return err } processor.Settings = updatedSettings - processor.ReconciliationState.ExistingSecretReferences = updatedSettings.SecretReferences processor.RefreshOptions.ConfigMapSettingPopulated = true if processor.Provider.Spec.Secret != nil { processor.RefreshOptions.SecretSettingPopulated = true @@ -160,7 +159,6 @@ func (processor *AppConfigurationProviderProcessor) processKeyValueRefresh(exist } processor.Settings = keyValueRefreshedSettings - reconcileState.ExistingSecretReferences = keyValueRefreshedSettings.SecretReferences processor.RefreshOptions.ConfigMapSettingPopulated = true if processor.Provider.Spec.Secret != nil { processor.RefreshOptions.SecretSettingPopulated = true @@ -201,17 +199,26 @@ func (processor *AppConfigurationProviderProcessor) processSecretReferenceRefres // Only resolve the secret references that not specified the secret version secretReferencesToSolve := make(map[string]*loader.TargetSecretReference) + copiedSecretReferences := make(map[string]*loader.TargetSecretReference) for secretName, reference := range reconcileState.ExistingSecretReferences { - for key, uriSegment := range reference.UriSegments { - if uriSegment.SecretVersion == "" { + for key, secretMetadata := range reference.SecretsMetadata { + if secretMetadata.SecretVersion == "" { if secretReferencesToSolve[secretName] == nil { secretReferencesToSolve[secretName] = &loader.TargetSecretReference{ - Type: reference.Type, - UriSegments: make(map[string]loader.KeyVaultSecretUriSegment), + Type: reference.Type, + SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata), } } - secretReferencesToSolve[secretName].UriSegments[key] = uriSegment + secretReferencesToSolve[secretName].SecretsMetadata[key] = secretMetadata } + + if copiedSecretReferences[secretName] == nil { + copiedSecretReferences[secretName] = &loader.TargetSecretReference{ + Type: reference.Type, + SecretsMetadata: make(map[string]loader.KeyVaultSecretMetadata), + } + } + copiedSecretReferences[secretName].SecretsMetadata[key] = secretMetadata } } @@ -220,13 +227,19 @@ func (processor *AppConfigurationProviderProcessor) processSecretReferenceRefres return err } - for secretName, resolvedSecret := range resolvedSecrets { + for secretName, resolvedSecret := range resolvedSecrets.SecretSettings { existingSecret, ok := existingSecrets[secretName] if ok { maps.Copy(existingSecret.Data, resolvedSecret.Data) } } + + for secretName, reference := range resolvedSecrets.SecretReferences { + maps.Copy(copiedSecretReferences[secretName].SecretsMetadata, reference.SecretsMetadata) + } + processor.Settings.SecretSettings = existingSecrets + processor.Settings.SecretReferences = copiedSecretReferences processor.RefreshOptions.SecretSettingPopulated = true // Update next refresh time only if settings updated successfully @@ -270,6 +283,11 @@ func (processor *AppConfigurationProviderProcessor) shouldReconcile( func (processor *AppConfigurationProviderProcessor) Finish() (ctrl.Result, error) { processor.ReconciliationState.Generation = processor.Provider.Generation + + if processor.RefreshOptions.SecretSettingPopulated { + processor.ReconciliationState.ExistingSecretReferences = processor.Settings.SecretReferences + } + if !processor.RefreshOptions.secretReferenceRefreshEnabled && !processor.RefreshOptions.sentinelBasedRefreshEnabled && !processor.RefreshOptions.featureFlagRefreshEnabled { diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 986fb81..aad5fc1 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -302,3 +302,32 @@ func verifySelectorObject(selector acpv1.Selector) error { return nil } + +func shouldCreateOrUpdate(processor *AppConfigurationProviderProcessor, secretName string) bool { + if processor.ShouldReconcile { + return true + } + + existingSecretReferences := processor.ReconciliationState.ExistingSecretReferences + if _, ok := existingSecretReferences[secretName]; !ok { + return true + } + + secretReference := processor.Settings.SecretReferences[secretName] + if len(existingSecretReferences[secretName].SecretsMetadata) != len(secretReference.SecretsMetadata) { + return true + } + + for key, secretMetadata := range secretReference.SecretsMetadata { + if _, ok := existingSecretReferences[secretName].SecretsMetadata[key]; !ok { + return true + } + if secretMetadata.SecretId != nil && + existingSecretReferences[secretName].SecretsMetadata[key].SecretId != nil && + *(existingSecretReferences[secretName].SecretsMetadata[key].SecretId) != *(secretMetadata.SecretId) { + return true + } + } + + return false +} diff --git a/internal/loader/configuraiton_setting_loader_test.go b/internal/loader/configuraiton_setting_loader_test.go index 869e461..673f114 100644 --- a/internal/loader/configuraiton_setting_loader_test.go +++ b/internal/loader/configuraiton_setting_loader_test.go @@ -140,7 +140,7 @@ func (m *MockResolveSecretReference) EXPECT() *MockResolveSecretReferenceMockRec } // Resolve mocks base method. -func (m *MockResolveSecretReference) Resolve(arg0 KeyVaultSecretUriSegment, arg1 context.Context) (azsecrets.GetSecretResponse, error) { +func (m *MockResolveSecretReference) Resolve(arg0 KeyVaultSecretMetadata, arg1 context.Context) (azsecrets.GetSecretResponse, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Resolve", arg0, arg1) ret0, _ := ret[0].(azsecrets.GetSecretResponse) @@ -406,7 +406,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -459,7 +459,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -515,7 +515,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -571,7 +571,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -627,7 +627,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -641,9 +641,9 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secrets, err := configurationProvider.ResolveSecretReferences(context.Background(), secretReferencesToResolve, mockResolveSecretReference) Expect(err).Should(BeNil()) - Expect(len(secrets)).Should(Equal(1)) - Expect(string(secrets[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) - Expect(string(secrets[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN PRIVATE KEY")) + Expect(len(secrets.SecretSettings)).Should(Equal(1)) + Expect(string(secrets.SecretSettings[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) + Expect(string(secrets.SecretSettings[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN PRIVATE KEY")) }) It("Succeeded to get target tls type secret", func() { @@ -686,7 +686,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -700,9 +700,9 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secrets, err := configurationProvider.ResolveSecretReferences(context.Background(), secretReferencesToResolve, mockResolveSecretReference) Expect(err).Should(BeNil()) - Expect(len(secrets)).Should(Equal(1)) - Expect(string(secrets[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) - Expect(string(secrets[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN RSA PRIVATE KEY")) + Expect(len(secrets.SecretSettings)).Should(Equal(1)) + Expect(string(secrets.SecretSettings[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) + Expect(string(secrets.SecretSettings[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN RSA PRIVATE KEY")) }) It("Succeeded to get tls type secret", func() { @@ -743,7 +743,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -757,9 +757,9 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secrets, err := configurationProvider.ResolveSecretReferences(context.Background(), secretReferencesToResolve, mockResolveSecretReference) Expect(err).Should(BeNil()) - Expect(len(secrets)).Should(Equal(1)) - Expect(string(secrets[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) - Expect(string(secrets[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN PRIVATE KEY")) + Expect(len(secrets.SecretSettings)).Should(Equal(1)) + Expect(string(secrets.SecretSettings[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) + Expect(string(secrets.SecretSettings[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN PRIVATE KEY")) }) It("Succeeded to get tls type secret", func() { @@ -800,7 +800,7 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secretReferencesToResolve := map[string]*TargetSecretReference{ secretName: { Type: corev1.SecretTypeTLS, - UriSegments: map[string]KeyVaultSecretUriSegment{ + SecretsMetadata: map[string]KeyVaultSecretMetadata{ secretName: { HostName: "fake-vault", SecretName: "fake-secret", @@ -814,9 +814,9 @@ var _ = Describe("AppConfiguationProvider Get All Settings", func() { secrets, err := configurationProvider.ResolveSecretReferences(context.Background(), secretReferencesToResolve, mockResolveSecretReference) Expect(err).Should(BeNil()) - Expect(len(secrets)).Should(Equal(1)) - Expect(string(secrets[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) - Expect(string(secrets[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN RSA PRIVATE KEY")) + Expect(len(secrets.SecretSettings)).Should(Equal(1)) + Expect(string(secrets.SecretSettings[secretName].Data["tls.crt"])).Should(ContainSubstring("BEGIN CERTIFICATE")) + Expect(string(secrets.SecretSettings[secretName].Data["tls.key"])).Should(ContainSubstring("BEGIN RSA PRIVATE KEY")) }) }) diff --git a/internal/loader/configuration_setting_loader.go b/internal/loader/configuration_setting_loader.go index 5fc0391..56222e0 100644 --- a/internal/loader/configuration_setting_loader.go +++ b/internal/loader/configuration_setting_loader.go @@ -50,7 +50,7 @@ type TargetKeyValueSettings struct { type TargetSecretReference struct { Type corev1.SecretType - UriSegments map[string]KeyVaultSecretUriSegment + SecretsMetadata map[string]KeyVaultSecretMetadata SecretResourceVersion string } @@ -67,7 +67,7 @@ type ConfigurationSettingsRetriever interface { RefreshKeyValueSettings(ctx context.Context, existingConfigMapSettings *map[string]string, resolveSecretReference SecretReferenceResolver) (*TargetKeyValueSettings, error) RefreshFeatureFlagSettings(ctx context.Context, existingConfigMapSettings *map[string]string) (*TargetKeyValueSettings, error) CheckAndRefreshSentinels(ctx context.Context, provider *acpv1.AzureAppConfigurationProvider, eTags map[acpv1.Sentinel]*azcore.ETag) (bool, map[acpv1.Sentinel]*azcore.ETag, error) - ResolveSecretReferences(ctx context.Context, kvReferencesToResolve map[string]*TargetSecretReference, kvResolver SecretReferenceResolver) (map[string]corev1.Secret, error) + ResolveSecretReferences(ctx context.Context, kvReferencesToResolve map[string]*TargetSecretReference, kvResolver SecretReferenceResolver) (*TargetKeyValueSettings, error) } type ServicePrincipleAuthenticationParameters struct { @@ -196,8 +196,8 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex if csl.Spec.Secret != nil { rawSettings.SecretReferences[csl.Spec.Secret.Target.SecretName] = &TargetSecretReference{ - Type: corev1.SecretTypeOpaque, - UriSegments: make(map[string]KeyVaultSecretUriSegment), + Type: corev1.SecretTypeOpaque, + SecretsMetadata: make(map[string]KeyVaultSecretMetadata), } } @@ -245,7 +245,7 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex } currentUrl := *setting.Value - secretUriSegment, err := parse(currentUrl) + secretMetadata, err := parse(currentUrl) if err != nil { return nil, err } @@ -258,11 +258,11 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex if _, ok := rawSettings.SecretReferences[secretName]; !ok { rawSettings.SecretReferences[secretName] = &TargetSecretReference{ - Type: secretType, - UriSegments: make(map[string]KeyVaultSecretUriSegment), + Type: secretType, + SecretsMetadata: make(map[string]KeyVaultSecretMetadata), } } - rawSettings.SecretReferences[secretName].UriSegments[trimmedKey] = *secretUriSegment + rawSettings.SecretReferences[secretName].SecretsMetadata[trimmedKey] = *secretMetadata default: rawSettings.KeyValueSettings[trimmedKey] = setting.Value rawSettings.IsJsonContentTypeMap[trimmedKey] = isJsonContentType(setting.ContentType) @@ -273,7 +273,8 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex if resolvedSecret, err := csl.ResolveSecretReferences(ctx, rawSettings.SecretReferences, resolver); err != nil { return nil, err } else { - err = MergeSecret(rawSettings.SecretSettings, resolvedSecret) + rawSettings.SecretReferences = resolvedSecret.SecretReferences + err = MergeSecret(rawSettings.SecretSettings, resolvedSecret.SecretSettings) if err != nil { return nil, err } @@ -356,7 +357,7 @@ func (csl *ConfigurationSettingLoader) CheckAndRefreshSentinels( func (csl *ConfigurationSettingLoader) ResolveSecretReferences( ctx context.Context, secretReferencesToResolve map[string]*TargetSecretReference, - resolver SecretReferenceResolver) (map[string]corev1.Secret, error) { + resolver SecretReferenceResolver) (*TargetKeyValueSettings, error) { if resolver == nil { if kvResolver, err := csl.createSecretReferenceResolver(ctx); err != nil { return nil, err @@ -365,18 +366,18 @@ func (csl *ConfigurationSettingLoader) ResolveSecretReferences( } } - resolvedSecretReferences := make(map[string]corev1.Secret) + resolvedSecrets := make(map[string]corev1.Secret) for name, targetSecretReference := range secretReferencesToResolve { - resolvedSecretReferences[name] = corev1.Secret{ + resolvedSecrets[name] = corev1.Secret{ Data: make(map[string][]byte), Type: targetSecretReference.Type, } var eg errgroup.Group if targetSecretReference.Type == corev1.SecretTypeOpaque { - if len(targetSecretReference.UriSegments) > 0 { + if len(targetSecretReference.SecretsMetadata) > 0 { lock := &sync.Mutex{} - for key, kvReference := range targetSecretReference.UriSegments { + for key, kvReference := range targetSecretReference.SecretsMetadata { currentKey := key currentReference := kvReference eg.Go(func() error { @@ -386,7 +387,10 @@ func (csl *ConfigurationSettingLoader) ResolveSecretReferences( } lock.Lock() defer lock.Unlock() - resolvedSecretReferences[name].Data[currentKey] = []byte(*resolvedSecret.Value) + resolvedSecrets[name].Data[currentKey] = []byte(*resolvedSecret.Value) + currentSecretMetadata := targetSecretReference.SecretsMetadata[currentKey] + currentSecretMetadata.SecretId = resolvedSecret.ID + secretReferencesToResolve[name].SecretsMetadata[currentKey] = currentSecretMetadata return nil }) } @@ -397,7 +401,10 @@ func (csl *ConfigurationSettingLoader) ResolveSecretReferences( } } else if targetSecretReference.Type == corev1.SecretTypeTLS { eg.Go(func() error { - resolvedSecret, err := resolver.Resolve(targetSecretReference.UriSegments[name], ctx) + resolvedSecret, err := resolver.Resolve(targetSecretReference.SecretsMetadata[name], ctx) + currentSecretMetadata := targetSecretReference.SecretsMetadata[name] + currentSecretMetadata.SecretId = resolvedSecret.ID + secretReferencesToResolve[name].SecretsMetadata[name] = currentSecretMetadata if err != nil { return fmt.Errorf("fail to resolve the Key Vault reference type setting '%s': %s", name, err.Error()) } @@ -408,9 +415,9 @@ func (csl *ConfigurationSettingLoader) ResolveSecretReferences( switch *resolvedSecret.ContentType { case CertTypePfx: - resolvedSecretReferences[name].Data[TlsKey], resolvedSecretReferences[name].Data[TlsCrt], err = decodePkcs12(*resolvedSecret.Value) + resolvedSecrets[name].Data[TlsKey], resolvedSecrets[name].Data[TlsCrt], err = decodePkcs12(*resolvedSecret.Value) case CertTypePem: - resolvedSecretReferences[name].Data[TlsKey], resolvedSecretReferences[name].Data[TlsCrt], err = decodePem(*resolvedSecret.Value) + resolvedSecrets[name].Data[TlsKey], resolvedSecrets[name].Data[TlsCrt], err = decodePem(*resolvedSecret.Value) default: err = fmt.Errorf("unknown content type '%s'", *resolvedSecret.ContentType) } @@ -429,7 +436,10 @@ func (csl *ConfigurationSettingLoader) ResolveSecretReferences( } } - return resolvedSecretReferences, nil + return &TargetKeyValueSettings{ + SecretSettings: resolvedSecrets, + SecretReferences: secretReferencesToResolve, + }, nil } func (csl *ConfigurationSettingLoader) createSecretReferenceResolver(ctx context.Context) (SecretReferenceResolver, error) { diff --git a/internal/loader/keyvault_reference_resolver.go b/internal/loader/keyvault_reference_resolver.go index 42c04fb..1de6a84 100644 --- a/internal/loader/keyvault_reference_resolver.go +++ b/internal/loader/keyvault_reference_resolver.go @@ -24,34 +24,35 @@ type SecretReference struct { Uri string `json:"uri,omitempty"` } -type KeyVaultSecretUriSegment struct { +type KeyVaultSecretMetadata struct { HostName string SecretName string SecretVersion string + SecretId *azsecrets.ID } type SecretReferenceResolver interface { - Resolve(secretUriSegment KeyVaultSecretUriSegment, ctx context.Context) (azsecrets.GetSecretResponse, error) + Resolve(secretMetadata KeyVaultSecretMetadata, ctx context.Context) (azsecrets.GetSecretResponse, error) } func (resolver *KeyVaultConnector) Resolve( - secretUriSegment KeyVaultSecretUriSegment, + secretMetadata KeyVaultSecretMetadata, ctx context.Context) (azsecrets.GetSecretResponse, error) { var secretClient any var ok bool - if secretClient, ok = resolver.Clients.Load(secretUriSegment.HostName); !ok { - newSecretClient, err := azsecrets.NewClient("https://"+secretUriSegment.HostName, resolver.DefaultTokenCredential, nil) + if secretClient, ok = resolver.Clients.Load(secretMetadata.HostName); !ok { + newSecretClient, err := azsecrets.NewClient("https://"+secretMetadata.HostName, resolver.DefaultTokenCredential, nil) if err != nil { return azsecrets.GetSecretResponse{}, err } secretClient = newSecretClient - resolver.Clients.Store(secretUriSegment.HostName, newSecretClient) + resolver.Clients.Store(secretMetadata.HostName, newSecretClient) } - return secretClient.(*azsecrets.Client).GetSecret(ctx, secretUriSegment.SecretName, secretUriSegment.SecretVersion, nil) + return secretClient.(*azsecrets.Client).GetSecret(ctx, secretMetadata.SecretName, secretMetadata.SecretVersion, nil) } -func parse(settingValue string) (*KeyVaultSecretUriSegment, error) { +func parse(settingValue string) (*KeyVaultSecretMetadata, error) { var secretRef SecretReference // // Valid Key Vault Reference setting value to parse @@ -81,7 +82,7 @@ func parse(settingValue string) (*KeyVaultSecretUriSegment, error) { secretName := segments[1] hostName := strings.ToLower(secretUrl.Host) - result := &KeyVaultSecretUriSegment{ + result := &KeyVaultSecretMetadata{ HostName: hostName, SecretName: secretName, SecretVersion: secretVersion, diff --git a/internal/loader/keyvault_reference_resolver_test.go b/internal/loader/keyvault_reference_resolver_test.go index a7c937c..9bb0983 100644 --- a/internal/loader/keyvault_reference_resolver_test.go +++ b/internal/loader/keyvault_reference_resolver_test.go @@ -17,78 +17,78 @@ import ( func TestResolveNotValidSecretReference(t *testing.T) { valueToTest := "https://fake-vault/secrets/fake-secret/version" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Nil(t, secretUriSegment) + assert.Nil(t, secretMetadata) assert.Equal(t, "invalid character 'h' looking for beginning of value", err.Error()) } func TestResolveNotValidSecretReferenceUri(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Nil(t, secretUriSegment) + assert.Nil(t, secretMetadata) assert.Equal(t, "not a valid url in Key Vault reference type setting '{ \"uri\":\"https://fake-vault/\"}', not a valid item", err.Error()) } func TestResolveNotValidSecretReferenceUri2(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/secrets///fake-secret\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Nil(t, secretUriSegment) + assert.Nil(t, secretMetadata) assert.Equal(t, "not a valid url in Key Vault reference type setting '{ \"uri\":\"https://fake-vault/secrets///fake-secret\"}', not a valid item", err.Error()) } func TestResolveNotValidSecretReferenceUri3(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/test/test/test\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Nil(t, secretUriSegment) + assert.Nil(t, secretMetadata) assert.Equal(t, "not a valid url in Key Vault reference type setting '{ \"uri\":\"https://fake-vault/test/test/test\"}', not a valid item", err.Error()) } func TestResolveNotValidSecretReferenceUri4(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/test////test\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Nil(t, secretUriSegment) + assert.Nil(t, secretMetadata) assert.Equal(t, "not a valid url in Key Vault reference type setting '{ \"uri\":\"https://fake-vault/test////test\"}', not a valid item", err.Error()) } func TestResolveValidSecretReferenceUriEmptyVersion(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/secrets/fake-secret\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Equal(t, "fake-vault", secretUriSegment.HostName) - assert.Equal(t, "fake-secret", secretUriSegment.SecretName) - assert.Equal(t, "", secretUriSegment.SecretVersion) + assert.Equal(t, "fake-vault", secretMetadata.HostName) + assert.Equal(t, "fake-secret", secretMetadata.SecretName) + assert.Equal(t, "", secretMetadata.SecretVersion) assert.Nil(t, err) } func TestResolveValidSecretReferenceUri(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/secrets/fake-secret/testversion\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Equal(t, "fake-vault", secretUriSegment.HostName) - assert.Equal(t, "fake-secret", secretUriSegment.SecretName) - assert.Equal(t, "testversion", secretUriSegment.SecretVersion) + assert.Equal(t, "fake-vault", secretMetadata.HostName) + assert.Equal(t, "fake-secret", secretMetadata.SecretName) + assert.Equal(t, "testversion", secretMetadata.SecretVersion) assert.Nil(t, err) } func TestResolveValidSecretReferenceUri2(t *testing.T) { valueToTest := "{ \"uri\":\"https://fake-vault/secrets/fake-secret/testversion/notvalid\"}" - secretUriSegment, err := parse(valueToTest) + secretMetadata, err := parse(valueToTest) - assert.Equal(t, "fake-vault", secretUriSegment.HostName) - assert.Equal(t, "fake-secret", secretUriSegment.SecretName) - assert.Equal(t, "testversion", secretUriSegment.SecretVersion) + assert.Equal(t, "fake-vault", secretMetadata.HostName) + assert.Equal(t, "fake-secret", secretMetadata.SecretName) + assert.Equal(t, "testversion", secretMetadata.SecretVersion) assert.Nil(t, err) } @@ -101,8 +101,8 @@ func TestResolveSecretReferenceSetting(t *testing.T) { Clients: clients, } - secretUriSegment, _ := parse(valueToTest) - resolvedSecret, err := resolver.Resolve(*secretUriSegment, context.Background()) + secretMetadata, _ := parse(valueToTest) + resolvedSecret, err := resolver.Resolve(*secretMetadata, context.Background()) assert.Contains(t, err.Error(), "fake-vault") length := 0 diff --git a/internal/loader/mocks/mock_configuration_settings_retriever.go b/internal/loader/mocks/mock_configuration_settings_retriever.go index 18052e4..07e4f2b 100644 --- a/internal/loader/mocks/mock_configuration_settings_retriever.go +++ b/internal/loader/mocks/mock_configuration_settings_retriever.go @@ -12,7 +12,6 @@ import ( azcore "github.com/Azure/azure-sdk-for-go/sdk/azcore" gomock "github.com/golang/mock/gomock" - v10 "k8s.io/api/core/v1" ) // MockConfigurationSettingsRetriever is a mock of ConfigurationSettingsRetriever interface. @@ -100,10 +99,10 @@ func (mr *MockConfigurationSettingsRetrieverMockRecorder) RefreshKeyValueSetting } // ResolveSecretReferences mocks base method. -func (m *MockConfigurationSettingsRetriever) ResolveSecretReferences(arg0 context.Context, arg1 map[string]*loader.TargetSecretReference, arg2 loader.SecretReferenceResolver) (map[string]v10.Secret, error) { +func (m *MockConfigurationSettingsRetriever) ResolveSecretReferences(arg0 context.Context, arg1 map[string]*loader.TargetSecretReference, arg2 loader.SecretReferenceResolver) (*loader.TargetKeyValueSettings, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ResolveSecretReferences", arg0, arg1, arg2) - ret0, _ := ret[0].(map[string]v10.Secret) + ret0, _ := ret[0].(*loader.TargetKeyValueSettings) ret1, _ := ret[1].(error) return ret0, ret1 }