Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

fix: add iam:CreateServiceLinkedRole #12

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mpgo13
Copy link
Contributor

@mpgo13 mpgo13 commented Mar 22, 2019

See issue #7

@fquffio
Copy link
Contributor

fquffio commented Mar 22, 2019

Hi @mpgo13 ! Thanks (again) for your contributions!

I'm not sure about the approach of this solution… 🤔 I know that iam:CreateServiceLinkedRole is a relatively safe permission, but I'd prefer not to grant it to an EC2 instance anyway.

How about creating a service-linked role in the CloudFormation template? (see docs)

ServiceLinkedRole:
    Type: 'AWS::IAM::ServiceLinkedRole'
    Properties:
        AWSServiceName: !Sub 'ec2.${AWS::URLSuffix}' # TODO: check if this is correct!!
        CustomSuffix: !Sub '-${AWS::StackName}'
        Description: !Sub 'Service-Linked Role for EC2 Spot Instances - ${AWS::StackName}'
    Condition: 'UseSpotInstances'

Or, at least, grant that permission in EC2 Instance Profile only if UseSpotInstances is true. 🙃

@mpgo13
Copy link
Contributor Author

mpgo13 commented Mar 23, 2019

Good point @fquffio , I'll have a look at it.

@mpgo13
Copy link
Contributor Author

mpgo13 commented Mar 24, 2019

This worked now for me :) The fix will create the service linked role and cloudformation will automatically attach the managed policy AWSEC2SpotServiceRolePolicy to it.

@mpgo13
Copy link
Contributor Author

mpgo13 commented Mar 24, 2019

Tested creating two stacks and got:

SLR [AWSServiceRoleForEC2Spot] already exists but has a different description: [Service-Linked Role for EC2 Spot Instances - gitlab-runner] Please verify your SLR use case. If you are sure the use case is correct please modify your CloudFormation template and keep SLR description consistent.

@fquffio fquffio self-assigned this Mar 25, 2019
@fquffio fquffio added the bug Something isn't working label Mar 25, 2019
@fquffio
Copy link
Contributor

fquffio commented Mar 25, 2019

Thanks a lot, @mpgo13! I'll run a couple of tests today!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants