packages/serverless/lib/plugins/aws/package/lib/uses-dedicated-per-function-role.js (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace lodash with native JavaScript.

Use optional chaining and native object checks instead of _.get and _.isObject.

As per coding guidelines, “Prefer native JavaScript over lodash.”

Also applies to: 31-50

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/serverless/lib/plugins/aws/package/lib/uses-dedicated-per-function-role.js`
at line 1, Replace the lodash import and its usages in the
dedicated-per-function-role logic with native JavaScript: use optional chaining
for nested property access instead of _.get, and native object checks instead of
_.isObject. Preserve the existing behavior across the logic spanning the
referenced lines, then remove the unused lodash import.

Source: Coding guidelines

packages/serverless/lib/plugins/aws/package/lib/merge-iam-templates.js (1)

225-267: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the duplicated disableLogs-Deny construction into a helper.

The Deny-statement building logic (map function names → log-group ARNs → mergeStatements) here duplicates the existing block a few lines above (the hasOneOrMoreCanonicallyNamedFunctions branch). A future edit to one copy (e.g. Action list or ARN format) risks silently missing the other.

♻️ Proposed extraction

+    const emitDisableLogsDeny = (functionNames) => {
+      const arnOfDisableLogGroups = functionNames.map((fnName) => ({
+        'Fn::Sub':
+          'arn:${AWS::Partition}:logs:${AWS::Region}:${AWS::AccountId}' +
+          `:log-group:${this.provider.naming.getLogGroupName(fnName)}:*`,
+      }))
+      this.mergeStatements([
+        {
+          Effect: 'Deny',
+          Action: 'logs:PutLogEvents',
+          Resource: arnOfDisableLogGroups,
+        },
+      ])
+    }
+
     if (hasOneOrMoreCanonicallyNamedFunctions) {
       ...
       if (disableLogsCanonicallyNamedFunctions.length > 0) {
-        // add deny rule for disabled logs function
-        const arnOfDisableLogGroups = disableLogsCanonicallyNamedFunctions.map(...)
-        this.mergeStatements([...])
+        emitDisableLogsDeny(disableLogsCanonicallyNamedFunctions)
       }
     }
     ...
     if (skippedDedicatedLogContributions && ...) {
       ...
       if (disableLogsCanonicallyNamedFunctions.length > 0) {
-        const arnOfDisableLogGroups = disableLogsCanonicallyNamedFunctions.map(...)
-        this.mergeStatements([...])
+        emitDisableLogsDeny(disableLogsCanonicallyNamedFunctions)
       }
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/serverless/lib/plugins/aws/package/lib/merge-iam-templates.js`
around lines 225 - 267, Extract the duplicated disableLogs Deny construction
into a shared helper near the existing IAM merge logic, including mapping
function names to log-group ARNs and calling mergeStatements with the Deny
statement. Replace both the hasOneOrMoreCanonicallyNamedFunctions branch and the
skippedDedicatedLogContributions branch with calls to that helper, preserving
their existing conditions and behavior.

packages/serverless/test/unit/lib/plugins/aws/package/lib/per-function-iam-global-role.test.js (1)

59-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated real-pipeline test harness across two new sibling files. Both files independently define byte-identical buildService and runEventCompilers helpers (construct real Serverless+AwsProvider, wire functions, seed Lambda resources, run selected event compilers). One root cause: no shared test-utility module for this harness, risking future drift between the two copies.

• packages/serverless/test/unit/lib/plugins/aws/package/lib/per-function-iam-global-role.test.js#L59-L125: extract buildService/runEventCompilers into a shared test-helper module (e.g. alongside the existing test-lib utilities) and import it here.
• packages/serverless/test/unit/lib/plugins/aws/package/lib/iam-empty-collection-invariant.test.js#L72-L133: import the same shared helper module instead of redefining buildService/runEventCompilers locally.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/serverless/test/unit/lib/plugins/aws/package/lib/per-function-iam-global-role.test.js`
around lines 59 - 125, Extract the shared buildService and runEventCompilers
harness into a common test-helper module, preserving their current behavior and
dependencies. In
packages/serverless/test/unit/lib/plugins/aws/package/lib/per-function-iam-global-role.test.js
lines 59-125, remove the local definitions and import the helpers; do the same
in
packages/serverless/test/unit/lib/plugins/aws/package/lib/iam-empty-collection-invariant.test.js
lines 72-133, using the same shared module in both files.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/serverless/test/unit/lib/plugins/aws/package/lib/roles-per-function.test.js (1)

16-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shared test-fixture logic duplicated across two suites. FakeServerlessError, buildContext, and the isExistingRoleProvided mock are copy-pasted between both files, which both exercise the same dedicated-role-creation subsystem; keeping two independent copies risks the mocks drifting out of sync and masking real regressions in one suite but not the other.

• packages/serverless/test/unit/lib/plugins/aws/package/lib/roles-per-function.test.js#L16-L93: extract FakeServerlessError, buildContext, and isExistingRoleProvided into a shared test-helper module and import it here.
• packages/serverless/test/unit/lib/plugins/aws/package/lib/uses-dedicated-per-function-role.lockstep.test.js#L34-L111: import the same shared helper (extending it with mergeIamTemplatesMixin/utils.readFileSync wiring specific to this suite) instead of re-declaring the fixture.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/serverless/test/unit/lib/plugins/aws/package/lib/roles-per-function.test.js`
around lines 16 - 93, Extract FakeServerlessError, buildContext, and the
isExistingRoleProvided mock into one shared test-helper module. In
packages/serverless/test/unit/lib/plugins/aws/package/lib/roles-per-function.test.js
lines 16-93, import and use that helper instead of the duplicated fixture. In
packages/serverless/test/unit/lib/plugins/aws/package/lib/uses-dedicated-per-function-role.lockstep.test.js
lines 34-111, import the same helper and retain only the suite-specific
mergeIamTemplatesMixin and utils.readFileSync wiring.