Make ServiceConfiguration eagerly evalulated

src/ApiService/ApiService/Functions/Events.cs

@@ -1,16 +1,13 @@
 using Microsoft.Azure.Functions.Worker;
 using Microsoft.Azure.Functions.Worker.Http;
-using Microsoft.Extensions.Logging;
 using Microsoft.OneFuzz.Service.Auth;
 namespace Microsoft.OneFuzz.Service.Functions;
 
 public class EventsFunction {
-    private readonly ILogger _log;
     private readonly IOnefuzzContext _context;
 
-    public EventsFunction(ILogger<EventsFunction> log, IOnefuzzContext context) {
+    public EventsFunction(IOnefuzzContext context) {
         _context = context;
-        _log = log;
     }
 
     [Function("Events")]

src/ApiService/ApiService/Program.cs

@@ -70,17 +70,19 @@ public static async Async.Task Main() {
             .ConfigureAppConfiguration(builder => {
                 // Using a connection string in dev allows us to run the functions locally.
                 if (!string.IsNullOrEmpty(configuration.AppConfigurationConnectionString)) {
-                    var _ = builder.AddAzureAppConfiguration(options => {
-                        var _ = options
+                    builder.AddAzureAppConfiguration(options => {
+                        options
                             .Connect(configuration.AppConfigurationConnectionString)
                             .UseFeatureFlags(ffOptions => ffOptions.CacheExpirationInterval = TimeSpan.FromSeconds(30));
                     });
-                } else {
-                    var _ = builder.AddAzureAppConfiguration(options => {
-                        var _ = options
-                            .Connect(new Uri(configuration.AppConfigurationEndpoint!), new DefaultAzureCredential())
+                } else if (!string.IsNullOrEmpty(configuration.AppConfigurationEndpoint)) {
+                    builder.AddAzureAppConfiguration(options => {
+                        options
+                            .Connect(new Uri(configuration.AppConfigurationEndpoint), new DefaultAzureCredential())
                             .UseFeatureFlags(ffOptions => ffOptions.CacheExpirationInterval = TimeSpan.FromMinutes(1));
                     });
+                } else {
+                    throw new InvalidOperationException($"One of APPCONFIGURATION_CONNECTION_STRING or APPCONFIGURATION_ENDPOINT must be set");
                 }
             })
             .ConfigureServices((context, services) => {
@@ -201,13 +203,10 @@ public static async Async.Task SetupStorage(IStorage storage, IServiceConfig ser
             }
         }
 
-        var storageAccount = serviceConfig.OneFuzzFuncStorage;
-        if (storageAccount is not null) {
-            var tableClient = await storage.GetTableServiceClientForAccount(storageAccount);
-            await Async.Task.WhenAll(toCreate.Select(async t => {
-                // don't care if it was created or not
-                _ = await tableClient.CreateTableIfNotExistsAsync(serviceConfig.OneFuzzStoragePrefix + t.Name);
-            }));
-        }
+        var tableClient = await storage.GetTableServiceClientForAccount(serviceConfig.OneFuzzFuncStorage);
+        await Async.Task.WhenAll(toCreate.Select(async t => {
+            // don't care if it was created or not
+            _ = await tableClient.CreateTableIfNotExistsAsync(serviceConfig.OneFuzzStoragePrefix + t.Name);
+        }));
     }
 }

src/ApiService/ApiService/ServiceConfiguration.cs

@@ -10,57 +10,51 @@ public enum LogDestination {
 
 
 public interface IServiceConfig {
-    public LogDestination[] LogDestinations { get; set; }
-
+    #region Parameters for logging & application insights
+    public LogDestination[] LogDestinations { get; }
     public ApplicationInsights.DataContracts.SeverityLevel LogSeverityLevel { get; }
-
     public string? ApplicationInsightsAppId { get; }
     public string? ApplicationInsightsInstrumentationKey { get; }
+    #endregion
+
+    #region Parameters for feature flags
     public string? AppConfigurationEndpoint { get; }
     public string? AppConfigurationConnectionString { get; }
-    public string? AzureSignalRConnectionString { get; }
-    public string? AzureSignalRServiceTransportType { get; }
-
-    public string? AzureWebJobDisableHomePage { get; }
-    public string? AzureWebJobStorage { get; }
+    #endregion
 
-    public string? DiagnosticsAzureBlobContainerSasUrl { get; }
-    public string? DiagnosticsAzureBlobRetentionDays { get; }
+    #region Auth parameters for CLI app
     public string? CliAppId { get; }
     public string? Authority { get; }
     public string? TenantDomain { get; }
     public string? MultiTenantDomain { get; }
-    public ResourceIdentifier? OneFuzzDataStorage { get; }
-    public ResourceIdentifier? OneFuzzFuncStorage { get; }
-    public string? OneFuzzInstance { get; }
-    public string? OneFuzzInstanceName { get; }
-    public string? OneFuzzEndpoint { get; }
-    public string? OneFuzzKeyvault { get; }
-
+    #endregion
+
+    public ResourceIdentifier OneFuzzResourceGroup { get; }
+    public ResourceIdentifier OneFuzzDataStorage { get; }
+    public ResourceIdentifier OneFuzzFuncStorage { get; }
+    public Uri OneFuzzInstance { get; }
+    public string OneFuzzInstanceName { get; }
+    public Uri? OneFuzzEndpoint { get; }
+    public string OneFuzzKeyvault { get; }
     public string? OneFuzzMonitor { get; }
     public string? OneFuzzOwner { get; }
-
-    public string? OneFuzzResourceGroup { get; }
     public string? OneFuzzTelemetry { get; }
-
     public string OneFuzzVersion { get; }
-
     public string? OneFuzzAllowOutdatedAgent { get; }
 
     // Prefix to add to the name of any tables & containers created. This allows
     // multiple instances to run against the same storage account, which
     // is useful for things like integration testing.
     public string OneFuzzStoragePrefix { get; }
-
-    public Uri OneFuzzBaseAddress { get; }
 }
 
 public class ServiceConfiguration : IServiceConfig {
 
     // Version is baked into the assembly by the build process:
-    private static readonly string? _oneFuzzVersion =
+    private static readonly string _oneFuzzVersion =
         Assembly.GetExecutingAssembly()
-        .GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
+        .GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion
+        ?? throw new InvalidOperationException("Unable to read OneFuzz version from assembly");
 
     public ServiceConfiguration() {
 #if DEBUG
@@ -72,78 +66,57 @@ public ServiceConfiguration() {
 
     private static string? GetEnv(string name) {
         var v = Environment.GetEnvironmentVariable(name);
-        if (String.IsNullOrEmpty(v))
-            return null;
-
-        return v;
+        return string.IsNullOrEmpty(v) ? null : v;
     }
 
+    private static string MustGetEnv(string name)
+        => GetEnv(name) ?? throw new InvalidOperationException($"Environment variable {name} is required to be set");
+
     //TODO: Add environment variable to control where to write logs to
-    public LogDestination[] LogDestinations { get; set; }
+    public LogDestination[] LogDestinations { get; private set; }
 
     //TODO: Get this from Environment variable
     public ApplicationInsights.DataContracts.SeverityLevel LogSeverityLevel => ApplicationInsights.DataContracts.SeverityLevel.Verbose;
 
-    public string? ApplicationInsightsAppId => GetEnv("APPINSIGHTS_APPID");
-    public string? ApplicationInsightsInstrumentationKey => GetEnv("APPINSIGHTS_INSTRUMENTATIONKEY");
+    public string? ApplicationInsightsAppId { get; } = GetEnv("APPINSIGHTS_APPID");
 
-    public string? AppConfigurationEndpoint => GetEnv("APPCONFIGURATION_ENDPOINT");
+    public string? ApplicationInsightsInstrumentationKey { get; } = GetEnv("APPINSIGHTS_INSTRUMENTATIONKEY");
 
-    public string? AppConfigurationConnectionString => GetEnv("APPCONFIGURATION_CONNECTION_STRING");
+    public string? AppConfigurationEndpoint { get; } = GetEnv("APPCONFIGURATION_ENDPOINT");
 
-    public string? AzureSignalRConnectionString => GetEnv("AzureSignalRConnectionString");
-    public string? AzureSignalRServiceTransportType => GetEnv("AzureSignalRServiceTransportType");
+    public string? AppConfigurationConnectionString { get; } = GetEnv("APPCONFIGURATION_CONNECTION_STRING");
 
-    public string? AzureWebJobDisableHomePage { get => GetEnv("AzureWebJobsDisableHomepage"); }
-    public string? AzureWebJobStorage { get => GetEnv("AzureWebJobsStorage"); }
+    public string? CliAppId { get; } = GetEnv("CLI_APP_ID");
 
-    public string? DiagnosticsAzureBlobContainerSasUrl { get => GetEnv("DIAGNOSTICS_AZUREBLOBCONTAINERSASURL"); }
-    public string? DiagnosticsAzureBlobRetentionDays { get => GetEnv("DIAGNOSTICS_AZUREBLOBRETENTIONINDAYS"); }
-    public string? CliAppId { get => GetEnv("CLI_APP_ID"); }
-    public string? Authority { get => GetEnv("AUTHORITY"); }
-    public string? TenantDomain { get => GetEnv("TENANT_DOMAIN"); }
-    public string? MultiTenantDomain { get => GetEnv("MULTI_TENANT_DOMAIN"); }
+    public string? Authority { get; } = GetEnv("AUTHORITY");
 
-    public ResourceIdentifier? OneFuzzDataStorage {
-        get {
-            var env = GetEnv("ONEFUZZ_DATA_STORAGE");
-            return env is null ? null : new ResourceIdentifier(env);
-        }
-    }
+    public string? TenantDomain { get; } = GetEnv("TENANT_DOMAIN");
 
-    public ResourceIdentifier? OneFuzzFuncStorage {
-        get {
-            var env = GetEnv("ONEFUZZ_FUNC_STORAGE");
-            return env is null ? null : new ResourceIdentifier(env);
-        }
-    }
+    public string? MultiTenantDomain { get; } = GetEnv("MULTI_TENANT_DOMAIN");
 
-    public string? OneFuzzInstance { get => GetEnv("ONEFUZZ_INSTANCE"); }
-    public string? OneFuzzInstanceName { get => GetEnv("ONEFUZZ_INSTANCE_NAME"); }
-    public string? OneFuzzEndpoint { get => GetEnv("ONEFUZZ_ENDPOINT"); }
-    public string? OneFuzzKeyvault { get => GetEnv("ONEFUZZ_KEYVAULT"); }
-    public string? OneFuzzMonitor { get => GetEnv("ONEFUZZ_MONITOR"); }
-    public string? OneFuzzOwner { get => GetEnv("ONEFUZZ_OWNER"); }
-    public string? OneFuzzResourceGroup { get => GetEnv("ONEFUZZ_RESOURCE_GROUP"); }
-    public string? OneFuzzTelemetry { get => GetEnv("ONEFUZZ_TELEMETRY"); }
-
-    public string OneFuzzVersion {
-        get {
-            // version can be overridden by config:
-            return GetEnv("ONEFUZZ_VERSION")
-                ?? _oneFuzzVersion
-                ?? throw new InvalidOperationException("Unable to read OneFuzz version from assembly");
-        }
-    }
+    public ResourceIdentifier OneFuzzDataStorage { get; } = new(MustGetEnv("ONEFUZZ_DATA_STORAGE"));
 
-    public string? OneFuzzAllowOutdatedAgent => GetEnv("ONEFUZZ_ALLOW_OUTDATED_AGENT");
-    public string OneFuzzStoragePrefix => ""; // in production we never prefix the tables
+    public ResourceIdentifier OneFuzzFuncStorage { get; } = new(MustGetEnv("ONEFUZZ_FUNC_STORAGE"));
 
-    public Uri OneFuzzBaseAddress {
-        get {
-            var hostName = Environment.GetEnvironmentVariable("WEBSITE_HOSTNAME");
-            var scheme = Environment.GetEnvironmentVariable("HTTPS") != null ? "https" : "http";
-            return new Uri($"{scheme}://{hostName}");
-        }
-    }
+    public Uri OneFuzzInstance { get; } = new Uri(MustGetEnv("ONEFUZZ_INSTANCE"));
+
+    public string OneFuzzInstanceName { get; } = MustGetEnv("ONEFUZZ_INSTANCE_NAME");
+
+    public Uri? OneFuzzEndpoint { get; } = GetEnv("ONEFUZZ_ENDPOINT") is string value ? new Uri(value) : null;
+
+    public string OneFuzzKeyvault { get; } = MustGetEnv("ONEFUZZ_KEYVAULT");
+
+    public string? OneFuzzMonitor { get; } = GetEnv("ONEFUZZ_MONITOR");
+
+    public string? OneFuzzOwner { get; } = GetEnv("ONEFUZZ_OWNER");
+
+    public ResourceIdentifier OneFuzzResourceGroup { get; } = new(MustGetEnv("ONEFUZZ_RESOURCE_GROUP"));
+
+    public string? OneFuzzTelemetry { get; } = GetEnv("ONEFUZZ_TELEMETRY");
+
+    public string OneFuzzVersion { get; } = GetEnv("ONEFUZZ_VERSION") ?? _oneFuzzVersion;
+
+    public string? OneFuzzAllowOutdatedAgent { get; } = GetEnv("ONEFUZZ_ALLOW_OUTDATED_AGENT");
+
+    public string OneFuzzStoragePrefix => ""; // in production we never prefix the tables
 }

src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs

@@ -26,12 +26,12 @@ public ConfigOperations(ILogger<ConfigOperations> log, IOnefuzzContext context,
     public Task<InstanceConfig> Fetch()
         => _cache.GetOrCreateAsync(_instanceConfigCacheKey, async entry => {
             entry = entry.SetAbsoluteExpiration(TimeSpan.FromMinutes(1)); // cached for 1 minute
-            var key = _context.ServiceConfiguration.OneFuzzInstanceName ?? throw new Exception("Environment variable ONEFUZZ_INSTANCE_NAME is not set");
+            var key = _context.ServiceConfiguration.OneFuzzInstanceName;
             return await GetEntityAsync(key, key);
         })!; // NULLABLE: only this class inserts _instanceConfigCacheKey so it cannot be null
 
     public async Async.Task Save(InstanceConfig config, bool isNew = false, bool requireEtag = false) {
-        var newConfig = config with { InstanceName = _context.ServiceConfiguration.OneFuzzInstanceName ?? throw new Exception("Environment variable ONEFUZZ_INSTANCE_NAME is not set") };
+        var newConfig = config with { InstanceName = _context.ServiceConfiguration.OneFuzzInstanceName };
         ResultVoid<(HttpStatusCode Status, string Reason)> r;
         if (isNew) {
             r = await Insert(newConfig);

src/ApiService/ApiService/onefuzzlib/Containers.cs

@@ -266,7 +266,7 @@ public string AuthDownloadUrl(Container container, string filename) {
         queryString.Add("container", container.String);
         queryString.Add("filename", filename);
 
-        return $"{instance}/api/download?{queryString}";
+        return $"{instance}api/download?{queryString}";
     }
 
     public async Async.Task<OneFuzzResultVoid> DownloadAsZip(Container container, StorageType storageType, Stream stream, string? prefix = null) {

src/ApiService/ApiService/onefuzzlib/Creds.cs

@@ -50,50 +50,29 @@ public Creds(IServiceConfig config, IHttpClientFactory httpClientFactory, IMemor
         _httpClientFactory = httpClientFactory;
         _cache = cache;
         _azureCredential = new DefaultAzureCredential();
-        _armClient = new ArmClient(this.GetIdentity(), this.GetSubscription());
+        _armClient = new ArmClient(GetIdentity(), GetSubscription());
 
     }
 
-    public DefaultAzureCredential GetIdentity() {
-        return _azureCredential;
-    }
+    public DefaultAzureCredential GetIdentity() => _azureCredential;
 
-    public string GetSubscription() {
-        var storageResourceId = _config.OneFuzzDataStorage
-            ?? throw new System.Exception("Data storage env var is not present");
-        return storageResourceId.SubscriptionId
-            ?? throw new Exception("OneFuzzDataStorage did not have subscription ID");
-    }
+    public string GetSubscription() => _config.OneFuzzDataStorage.SubscriptionId
+            ?? throw new InvalidOperationException("OneFuzzDataStorage did not have subscription ID");
 
-    public string GetBaseResourceGroup() {
-        var storageResourceId = _config.OneFuzzDataStorage
-            ?? throw new System.Exception("Data storage env var is not present");
-        return storageResourceId.ResourceGroupName
-            ?? throw new Exception("OneFuzzDataStorage did not have resource group name");
-    }
+    public string GetBaseResourceGroup() => _config.OneFuzzDataStorage.ResourceGroupName
+            ?? throw new InvalidOperationException("OneFuzzDataStorage did not have resource group name");
 
-    public ResourceIdentifier GetResourceGroupResourceIdentifier() {
-        var resourceId = _config.OneFuzzResourceGroup
-            ?? throw new System.Exception("Resource group env var is not present");
-        return new ResourceIdentifier(resourceId);
-    }
+    public ResourceIdentifier GetResourceGroupResourceIdentifier() => _config.OneFuzzResourceGroup;
 
-    public string GetInstanceName() {
-        var instanceName = _config.OneFuzzInstanceName
-            ?? throw new System.Exception("Instance Name env var is not present");
+    public string GetInstanceName() => _config.OneFuzzInstanceName;
 
-        return instanceName;
-    }
+    public ResourceGroupResource GetResourceGroupResource()
+        => ArmClient.GetResourceGroupResource(GetResourceGroupResourceIdentifier());
 
-    public ResourceGroupResource GetResourceGroupResource() {
-        var resourceId = GetResourceGroupResourceIdentifier();
-        return ArmClient.GetResourceGroupResource(resourceId);
-    }
+    public SubscriptionResource GetSubscriptionResource()
+        => ArmClient.GetSubscriptionResource(SubscriptionResource.CreateResourceIdentifier(GetSubscription()));
 
-    public SubscriptionResource GetSubscriptionResource() {
-        var id = SubscriptionResource.CreateResourceIdentifier(GetSubscription());
-        return ArmClient.GetSubscriptionResource(id);
-    }
+    public Uri GetInstanceUrl() => _config.OneFuzzEndpoint ?? new($"https://{GetInstanceName()}.azurewebsites.net");
 
     private static readonly object _baseRegionKey = new(); // we only need equality/hashcode
     public Async.Task<Region> GetBaseRegion() {
@@ -106,11 +85,6 @@ public Async.Task<Region> GetBaseRegion() {
         })!; // NULLABLE: only this method inserts _baseRegionKey so it cannot be null
     }
 
-    public Uri GetInstanceUrl() {
-        var onefuzzEndpoint = _config.OneFuzzEndpoint;
-        return onefuzzEndpoint != null ? new Uri(onefuzzEndpoint) : new($"https://{GetInstanceName()}.azurewebsites.net");
-    }
-
     public record ScaleSetIdentity(string principalId);
 
     public Async.Task<Guid> GetScalesetPrincipalId() {
@@ -128,16 +102,14 @@ public ResourceIdentifier GetScalesetIdentityResourcePath() {
         var scalesetIdName = $"{GetInstanceName()}-scalesetid";
         var resourceGroupPath = $"/subscriptions/{GetSubscription()}/resourceGroups/{GetBaseResourceGroup()}/providers";
 
-        return new ResourceIdentifier($"{resourceGroupPath}/Microsoft.ManagedIdentity/userAssignedIdentities/{scalesetIdName}");
+        return new($"{resourceGroupPath}/Microsoft.ManagedIdentity/userAssignedIdentities/{scalesetIdName}");
     }
 
-    public GenericResource ParseResourceId(ResourceIdentifier resourceId) {
-        return ArmClient.GetGenericResource(resourceId);
-    }
+    public GenericResource ParseResourceId(ResourceIdentifier resourceId)
+        => ArmClient.GetGenericResource(resourceId);
 
-    public GenericResource ParseResourceId(string resourceId) {
-        return ArmClient.GetGenericResource(new ResourceIdentifier(resourceId));
-    }
+    public GenericResource ParseResourceId(string resourceId)
+        => ArmClient.GetGenericResource(new ResourceIdentifier(resourceId));
 
     public async Async.Task<GenericResource> GetData(GenericResource resource) {
         if (!resource.HasData) {

src/ApiService/ApiService/onefuzzlib/Secrets.cs

@@ -75,8 +75,7 @@ public async Task<Uri> StoreSecret(ISecret secret) {
 
     public Uri GetKeyvaultAddress() {
         // https://docs.microsoft.com/en-us/azure/key-vault/general/about-keys-secrets-certificates#vault-name-and-object-name
-        var keyvaultName = _config!.OneFuzzKeyvault;
-        return new Uri($"https://{keyvaultName}.vault.azure.net");
+        return new Uri($"https://{_config.OneFuzzKeyvault}.vault.azure.net");
     }