feat:[PEA-358] Allow k8s lambda to assume 'ro' role#2399
Conversation
WalkthroughThe IAM assume-role policy in ChangesIAM Policy Update
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@installer/resources/iam/all_read_role.py`:
- Line 13: The role ARN assembly in the class body references
Settings.ACCOUNT_ID and Settings.STACK_ENV before Settings is imported, which
will cause a NameError during module load. Add the missing module-level import
for Settings in all_read_role.py before the class definition that builds the
ARN, and keep the reference in the policy string unchanged so policy generation
can complete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 60244640-1d30-4846-87b3-4b94280b77aa
📒 Files selected for processing (1)
installer/resources/iam/all_read_role.py
| 'identifiers': [ECSRole.get_output_attr('arn')] | ||
| 'identifiers': [ | ||
| ECSRole.get_output_attr('arn'), | ||
| "arn:aws:iam::" + Settings.ACCOUNT_ID + ":role/" + Settings.STACK_ENV + "-svc-plugin-k8s-containers-role" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the canonical Settings definition/import pattern, then add the matching import
# to installer/resources/iam/all_read_role.py.
rg -n --type py -C2 'class Settings\b|Settings\s*=' .
rg -n --type py -C2 'from .* import .*Settings|import .*Settings' .Repository: PaladinCloud/CE
Length of output: 29629
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and the closest matching IAM resource for import patterns.
sed -n '1,120p' installer/resources/iam/all_read_role.py
printf '\n--- base_role.py ---\n'
sed -n '1,80p' installer/resources/iam/base_role.py
printf '\n--- ecs_role.py ---\n'
sed -n '1,80p' installer/resources/iam/ecs_role.py
# Show exact references to Settings in the target file, if any.
printf '\n--- Settings refs in all_read_role.py ---\n'
rg -n 'Settings|ECSRole|principals|identifiers' installer/resources/iam/all_read_role.pyRepository: PaladinCloud/CE
Length of output: 9701
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the reviewed file and dependency files to determine whether Settings is imported.
for f in installer/resources/iam/all_read_role.py installer/resources/iam/base_role.py installer/resources/iam/ecs_role.py; do
echo "--- $f ---"
cat -n "$f"
echo
done
# Check whether the target file imports Settings directly or via wildcard.
echo "--- import lines in all_read_role.py ---"
rg -n '^(from|import) ' installer/resources/iam/all_read_role.pyRepository: PaladinCloud/CE
Length of output: 15408
Import Settings before using it here.
Settings.ACCOUNT_ID and Settings.STACK_ENV are referenced without a module-level import, so this class body will raise NameError during import and block policy generation.
🧰 Tools
🪛 Ruff (0.15.20)
[error] 13-13: Undefined name Settings
(F821)
[error] 13-13: Undefined name Settings
(F821)
🤖 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 `@installer/resources/iam/all_read_role.py` at line 13, The role ARN assembly
in the class body references Settings.ACCOUNT_ID and Settings.STACK_ENV before
Settings is imported, which will cause a NameError during module load. Add the
missing module-level import for Settings in all_read_role.py before the class
definition that builds the ARN, and keep the reference in the policy string
unchanged so policy generation can complete.
Source: Linters/SAST tools
1 participant
Add the k8s lambda role to the
rorole, allowing the k8s lambda to assume thePaladinCloudIntegrationRolein the remote account, which is needed to get a valid K8s token.Summary by CodeRabbit