Skip to content

Commit

Permalink
Refresh secret when data change (#53)
Browse files Browse the repository at this point in the history 
* resolve confict

* update

* resolve comments

* add test

* update

* update

* update

* rename secretMetadata to secretsMetadata

* update
  • Loading branch information
@linglingye001
linglingye001 committed Nov 8, 2024
1 parent 8408e9e commit 92e7a79
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 108 deletions.
20 changes: 14 additions & 6 deletions internal/controller/appconfigurationprovider_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,15 @@ 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
}
}

// 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
}
Expand Down Expand Up @@ -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")
}

Expand All @@ -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,
Expand Down Expand Up @@ -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))
}

Expand Down
70 changes: 52 additions & 18 deletions internal/controller/appconfigurationprovider_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
},
},
}
Expand Down Expand Up @@ -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),
},
},
}
Expand Down Expand Up @@ -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"
Expand All @@ -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: {
Expand All @@ -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,
},
},
}
Expand Down Expand Up @@ -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: {
Expand All @@ -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)

Expand All @@ -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")))
})
})
Expand Down
34 changes: 26 additions & 8 deletions internal/controller/processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 29 additions & 0 deletions internal/controller/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 92e7a79

Please sign in to comment.