Skip to content

Conversation

@alexchristy
Copy link
Member

@alexchristy alexchristy commented Jul 25, 2025

Checklist

  • I have added a version label to my PR (e.g., patch, minor, major).
  • I have tested my changes locally and verified they work as expected.
  • I have added relevant tests to cover my changes.
  • I have made any necessary updates to the documentation.
  • I have made any necessary updates to the CLI.
  • I have made any necessary updates to the frontend.

Description

This PR builds on the solution in PR #139 by creating a function that generates unique logical ids for resources by normalizing them (like before) and then appending suffixes to prevent name collisions. For example, the names My Subnet and my subnet --> my-subnet and my-subnet-1 which would collide before.

Additionally, this PR improves the normalization function to create kebab case names for better readability and cross cloud compatibility.

Fixes known issue 1 in: #139

@alexchristy alexchristy added patch Increment the patch version when merged backend Related to the OpenLabs backend/API labels Jul 25, 2025
@alexchristy alexchristy marked this pull request as ready for review July 25, 2025 20:43
Copilot AI review requested due to automatic review settings July 25, 2025 20:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses normalized name collisions by implementing a robust collision handling system for resource logical IDs. The solution improves upon the previous normalization approach by creating unique kebab-case identifiers with numeric suffixes when conflicts arise.

  • Introduces a collision-aware ID generation function that appends suffixes to prevent conflicts
  • Enhances the normalization function to create kebab-case names with better readability
  • Updates AWS stack generation to use the new collision-safe ID system

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
api/src/app/utils/name_utils.py Enhanced normalization function to create kebab-case names and validate non-empty results
api/src/app/utils/cdktf_utils.py Added gen_resource_logical_ids function for collision-safe ID generation with deterministic suffix assignment
api/src/app/core/cdktf/stacks/aws_stack.py Updated to use new collision-safe logical ID generation for VPCs and subnets
api/src/app/core/cdktf/ranges/base_range.py Modified Terraform output parsing to use new logical ID system
api/tests/unit/utils/test_name_utils.py Comprehensive test suite for the enhanced normalization function
api/tests/unit/utils/test_cdktf_utils.py Test coverage for collision handling and logical ID generation
api/tests/common/api/v1/config.py Added test case for normalized name collision scenario
Comments suppressed due to low confidence (2)

api/tests/unit/utils/test_name_utils.py:78

  • The test only checks for the word 'empty' in the error message, but the actual error message contains 'Name is empty after normalization'. Consider using a more specific match pattern like 'Name is empty after normalization' to ensure the correct error is being raised.
    with pytest.raises(ValueError, match="empty"):

api/tests/unit/utils/test_cdktf_utils.py:85

  • Similar to the name_utils test, consider using a more specific match pattern like 'Input list contains duplicate names' to ensure the correct error message is being tested.
    with pytest.raises(ValueError, match="duplicate"):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Related to the OpenLabs backend/API patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants