Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
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.

Sign up for GitHub

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

Jump to bottom

Azure role assignment name needs to be GUID #121

Closed
carterwilliamson opened this issue Sep 21, 2018 · 4 comments
Closed

Azure role assignment name needs to be GUID #121

carterwilliamson opened this issue Sep 21, 2018 · 4 comments
Assignees
@lukehoban
Milestone
0.18

Comments

@carterwilliamson
Copy link

When trying to assign a role to a service principal, the name needs to be a GUID.

The following code -

 let roleAssignment = new azure.role.assignment("ra", {
      principalId: servicePrincipal.id,
      roleDefinitionName: "Contributor",
      scope: `/subscriptions/${azure.config.subscriptionId}`
})

Will return a 400 error from azure saying that the name must be a valid GUID.

I tested with the uuid package and it worked, however this will trigger a change every time pulumi up is ran.

import * as uuid from "uuid"
let role = new azure.role.assignment("ra", {
     name: uuid.v4(),
     principalId: _sp.id,
     roleDefinitionName: "Contributor",
     scope: `/subscriptions/${azure.config.subscriptionId}`
})
@lukehoban
Copy link
Member

It would be nice to have something like new pulumi.Uuid() which persisted the UUID as a resource. This is similar to pulumi/pulumi#381.

I believe this can technically be implemented already today with Dynamic Providers. cc @pgavlin who may have an example of this.

But we should also consider adding a simpler API for managing some state in the checkpoint file that is initialized from a callback.

@lukehoban lukehoban added this to the 0.18 milestone Sep 24, 2018
@lukehoban lukehoban assigned lukehoban and unassigned pgavlin Sep 24, 2018
@mpp-oliverh
Copy link

mpp-oliverh commented Sep 26, 2018 •
edited
Loading

There's some general weirdness with this, I've created some static GUIDs but pulumi seems to get confused still.

I was initially creating the role assignment like this:

const mppreleaseRoleAssignment = new azure.role.assignment("k8s-role", {
    principalId: adServicePrincipal.id,
    scope: config.mpprelease.then(x => x.id),
    roleDefinitionName: "Reader"
});

error: Plan apply failed: authorization.RoleAssignmentsClient#Create: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned
an error. Status=400 Code="InvalidRoleAssignmentId" Message="The role assignment ID 'k8s-role129594be' is not valid. The role assignment ID must be a GUID."

So I specified an empty GUID as follows (taken from a terraform example), which worked:

const mppreleaseRoleAssignment = new azure.role.assignment("k8s-role", {
    name: "00000000-0000-0000-0000-000000000000",
    principalId: adServicePrincipal.id,
    scope: config.mpprelease.then(x => x.id),
    roleDefinitionName: "Reader"
});

But when it came to granting a second permission I changed it to the following.

const mppreleaseRoleAssignment = new azure.role.assignment("mpprelease", {
    name: "00000000-0000-0000-0000-000000000000",
    principalId: adServicePrincipal.id,
    scope: config.mpprelease.then(x => x.id),
    roleDefinitionName: "Reader"
});

const mppbuildRoleAssignment = new azure.role.assignment("mppbuild", {
    name: "00000000-0000-0000-0000-000000000000",
    principalId: adServicePrincipal.id,
    scope: config.mppbuild.then(x => x.id),
    roleDefinitionName: "Reader"
});

azure:role:assignment: mppbuild
error: Plan apply failed: authorization.RoleAssignmentsClient#Create: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned
an error. Status=400 Code="RoleAssignmentUpdateNotPermitted" Message="Tenant ID, application ID, principal ID, and scope are not allowed to be updated."

I believe the two roles were conflicting because of the same GUID so pulumi was granting the 'mpprelease' one, and then attempting to change the role assignment to 'mppbuild' despite the fact the pulumi name was different.

At this point I tried creating static GUIDs :

const mppreleaseGuid = "70db97a6-d823-461d-befb-d06c79f93241";
const mppreleaseRoleAssignment = new azure.role.assignment("mpprelease", {
    name: mppreleaseGuid,
    principalId: adServicePrincipal.id,
    scope: config.mpprelease.then(x => x.id),
    roleDefinitionName: "Reader"
});

const mppbuildGuid = "9b819003-7439-45af-985f-45da1e939270";
const mppbuildRoleAssignment = new azure.role.assignment("mppbuild", {
    name: mppbuildGuid,
    principalId: adServicePrincipal.id,
    scope: config.mppbuild.then(x => x.id),
    roleDefinitionName: "Reader"
});

error: Plan apply failed: authorization.RoleAssignmentsClient#Create: Failure responding to request: StatusCode=409 -- Original Error: autorest/azure: Service returned
an error. Status=409 Code="RoleAssignmentExists" Message="The role assignment already exists."

Pulumi's trying to add the new 'mpprelease' role before deleting the previous one ('k8s-role' from above) which grants the same privileges because it seems to be unaware that the same role can't be granted twice even with different names or GUIDs.

@lukehoban lukehoban mentioned this issue Oct 1, 2018
Closed
@lukehoban
Copy link
Member

Looks like the underlying provider was already auto-UUID-ing these name properties - but the Pulumi provider was overwriting that with it's own default. #126 is a fix that would avoid doing that.

@lukehoban
Copy link
Member

Fixed with #127.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukehoban lukehoban

Labels
None yet
Projects
None yet
Milestone
0.18
Development

Successfully merging a pull request may close this issue.

Don't auto-name when 'name' is optional pulumi/pulumi-azure
4 participants
@lukehoban @pgavlin @carterwilliamson @mpp-oliverh