initial checkin for Azure.Provisioning.CloudMachine (work in progress) #46077
Conversation
sdk/provisioning/Azure.Provisioning.CloudMachine/src/CloudMachineInfrastructure.cs
Show resolved
Hide resolved
sdk/provisioning/Azure.Provisioning.CloudMachine/src/CloudMachineInfrastructure.cs
Outdated
Show resolved
Hide resolved
|
/azp run net - provisioning |
|
Azure Pipelines failed to run 1 pipeline(s). |
christothes
force-pushed
the
chriss/cm_infra
branch
from
4e4e3f1
to
1a7e1b6
Compare
|
/azp run net - provisioning |
|
Azure Pipelines failed to run 1 pipeline(s). |
| /// <summary> | ||
| /// The common principalId parameter. | ||
| /// </summary> | ||
| public BicepParameter PrincipalIdParameter => new BicepParameter("principalId", typeof(string)); |
There was a problem hiding this comment.
Why are these public while everything else is private? Just a point in time thing or ...?
|
|
||
| public CloudMachineInfrastructure(string? name = "cm") : base(name!) | ||
| { | ||
| _name = name ?? "cm"; |
There was a problem hiding this comment.
It feels a little weird to make it nullable but give it a default but then add an explicit override here. I think this would be cleaner with a non-nullable param that defaults to "cm".
| { | ||
| _name = name ?? "cm"; | ||
| _identity = new($"{_name}_identity"); | ||
| var managedServiceIdentity = new ManagedServiceIdentity |
There was a problem hiding this comment.
Style nit - most of this file is doing Foo f = new() rather than var f = new Foo().
| var managedServiceIdentity = new ManagedServiceIdentity | ||
| { | ||
| ManagedServiceIdentityType = ManagedServiceIdentityType.UserAssigned, | ||
| UserAssignedIdentities = { { BicepFunction.Interpolate($"{_identity!.Id}").Compile().ToString(), new UserAssignedIdentityDetails() } } |
There was a problem hiding this comment.
Do you need the ! in _identity! after instantiating it above?
| _serviceBusTopic_main = new($"{_name}_sb_topic_main", "2021-11-01") | ||
| { | ||
| Parent = _serviceBusNamespace, | ||
| // Name = "default", |
There was a problem hiding this comment.
Why is this commented out? I don't know if I'm aware of whatever's blocking this.
| Add(_storage.AssignRole(StorageBuiltInRole.StorageBlobDataContributor, RoleManagementPrincipalType.User, PrincipalIdParameter)); | ||
| Add(_storage.AssignRole(StorageBuiltInRole.StorageTableDataContributor, RoleManagementPrincipalType.User, PrincipalIdParameter)); | ||
| Add(_container); | ||
| Add(_blobs); |
There was a problem hiding this comment.
It might be better to parent this to the storage account here to prepare for when things open up.
| Add(_eventGridTopic_Blobs); | ||
|
|
||
| // Placeholders for now. | ||
| Add(new BicepOutput($"storage_name", typeof(string)) { Value = _storage.Name }); |
There was a problem hiding this comment.
Do you want the name or the primary endpoint like _storage.PrimaryEndpoints.Value!.BlobUri?
| @@ -11,6 +11,7 @@ trigger: | |||
| - sdk/provisioning/ci.data.yml | |||
| - sdk/provisioning/Azure.Provisioning | |||
| - sdk/provisioning/Azure.Provisioning.AppConfiguration | |||
| - sdk/provisioning/Azure.Provisioning.CloudMachine | |||
There was a problem hiding this comment.
Can you do me a favor and move this into ci.compute.yml instead? I'm trying to keep the pipelines balanced because I'm worried about blowing out azdo again with how big this one is getting.
There was a problem hiding this comment.
(Or consider adding your own release pipeline since I don't think this will be going out with everything else.)
Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.