Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions deploy/parameter/helm-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ autoscaling:
targetCPUUtilizationPercentage: 80
targetMemoryUtilizationPercentage: 80

audience: ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about put it into env section and name it "azureAppconfigAudience"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use full product name. If put it to the env section , the command will be --set env.azureAppConfigurationAudience, what do you think? @zhenlan

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which one is more consistent with existing naming conventions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helm best practice recommended camelCase, and azureAppConfigurationAudience is more descriptive, consistent with how azureClientId and azureTenantId are already done.


env:
azureClientId: ""
azureTenantId: ""
Expand Down
4 changes: 4 additions & 0 deletions deploy/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ spec:
{{- end }}
- name: REQUEST_TRACING_ENABLED
value: "{{ .Values.requestTracing.enabled }}"
{{- if .Values.audience }}
- name: AZURE_APPCONFIG_AUDIENCE
value: {{ .Values.audience | quote }}
{{- end }}
livenessProbe:
httpGet:
path: /healthz
Expand Down
28 changes: 21 additions & 7 deletions internal/loader/configuration_client_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
acpv1 "azappconfig/provider/api/v1"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
azappconfig "github.com/Azure/azure-sdk-for-go/sdk/data/azappconfig/v2"
Expand Down Expand Up @@ -80,17 +81,30 @@ const (
ApiTokenExchangeAudience string = "api://AzureADTokenExchange"
AnnotationClientID string = "azure.workload.identity/client-id"
AnnotationTenantID string = "azure.workload.identity/tenant-id"
AzureAppConfigAudience string = "AZURE_APPCONFIG_AUDIENCE"
)

var (
clientOptionWithModuleInfo *azappconfig.ClientOptions = &azappconfig.ClientOptions{
func newClientOptions() *azappconfig.ClientOptions {
options := &azappconfig.ClientOptions{
ClientOptions: policy.ClientOptions{
Telemetry: policy.TelemetryOptions{
ApplicationID: fmt.Sprintf("%s/%s", properties.ModuleName, properties.ModuleVersion),
},
},
}
)

if audience, ok := os.LookupEnv(AzureAppConfigAudience); ok && audience != "" {
options.ClientOptions.Cloud = cloud.Configuration{
Services: map[cloud.ServiceName]cloud.ServiceConfiguration{
azappconfig.ServiceName: {
Audience: audience,
},
},
}
}

return options
}

func NewConfigurationClientManager(ctx context.Context, provider acpv1.AzureAppConfigurationProvider) (ClientManager, error) {
manager := &ConfigurationClientManager{
Expand Down Expand Up @@ -118,14 +132,14 @@ func NewConfigurationClientManager(ctx context.Context, provider acpv1.AzureAppC
if manager.id, err = parseConnectionString(connectionString, IdSection); err != nil {
return nil, err
}
if staticClient, err = azappconfig.NewClientFromConnectionString(connectionString, clientOptionWithModuleInfo); err != nil {
if staticClient, err = azappconfig.NewClientFromConnectionString(connectionString, newClientOptions()); err != nil {
return nil, err
}
} else {
if manager.credential, err = CreateTokenCredential(ctx, provider.Spec.Auth, provider.Namespace); err != nil {
return nil, err
}
if staticClient, err = azappconfig.NewClient(*provider.Spec.Endpoint, manager.credential, clientOptionWithModuleInfo); err != nil {
if staticClient, err = azappconfig.NewClient(*provider.Spec.Endpoint, manager.credential, newClientOptions()); err != nil {
return nil, err
}
manager.endpoint = *provider.Spec.Endpoint
Expand Down Expand Up @@ -286,15 +300,15 @@ func QuerySrvTargetHost(ctx context.Context, host string) ([]string, error) {

func (manager *ConfigurationClientManager) newConfigurationClient(endpoint string) (*azappconfig.Client, error) {
if manager.credential != nil {
return azappconfig.NewClient(endpoint, manager.credential, clientOptionWithModuleInfo)
return azappconfig.NewClient(endpoint, manager.credential, newClientOptions())
}

connectionStr := buildConnectionString(endpoint, manager.secret, manager.id)
if connectionStr == "" {
return nil, fmt.Errorf("failed to build connection string for fallback client")
}

return azappconfig.NewClientFromConnectionString(connectionStr, clientOptionWithModuleInfo)
return azappconfig.NewClientFromConnectionString(connectionStr, newClientOptions())
}

func isValidEndpoint(host string, validDomain string) bool {
Expand Down
108 changes: 108 additions & 0 deletions internal/loader/configuration_client_manager_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

package loader

import (
"azappconfig/provider/internal/properties"
"fmt"
"os"
"testing"

azappconfig "github.com/Azure/azure-sdk-for-go/sdk/data/azappconfig/v2"
)

func TestNewClientOptions(t *testing.T) {
expectedAppID := fmt.Sprintf("%s/%s", properties.ModuleName, properties.ModuleVersion)

tests := []struct {
name string
envVars map[string]string
expectedAudience string
hasCloudConfig bool
}{
{
name: "no audience set - default behavior",
envVars: nil,
expectedAudience: "",
hasCloudConfig: false,
},
{
name: "audience set to Azure Government",
envVars: map[string]string{
AzureAppConfigAudience: "https://appconfig.azure.us",
},
expectedAudience: "https://appconfig.azure.us",
hasCloudConfig: true,
},
{
name: "audience set to Azure China",
envVars: map[string]string{
AzureAppConfigAudience: "https://appconfig.azure.cn",
},
expectedAudience: "https://appconfig.azure.cn",
hasCloudConfig: true,
},
{
name: "audience set to empty string",
envVars: map[string]string{
AzureAppConfigAudience: "",
},
expectedAudience: "",
hasCloudConfig: false,
},
{
name: "audience set to custom value",
envVars: map[string]string{
AzureAppConfigAudience: "https://custom.audience.example",
},
expectedAudience: "https://custom.audience.example",
hasCloudConfig: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Clean up the env var before each test
if err := os.Unsetenv(AzureAppConfigAudience); err != nil {
t.Fatalf("Failed to unset environment variable: %v", err)
}

// Setup environment variables
if tt.envVars != nil {
for k, v := range tt.envVars {
if err := os.Setenv(k, v); err != nil {
t.Fatalf("Failed to set environment variable: %v", err)
}
defer func(key string) {
if err := os.Unsetenv(key); err != nil {
t.Errorf("Failed to unset environment variable: %v", err)
}
}(k)
}
}

options := newClientOptions()

// Verify telemetry ApplicationID is always set
if options.ClientOptions.Telemetry.ApplicationID != expectedAppID {
t.Errorf("Expected ApplicationID %q, got %q", expectedAppID, options.ClientOptions.Telemetry.ApplicationID)
}

// Verify cloud configuration / audience
if tt.hasCloudConfig {
serviceConfig, exists := options.ClientOptions.Cloud.Services[azappconfig.ServiceName]
if !exists {
t.Fatal("Expected cloud service configuration to be set for azappconfig.ServiceName, but it was not found")
}
if serviceConfig.Audience != tt.expectedAudience {
t.Errorf("Expected audience %q, got %q", tt.expectedAudience, serviceConfig.Audience)
}
} else {
if len(options.ClientOptions.Cloud.Services) != 0 {
t.Errorf("Expected no cloud service configuration, but got %v", options.ClientOptions.Cloud.Services)
}
}
})
}
}
Loading