Skip to content

Conversation

@sigJoe
Copy link
Contributor

@sigJoe sigJoe commented Jan 28, 2026

Which issue(s) does this change fix?

This PR adds two enhancements to the parameter override file functionality introduced in #7876:

  1. Relative path resolution for nested includes - Parameter files can now reference other files using paths relative to their own location, rather than requiring paths relative to the project root
  2. $include key support - Dict-based parameter files (YAML/TOML) can now include other files using a special $include key, providing the same composition capabilities that list-based files already had

Why is this change necessary?

  1. Improved ergonomics for nested includes: When organizing config files in subdirectories (e.g., ./config/a.yaml and ./config/b.yaml), it's unintuitive for a.yaml to reference b.yaml as file://config/b.yaml instead of simply file://b.yaml. Relative path resolution makes file organization more natural and maintainable.
  2. TOML parity with YAML: The previous implementation only supported file includes via list syntax, which worked for YAML but did not allow TOML dict-based files to include other files.

How does it address the issue?

  1. Relative path resolution: Added a current_file parameter to track the file being processed. When a file:// reference is encountered, relative paths are resolved against current_file.parent instead of the project root.
  2. $include key: Added support for a special $include key in dict-based parameter files that accepts either a single file path or a list of file paths. I picked this key name because it is not a valid cloudformation parameter so it should be fully backwards compatible. Another option was __include which didn't need to be quote-escaped in TOML but overall didn't look as pretty.

What side effects does this change have?

This changes how file:// references work for parameter overrides in a way that is not backwards compatible. However, impact is limited since:

  • The feature was introduced very recently (Expand parameter overrides #7876)
  • It has not yet been documented in official AWS documentation
  • The new behavior is more intuitive and aligns with user expectations

Mandatory Checklist

PRs will only be reviewed after checklist is complete

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Documentation

Old and busted

When using file:// to include external parameter overrides, there were limitations to include further levels of nested files. YAML had to be a list:

- StringParam: Pam Param
- NumberParam: 42
- file://other_yaml_params.yaml
- file://toml_params_because_I_can.toml

TOML just didn't support it:

StringParam = "Pam Param"
NumberParam = 42
file://another_toml_file.toml # This will fail because it is not valid TOML

New hotness

YAML can now be a dict (if you want) using $include:

$include:
  - file://default.yaml
  - file://prod.yaml
Environment: prod-b

Or single file inclusion:

$include: file://base.yaml
Environment: prod-b

And TOML can now join the party too:

'$include' = 'file://default.toml'
Environment = 'prod-b'

And bring multiple friends:

'$include' = ['file://default.toml', 'file://prod.toml']
Environment = 'prod-b'

@sigJoe sigJoe requested a review from a team as a code owner January 28, 2026 19:27
@github-actions github-actions bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Jan 28, 2026
Copy link
Contributor

@bnusunny bnusunny left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @sigJoe! This is a nice follow-up to #7876. The relative path resolution and $include support are both sensible additions that improve the ergonomics of nested config files.

Implementation looks good:

  • Clean use of current_file parameter to track context for relative path resolution
  • file_path.resolve() properly normalizes paths
  • Reuses existing seen_files mechanism for recursion detection
    Good error message for invalid $include` types

A few suggestions:

. Add a test for $includecircular references - The existingtest_infinite_recursion_protectionuses list syntax. Would be good to add a similar test using$include`:

def test_include_key_infinite_recursion(self):
    mock_files = {
        "A.yaml": "$include: file://B.yaml",
        "B.yaml": "$include: file://A.yaml",
    }
    # ... should raise BadParameter with "Infinite recursion detected"
  1. Consider adding type hint for current_file:
def convert(self, values, param, ctx, seen_files=None, current_file: Optional[Path] = None):
  1. Edge case question: What happens if someone writes $include: "NotAFileReference" (without file:// prefix)? It looks like it would fall through to the legacy parameter matching and fail with a potentially confusing error. Might be worth either:

    • Adding validation that $include values must start with file://
    • Or just documenting the expected format

Overall this is solid work. Once the circular reference test is added, this should be good to go.

@bnusunny bnusunny added area/sam-config and removed stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants