Skip to content

Commit

Permalink
fix: updated compileCloudWatchLogGroup to not create new role when ro…
Browse files Browse the repository at this point in the history
…leArn is provided
  • Loading branch information
Lorenzohidalgo committed Dec 13, 2023
1 parent 05164d8 commit ab81575
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 8 deletions.
39 changes: 39 additions & 0 deletions src/__tests__/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,45 @@ describe('Api', () => {
}
`);
});

it('should compile CloudWatch Resources without additional IAM Role when logging roleARN is provided', () => {
const api = new Api(
given.appSyncConfig({
logging: {
level: 'ERROR',
retentionInDays: 14,
enabled: true,
roleArn: 'arn:',
},
}),
plugin,
);

expect(api.compileCloudWatchLogGroup()).toMatchInlineSnapshot(`
Object {
"GraphQlApiLogGroup": Object {
"Properties": Object {
"LogGroupName": Object {
"Fn::Join": Array [
"/",
Array [
"/aws/appsync/apis",
Object {
"Fn::GetAtt": Array [
"GraphQlApi",
"ApiId",
],
},
],
],
},
"RetentionInDays": 14,
},
"Type": "AWS::Logs::LogGroup",
},
}
`);
});
});

describe('apiKeys', () => {
Expand Down
23 changes: 15 additions & 8 deletions src/resources/Api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,9 @@ export class Api {
}

const logGroupLogicalId = this.naming.getLogGroupLogicalId();
const roleLogicalId = this.naming.getLogGroupRoleLogicalId();
const policyLogicalId = this.naming.getLogGroupPolicyLogicalId();
const apiLogicalId = this.naming.getApiLogicalId();

return {
const logGroupCF = {
[logGroupLogicalId]: {
Type: 'AWS::Logs::LogGroup',
Properties: {
Expand All @@ -133,10 +131,19 @@ export class Api {
],
},
RetentionInDays:
this.config.logging.retentionInDays ||
this.config.logging.retentionInDays ??
this.plugin.serverless.service.provider.logRetentionInDays,
},
},
};

if (this.config.logging.roleArn) return logGroupCF;

const roleLogicalId = this.naming.getLogGroupRoleLogicalId();
const policyLogicalId = this.naming.getLogGroupPolicyLogicalId();

return {
...logGroupCF,
[policyLogicalId]: {
Type: 'AWS::IAM::Policy',
Properties: {
Expand Down Expand Up @@ -418,10 +425,10 @@ export class Api {
AppIdClientRegex: auth.config.appIdClientRegex,
...(!isAdditionalAuth
? {
// Default action is the one passed in the config
// or 'ALLOW'
DefaultAction: auth.config.defaultAction || 'ALLOW',
}
// Default action is the one passed in the config
// or 'ALLOW'
DefaultAction: auth.config.defaultAction || 'ALLOW',
}
: {}),
};

Check failure on line 434 in src/resources/Api.ts

View workflow job for this annotation

GitHub Actions / tests (16)

Insert `··`

Check failure on line 434 in src/resources/Api.ts

View workflow job for this annotation

GitHub Actions / tests (18)

Insert `··`
Expand Down

0 comments on commit ab81575

Please sign in to comment.