Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Commit

Permalink
Expand valid scaleset names (#3045)
Browse files Browse the repository at this point in the history 
Scaleset names are now permitted to be any (valid) strings, instead of only GUIDs. When we generate a scaleset name it is now based upon the pool name; for example the pool `pool` might get a scaleset named `pool-3b24ba211cad4b078655914754485838`.

This should be backwards-compatible since GUIDs are [already serialized to table storage as strings](https://github.com/microsoft/onefuzz/blob/dddcfa49490e44834afc9a6910c350698022792c/src/ApiService/ApiService/onefuzzlib/orm/EntityConverter.cs#L190-L191), so this simply loosens the restrictions placed upon them.

Scaleset IDs now have a strong type in the same way as other IDs; this helps to avoid mixing them up with other strings. Because of this I found one bug in the scaleset search query logic due to Pool ID/VMSS ID confusion. As part of fixing this I've changed the scaleset search query to only return nodes from the table rather than querying Azure to find a list; this seems to be sufficient for the CLI.
  • Loading branch information
@Porges
Porges authored May 16, 2023
1 parent d84b72b commit 2f478d6
Show file tree
Hide file tree
Showing 38 changed files with 380 additions and 291 deletions.
28 changes: 5 additions & 23 deletions docs/webhook_events.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand Down Expand Up @@ -1369,7 +1368,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand Down Expand Up @@ -1429,7 +1427,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand Down Expand Up @@ -1487,7 +1484,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand Down Expand Up @@ -2597,7 +2593,7 @@ If webhook is set to have Event Grid message format then the payload will look a
"image": "Canonical:0001-com-ubuntu-server-focal:20_04-lts:latest",
"pool_name": "example",
"region": "eastus",
"scaleset_id": "00000000-0000-0000-0000-000000000000",
"scaleset_id": "example-000",
"size": 10,
"vm_sku": "Standard_D2s_v3"
}
Expand All @@ -2621,7 +2617,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand Down Expand Up @@ -2654,7 +2649,7 @@ If webhook is set to have Event Grid message format then the payload will look a
```json
{
"pool_name": "example",
"scaleset_id": "00000000-0000-0000-0000-000000000000"
"scaleset_id": "example-000"
}
```

Expand All @@ -2668,7 +2663,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -2695,7 +2689,7 @@ If webhook is set to have Event Grid message format then the payload will look a
]
},
"pool_name": "example",
"scaleset_id": "00000000-0000-0000-0000-000000000000"
"scaleset_id": "example-000"
}
```

Expand Down Expand Up @@ -2764,7 +2758,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -2786,7 +2779,7 @@ If webhook is set to have Event Grid message format then the payload will look a
```json
{
"pool_name": "example",
"scaleset_id": "00000000-0000-0000-0000-000000000000",
"scaleset_id": "example-000",
"size": 0
}
```
Expand All @@ -2801,7 +2794,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand All @@ -2827,7 +2819,7 @@ If webhook is set to have Event Grid message format then the payload will look a
```json
{
"pool_name": "example",
"scaleset_id": "00000000-0000-0000-0000-000000000000",
"scaleset_id": "example-000",
"state": "init"
}
```
Expand Down Expand Up @@ -2857,7 +2849,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand Down Expand Up @@ -5658,7 +5649,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -5682,7 +5672,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -5709,7 +5698,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -5733,7 +5721,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand Down Expand Up @@ -5926,7 +5913,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand Down Expand Up @@ -5957,7 +5943,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -5979,7 +5964,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
}
Expand All @@ -5999,7 +5983,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand All @@ -6023,7 +6006,6 @@ If webhook is set to have Event Grid message format then the payload will look a
"type": "string"
},
"scaleset_id": {
"format": "uuid",
"title": "Scaleset Id",
"type": "string"
},
Expand Down
2 changes: 1 addition & 1 deletion src/ApiService/ApiService/Functions/Proxy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ private async Async.Task<HttpResponseData> Get(HttpRequestData req) {

var proxyGet = request.OkV;
switch ((proxyGet.ScalesetId, proxyGet.MachineId, proxyGet.DstPort)) {
case (Guid scalesetId, Guid machineId, int dstPort):
case (ScalesetId scalesetId, Guid machineId, int dstPort):
var scaleset = await _context.ScalesetOperations.GetById(scalesetId);
if (!scaleset.IsOk) {
return await _context.RequestHandling.NotOk(req, scaleset.ErrorV, "ProxyGet");
Expand Down
4 changes: 2 additions & 2 deletions src/ApiService/ApiService/Functions/Scaleset.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private async Task<HttpResponseData> Post(HttpRequestData req) {
}

var scaleset = new Service.Scaleset(
ScalesetId: Guid.NewGuid(),
ScalesetId: Service.Scaleset.GenerateNewScalesetId(create.PoolName),
State: ScalesetState.Init,
NeedsConfigUpdate: false,
Auth: await Auth.BuildAuth(_log),
Expand Down Expand Up @@ -206,7 +206,7 @@ private async Task<HttpResponseData> Get(HttpRequestData req) {
}

var search = request.OkV;
if (search.ScalesetId is Guid id) {
if (search.ScalesetId is ScalesetId id) {
var scalesetResult = await _context.ScalesetOperations.GetById(id);
if (!scalesetResult.IsOk) {
return await _context.RequestHandling.NotOk(req, scalesetResult.ErrorV, "ScalesetSearch");
Expand Down
18 changes: 9 additions & 9 deletions src/ApiService/ApiService/OneFuzzTypes/Events.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ Guid PingId

[EventType(EventType.ScalesetCreated)]
public record EventScalesetCreated(
Guid ScalesetId,
ScalesetId ScalesetId,
PoolName PoolName,
string VmSku,
string Image,
Expand All @@ -195,23 +195,23 @@ public record EventScalesetCreated(

[EventType(EventType.ScalesetFailed)]
public sealed record EventScalesetFailed(
Guid ScalesetId,
ScalesetId ScalesetId,
PoolName PoolName,
Error Error
) : BaseEvent();


[EventType(EventType.ScalesetDeleted)]
public record EventScalesetDeleted(
Guid ScalesetId,
ScalesetId ScalesetId,
PoolName PoolName

) : BaseEvent();


[EventType(EventType.ScalesetResizeScheduled)]
public record EventScalesetResizeScheduled(
Guid ScalesetId,
ScalesetId ScalesetId,
PoolName PoolName,
long size
) : BaseEvent();
Expand Down Expand Up @@ -267,14 +267,14 @@ VmState State
[EventType(EventType.NodeCreated)]
public record EventNodeCreated(
Guid MachineId,
Guid? ScalesetId,
ScalesetId? ScalesetId,
PoolName PoolName
) : BaseEvent();

[EventType(EventType.NodeHeartbeat)]
public record EventNodeHeartbeat(
Guid MachineId,
Guid? ScalesetId,
ScalesetId? ScalesetId,
PoolName PoolName,
NodeState state
) : BaseEvent();
Expand All @@ -283,23 +283,23 @@ NodeState state
[EventType(EventType.NodeDeleted)]
public record EventNodeDeleted(
Guid MachineId,
Guid? ScalesetId,
ScalesetId? ScalesetId,
PoolName PoolName,
NodeState? MachineState
) : BaseEvent();


[EventType(EventType.ScalesetStateUpdated)]
public record EventScalesetStateUpdated(
Guid ScalesetId,
ScalesetId ScalesetId,
PoolName PoolName,
ScalesetState State
) : BaseEvent();

[EventType(EventType.NodeStateUpdated)]
public record EventNodeStateUpdated(
Guid MachineId,
Guid? ScalesetId,
ScalesetId? ScalesetId,
PoolName PoolName,
NodeState state
) : BaseEvent();
Expand Down
46 changes: 33 additions & 13 deletions src/ApiService/ApiService/OneFuzzTypes/Model.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
using Endpoint = System.String;
Expand Down Expand Up @@ -107,7 +108,7 @@ public record Node
// a string internally.
string? InstanceId = null,

Guid? ScalesetId = null,
ScalesetId? ScalesetId = null,

bool ReimageRequested = false,
bool DeleteRequested = false,
Expand All @@ -132,7 +133,7 @@ public record ProxyForward
(
[PartitionKey] Region Region,
[RowKey] long Port,
Guid ScalesetId,
ScalesetId ScalesetId,
Guid MachineId,
Guid? ProxyId,
long DstPort,
Expand Down Expand Up @@ -263,7 +264,7 @@ string EventType

public record NodeAssignment(
Guid NodeId,
Guid? ScalesetId,
ScalesetId? ScalesetId,
NodeTaskState State
);

Expand Down Expand Up @@ -392,7 +393,7 @@ public ResultVoid<List<string>> CheckInstanceConfig() {
}

public record AutoScale(
[PartitionKey, RowKey] Guid ScalesetId,
[PartitionKey, RowKey] ScalesetId ScalesetId,
long Min,
long Max,
long Default,
Expand All @@ -402,15 +403,10 @@ public record AutoScale(
long ScaleInCooldown
) : EntityBase;

public record ScalesetNodeState(
Guid MachineId,
string InstanceId,
NodeState? State
);

public record Scaleset(
public partial record Scaleset(
[PartitionKey] PoolName PoolName,
[RowKey] Guid ScalesetId,
[RowKey] ScalesetId ScalesetId,
ScalesetState State,
string VmSku,
ImageReference Image,
Expand All @@ -425,7 +421,31 @@ public record Scaleset(
Guid? ClientId = null,
Guid? ClientObjectId = null
// 'Nodes' removed when porting from Python: only used in search response
) : StatefulEntityBase<ScalesetState>(State);
) : StatefulEntityBase<ScalesetState>(State) {

[GeneratedRegex(@"[^a-zA-Z0-9\-]+")]
private static partial Regex InvalidCharacterRegex();

public static ScalesetId GenerateNewScalesetId(PoolName poolName)
=> GenerateNewScalesetIdUsingGuid(poolName, Guid.NewGuid());

public static ScalesetId GenerateNewScalesetIdUsingGuid(PoolName poolName, Guid guid) {
// poolnames permit underscores but not scaleset names; use hyphen instead:
var name = poolName.ToString().Replace("_", "-");

// since poolnames are not actually validated, take only the valid characters:
name = InvalidCharacterRegex().Replace(name, "");

// trim off any starting and ending dashes:
name = name.Trim('-');

// this should now be a valid name; generate a unique suffix:
// max length is 64; length of Guid in "N" format is 32, -1 for the hyphen
name = name[..Math.Min(64 - 32 - 1, name.Length)] + "-" + guid.ToString("N");

return ScalesetId.Parse(name);
}
}

public record Notification(
[PartitionKey] Guid NotificationId,
Expand Down Expand Up @@ -733,7 +753,7 @@ List<WorkUnitSummary> WorkUnits
);

public record ScalesetSummary(
Guid ScalesetId,
ScalesetId ScalesetId,
ScalesetState State
);

Expand Down
Loading

0 comments on commit 2f478d6

Please sign in to comment.