CSharp Refactor - Instance Config Endpoint (#2347)

* CSharp Refactor - Instance Config Endpoint

* Finshing config update.

* Formatting.

* Formatting.

* formatting.

* Fixing encoding.

* Fixing config references.

* Fixing refs.

* Trying location.

* Trying ref to location.

* Passing nsg.

* Passing nsg.

* Setting nsg to not null.

* Fixing ok reference.

* Adding Instance Config Response.

* Setting required attribute.

* Adding route specifier.

* Formatting.

* Fixing route.

* Fixing optionals.

* Trying to set default

* Trying again.

* Setting require admins

* Removing optioanl.

* Testing with instancename.

* Updating instanceconfig model.

* Updating instance config response.

* Formatting.

* Removing AllowPoolManagement.

* Readding.

* Removing arg.

* Replacing with RequireAdminPrivs.

* Fix orm test.

* Setting requireadminprivs to true.

* Requiring admin privs.

* Fix formatting.

* fix test.

* Fixing.

* Changing error message.

* Changing.

* Reordering test args.

* Flipping.

* Fixing args.

* Fixing again.

* Removing false.

* Removing from constructor.

* Setting.

* Setting string to optional.

* Formatting.

* Adding default value.

* PUshing changes to OrmModelsTest

* Updating test to not pass null.

* George's suggestions.

* Removing entityconverter changes.

* Fixing import.

src/ApiService/ApiService/Functions/InstanceConfig.cs

@@ -0,0 +1,80 @@
+using System.Net;
+using System.Threading.Tasks;
+using Microsoft.Azure.Functions.Worker;
+using Microsoft.Azure.Functions.Worker.Http;
+
+namespace Microsoft.OneFuzz.Service.Functions;
+
+public class InstanceConfig {
+    private readonly ILogTracer _log;
+    private readonly IEndpointAuthorization _auth;
+    private readonly IOnefuzzContext _context;
+
+    public InstanceConfig(ILogTracer log, IEndpointAuthorization auth, IOnefuzzContext context) {
+        _log = log;
+        _auth = auth;
+        _context = context;
+    }
+
+    [Function("InstanceConfig")]
+    public Async.Task<HttpResponseData> Run(
+        [HttpTrigger(AuthorizationLevel.Anonymous, "GET", "POST", Route = "instance_config")] HttpRequestData req) {
+        return _auth.CallIfUser(req, r => r.Method switch {
+            "GET" => Get(r),
+            "POST" => Post(r),
+            _ => throw new InvalidOperationException("Unsupported HTTP method"),
+        });
+    }
+    public async Async.Task<HttpResponseData> Get(HttpRequestData req) {
+        _log.Info($"getting instance_config");
+        var config = await _context.ConfigOperations.Fetch();
+
+        var response = req.CreateResponse(HttpStatusCode.OK);
+        await response.WriteAsJsonAsync(config);
+        return response;
+    }
+
+    public async Async.Task<HttpResponseData> Post(HttpRequestData req) {
+        _log.Info($"attempting instance_config update");
+        var request = await RequestHandling.ParseRequest<InstanceConfigUpdate>(req);
+
+        if (!request.IsOk) {
+            return await _context.RequestHandling.NotOk(
+                req,
+                request.ErrorV,
+                context: "instance_config update");
+        }
+        var (config, answer) = await (
+            _context.ConfigOperations.Fetch(),
+            _auth.CheckRequireAdmins(req));
+        if (!answer.IsOk) {
+            return await _context.RequestHandling.NotOk(req, answer.ErrorV, "instance_config update");
+        }
+        var updateNsg = false;
+        if (request.OkV.config.ProxyNsgConfig is NetworkSecurityGroupConfig requestConfig
+            && config.ProxyNsgConfig is NetworkSecurityGroupConfig currentConfig) {
+            if (!requestConfig.AllowedServiceTags.ToHashSet().SetEquals(currentConfig.AllowedServiceTags)
+                || !requestConfig.AllowedIps.ToHashSet().SetEquals(currentConfig.AllowedIps)) {
+                updateNsg = true;
+            }
+        }
+        await _context.ConfigOperations.Save(request.OkV.config, false, false);
+        if (updateNsg) {
+            await foreach (var nsg in _context.NsgOperations.ListNsgs()) {
+                _log.Info($"Checking if nsg: {nsg.Data.Location!} ({nsg.Data.Name}) owned by OneFuzz");
+                if (nsg.Data.Location! == nsg.Data.Name) {
+                    var result = await _context.NsgOperations.SetAllowedSources(new Nsg(nsg.Data.Location!, nsg.Data.Location!), request.OkV.config.ProxyNsgConfig!);
+                    if (!result.IsOk) {
+                        return await _context.RequestHandling.NotOk(
+                        req,
+                        result.ErrorV,
+                        context: "instance_config update");
+                    }
+                }
+            }
+        }
+        var instanceConfigResponse = req.CreateResponse(HttpStatusCode.OK);
+        await instanceConfigResponse.WriteAsJsonAsync(request.OkV.config);
+        return instanceConfigResponse;
+    }
+}

src/ApiService/ApiService/OneFuzzTypes/Model.cs

@@ -325,13 +325,12 @@ public record InstanceConfig
     [DefaultValue(InitMethod.DefaultConstructor)] NetworkConfig NetworkConfig,
     [DefaultValue(InitMethod.DefaultConstructor)] NetworkSecurityGroupConfig ProxyNsgConfig,
     AzureVmExtensionConfig? Extensions,
-    string ProxyVmSku,
+    string ProxyVmSku = "Standard_B2s",
-    bool AllowPoolManagement = true,
+    bool RequireAdminPrivileges = false,
     IDictionary<Endpoint, ApiAccessRule>? ApiAccessRules = null,
     IDictionary<PrincipalId, GroupId[]>? GroupMembership = null,
     IDictionary<string, string>? VmTags = null,
-    IDictionary<string, string>? VmssTags = null,
+    IDictionary<string, string>? VmssTags = null
-    bool? RequireAdminPrivileges = null
 ) : EntityBase() {
     public InstanceConfig(string instanceName) : this(
         instanceName,
@@ -341,7 +340,7 @@ public record InstanceConfig
         new NetworkSecurityGroupConfig(),
         null,
         "Standard_B2s",
-        true
+        false
         ) { }
 
     public static List<Guid>? CheckAdmins(List<Guid>? value) {
@@ -725,7 +724,6 @@ public record WorkUnit(
     Guid JobId,
     Guid TaskId,
     TaskType TaskType,
-
     // JSON-serialized `TaskUnitConfig`.
     [property: JsonConverter(typeof(TaskUnitConfigConverter))] TaskUnitConfig Config
 );

src/ApiService/ApiService/OneFuzzTypes/Requests.cs

@@ -286,3 +286,7 @@ public record WebhookUpdate(
     string? SecretToken,
     WebhookMessageFormat? MessageFormat
 ) : BaseRequest;
+
+public record InstanceConfigUpdate(
+    [property: Required] InstanceConfig config
+) : BaseRequest;

src/ApiService/ApiService/az-local.settings.json

@@ -4,4 +4,4 @@
       "FUNCTIONS_WORKER_RUNTIME": "dotnet-isolated",
       "linux_fx_version": "DOTNET-ISOLATED|6.0"
     }
-}
+}

src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs

@@ -31,29 +31,27 @@ public class ConfigOperations : Orm<InstanceConfig>, IConfigOperations {
         });
 
     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") };
         ResultVoid<(int, string)> r;
         if (isNew) {
-            r = await Insert(config);
+            r = await Insert(newConfig);
             if (!r.IsOk) {
                 _log.WithHttpStatus(r.ErrorV).Error($"Failed to save new instance config record");
             }
         } else if (requireEtag && config.ETag.HasValue) {
-            r = await Update(config);
+            r = await Update(newConfig);
             if (!r.IsOk) {
                 _log.WithHttpStatus(r.ErrorV).Error($"Failed to update instance config record");
             }
         } else {
-            r = await Replace(config);
+            r = await Replace(newConfig);
             if (!r.IsOk) {
                 _log.WithHttpStatus(r.ErrorV).Error($"Failed to replace instance config record");
             }
         }
-
         if (r.IsOk) {
-            _cache.Set(_key, config);
+            _cache.Set(_key, newConfig);
         }
-
+        await _context.Events.SendEvent(new EventInstanceConfigUpdated(newConfig));
-        await _context.Events.SendEvent(new EventInstanceConfigUpdated(config));
     }
 }

src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs

@@ -127,7 +127,7 @@ public class EndpointAuthorization : IEndpointAuthorization {
 
             return new Error(
                 Code: ErrorCode.UNAUTHORIZED,
-                Errors: new string[] { "not authorized to manage pools" });
+                Errors: new string[] { "not authorized to manage instance" });
         } else {
             return new Error(
                 Code: ErrorCode.UNAUTHORIZED,

src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs

@@ -231,12 +231,18 @@ public class ProxyOperations : StatefulOrm<Proxy, VmState, ProxyOperations>, IPr
 
     public static Vm GetVm(Proxy proxy, InstanceConfig config) {
         var tags = config.VmssTags;
+        var proxyVmSku = "";
         const string PROXY_IMAGE = "Canonical:UbuntuServer:18.04-LTS:latest";
+        if (config.ProxyVmSku == null) {
+            proxyVmSku = "Standard_B2s";
+        } else {
+            proxyVmSku = config.ProxyVmSku;
+        }
         return new Vm(
             // name should be less than 40 chars otherwise it gets truncated by azure
             Name: $"proxy-{proxy.ProxyId:N}",
             Region: proxy.Region,
-            Sku: config.ProxyVmSku,
+            Sku: proxyVmSku,
             Image: PROXY_IMAGE,
             Auth: proxy.Auth,
             Tags: tags,

src/ApiService/IntegrationTests/NodeTests.cs

@@ -155,7 +155,9 @@ public abstract class NodeTestBase : FunctionTestBase {
     public async Async.Task RequiresAdmin(string method) {
         // config must be found
         await Context.InsertAll(
-            new InstanceConfig(Context.ServiceConfiguration.OneFuzzInstanceName!));
+            new InstanceConfig(Context.ServiceConfiguration.OneFuzzInstanceName!) {
+                RequireAdminPrivileges = true
+            });
 
         // must be a user to auth
         var auth = new TestEndpointAuthorization(RequestType.User, Logger, Context);
@@ -243,7 +245,7 @@ public abstract class NodeTestBase : FunctionTestBase {
         // config specifies that a different user is admin
         await Context.InsertAll(
             new InstanceConfig(Context.ServiceConfiguration.OneFuzzInstanceName!) {
-                Admins = new[] { otherObjectId }
+                Admins = new[] { otherObjectId }, RequireAdminPrivileges = true
             });
 
         // must be a user to auth
@@ -260,7 +262,7 @@ public abstract class NodeTestBase : FunctionTestBase {
 
         var err = BodyAs<Error>(result);
         Assert.Equal(ErrorCode.UNAUTHORIZED, err.Code);
-        Assert.Contains("not authorized to manage pools", err.Errors?.Single());
+        Assert.Contains("not authorized to manage instance", err.Errors?.Single());
     }
 
     [Theory]

src/ApiService/Tests/OrmModelsTest.cs

@@ -188,26 +188,27 @@ namespace Tests {
         }
 
         public static Gen<InstanceConfig> InstanceConfig() {
-            return Arb.Generate<Tuple<
+            var config = Arb.Generate<Tuple<
-                Tuple<string, Guid[]?, bool, string[], NetworkConfig, NetworkSecurityGroupConfig, AzureVmExtensionConfig?>,
+                Tuple<string, Guid[]?, string[], NetworkConfig, NetworkSecurityGroupConfig, AzureVmExtensionConfig?, NonNull<string>>,
-                Tuple<string, IDictionary<string, ApiAccessRule>?, IDictionary<Guid, Guid[]>?, IDictionary<string, string>?, IDictionary<string, string>?>>>().Select(
+                Tuple<bool, IDictionary<string, ApiAccessRule>?, IDictionary<Guid, Guid[]>?, IDictionary<string, string>?, IDictionary<string, string>?>>>().Select(
                 arg =>
                     new InstanceConfig(
                         InstanceName: arg.Item1.Item1,
                         Admins: arg.Item1.Item2,
-                        AllowPoolManagement: arg.Item1.Item3,
+                        AllowedAadTenants: arg.Item1.Item3,
-                        AllowedAadTenants: arg.Item1.Item4,
+                        NetworkConfig: arg.Item1.Item4,
-                        NetworkConfig: arg.Item1.Item5,
+                        ProxyNsgConfig: arg.Item1.Item5,
-                        ProxyNsgConfig: arg.Item1.Item6,
+                        Extensions: arg.Item1.Item6,
-                        Extensions: arg.Item1.Item7,
+                        ProxyVmSku: arg.Item1.Item7.Item,
 
-                        ProxyVmSku: arg.Item2.Item1,
+                        RequireAdminPrivileges: arg.Item2.Item1,
                         ApiAccessRules: arg.Item2.Item2,
                         GroupMembership: arg.Item2.Item3,
                         VmTags: arg.Item2.Item4,
                         VmssTags: arg.Item2.Item5
                     )
-            );
+                );
+            return config;
         }
 
         public static Gen<Task> Task() {
@@ -682,7 +683,7 @@ namespace Tests {
         [Property]
         void Replay()
         {
-            var seed = FsCheck.Random.StdGen.NewStdGen(515508280, 297027790);
+            var seed = FsCheck.Random.StdGen.NewStdGen(610100457,297085446);
             var p = Prop.ForAll((InstanceConfig x) => InstanceConfig(x) );
             p.Check(new Configuration { Replay = seed });
         }