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

fix(secretsmanager): hosted rotation with fromSecretNameV2() does not create correct iam policy #28379

Merged
merged 9 commits into from
Dec 20, 2023
Merged

fix(secretsmanager): hosted rotation with fromSecretNameV2() does not create correct iam policy #28379

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
85ba900
fix(secretsmanager): hosted rotation does not generate correct policy…
go-to-k Dec 15, 2023
c7c0b9c
Merge branch 'main' of https://github.com/go-to-k/aws-cdk into fix/se…
go-to-k Dec 15, 2023
f82667a
modify for fromArn methods
go-to-k Dec 15, 2023
0f7b8da
wip
go-to-k Dec 15, 2023
97586f8
snapshot
go-to-k Dec 15, 2023
e00eae0
Merge branch 'main' into fix/secret-hosted-schedule-from-method
go-to-k Dec 16, 2023
efd98b6
change test names
go-to-k Dec 16, 2023
e6911ab
change test names again
go-to-k Dec 16, 2023
f359c12
Merge branch 'main' into fix/secret-hosted-schedule-from-method
paulhcsun Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ...es/@aws-cdk-testing/framework-integ/test/aws-secretsmanager/test/integ.hosted-rotation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ class TestStack extends cdk.Stack {
hostedRotation: secretsmanager.HostedRotation.mysqlSingleUser(),
rotateImmediatelyOnUpdate: false,
});

const mySecret = new secretsmanager.Secret(this, 'MySecret');
const masterSecret = new secretsmanager.Secret(this, 'MasterSecret');
const importedSecret = secretsmanager.Secret.fromSecretNameV2(this, 'MasterSecretImported', masterSecret.secretName);
mySecret.addRotationSchedule('RotationSchedule', {
hostedRotation: secretsmanager.HostedRotation.postgreSqlMultiUser({
masterSecret: importedSecret,
}),
});
}
}

Expand Down
9 changes: 8 additions & 1 deletion packages/aws-cdk-lib/aws-secretsmanager/lib/rotation-schedule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,14 +325,21 @@ export class HostedRotation implements ec2.IConnectable {
this.masterSecret.denyAccountRootDelete();
}

let masterSecretArn: string | undefined;
if (this.masterSecret?.secretFullArn) {
masterSecretArn = this.masterSecret.secretArn;
} else if (this.masterSecret) { // ISecret as an imported secret with partial ARN
masterSecretArn = this.masterSecret.secretArn + '-??????';
}
Comment on lines +329 to +333
Copy link
Contributor Author

@go-to-k go-to-k Dec 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • if (this.masterSecret?.secretFullArn)
    • If we call a secret construct for this.masterSecret in the stack, here is true.
    • Otherwise this.masterSecret is generated from fromSecretArn(), fromSecretCompleteArn() methods, etc. They also have a full ARN, so here is also true.
      • This if statement will be true even if the arg (so it is the arn) for the methods is a token or not.
  • else if (this.masterSecret)
    • The secret is ISecret as an imported secret with partial ARN. It is generated from fromSecretNameV2().
    • The secretArn in this block will be a token. But processing this string is not a problem because the CloudFormation template can use Fn::Join to combine them properly. (see the unit tests.)


const defaultExcludeCharacters = Secret.isSecret(secret)
? secret.excludeCharacters ?? DEFAULT_PASSWORD_EXCLUDE_CHARS
: DEFAULT_PASSWORD_EXCLUDE_CHARS;

return {
rotationType: this.type.name,
kmsKeyArn: secret.encryptionKey?.keyArn,
masterSecretArn: this.masterSecret?.secretArn,
masterSecretArn: masterSecretArn,
masterSecretKmsKeyArn: this.masterSecret?.encryptionKey?.keyArn,
rotationLambdaName: this.props.functionName,
vpcSecurityGroupIds: this._connections?.securityGroups?.map(s => s.securityGroupId).join(','),
Expand Down
59 changes: 59 additions & 0 deletions packages/aws-cdk-lib/aws-secretsmanager/test/rotation-schedule.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,65 @@ describe('hosted rotation', () => {
},
});
});

test('generate correct IAM policy for masterSecret as an imported secret with full arn', () => {
// GIVEN
const secret = new secretsmanager.Secret(stack, 'Secret');
const importedSecret = secretsmanager.Secret.fromSecretCompleteArn(stack, 'MasterSecretImported', 'arn:aws:secretsmanager:us-east-1:123456789012:secret:MySecret-123456');

// WHEN
secret.addRotationSchedule('RotationSchedule', {
hostedRotation: secretsmanager.HostedRotation.postgreSqlMultiUser({
masterSecret: importedSecret,
}),
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::SecretsManager::RotationSchedule', {
SecretId: {
Ref: 'SecretA720EF05',
},
HostedRotationLambda: {
MasterSecretArn: 'arn:aws:secretsmanager:us-east-1:123456789012:secret:MySecret-123456',
},
});
});

test('generate correct IAM policy for masterSecret as an imported secret with partial arn', () => {
// GIVEN
const secret = new secretsmanager.Secret(stack, 'Secret');
const importedSecret = secretsmanager.Secret.fromSecretNameV2(stack, 'MasterSecretImported', 'MySecret');

// WHEN
secret.addRotationSchedule('RotationSchedule', {
hostedRotation: secretsmanager.HostedRotation.postgreSqlMultiUser({
masterSecret: importedSecret,
}),
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::SecretsManager::RotationSchedule', {
SecretId: {
Ref: 'SecretA720EF05',
},
HostedRotationLambda: {
MasterSecretArn: {
'Fn::Join': [
'',
[
'arn:',
{ Ref: 'AWS::Partition' },
':secretsmanager:',
{ Ref: 'AWS::Region' },
':',
{ Ref: 'AWS::AccountId' },
':secret:MySecret-??????',
],
],
},
},
});
});
});

describe('manual rotations', () => {
Expand Down
Loading