resolve comments

Azure · Oct 9, 2024 · a573d71 · a573d71
1 parent 588dd07
commit a573d71
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 20 deletions.
diff --git a/internal/loader/configuraiton_setting_loader_test.go b/internal/loader/configuraiton_setting_loader_test.go
@@ -1292,8 +1292,8 @@ func TestGetFilters(t *testing.T) {
 	assert.Len(t, featureFlagFilters4, 0)
 	assert.Equal(t, "two", *filters4[0].KeyFilter)
 	assert.Equal(t, "test", *filters4[0].LabelFilter)
-	assert.Equal(t, `one`, *filters4[1].KeyFilter)
-	assert.Nil(t, filters4[1].LabelFilter)
+	assert.Equal(t, "one", *filters4[1].KeyFilter)
+	assert.Equal(t, "\x00", *filters4[1].LabelFilter)
 
 	testSpec5 := acpv1.AzureAppConfigurationProviderSpec{
 		Configuration: acpv1.AzureAppConfigurationKeyValueOptions{
@@ -1324,7 +1324,7 @@ func TestGetFilters(t *testing.T) {
 	assert.Equal(t, "one", *filters6[0].KeyFilter)
 	assert.Equal(t, "test", *filters6[0].LabelFilter)
 	assert.Equal(t, "one", *filters6[1].KeyFilter)
-	assert.Nil(t, filters6[1].LabelFilter)
+	assert.Equal(t, "\x00", *filters6[1].LabelFilter)
 
 	testSpec7 := acpv1.AzureAppConfigurationProviderSpec{
 		Configuration: acpv1.AzureAppConfigurationKeyValueOptions{
@@ -1337,7 +1337,7 @@ func TestGetFilters(t *testing.T) {
 	filters7 := GetKeyValueFilters(testSpec7)
 	assert.Len(t, filters7, 1)
 	assert.Equal(t, "one", *filters7[0].KeyFilter)
-	assert.Nil(t, filters7[0].LabelFilter)
+	assert.Equal(t, "\x00", *filters7[0].LabelFilter)
 
 	testSpec8 := acpv1.AzureAppConfigurationProviderSpec{
 		Configuration: acpv1.AzureAppConfigurationKeyValueOptions{
@@ -1358,12 +1358,12 @@ func TestGetFilters(t *testing.T) {
 		},
 	}
 
-	sentinels := getSentinels(testSpec8.Configuration.Refresh.Monitoring.Sentinels)
+	sentinels := normalizeSentinels(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, "\x00", *sentinels[0].Label)
 	assert.Equal(t, "two", sentinels[1].Key)
-	assert.Nil(t, sentinels[1].Label)
+	assert.Equal(t, "\x00", *sentinels[1].Label)
 }
 
 func TestCompare(t *testing.T) {

diff --git a/internal/loader/configuration_setting_loader.go b/internal/loader/configuration_setting_loader.go
@@ -303,7 +303,7 @@ 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)
+	sentinels := normalizeSentinels(provider.Spec.Configuration.Refresh.Monitoring.Sentinels)
 
 	for _, sentinel := range sentinels {
 		if eTag, ok := eTags[sentinel]; ok {
@@ -625,14 +625,14 @@ func GetSecret(ctx context.Context,
 }
 
 func GetKeyValueFilters(acpSpec acpv1.AzureAppConfigurationProviderSpec) []acpv1.Selector {
-	return deduplicateFilters(processEmptyLabelFilter(acpSpec.Configuration.Selectors))
+	return deduplicateFilters(normalizeLabelFilter(acpSpec.Configuration.Selectors))
 }
 
 func GetFeatureFlagFilters(acpSpec acpv1.AzureAppConfigurationProviderSpec) []acpv1.Selector {
 	featureFlagFilters := make([]acpv1.Selector, 0)
 
 	if acpSpec.FeatureFlag != nil {
-		featureFlagFilters = deduplicateFilters(processEmptyLabelFilter(acpSpec.FeatureFlag.Selectors))
+		featureFlagFilters = deduplicateFilters(normalizeLabelFilter(acpSpec.FeatureFlag.Selectors))
 		for i := 0; i < len(featureFlagFilters); i++ {
 			if featureFlagFilters[i].KeyFilter != nil {
 				prefixedFeatureFlagFilter := FeatureFlagKeyPrefix + *featureFlagFilters[i].KeyFilter
@@ -644,12 +644,13 @@ func GetFeatureFlagFilters(acpSpec acpv1.AzureAppConfigurationProviderSpec) []ac
 	return featureFlagFilters
 }
 
-func getSentinels(sentinels []acpv1.Sentinel) []acpv1.Sentinel {
+func normalizeSentinels(sentinels []acpv1.Sentinel) []acpv1.Sentinel {
 	var results []acpv1.Sentinel
+	nullString := "\x00"
 	for _, sentinel := range sentinels {
 		label := sentinel.Label
-		if sentinel.Label != nil && len(*sentinel.Label) == 0 {
-			label = nil
+		if sentinel.Label == nil || len(*sentinel.Label) == 0 {
+			label = &nullString
 		}
 
 		results = append(results, acpv1.Sentinel{
@@ -661,12 +662,13 @@ func getSentinels(sentinels []acpv1.Sentinel) []acpv1.Sentinel {
 	return results
 }
 
-func processEmptyLabelFilter(filters []acpv1.Selector) []acpv1.Selector {
+func normalizeLabelFilter(filters []acpv1.Selector) []acpv1.Selector {
 	var result []acpv1.Selector
+	nullString := "\x00"
 	for i := 0; i < len(filters); i++ {
 		labelFilter := filters[i].LabelFilter
-		if filters[i].LabelFilter != nil && len(*filters[i].LabelFilter) == 0 {
-			labelFilter = nil
+		if filters[i].LabelFilter == nil || len(*filters[i].LabelFilter) == 0 {
+			labelFilter = &nullString
 		}
 
 		result = append(result, acpv1.Selector{

diff --git a/internal/loader/settings_client.go b/internal/loader/settings_client.go
@@ -119,15 +119,11 @@ func (s *SentinelSettingsClient) GetSettings(ctx context.Context, client *azappc
 }
 
 func (s *SelectorSettingsClient) GetSettings(ctx context.Context, client *azappconfig.Client) (*SettingsResponse, error) {
-	nullString := "\x00"
 	settings := make([]azappconfig.Setting, 0)
 	pageEtags := make(map[acpv1.Selector][]*azcore.ETag)
 
 	for _, filter := range s.selectors {
 		if filter.KeyFilter != nil {
-			if filter.LabelFilter == nil {
-				filter.LabelFilter = &nullString // NUL is escaped to \x00 in golang
-			}
 			selector := azappconfig.SettingSelector{
 				KeyFilter:   filter.KeyFilter,
 				LabelFilter: filter.LabelFilter,