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

aws-iam: service principal name mismatch in CN partition #31767

Closed
1 task done
jy19 opened this issue Oct 15, 2024 · 7 comments · Fixed by #31793
Closed
1 task done

aws-iam: service principal name mismatch in CN partition #31767

jy19 opened this issue Oct 15, 2024 · 7 comments · Fixed by #31793
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. potential-regression Marking this issue as a potential regression to be checked by team member

Comments

@jy19
Copy link

jy19 commented Oct 15, 2024

Describe the bug

hey folks, we have a CDK stack that currently create an IAM role that does this:

assumedBy: new ServicePrincipal("elasticmapreduce.amazonaws.com"),
(this uses iam.ServicePrincipal)

when we upgrade our CDK version, infrastructure that uses this role starts failing being able to create EMR clusters in CN partition because we noticed that the role changes from elasticmapreduce.amazonaws.com.cn to elasticmapreduce.amazonaws.com. this seems related to this CDK change that removes "deprecated SP mappings" . i am trying to work around this by creating a temporary mapping for EMR, so i explicitly specify the endpoint like so:
assumedBy: new ServicePrincipal("elasticmapreduce.amazonaws.com.cn"),

but when i run cdk diff against my CN stack i see this:

 [-]       "Service": {
 [-]         "Fn::Join": [
 [-]           "",
 [-]           [
 [-]             "elasticmapreduce.",
 [-]             {
 [-]               "Ref": "AWS::URLSuffix"
 [-]             }
 [-]           ]
 [-]         ]
 [-]       }
 [+]       "Service": "elasticmapreduce.amazonaws.com"

why does it ignore the name? i see in the cdk file it says the format should still be supported.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

the cdk version in our CN regions is ~2.130.0, we are trying to upgrade to ~2.150.0

Expected Behavior

I expected that specifying elasticmapreduce.amazonaws.com.cn on ServicePrincipal to put that same string into the created iam role.

Current Behavior

Specifying elasticmapreduce.amazonaws.com.cn on ServicePrincipal gets translated to elasticmapreduce.amazonaws.com in the iam role.

Reproduction Steps

repro steps:
create an iam role with trust relationship to service principal elasticmapreduce.amazonaws.com.cn in a CN region.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.158.0

Framework Version

No response

Node.js Version

^20.11.22

OS

amazon linux

Language

TypeScript

Language Version

^5.3.3

Other information

No response

@jy19 jy19 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2024
@github-actions github-actions bot added @aws-cdk/aws-iam Related to AWS Identity and Access Management potential-regression Marking this issue as a potential regression to be checked by team member labels Oct 15, 2024
@khushail khushail added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2024
@jy19
Copy link
Author

jy19 commented Oct 15, 2024

actually - based on this - should I be moving to use new ServicePrincipal(ServicePrincipal.servicePrincipalName('elasticmapreduce.amazonaws.com.cn') instead

@khushail
Copy link
Contributor

@jy19 , thanks for reaching out.

Parsing through the recent CDK Changes, it seems that this change-

https:/aws/aws-cdk/blob/7d6bf773d3a8f17d94c4aa5d5aa9025270c254aa/packages/aws-cdk-lib/region-info/lib/region-info.ts#L124C1-L132C4

I am not able to repro this due to access rights but sure, please give it a try and see if that works.

@jy19
Copy link
Author

jy19 commented Oct 15, 2024

i changed the call pattern to:

assumedBy: new ServicePrincipal(
        ServicePrincipal.servicePrincipalName('elasticmapreduce.amazonaws.com.cn')
      ),

but unfortunately still see the same issue

@khushail khushail added p0 and removed p1 labels Oct 15, 2024
@khushail
Copy link
Contributor

Internal Ticket reference- V1549524507

@mrgrain
Copy link
Contributor

mrgrain commented Oct 16, 2024

Irrespectively of the observed failure, the service principal appears to be documented as elasticmapreduce.amazonaws.com even for CN regions:

https://docs.amazonaws.cn/en_us/emr/latest/ManagementGuide/emr-iam-role.html
https://docs.amazonaws.cn/en_us/emr/latest/ManagementGuide/emr-iam-roles.html

@pahud pahud added p0 and removed p0 labels Oct 16, 2024
@moelasmar
Copy link
Contributor

Thanks @jy19 for raising this issue. Unfortunately, I am not able to reproduce this issue. This is the sample application I am using to reproduce this issue:

class EmrServicePrincipalTestStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const subnetId = process.env.SUBNET_ID;
   
    const emrServiceRole = new iam.Role(this, 'EMRServiceRole', {
      assumedBy: new iam.ServicePrincipal('elasticmapreduce.amazonaws.com'),
      managedPolicies: [
        iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonElasticMapReduceRole'),
      ],
    });

    const emrJobFlowRole = new iam.Role(this, 'EMRJobFlowRole', {
      assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'),
      managedPolicies: [
        iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonElasticMapReduceforEC2Role'),
      ],
    });

    const emrJobFlowProfile = new iam.CfnInstanceProfile(this, 'EMRJobFlowProfile', {
      roles: [emrJobFlowRole.roleName],
      instanceProfileName: 'EMRJobFlowProfile_',
    });

    const sshKey = new ec2.CfnKeyPair(this, 'SSHKey', {
      keyName: 'TestingSSHKey',
    });

    new emr.CfnCluster(this, 'EMRCluster', {
      name: 'My first cluster',
      instances: {
        coreInstanceGroup: {
          instanceCount: 1,
          instanceType: 'm5.xlarge',
        },
        ec2SubnetId: subnetId,
        hadoopVersion: 'Amazon',
        keepJobFlowAliveWhenNoSteps: false,
        ec2KeyName: sshKey.ref,
        terminationProtected: false,
        masterInstanceGroup: {
          instanceCount: 1,
          instanceType: 'm5.xlarge',
        },
      },
      jobFlowRole: emrJobFlowProfile.ref,
      applications: [
        { name: 'Spark' },
      ],
      serviceRole: emrServiceRole.roleName,
      releaseLabel: 'emr-6.4.0',
    });
  }
}

const app = new cdk.App();
const testingStack = new EmrServicePrincipalTestStack(app, 'EmrServicePrincipalTestingStack2', {
  env: {
    region: 'cn-north-1',
  },
});

and this is the CFN template for the generated service role:

EMRServiceRole2CF24E74:
  Type: AWS::IAM::Role
  Properties:
    AssumeRolePolicyDocument:
      Statement:
      - Action: sts:AssumeRole
        Effect: Allow
        Principal:
          Service: elasticmapreduce.amazonaws.com
      Version: '2012-10-17'
    ManagedPolicyArns:
    - Fn::Join:
      - ''
      - - 'arn:'
        - Ref: AWS::Partition
        - ":iam::aws:policy/service-role/AmazonElasticMapReduceRole"

The stack created successfully, and the EMR cluster started, created EC2 instances, and then terminated successfully.

Could you please retry the deployment from your side, and if it is still failing, please share the code of your CDK app so we can replicate the issue.

@moelasmar moelasmar added needs-reproduction This issue needs reproduction. and removed p0 labels Oct 17, 2024
@mergify mergify bot closed this as completed in #31793 Oct 17, 2024
mergify bot pushed a commit that referenced this issue Oct 17, 2024
### Issue # (if applicable)

Closes #31767

### Reason for this change
To add a function that allow customers to create ServicePrinciple construct using custom name as an escape hatch if some service is using principle name that does not follow the IAM recommended pattern which is `<service>.amazonaws.com`

### Checklist
- [X] My code adheres to the [CONTRIBUTING GUIDE](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https:/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management bug This issue is a bug. effort/small Small work item – less than a day of effort needs-reproduction This issue needs reproduction. potential-regression Marking this issue as a potential regression to be checked by team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants