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

Allow option to not inject region into iam.ServicePrincipal's Principal String #2999

Closed
1 of 5 tasks
KingOfPoptart opened this issue Jun 21, 2019 · 10 comments
Closed
1 of 5 tasks
Assignees
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management feature-request A feature should be added or improved.

Comments

@KingOfPoptart
Copy link
Contributor

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

  • I'm submitting a ...

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    If the current behavior is a 🪲bug🪲: Please provide the steps to reproduce

When using new iam.ServicePrincipal() - the CloudFormation that is output injects the region into the Principal and there is no option to disable this behavior.

# Create a service principal, point it to "codedeploy.amazonaws.com"
new iam.Role(this, 'IamRoleWithServicePrincipal', {
    assumedBy: new iam.ServicePrincipal('codedeploy.amazonaws.com'),
    managedPolicyArns: ['arn:aws:iam::aws:policy/service-role/AWSCodeDeployRole',
        'arn:aws:iam::aws:policy/AWSCodeDeployRoleForECS'],
    roleName: 'myrole'
});


# This is what gets output from cdk synth - Note that `Ref: AWS::Region` 
# gets included as part of the Service Principal
Resources:
  myroleD153DA9E:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service:
                Fn::Join:
                  - ""
                  - - codedeploy.
                    - Ref: AWS::Region
                    - "."
                    - Ref: AWS::URLSuffix
        Version: "2012-10-17"
      ManagedPolicyArns:
        - arn:aws:iam::aws:policy/service-role/AWSCodeDeployRole
        - arn:aws:iam::aws:policy/AWSCodeDeployRoleForECS
      RoleName: myrole
  • What is the expected behavior (or behavior of feature suggested)?

There should be an optional parameter to not inject the region into the principal in the properties passed into new iam.ServicePrincipal().

  • What is the motivation / use case for changing the behavior or adding this feature?

Sometimes the console ignores roles with this region set in specific scenarios (certain CodeDeploy stuff). It is also useful to give devs the option to have this flexibility

  • Please tell us about your environment:

    • CDK CLI Version: 0.35.0
    • Module Version: 0.35.0
    • OS: OSX
    • Language: TypeScript
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

This expands on the discussion here #2622

@NGL321
Copy link
Contributor

NGL321 commented Jun 21, 2019

Thank you for converting this. It has been received and noted!

We are currently pausing work on most new feature requests and community PRs for a few weeks while we are working on increasing stability and tuning to meet our consistency guidelines, however feel free to continue the conversation here in the interim!

@NGL321 NGL321 added feature-request A feature should be added or improved. gap @aws-cdk/aws-iam Related to AWS Identity and Access Management labels Jun 21, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Jan 23, 2020

I'm going to close this, as something you shouldn't need to want to do. You are asking to implement your solution, rather than describe your problem.

Please provide an example of a use case that's broken with the current behavior, and we will make the appropriate changes to unbreak you.

@rix0rrr rix0rrr closed this as completed Jan 23, 2020
@james-portman
Copy link

Please re-open this.
as per the original message, it seems impossible to specify a service principal without it being messed with.
In my work I wanted to specify "logs.amazonaws.com" but it is inserting the region after "logs"

@james-portman
Copy link

It seems like you are just going out of your way to be awkward and close issues.
It seems really obvious from the original post what the issue was, even if it was not worded in the way you like.
It is strange that you are telling people what they should want to do or not

@james-portman
Copy link

Here is an example of why it's an issue, if you don't believe the two of us,
https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/enable-access-logs.html
In this example AWS specify to use "delivery.logs.amazonaws.com" as the service principal, NOT something like "delivery.logs.eu-west-1.amazonaws.com" with the region in

@melnikalex
Copy link

would like to re-open this as well, or get any suggestions for workarounds

@aidandeno
Copy link

This is still an issue and affects other services like SSM

@danielenricocahall
Copy link

Just following up here for anyone who may find this page - a feature flag was added in 2.51.0 which controls the injection of region in the ServicePrincipal.

@james-portman
Copy link

5 years is a solid turnaround, I wonder why people hate CDK so much..

@avoidik
Copy link

avoidik commented Aug 14, 2024

the flag in question is:

{
    "@aws-cdk/aws-iam:standardizedServicePrincipals": true
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

8 participants