Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Example: SIG with Image Definition and Role Assignment #1251

Merged
merged 3 commits into from
Jan 3, 2021
Merged

New Example: SIG with Image Definition and Role Assignment #1251

merged 3 commits into from
Jan 3, 2021

Conversation

fberson
Copy link
Contributor

@fberson fberson commented Jan 3, 2021

New Example: Shared Image Gallery with Image Definition and Role Assignment

…ignment

New Example: Shared Image Gallery with Image Defintition and Role Assignment
@codecov-io
Copy link

codecov-io commented Jan 3, 2021

Codecov Report

Merging #1251 (0d666de) into main (700770b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1251   +/-   ##
=======================================
  Coverage   94.49%   94.49%           
=======================================
  Files         336      336           
  Lines       16835    16835           
  Branches       14       14           
=======================================
  Hits        15908    15908           
  Misses        927      927           
Flag Coverage Δ
dotnet 95.04% <ø> (ø)
typescript 27.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

param principalID string
param templateImageResourceGroup string

var assignableScopes = '/subscriptions/${azureSubscriptionID}/resourcegroups/${templateImageResourceGroup}'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is just a single value, I would say assignableScope or possibly something like imageResourceGroupId to be more specific.

I'm assuming that this resourceGroup could be in a different rg than the target of the deployment? Otherwise, you could use resourceGroup().id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense! I changed it to templateImageResourceGroupId to match the existing parameter templateImageResourceGroup.


//create role assignment
resource galleryass 'Microsoft.Authorization/roleAssignments@2020-04-01-preview' = {
name: guid(resourceGroup().id)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guid() function needs some more arguments. If I did another role assignment to the same rg with different properties and used only the rg id as an argument, I would get a conflict. I'd recommend adding the roleDefinition id and principal id:

guid(resourceGroup().id, gallerydef.id, principalId)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated!

Copy link
Collaborator

@alex-frankel alex-frankel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small tweaks

Processed suggested changes
Now also updating the transpiled json result
@alex-frankel alex-frankel merged commit f998860 into Azure:main Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants