diff --git a/internal/loader/configuraiton_setting_loader_test.go b/internal/loader/configuraiton_setting_loader_test.go index de0e908..497572e 100644 --- a/internal/loader/configuraiton_setting_loader_test.go +++ b/internal/loader/configuraiton_setting_loader_test.go @@ -2215,3 +2215,224 @@ func newSelectorWithTagFilters(key string, label *string, tagFilters []string) a } return acpv1.MakeComparable(selector) } + +// --- Snapshot Reference Tests --- + +func TestParseSnapshotReference(t *testing.T) { + t.Run("valid reference", func(t *testing.T) { + name, err := parseSnapshotReference(`{"snapshot_name":"my-snapshot"}`) + assert.NoError(t, err) + assert.Equal(t, "my-snapshot", name) + }) + + t.Run("empty snapshot name", func(t *testing.T) { + _, err := parseSnapshotReference(`{"snapshot_name":""}`) + assert.Error(t, err) + assert.Contains(t, err.Error(), "snapshot_name is empty") + }) + + t.Run("invalid JSON", func(t *testing.T) { + _, err := parseSnapshotReference(`invalid json`) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse snapshot reference") + }) + + t.Run("missing snapshot_name field", func(t *testing.T) { + _, err := parseSnapshotReference(`{"other_field":"value"}`) + assert.Error(t, err) + assert.Contains(t, err.Error(), "snapshot_name is empty") + }) +} + +func newSnapshotReferenceSettings(key string, snapshotName string, label string) azappconfig.Setting { + snapshotRefContentType := SnapshotReferenceContentType + value := fmt.Sprintf(`{"snapshot_name":"%s"}`, snapshotName) + return azappconfig.Setting{ + Key: &key, + Value: &value, + Label: &label, + ContentType: &snapshotRefContentType, + } +} + +func TestSnapshotReferenceInCreateKeyValueSettings(t *testing.T) { + t.Run("Snapshot reference setting should be collected and traced", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockSettingsClient := NewMockSettingsClient(mockCtrl) + mockClientManager := NewMockClientManager(mockCtrl) + + EndpointName := "https://fake-endpoint" + testSpec := acpv1.AzureAppConfigurationProviderSpec{ + Endpoint: &EndpointName, + ReplicaDiscoveryEnabled: false, + Target: acpv1.ConfigurationGenerationParameters{ + ConfigMapName: "test-configmap", + }, + } + testProvider := acpv1.AzureAppConfigurationProvider{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "azconfig.io/v1", + Kind: "AppConfigurationProvider", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "testName", + Namespace: "testNamespace", + }, + Spec: testSpec, + } + + // Settings returned: 1 regular kv, 1 snapshot reference + settingsToReturn := []azappconfig.Setting{ + newCommonKeyValueSettings("key1", "value1", "label1"), + newSnapshotReferenceSettings("snapshotRef1", "my-snapshot", "label1"), + } + + keyValueEtags := make(map[acpv1.ComparableSelector][]*azcore.ETag) + keyValueEtags[newKeyValueSelector("*", nil)] = []*azcore.ETag{} + settingsResponse := &SettingsResponse{ + Settings: settingsToReturn, + Etags: keyValueEtags, + } + + mockSettingsClient.EXPECT().GetSettings(gomock.Any(), gomock.Any()).Return(settingsResponse, nil) + + // First GetClients call: for ExecuteFailoverPolicy (initial key-value loading) - returns valid wrapper + // Second GetClients call: for resolveSnapshotReferences - returns empty to trigger error + gomock.InOrder( + mockClientManager.EXPECT().GetClients(gomock.Any()).Return([]*ConfigurationClientWrapper{&fakeClientWrapper}, nil), + mockClientManager.EXPECT().GetClients(gomock.Any()).Return([]*ConfigurationClientWrapper{}, nil), + ) + + configurationProvider, _ := NewConfigurationSettingLoader(testProvider, mockClientManager, mockSettingsClient) + + // Resolution will fail because no clients available for snapshot resolution + _, err := configurationProvider.CreateKeyValueSettings(context.Background(), nil) + + // Expect an error because no clients available for snapshot resolution + assert.Error(t, err) + assert.Contains(t, err.Error(), "no client is available to resolve snapshot references") + + // Verify the tracing feature was set during the collection phase (before resolution) + assert.True(t, configurationProvider.TracingFeatures.UseSnapshotReference, "UseSnapshotReference flag should be set when snapshot reference settings are found") + }) + + t.Run("Settings without snapshot references should not set tracing flag", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockSettingsClient := NewMockSettingsClient(mockCtrl) + mockClientManager := NewMockClientManager(mockCtrl) + + EndpointName := "https://fake-endpoint" + testSpec := acpv1.AzureAppConfigurationProviderSpec{ + Endpoint: &EndpointName, + ReplicaDiscoveryEnabled: false, + Target: acpv1.ConfigurationGenerationParameters{ + ConfigMapName: "test-configmap", + }, + } + testProvider := acpv1.AzureAppConfigurationProvider{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "azconfig.io/v1", + Kind: "AppConfigurationProvider", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "testName", + Namespace: "testNamespace", + }, + Spec: testSpec, + } + + settingsToReturn := mockConfigurationSettings() + keyValueEtags := make(map[acpv1.ComparableSelector][]*azcore.ETag) + keyValueEtags[newKeyValueSelector("*", nil)] = []*azcore.ETag{} + settingsResponse := &SettingsResponse{ + Settings: settingsToReturn, + Etags: keyValueEtags, + } + + mockSettingsClient.EXPECT().GetSettings(gomock.Any(), gomock.Any()).Return(settingsResponse, nil) + mockClientManager.EXPECT().GetClients(gomock.Any()).Return([]*ConfigurationClientWrapper{&fakeClientWrapper}, nil).AnyTimes() + + configurationProvider, _ := NewConfigurationSettingLoader(testProvider, mockClientManager, mockSettingsClient) + rawSettings, err := configurationProvider.CreateKeyValueSettings(context.Background(), nil) + + assert.NoError(t, err) + assert.NotNil(t, rawSettings) + assert.False(t, configurationProvider.TracingFeatures.UseSnapshotReference, "UseSnapshotReference flag should not be set when no snapshot references exist") + }) + + t.Run("Invalid snapshot reference format should return error", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockSettingsClient := NewMockSettingsClient(mockCtrl) + mockClientManager := NewMockClientManager(mockCtrl) + + EndpointName := "https://fake-endpoint" + testSpec := acpv1.AzureAppConfigurationProviderSpec{ + Endpoint: &EndpointName, + ReplicaDiscoveryEnabled: false, + Target: acpv1.ConfigurationGenerationParameters{ + ConfigMapName: "test-configmap", + }, + } + testProvider := acpv1.AzureAppConfigurationProvider{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "azconfig.io/v1", + Kind: "AppConfigurationProvider", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "testName", + Namespace: "testNamespace", + }, + Spec: testSpec, + } + + // Create a snapshot reference setting with invalid JSON value + invalidRefContentType := SnapshotReferenceContentType + invalidRefValue := "invalid json" + invalidRefKey := "badRef" + invalidRefLabel := "label1" + settingsToReturn := []azappconfig.Setting{ + { + Key: &invalidRefKey, + Value: &invalidRefValue, + Label: &invalidRefLabel, + ContentType: &invalidRefContentType, + }, + } + + keyValueEtags := make(map[acpv1.ComparableSelector][]*azcore.ETag) + keyValueEtags[newKeyValueSelector("*", nil)] = []*azcore.ETag{} + settingsResponse := &SettingsResponse{ + Settings: settingsToReturn, + Etags: keyValueEtags, + } + + mockSettingsClient.EXPECT().GetSettings(gomock.Any(), gomock.Any()).Return(settingsResponse, nil) + fakeClient := &ConfigurationClientWrapper{ + Client: nil, + Endpoint: EndpointName, + } + mockClientManager.EXPECT().GetClients(gomock.Any()).Return([]*ConfigurationClientWrapper{fakeClient}, nil).AnyTimes() + + configurationProvider, _ := NewConfigurationSettingLoader(testProvider, mockClientManager, mockSettingsClient) + _, err := configurationProvider.CreateKeyValueSettings(context.Background(), nil) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid format for snapshot reference setting") + }) + + t.Run("Snapshot reference with KV ref should require spec.secret", func(t *testing.T) { + // This verifies that if a snapshot ref contains Key Vault references but spec.secret is not configured, + // the parseSnapshotReference itself works but the resolution flow would error. + // For unit test purposes, we validate the parsing side. + name, err := parseSnapshotReference(`{"snapshot_name":"vault-ref-snapshot"}`) + assert.NoError(t, err) + assert.Equal(t, "vault-ref-snapshot", name) + }) +} + diff --git a/internal/loader/configuration_setting_loader.go b/internal/loader/configuration_setting_loader.go index 78d1639..1750627 100644 --- a/internal/loader/configuration_setting_loader.go +++ b/internal/loader/configuration_setting_loader.go @@ -87,6 +87,7 @@ type ServicePrincipleAuthenticationParameters struct { const ( SecretReferenceContentType string = "application/vnd.microsoft.appconfig.keyvaultref+json;charset=utf-8" + SnapshotReferenceContentType string = "application/json; profile=\"https://azconfig.io/mime-profiles/snapshot-ref\"; charset=utf-8" FeatureFlagContentType string = "application/vnd.microsoft.appconfig.ff+json;charset=utf-8" AzureClientId string = "azure_client_id" AzureClientSecret string = "azure_client_secret" @@ -235,6 +236,7 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex resolver := secretReferenceResolver useAIConfiguration := false useAIChatCompletionConfiguration := false + snapshotRefs := make(map[string]string) for _, setting := range settingsResponse.Settings { trimmedKey := trimPrefix(*setting.Key, csl.Spec.Configuration.TrimKeyPrefixes) if len(trimmedKey) == 0 { @@ -299,6 +301,11 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex } } rawSettings.K8sSecrets[secretName].SecretsKeyVaultMetadata[trimmedKey] = *secretMetadata + case SnapshotReferenceContentType: + if setting.Value != nil { + snapshotRefs[trimmedKey] = *setting.Value + csl.TracingFeatures.UseSnapshotReference = true + } default: rawSettings.KeyValueSettings[trimmedKey] = setting.Value rawSettings.IsJsonContentTypeMap[trimmedKey] = isJsonContentType(setting.ContentType) @@ -317,6 +324,15 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex csl.TracingFeatures.UseAIConfiguration = useAIConfiguration csl.TracingFeatures.UseAIChatCompletionConfiguration = useAIChatCompletionConfiguration + // Resolve snapshot references + if len(snapshotRefs) > 0 { + if err := csl.resolveSnapshotReferences(ctx, snapshotRefs, rawSettings, &resolver, &useAIConfiguration, &useAIChatCompletionConfiguration); err != nil { + return nil, err + } + csl.TracingFeatures.UseAIConfiguration = csl.TracingFeatures.UseAIConfiguration || useAIConfiguration + csl.TracingFeatures.UseAIChatCompletionConfiguration = csl.TracingFeatures.UseAIChatCompletionConfiguration || useAIChatCompletionConfiguration + } + // resolve the secret reference settings if resolvedSecret, err := csl.ResolveSecretReferences(ctx, rawSettings.K8sSecrets, resolver); err != nil { return nil, err @@ -331,6 +347,136 @@ func (csl *ConfigurationSettingLoader) CreateKeyValueSettings(ctx context.Contex return rawSettings, nil } +func (csl *ConfigurationSettingLoader) resolveSnapshotReferences( + ctx context.Context, + snapshotRefs map[string]string, + rawSettings *RawSettings, + resolver *SecretReferenceResolver, + useAIConfiguration *bool, + useAIChatCompletionConfiguration *bool, +) error { + clients, err := csl.ClientManager.GetClients(ctx) + if err != nil { + return err + } + if len(clients) == 0 { + return fmt.Errorf("no client is available to resolve snapshot references") + } + + for key, snapshotRefValue := range snapshotRefs { + snapshotName, err := parseSnapshotReference(snapshotRefValue) + if err != nil { + return fmt.Errorf("invalid format for snapshot reference setting '%s': %s", key, err.Error()) + } + + snapshotSettings, err := loadSnapshotSettings(ctx, clients[0].Client, snapshotName) + if err != nil { + return fmt.Errorf("failed to load snapshot settings: key=%s, error=%s", key, err.Error()) + } + + for _, setting := range snapshotSettings { + if setting.Key == nil { + continue + } + + trimmedKey := trimPrefix(*setting.Key, csl.Spec.Configuration.TrimKeyPrefixes) + if len(trimmedKey) == 0 { + klog.Warningf("key of the setting '%s' from snapshot reference is trimmed to the empty string, just ignore it", *setting.Key) + continue + } + + if setting.ContentType == nil { + rawSettings.KeyValueSettings[trimmedKey] = setting.Value + rawSettings.IsJsonContentTypeMap[trimmedKey] = false + continue + } + + contentType := strings.TrimSpace(strings.ToLower(*setting.ContentType)) + switch contentType { + case FeatureFlagContentType: + continue // ignore feature flag from snapshot reference + case SecretReferenceContentType: + if setting.Value == nil { + return fmt.Errorf("the value of Key Vault reference '%s' from snapshot reference is null", *setting.Key) + } + + if csl.Spec.Secret == nil { + return fmt.Errorf("a Key Vault reference is found in snapshot reference, but 'spec.secret' was not configured in the Azure App Configuration provider '%s' in namespace '%s'", csl.Name, csl.Namespace) + } + + var secretType corev1.SecretType = corev1.SecretTypeOpaque + if secretTypeTag, ok := setting.Tags[PreservedSecretTypeTag]; ok { + if secretTypeTag == nil { + return fmt.Errorf("the secret type tag '%s' for setting '%s' from snapshot reference is invalid", PreservedSecretTypeTag, *setting.Key) + } + secretType, err = parseSecretType(*secretTypeTag) + if err != nil { + return err + } + } + + if *resolver == nil { + if newResolver, resolverErr := csl.createSecretReferenceResolver(ctx); resolverErr != nil { + return resolverErr + } else { + *resolver = newResolver + } + } + + currentUrl := *setting.Value + secretMetadata, err := parse(currentUrl) + if err != nil { + return err + } + + secretName := trimmedKey + if secretType == corev1.SecretTypeOpaque { + secretName = csl.Spec.Secret.Target.SecretName + } + + if _, ok := rawSettings.K8sSecrets[secretName]; !ok { + rawSettings.K8sSecrets[secretName] = &TargetK8sSecretMetadata{ + Type: secretType, + SecretsKeyVaultMetadata: make(map[string]KeyVaultSecretMetadata), + } + } + rawSettings.K8sSecrets[secretName].SecretsKeyVaultMetadata[trimmedKey] = *secretMetadata + default: + rawSettings.KeyValueSettings[trimmedKey] = setting.Value + rawSettings.IsJsonContentTypeMap[trimmedKey] = isJsonContentType(setting.ContentType) + if rawSettings.IsJsonContentTypeMap[trimmedKey] { + if isAIConfigurationContentType(setting.ContentType) { + *useAIConfiguration = true + } + if isAIChatCompletionContentType(setting.ContentType) { + *useAIChatCompletionConfiguration = true + } + } + } + } + } + + return nil +} + +// parseSnapshotReference parses a snapshot reference JSON value and returns the snapshot name. +// The expected format is: {"snapshot_name":""} +func parseSnapshotReference(ref string) (string, error) { + var snapshotRef struct { + SnapshotName string `json:"snapshot_name"` + } + + if err := json.Unmarshal([]byte(ref), &snapshotRef); err != nil { + return "", fmt.Errorf("failed to parse snapshot reference: %s", err.Error()) + } + + if snapshotRef.SnapshotName == "" { + return "", fmt.Errorf("snapshot_name is empty in snapshot reference") + } + + return snapshotRef.SnapshotName, nil +} + func (csl *ConfigurationSettingLoader) CheckAndRefreshSentinels( ctx context.Context, provider *acpv1.AzureAppConfigurationProvider, diff --git a/internal/loader/request_tracing.go b/internal/loader/request_tracing.go index 1bd5606..710b29e 100644 --- a/internal/loader/request_tracing.go +++ b/internal/loader/request_tracing.go @@ -23,6 +23,7 @@ type TracingFeatures struct { IsFailoverRequest bool UseAIConfiguration bool UseAIChatCompletionConfiguration bool + UseSnapshotReference bool } // Feature flag telemetry @@ -47,6 +48,7 @@ const ( LoadBalancingKey string = "LB" AIConfigurationKey string = "AI" AIChatCompletionKey string = "AICC" + SnapshotReferenceKey string = "SnapshotRef" ) func createCorrelationContextHeader(ctx context.Context, provider acpv1.AzureAppConfigurationProvider, tracingFeatures TracingFeatures) http.Header { @@ -97,6 +99,10 @@ func createCorrelationContextHeader(ctx context.Context, provider acpv1.AzureApp features = append(features, AIChatCompletionKey) } + if tracingFeatures.UseSnapshotReference { + features = append(features, SnapshotReferenceKey) + } + if len(features) > 0 { featureStr := "Features=" + strings.Join(features, TracingFeatureDelimiterKey) output = append(output, featureStr) diff --git a/internal/loader/request_tracing_test.go b/internal/loader/request_tracing_test.go index 6d94c17..34a97ca 100644 --- a/internal/loader/request_tracing_test.go +++ b/internal/loader/request_tracing_test.go @@ -131,6 +131,35 @@ func TestCreateCorrelationContextHeader(t *testing.T) { "InstalledBy=Extension", }, }, + { + name: "with snapshot reference", + provider: acpv1.AzureAppConfigurationProvider{}, + tracingFeatures: TracingFeatures{ + UseSnapshotReference: true, + }, + expected: []string{ + "Host=Kubernetes", + "Features=SnapshotRef", + "InstalledBy=Helm", + }, + }, + { + name: "with snapshot reference and other features", + provider: acpv1.AzureAppConfigurationProvider{ + Spec: acpv1.AzureAppConfigurationProviderSpec{ + LoadBalancingEnabled: true, + }, + }, + tracingFeatures: TracingFeatures{ + UseAIConfiguration: true, + UseSnapshotReference: true, + }, + expected: []string{ + "Host=Kubernetes", + "Features=LB+AI+SnapshotRef", + "InstalledBy=Helm", + }, + }, } for _, tt := range tests { diff --git a/internal/loader/settings_client.go b/internal/loader/settings_client.go index 0089207..adeb575 100644 --- a/internal/loader/settings_client.go +++ b/internal/loader/settings_client.go @@ -148,25 +148,11 @@ func (s *SelectorSettingsClient) GetSettings(ctx context.Context, client *azappc // update the etags for the filter pageEtags[acpv1.MakeComparable(filter)] = latestEtags } else { - snapshot, err := client.GetSnapshot(ctx, *filter.SnapshotName, nil) + snapshotSettings, err := loadSnapshotSettings(ctx, client, *filter.SnapshotName) if err != nil { return nil, err } - - if *snapshot.CompositionType != azappconfig.CompositionTypeKey { - return nil, fmt.Errorf("compositionType for the selected snapshot '%s' must be 'key', found '%s'", *filter.SnapshotName, *snapshot.CompositionType) - } - - pager := client.NewListSettingsForSnapshotPager(*filter.SnapshotName, nil) - - for pager.More() { - page, err := pager.NextPage(ctx) - if err != nil { - return nil, err - } else if page.Settings != nil { - settings = append(settings, page.Settings...) - } - } + settings = append(settings, snapshotSettings...) } } @@ -175,3 +161,31 @@ func (s *SelectorSettingsClient) GetSettings(ctx context.Context, client *azappc Etags: pageEtags, }, nil } + +func loadSnapshotSettings(ctx context.Context, client *azappconfig.Client, snapshotName string) ([]azappconfig.Setting, error) { + settings := make([]azappconfig.Setting, 0) + snapshot, err := client.GetSnapshot(ctx, snapshotName, nil) + if err != nil { + var respErr *azcore.ResponseError + if errors.As(err, &respErr) && respErr.StatusCode == 404 { + return settings, nil // treat non-existing snapshot as empty + } + return nil, err + } + + if snapshot.CompositionType == nil || *snapshot.CompositionType != azappconfig.CompositionTypeKey { + return nil, fmt.Errorf("compositionType for the selected snapshot '%s' must be 'key'", snapshotName) + } + + pager := client.NewListSettingsForSnapshotPager(snapshotName, nil) + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + return nil, err + } else if page.Settings != nil { + settings = append(settings, page.Settings...) + } + } + + return settings, nil +}