Skip to content

Commit

Permalink
process empty label of sentinels
Browse files Browse the repository at this point in the history
  • Loading branch information
linglingye001 committed Sep 24, 2024
1 parent 09ae2e0 commit 1ebc998
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 48 deletions.
4 changes: 2 additions & 2 deletions api/v1/azureappconfigurationprovider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ type RefreshMonitoring struct {

// Defines the keyValues to be watched.
type Sentinel struct {
Key string `json:"key"`
Key *string `json:"key"`
// +kubebuilder:default="\x00"
Label string `json:"label,omitempty"`
Label *string `json:"label,omitempty"`
}

// ConfigMapDataOptions defines the options of generating ConfigMap data
Expand Down
14 changes: 13 additions & 1 deletion api/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 39 additions & 20 deletions internal/controller/appconfigurationprovider_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,8 @@ var _ = Describe("AppConfiguationProvider controller", func() {
mockConfigurationSettings.EXPECT().RefreshKeyValueSettings(gomock.Any(), gomock.Any(), gomock.Any()).Return(allSettings2, nil)

ctx := context.Background()
testKey := "testKey"
testLabel := "testLabel"
providerName := "refresh-appconfigurationprovider-2"
configMapName := "configmap-to-be-refresh-2"
configProvider := &acpv1.AzureAppConfigurationProvider{
Expand All @@ -651,7 +653,7 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Enabled: true,
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{Key: "testKey", Label: "testLabel"},
{Key: &testKey, Label: &testLabel},
},
},
},
Expand Down Expand Up @@ -783,6 +785,8 @@ var _ = Describe("AppConfiguationProvider controller", func() {
mockConfigurationSettings.EXPECT().CheckAndRefreshSentinels(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil, nil)

ctx := context.Background()
testNewKey := "testNewKey"
testNewLabel := "testNewLabel"
providerName := "refresh-appconfigurationprovider-2a"
configMapName := "configmap-to-be-refresh-2a"
configProvider := &acpv1.AzureAppConfigurationProvider{
Expand All @@ -806,7 +810,7 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Enabled: true,
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{Key: "testNewKey", Label: "testNewLabel"},
{Key: &testNewKey, Label: &testNewLabel},
},
},
},
Expand Down Expand Up @@ -857,6 +861,8 @@ var _ = Describe("AppConfiguationProvider controller", func() {
mockConfigurationSettings.EXPECT().CreateTargetSettings(gomock.Any(), gomock.Any()).Return(allSettings, nil)

ctx := context.Background()
testNewKey := "testNewKey"
testNewLabel := "testNewLabel"
providerName := "refresh-appconfigurationprovider-2b"
configMapName := "configmap-to-be-refresh-2b"
configProvider := &acpv1.AzureAppConfigurationProvider{
Expand All @@ -880,7 +886,7 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Enabled: false,
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{Key: "testNewKey", Label: "testNewLabel"},
{Key: &testNewKey, Label: &testNewLabel},
},
},
},
Expand Down Expand Up @@ -942,6 +948,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
mockConfigurationSettings.EXPECT().CreateTargetSettings(gomock.Any(), gomock.Any()).Return(allSettings2, nil)

ctx := context.Background()
testKeyOne := "testKeyOne"
testLabelOne := "testNewLabel"
testKeyTwo := "testKeyTwo"
testLabelTwo := "testLabel"
providerName := "refresh-appconfigurationprovider-2c"
configMapName := "configmap-to-be-refresh-2c"
configProvider := &acpv1.AzureAppConfigurationProvider{
Expand All @@ -965,8 +975,8 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Enabled: true,
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{Key: "testKeyOne", Label: "testNewLabel"},
{Key: "testKeyTwo", Label: "testLabel"},
{Key: &testKeyOne, Label: &testLabelOne},
{Key: &testKeyTwo, Label: &testLabelTwo},
},
},
},
Expand Down Expand Up @@ -2256,6 +2266,9 @@ var _ = Describe("AppConfiguationProvider controller", func() {
It("Should return error when duplicated sentinel key are set", func() {
configMapName := "test-configmap"
connectionStringReference := "fakeSecret"
testKey := "testKey"
testLabelOne := "testValue"
testLabelTwo := "testValue1"
configProviderSpec := acpv1.AzureAppConfigurationProviderSpec{
ConnectionStringReference: &connectionStringReference,
Target: acpv1.ConfigurationGenerationParameters{
Expand All @@ -2266,16 +2279,16 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{
Key: "testKey",
Label: "testValue",
Key: &testKey,
Label: &testLabelOne,
},
{
Key: "testKey",
Label: "testValue1",
Key: &testKey,
Label: &testLabelTwo,
},
{
Key: "testKey",
Label: "testValue",
Key: &testKey,
Label: &testLabelOne,
},
},
},
Expand All @@ -2289,6 +2302,10 @@ var _ = Describe("AppConfiguationProvider controller", func() {
It("Should return no error when all sentinel are unique", func() {
configMapName := "test-configmap"
connectionStringReference := "fakeSecret"
testKey := "testKey"
testKey2 := "testKey2"
testLabel := "testValue"
emptyLabel := ""
configProviderSpec := acpv1.AzureAppConfigurationProviderSpec{
ConnectionStringReference: &connectionStringReference,
Target: acpv1.ConfigurationGenerationParameters{
Expand All @@ -2299,20 +2316,20 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{
Key: "testKey",
Label: "\x00",
Key: &testKey,
Label: nil,
},
{
Key: "testKey",
Label: "testValue1",
Key: &testKey,
Label: &testLabel,
},
{
Key: "testKey2",
Label: "\x00",
Key: &testKey,
Label: nil,
},
{
Key: "testKey2",
Label: "",
Key: &testKey2,
Label: &emptyLabel,
},
},
},
Expand Down Expand Up @@ -2382,6 +2399,8 @@ var _ = Describe("AppConfiguationProvider controller", func() {

It("Should return error when configuration.refresh.interval is less than 1 second", func() {
configMapName := "test-configmap"
testKey := "testKey"
testLabel := "testLabel"
configProviderSpec := acpv1.AzureAppConfigurationProviderSpec{
Endpoint: &EndpointName,
Target: acpv1.ConfigurationGenerationParameters{
Expand All @@ -2393,7 +2412,7 @@ var _ = Describe("AppConfiguationProvider controller", func() {
Enabled: true,
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{Key: "testKey", Label: "testLabel"},
{Key: &testKey, Label: &testLabel},
},
},
},
Expand Down
14 changes: 7 additions & 7 deletions internal/controller/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ var _ = Describe("AppConfiguationProvider processor", func() {
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{
Key: sentinelKey1,
Key: &sentinelKey1,
},
{
Key: sentinelKey2,
Key: &sentinelKey2,
},
},
},
Expand All @@ -104,7 +104,7 @@ var _ = Describe("AppConfiguationProvider processor", func() {
NextKeyValueRefreshReconcileTime: metav1.Now(),
SentinelETags: map[acpv1.Sentinel]*azcore.ETag{
{
Key: sentinelKey1,
Key: &sentinelKey1,
}: &fakeEtag,
},
Generation: 1,
Expand All @@ -123,10 +123,10 @@ var _ = Describe("AppConfiguationProvider processor", func() {
true,
map[acpv1.Sentinel]*azcore.ETag{
{
Key: sentinelKey1,
Key: &sentinelKey1,
}: &newFakeEtag1,
{
Key: sentinelKey2,
Key: &sentinelKey2,
}: &newFakeEtag2,
},
nil,
Expand All @@ -141,10 +141,10 @@ var _ = Describe("AppConfiguationProvider processor", func() {
_, _ = processor.Finish()

Expect(processor.ReconciliationState.SentinelETags[acpv1.Sentinel{
Key: sentinelKey1,
Key: &sentinelKey1,
}]).Should(Equal(&newFakeEtag1))
Expect(processor.ReconciliationState.SentinelETags[acpv1.Sentinel{
Key: sentinelKey2,
Key: &sentinelKey2,
}]).Should(Equal(&newFakeEtag2))
Expect(processor.ReconciliationState.NextKeyValueRefreshReconcileTime).Should(Equal(expectedNextKeyValueRefreshReconcileTime))
})
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func verifyObject(spec acpv1.AzureAppConfigurationProviderSpec) error {
sentinelMap := make(map[acpv1.Sentinel]bool)
for _, sentinel := range spec.Configuration.Refresh.Monitoring.Sentinels {
if _, ok := sentinelMap[sentinel]; ok {
return loader.NewArgumentError("spec.configuration.refresh.monitoring.keyValues", fmt.Errorf("monitoring duplicated key '%s'", sentinel.Key))
return loader.NewArgumentError("spec.configuration.refresh.monitoring.keyValues", fmt.Errorf("monitoring duplicated key '%s'", *sentinel.Key))
}
sentinelMap[sentinel] = true
}
Expand Down
26 changes: 26 additions & 0 deletions internal/loader/configuraiton_setting_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1338,6 +1338,32 @@ func TestGetFilters(t *testing.T) {
assert.Len(t, filters7, 1)
assert.Equal(t, "one", *filters7[0].KeyFilter)
assert.Nil(t, filters7[0].LabelFilter)

testSpec8 := acpv1.AzureAppConfigurationProviderSpec{
Configuration: acpv1.AzureAppConfigurationKeyValueOptions{
Refresh: &acpv1.DynamicConfigurationRefreshParameters{
Monitoring: &acpv1.RefreshMonitoring{
Sentinels: []acpv1.Sentinel{
{
Key: &one,
Label: nil,
},
{
Key: &two,
Label: &emptyLabel,
},
},
},
},
},
}

sentinels := getSentinels(testSpec8.Configuration.Refresh.Monitoring.Sentinels)
assert.Len(t, sentinels, 2)
assert.Equal(t, "one", *sentinels[0].Key)
assert.Nil(t, sentinels[0].Label)
assert.Equal(t, "two", *sentinels[1].Key)
assert.Nil(t, sentinels[1].Label)
}

func TestCompare(t *testing.T) {
Expand Down
51 changes: 39 additions & 12 deletions internal/loader/configuration_setting_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,9 @@ func (csl *ConfigurationSettingLoader) CheckAndRefreshSentinels(
return sentinelChanged, eTags, NewArgumentError("spec.configuration.refresh", fmt.Errorf("refresh is not specified"))
}
refreshedETags := make(map[acpv1.Sentinel]*azcore.ETag)
sentinels := getSentinels(provider.Spec.Configuration.Refresh.Monitoring.Sentinels)

for _, sentinel := range provider.Spec.Configuration.Refresh.Monitoring.Sentinels {
for _, sentinel := range sentinels {
if eTag, ok := eTags[sentinel]; ok {
// Initialize the updatedETags with the current eTags
refreshedETags[sentinel] = eTag
Expand Down Expand Up @@ -624,14 +625,14 @@ func GetSecret(ctx context.Context,
}

func GetKeyValueFilters(acpSpec acpv1.AzureAppConfigurationProviderSpec) []acpv1.Selector {
return deduplicateFilters(acpSpec.Configuration.Selectors)
return deduplicateFilters(processEmptyLabelFilter(acpSpec.Configuration.Selectors))
}

func GetFeatureFlagFilters(acpSpec acpv1.AzureAppConfigurationProviderSpec) []acpv1.Selector {
featureFlagFilters := make([]acpv1.Selector, 0)

if acpSpec.FeatureFlag != nil {
featureFlagFilters = deduplicateFilters(acpSpec.FeatureFlag.Selectors)
featureFlagFilters = deduplicateFilters(processEmptyLabelFilter(acpSpec.FeatureFlag.Selectors))
for i := 0; i < len(featureFlagFilters); i++ {
if featureFlagFilters[i].KeyFilter != nil {
prefixedFeatureFlagFilter := FeatureFlagKeyPrefix + *featureFlagFilters[i].KeyFilter
Expand All @@ -643,6 +644,40 @@ func GetFeatureFlagFilters(acpSpec acpv1.AzureAppConfigurationProviderSpec) []ac
return featureFlagFilters
}

func getSentinels(sentinels []acpv1.Sentinel) []acpv1.Sentinel {
results := make([]acpv1.Sentinel, 0)
for _, sentinel := range sentinels {
label := sentinel.Label
if sentinel.Label != nil && len(*sentinel.Label) == 0 {
label = nil
}

results = append(results, acpv1.Sentinel{

Check failure on line 655 in internal/loader/configuration_setting_loader.go

View workflow job for this annotation

GitHub Actions / lint

SA4010: this result of append is never used, except maybe in other appends (staticcheck)
Key: sentinel.Key,
Label: label,
})

}
return sentinels
}

func processEmptyLabelFilter(filters []acpv1.Selector) []acpv1.Selector {
var result []acpv1.Selector
for i := 0; i < len(filters); i++ {
labelFilter := filters[i].LabelFilter
if filters[i].LabelFilter != nil && len(*filters[i].LabelFilter) == 0 {
labelFilter = nil
}

result = append(result, acpv1.Selector{
KeyFilter: filters[i].KeyFilter,
LabelFilter: labelFilter,
})
}

return result
}

func deduplicateFilters(filters []acpv1.Selector) []acpv1.Selector {
var result []acpv1.Selector
findDuplicate := false
Expand All @@ -664,15 +699,7 @@ func deduplicateFilters(filters []acpv1.Selector) []acpv1.Selector {
}
}
if !findDuplicate {
labelFilter := filters[i].LabelFilter
// need to check if labelFilter is empty, if so, set it to nil
if labelFilter != nil && len(*labelFilter) == 0 {
labelFilter = nil
}
result = append(result, acpv1.Selector{
KeyFilter: filters[i].KeyFilter,
LabelFilter: labelFilter,
})
result = append(result, filters[i])
}
}
reverse(result)
Expand Down
Loading

0 comments on commit 1ebc998

Please sign in to comment.