From f7f91df622112b7e4f642aa94e7d0e3553e630d3 Mon Sep 17 00:00:00 2001 From: Noah McGregor Harper <74685766+nharper285@users.noreply.github.com> Date: Tue, 13 Sep 2022 08:55:40 -0700 Subject: [PATCH] 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. --- .../ApiService/Functions/InstanceConfig.cs | 80 +++++++++++++++++++ .../ApiService/OneFuzzTypes/Model.cs | 10 +-- .../ApiService/OneFuzzTypes/Requests.cs | 4 + .../ApiService/az-local.settings.json | 2 +- ...{InstanceConfig.cs => ConfigOperations.cs} | 14 ++-- .../onefuzzlib/EndpointAuthorization.cs | 2 +- .../ApiService/onefuzzlib/ProxyOperations.cs | 8 +- src/ApiService/IntegrationTests/NodeTests.cs | 8 +- src/ApiService/Tests/OrmModelsTest.cs | 23 +++--- 9 files changed, 120 insertions(+), 31 deletions(-) create mode 100644 src/ApiService/ApiService/Functions/InstanceConfig.cs rename src/ApiService/ApiService/onefuzzlib/{InstanceConfig.cs => ConfigOperations.cs} (84%) diff --git a/src/ApiService/ApiService/Functions/InstanceConfig.cs b/src/ApiService/ApiService/Functions/InstanceConfig.cs new file mode 100644 index 000000000..effef93cd --- /dev/null +++ b/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 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 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 Post(HttpRequestData req) { + _log.Info($"attempting instance_config update"); + var request = await RequestHandling.ParseRequest(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; + } +} diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index fae2e51d6..77c4f4435 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/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, - bool AllowPoolManagement = true, + string ProxyVmSku = "Standard_B2s", + bool RequireAdminPrivileges = false, IDictionary? ApiAccessRules = null, IDictionary? GroupMembership = null, IDictionary? VmTags = null, - IDictionary? VmssTags = null, - bool? RequireAdminPrivileges = null + IDictionary? VmssTags = 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? CheckAdmins(List? value) { @@ -725,7 +724,6 @@ public record WorkUnit( Guid JobId, Guid TaskId, TaskType TaskType, - // JSON-serialized `TaskUnitConfig`. [property: JsonConverter(typeof(TaskUnitConfigConverter))] TaskUnitConfig Config ); diff --git a/src/ApiService/ApiService/OneFuzzTypes/Requests.cs b/src/ApiService/ApiService/OneFuzzTypes/Requests.cs index adccbfa9a..320470be7 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Requests.cs +++ b/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; diff --git a/src/ApiService/ApiService/az-local.settings.json b/src/ApiService/ApiService/az-local.settings.json index 2d7a2f73c..8f6ae1e0b 100644 --- a/src/ApiService/ApiService/az-local.settings.json +++ b/src/ApiService/ApiService/az-local.settings.json @@ -4,4 +4,4 @@ "FUNCTIONS_WORKER_RUNTIME": "dotnet-isolated", "linux_fx_version": "DOTNET-ISOLATED|6.0" } -} +} \ No newline at end of file diff --git a/src/ApiService/ApiService/onefuzzlib/InstanceConfig.cs b/src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs similarity index 84% rename from src/ApiService/ApiService/onefuzzlib/InstanceConfig.cs rename to src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs index c2706b58d..001cc7f0f 100644 --- a/src/ApiService/ApiService/onefuzzlib/InstanceConfig.cs +++ b/src/ApiService/ApiService/onefuzzlib/ConfigOperations.cs @@ -31,29 +31,27 @@ public class ConfigOperations : Orm, 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(config)); + await _context.Events.SendEvent(new EventInstanceConfigUpdated(newConfig)); } } diff --git a/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs b/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs index 82b82574d..3cdb7875f 100644 --- a/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs +++ b/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, diff --git a/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs b/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs index c76719d46..8080e1122 100644 --- a/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs @@ -231,12 +231,18 @@ public class ProxyOperations : StatefulOrm, 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, diff --git a/src/ApiService/IntegrationTests/NodeTests.cs b/src/ApiService/IntegrationTests/NodeTests.cs index 69da45061..4265113e8 100644 --- a/src/ApiService/IntegrationTests/NodeTests.cs +++ b/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(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] diff --git a/src/ApiService/Tests/OrmModelsTest.cs b/src/ApiService/Tests/OrmModelsTest.cs index d86072bc0..3dbc03cbe 100644 --- a/src/ApiService/Tests/OrmModelsTest.cs +++ b/src/ApiService/Tests/OrmModelsTest.cs @@ -188,26 +188,27 @@ namespace Tests { } public static Gen InstanceConfig() { - return Arb.Generate, - Tuple?, IDictionary?, IDictionary?, IDictionary?>>>().Select( + var config = Arb.Generate>, + Tuple?, IDictionary?, IDictionary?, IDictionary?>>>().Select( arg => new InstanceConfig( InstanceName: arg.Item1.Item1, Admins: arg.Item1.Item2, - AllowPoolManagement: arg.Item1.Item3, - AllowedAadTenants: arg.Item1.Item4, - NetworkConfig: arg.Item1.Item5, - ProxyNsgConfig: arg.Item1.Item6, - Extensions: arg.Item1.Item7, + AllowedAadTenants: arg.Item1.Item3, + NetworkConfig: arg.Item1.Item4, + ProxyNsgConfig: arg.Item1.Item5, + Extensions: arg.Item1.Item6, + 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() { @@ -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 }); }