Skip to content

Conversation

@d-padmanabhan
Copy link

Summary

Fixed a bug where security group egress rules were processed multiple times due to incorrect loop nesting. Also added standard repository configuration files (pre-commit hooks, linting, dependabot) to make contributing easier.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Configuration change
  • Build/CI change

Changes

Bug Fixes

  • Security group egress loop bug - The egress rules loop was nested inside the ingress loop in vpc.py, causing egress rules to be processed N times instead of once. Moved it to the correct scope.
  • Added missing imports (Table, asdict, get_theme_dir) that were causing runtime errors
  • Replaced bare except: with specific exception types (e.g., except ValueError:, except KeyError:)
  • Cleaned up unused imports and fixed empty f-strings
  • Renamed single-letter variable l to lis in test files for clarity

Repository Configuration

Added the following config files:

  • .gitattributes - line ending normalization
  • .editorconfig - consistent indentation/formatting
  • .markdownlint.yaml - markdown linting
  • .commitlintrc.yaml - conventional commit validation
  • .github/dependabot.yml - automated dependency updates
  • .github/CODEOWNERS - ownership for PR reviews
  • .github/PULL_REQUEST_TEMPLATE.md - PR template

Updated:

  • .pre-commit-config.yaml - added name: fields, shellcheck, markdownlint hooks
  • .gitignore - added pyright cache, coverage files

Housekeeping

  • Prefixed unused variables with _ to indicate intentional non-use
  • Added allowlist entries for test fixture AWS IDs in secrets baseline
  • Set executable bit on shell scripts
  • Fixed trailing whitespace in markdown files

Testing

  • Unit tests pass (pytest tests/)
  • Pre-commit hooks pass (pre-commit run --all-files)

Test Commands Run

pytest tests/ -v
pre-commit run --all-files

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

Security Considerations

  • No hardcoded secrets or credentials
  • No sensitive data logged or exposed
  • AWS credentials handled securely

Additional Notes

Commits included:

  • 5acd3a8 fix(core): fix critical bugs and code quality issues
  • 24a9c25 chore(repo): add repository scaffolding and configuration files

The security group bug was the main driver for this PR - wanted to get that fix in along with some repo hygiene.

- Fix security group egress loop incorrectly nested inside ingress loop in vpc.py
- Add missing imports (Table, asdict, get_theme_dir) causing runtime errors
- Replace bare except clauses with specific exception types
- Remove unused imports and fix f-strings without placeholders
- Fix ambiguous variable names (l -> lis) in test files
- Mark intentionally unused variables with underscore prefix
- Mark test fixture AWS example IDs as allowlisted secrets
- Make scripts with shebangs executable

Fixes security group data processing bug that caused egress rules
to be processed N times (once per ingress rule) instead of once.
- Add .gitattributes for line ending normalization and binary handling
- Add .editorconfig for consistent formatting across editors
- Add .markdownlint.yaml for markdown linting rules
- Add .commitlintrc.yaml for conventional commit enforcement
- Add .github/dependabot.yml for automated dependency updates
- Add .github/CODEOWNERS for code ownership assignments
- Add .github/PULL_REQUEST_TEMPLATE.md for standardized PR descriptions
- Update .pre-commit-config.yaml with named hooks, shellcheck, markdownlint
- Update .gitignore with pyright cache and coverage file patterns
- Auto-fix markdown files (trailing whitespace, newlines)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant