Skip to content

Commit

Permalink
fix(codeguruprofiler): incorrect profiling group name is returned whe…
Browse files Browse the repository at this point in the history
…n using importing (#22554)

Closes #22546. Verified via TDD with new unit test. Before the change, the unit test failed, replicating the bug in the issue with output:

```plain
FAIL test/profiling-group.test.js
  profiling group
    ✓ attach read permission to Profiling group via fromProfilingGroupArn (71 ms)
    ✓ attach publish permission to Profiling group via fromProfilingGroupName (27 ms)
    ✕ use name specified via fromProfilingGroupName (8 ms)
    ✓ default profiling group (21 ms)
    ✓ allows setting its ComputePlatform (14 ms)
    ✓ default profiling group without name (13 ms)
    ✓ default profiling group without name when name exceeding limit is generated (18 ms)
    ✓ grant publish permissions profiling group (25 ms)
    ✓ grant read permissions profiling group (25 ms)

  ● profiling group › use specified via name via fromProfilingGroupName

    expect(received).toEqual(expected) // deep equality

    Expected: "MyAwesomeProfilingGroup"
    Received: "profilingGroup"

      174 |
      175 |     const profilingGroup = ProfilingGroup.fromProfilingGroupName(stack, 'MyProfilingGroup', 'MyAwesomeProfilingGroup');
    > 176 |     expect(profilingGroup.profilingGroupName).toEqual('MyAwesomeProfilingGroup');
          |                                               ^
      177 |   });
      178 |
      179 |   test('default profiling group', () => {

      at Object.<anonymous> (test/profiling-group.test.ts:176:47)
```

----

### All Submissions:

* [Yes] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [No] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [N/A] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
openwebsolns authored Oct 19, 2022
1 parent 9139ca9 commit 9934619
Show file tree
Hide file tree
Showing 13 changed files with 1,665 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export class ProfilingGroup extends ProfilingGroupBase {
*/
public static fromProfilingGroupArn(scope: Construct, id: string, profilingGroupArn: string): IProfilingGroup {
class Import extends ProfilingGroupBase {
public readonly profilingGroupName = Stack.of(scope).splitArn(profilingGroupArn, ArnFormat.SLASH_RESOURCE_NAME).resource;
public readonly profilingGroupName = Stack.of(scope).splitArn(profilingGroupArn, ArnFormat.SLASH_RESOURCE_NAME).resourceName!;
public readonly profilingGroupArn = profilingGroupArn;
}

Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-codeguruprofiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.2"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { AccountRootPrincipal, Role } from '@aws-cdk/aws-iam';
import { App, CfnOutput, Stack } from '@aws-cdk/core';
import { ExpectedResult, IntegTest } from '@aws-cdk/integ-tests';
import { ProfilingGroup } from '../lib';

const app = new App();

const stack = new Stack(app, 'ProfilingGroupTestStack');

const profilingGroup1 = new ProfilingGroup(stack, 'ProfilingGroupWithExplicitlySetName', {
profilingGroupName: 'ExplicitlySetName',
});
const profilingGroup2 = new ProfilingGroup(stack, 'ProfilingGroupWithImplicitlySetName');

const publishAppRole = new Role(stack, 'PublishAppRole', {
assumedBy: new AccountRootPrincipal(),
});
profilingGroup1.grantPublish(publishAppRole);
profilingGroup2.grantPublish(publishAppRole);

const importedGroupWithExplicitlySetName = ProfilingGroup.fromProfilingGroupName(
stack,
'ImportedProfilingGroupWithExplicitlySetName',
profilingGroup1.profilingGroupName,
);

const importedGroupWithImplicitlySetName = ProfilingGroup.fromProfilingGroupName(
stack,
'ImportedProfilingGroupWithImplicitlySetName',
profilingGroup2.profilingGroupName,
);

new CfnOutput(stack, 'ExplicitlySetProfilingGroupName', {
value: importedGroupWithExplicitlySetName.profilingGroupName,
});

new CfnOutput(stack, 'ImplicitlySetProfilingGroupName', {
value: importedGroupWithImplicitlySetName.profilingGroupName,
});

const testCase = new IntegTest(app, 'test', {
testCases: [stack],
});

const describe = testCase.assertions.awsApiCall('CloudFormation', 'describeStacks', {
StackName: 'ProfilingGroupTestStack',
});

describe.assertAtPath('Stacks.0.Outputs.0.OutputKey', ExpectedResult.stringLikeRegexp('ExplicitlySetProfilingGroupName'));
describe.assertAtPath('Stacks.0.Outputs.0.OutputValue', ExpectedResult.stringLikeRegexp('ExplicitlySetName'));

describe.assertAtPath('Stacks.0.Outputs.1.OutputKey', ExpectedResult.stringLikeRegexp('ImplicitlySetProfilingGroupName'));
describe.assertAtPath('Stacks.0.Outputs.1.OutputValue', ExpectedResult.stringLikeRegexp('ProfilingGroupTestStackProfilingGroupWithImplicitlySetName98463923'));

app.synth();
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "21.0.0",
"files": {
"49d6a3151509f39124c2f82b21cf55a8a1364289fce8b6f8b764af6e204c6647": {
"source": {
"path": "ProfilingGroupTestStack.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "49d6a3151509f39124c2f82b21cf55a8a1364289fce8b6f8b764af6e204c6647.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
{
"Resources": {
"ProfilingGroupWithExplicitlySetNameProfilingGroup20552EAE": {
"Type": "AWS::CodeGuruProfiler::ProfilingGroup",
"Properties": {
"ProfilingGroupName": "ExplicitlySetName"
}
},
"ProfilingGroupWithImplicitlySetNameProfilingGroup21CDF1FC": {
"Type": "AWS::CodeGuruProfiler::ProfilingGroup",
"Properties": {
"ProfilingGroupName": "ProfilingGroupTestStackProfilingGroupWithImplicitlySetName98463923"
}
},
"PublishAppRole9FEBD682": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"AWS": {
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":iam::",
{
"Ref": "AWS::AccountId"
},
":root"
]
]
}
}
}
],
"Version": "2012-10-17"
}
}
},
"PublishAppRoleDefaultPolicyCA1E15C3": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": [
"codeguru-profiler:ConfigureAgent",
"codeguru-profiler:PostAgentProfile"
],
"Effect": "Allow",
"Resource": [
{
"Fn::GetAtt": [
"ProfilingGroupWithExplicitlySetNameProfilingGroup20552EAE",
"Arn"
]
},
{
"Fn::GetAtt": [
"ProfilingGroupWithImplicitlySetNameProfilingGroup21CDF1FC",
"Arn"
]
}
]
}
],
"Version": "2012-10-17"
},
"PolicyName": "PublishAppRoleDefaultPolicyCA1E15C3",
"Roles": [
{
"Ref": "PublishAppRole9FEBD682"
}
]
}
}
},
"Outputs": {
"ExplicitlySetProfilingGroupName": {
"Value": {
"Ref": "ProfilingGroupWithExplicitlySetNameProfilingGroup20552EAE"
}
},
"ImplicitlySetProfilingGroupName": {
"Value": {
"Ref": "ProfilingGroupWithImplicitlySetNameProfilingGroup21CDF1FC"
}
}
},
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
Loading

0 comments on commit 9934619

Please sign in to comment.