Skip to content

feat: directory support, map[string]yaml var type, migration#1685

Merged
DavSanchez merged 12 commits intomainfrom
feat/vartype-map-string-yaml
Oct 2, 2025
Merged

feat: directory support, map[string]yaml var type, migration#1685
DavSanchez merged 12 commits intomainfrom
feat/vartype-map-string-yaml

Conversation

@DavSanchez
Copy link
Contributor

@DavSanchez DavSanchez commented Sep 16, 2025

What this PR does / why we need it

This PR enhances the recently added support for filesystem operations for on-host by:

  • Adding a new variable type: map[string]yaml (d8374ee).
  • Extending the filesystem definition on the agent type mimicking a directory structure with its own ways of templating and finally emitting a list of files with their contents, to be created when spawning a sub-agent, see filesystem.rs module documentation for details! (d2d9d49)
  • Adapting the agent types for the Infrastructure Agent and the OTel Collector to be managed with this new method (5ffe820).

Which issue this PR fixes

Special notes for your reviewer

Sadly I still need to perform changes to the migration program and the changes are not straightforward. I'll follow the same principle as in #1640, probably continuing from there as well and merging it here as a single commit so you can review easily.

Also, commit a689506 adds trace logs with the paths to the files AC will create. If you see any risk on this (security, style, etc) let me know and I can just remove it.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Provided a meaningful title following conventional commit style.
  • Included a detailed description for the Pull Request.
  • Documentation under docs is aligned with the change. PENDING, BUT YOU CAN START REVIEWING!
  • Follows guidelines for Pull Requests in CONTRIBUTING.md.
  • Follows log level guidelines.

@DavSanchez DavSanchez force-pushed the feat/vartype-map-string-yaml branch 3 times, most recently from 6c430ae to d2d9d49 Compare September 19, 2025 14:32
@DavSanchez DavSanchez changed the title feat: add map[string]yaml var type feat: directory support, map[string]yaml var type, migration Sep 19, 2025
@DavSanchez DavSanchez force-pushed the feat/vartype-map-string-yaml branch 2 times, most recently from fded70a to ffc1b8f Compare September 19, 2025 18:13
@DavSanchez DavSanchez force-pushed the feat/vartype-map-string-yaml branch from ffc1b8f to f251eb1 Compare September 23, 2025 11:02
Copy link
Contributor

@gsanchezgavier gsanchezgavier left a comment

Choose a reason for hiding this comment

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

Awesome! I like how you resolve it !

* fix: use `relative_path` for file entry info

* docs: type

* feat: extend filesystem to fully support directories

* feat: a specific dir for each filesystem entity (files, dirs)

* test: templating

* test: move test-only impls to test module

* test: templating

* test: proper yaml rendering

* docs: detail how this module works

* feat: SafePath -> PathBuf

* feat(rendering): generate writable file list

* feat: rendering filesystem entry list

* style: rename path field

* test: rendering expected file entries

* test(integration): fix filesystem ops
@DavSanchez DavSanchez force-pushed the feat/vartype-map-string-yaml branch 2 times, most recently from 54861bc to 3ad1536 Compare September 25, 2025 13:58
@DavSanchez DavSanchez force-pushed the feat/vartype-map-string-yaml branch from 3ad1536 to 8383858 Compare September 25, 2025 13:58
The function `FileSystem::rendered` was public before, which could cause access to the internal `HashMap<PathBuf, String>`. If bound as mutable, it could be modified to write to unintended locations. Now the creation of `RenderedFileSystemEntries` is safer and doesn't provide visibility to the paths, only to write.
@DavSanchez DavSanchez force-pushed the feat/vartype-map-string-yaml branch from e5b3861 to e2792a5 Compare September 26, 2025 08:33
@DavSanchez DavSanchez marked this pull request as ready for review September 26, 2025 08:34
@DavSanchez DavSanchez requested a review from a team as a code owner September 26, 2025 08:34
path.components().try_for_each(|comp| match comp {
Component::Normal(_) | Component::CurDir => Ok(()),
// Disallow other non-supported variants like roots or prefixes
Component::ParentDir | Component::RootDir | Component::Prefix(_) => Err(format!(
Copy link
Contributor

Choose a reason for hiding this comment

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

cool is this also checking for ~ for instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly. Since ~ is a shell expansion if it's passed via the shell the character would be translated into the $HOME dir before reaching this. If somehow the literal character ~ reaches this point then a file or directory with that name will be created (from a SafePath("~")). It's not an error.

gsanchezgavier
gsanchezgavier previously approved these changes Sep 26, 2025
Copy link
Contributor

@gsanchezgavier gsanchezgavier left a comment

Choose a reason for hiding this comment

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

🚀 thanks!

danielorihuela
danielorihuela previously approved these changes Sep 26, 2025
@danielorihuela
Copy link
Contributor

@DavSanchez What about updating docs? In this PR or another?

@DavSanchez
Copy link
Contributor Author

@DavSanchez What about updating docs? In this PR or another?

Another, after I get rid of the deprecated variable types (file and map[string]file) I'll change the docs!

@DavSanchez DavSanchez merged commit c1e5c88 into main Oct 2, 2025
26 checks passed
@DavSanchez DavSanchez deleted the feat/vartype-map-string-yaml branch October 2, 2025 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments