feat: add CoreFileObject support and object_file_fetch module#317
Conversation
… module Add file_path and fetch_file parameters to the node module for creating, updating, and downloading CoreFileObject nodes. CoreFileObject kinds require file_path or fetch_file (mutually exclusive). Nodes without an HFID are looked up by file_name derived from file_path. Add object_file_fetch module for fetching file content by UUID or HFID with optional local save via dest parameter. Includes unit tests, conftest.py for pytest collection imports, and changelog.
There was a problem hiding this comment.
6 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/action/object_file_fetch.py">
<violation number="1" location="plugins/action/object_file_fetch.py:99">
P1: `node.is_file_object()` does not exist on `InfrahubNodeSync`—no such method is defined in the codebase. This will raise `AttributeError` at runtime for every successful fetch. The existing pattern in `node.py` checks via schema introspection: `hasattr(schema, "inherit_from") and "CoreFileObject" in (schema.inherit_from or [])`. Consider either fetching the schema to validate before download, or removing this post-download check since `download_file()` would already fail for non-file-object kinds.</violation>
</file>
<file name="plugins/modules/node.py">
<violation number="1" location="plugins/modules/node.py:154">
P2: Declare `mutually_exclusive=[["file_path", "fetch_file"]]` on the `AnsibleModule` constructor. Both parameters are documented as mutually exclusive, but without this declaration Ansible won't enforce it upfront—the error surfaces only after the schema fetch in `NodeModule.run()`.</violation>
</file>
<file name="plugins/modules/object_file_fetch.py">
<violation number="1" location="plugins/modules/object_file_fetch.py:162">
P1: Missing `env_fallback` for `api_endpoint` and `token` — environment variables `INFRAHUB_ADDRESS` / `INFRAHUB_API_TOKEN` won't work. Use `deepcopy(INFRAHUB_ARG_SPEC)` and `.pop("state")` like `schema.py` does, then `.update()` with module-specific args.</violation>
</file>
<file name="conftest.py">
<violation number="1" location="conftest.py:21">
P2: `Path.exists()` follows symlinks, so a stale symlink (target deleted/moved) makes this return `False` while the symlink file still exists on disk. The subsequent `symlink_to()` then raises `FileExistsError`. Use `is_symlink()` in the check to detect the symlink regardless of target validity.</violation>
</file>
<file name="plugins/module_utils/infrahub_utils.py">
<violation number="1" location="plugins/module_utils/infrahub_utils.py:1252">
P1: After unwrapping `{"value": None}`, `value` becomes `None` but the null-check already passed, so `str(None)` appends the literal string `"None"` as an HFID component. Add a second null-check after unwrapping to return `None` (missing HFID) as intended.</violation>
</file>
<file name="plugins/module_utils/node.py">
<violation number="1" location="plugins/module_utils/node.py:111">
P1: `Path(file_path).name` will crash with `TypeError` when `fetch_file=True` (so `file_path` is `None`), the schema has no HFID, and no `id` is in `data`. Add a guard for `file_path` being `None` before deriving `file_name`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| if isinstance(value, dict) and "value" in value: | ||
| value = value["value"] |
There was a problem hiding this comment.
P1: After unwrapping {"value": None}, value becomes None but the null-check already passed, so str(None) appends the literal string "None" as an HFID component. Add a second null-check after unwrapping to return None (missing HFID) as intended.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/module_utils/infrahub_utils.py, line 1252:
<comment>After unwrapping `{"value": None}`, `value` becomes `None` but the null-check already passed, so `str(None)` appends the literal string `"None"` as an HFID component. Add a second null-check after unwrapping to return `None` (missing HFID) as intended.</comment>
<file context>
@@ -1170,6 +1248,9 @@ def rebuild_hfid_from_data(
if value is None:
return None
+ # Unwrap nested {"value": ...} dicts used by Ansible data format
+ if isinstance(value, dict) and "value" in value:
+ value = value["value"]
hfid_values.append(str(value))
</file context>
| if isinstance(value, dict) and "value" in value: | |
| value = value["value"] | |
| if isinstance(value, dict) and "value" in value: | |
| value = value["value"] | |
| if value is None: | |
| return None |
| collections_dir = repo_root / ".pytest_collections" | ||
| link_target = collections_dir / "ansible_collections" / "opsmill" / "infrahub" | ||
| link_target.parent.mkdir(parents=True, exist_ok=True) | ||
| if not link_target.exists(): |
There was a problem hiding this comment.
P2: Path.exists() follows symlinks, so a stale symlink (target deleted/moved) makes this return False while the symlink file still exists on disk. The subsequent symlink_to() then raises FileExistsError. Use is_symlink() in the check to detect the symlink regardless of target validity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At conftest.py, line 21:
<comment>`Path.exists()` follows symlinks, so a stale symlink (target deleted/moved) makes this return `False` while the symlink file still exists on disk. The subsequent `symlink_to()` then raises `FileExistsError`. Use `is_symlink()` in the check to detect the symlink regardless of target validity.</comment>
<file context>
@@ -0,0 +1,25 @@
+ collections_dir = repo_root / ".pytest_collections"
+ link_target = collections_dir / "ansible_collections" / "opsmill" / "infrahub"
+ link_target.parent.mkdir(parents=True, exist_ok=True)
+ if not link_target.exists():
+ link_target.symlink_to(repo_root)
+ collections_str = str(collections_dir)
</file context>
Add per-file ruff ignores for tests and action plugin. Fix loop variable overwrite (PLW2901), unused noqa directive, raw string pattern, and formatting issues. Add conftest.py type annotation for mypy.
Deploying infrahub-ansible with
|
| Latest commit: |
a36cddd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://16a7705c.infrahub-ansible.pages.dev |
| Branch Preview URL: | https://001-feature-object-file.infrahub-ansible.pages.dev |
- Replace node.is_file_object() with schema introspection in object_file_fetch action plugin (method does not exist on InfrahubNodeSync) - Add mutually_exclusive for file_path/fetch_file on AnsibleModule constructor - Use INFRAHUB_ARG_SPEC with env_fallback in object_file_fetch module stub - Remove no_log from DOCUMENTATION docstring (enforced via argument_spec)
Add type annotations to _lookup_node, _ensure_object_exists, _create_object_with_file, and _update_object_with_file. Type self.result as dict[str, Any]. Exclude tests/ from mypy checking.
There was a problem hiding this comment.
You may need to reload from stable, missing 1.8 here
Summary
file_pathandfetch_fileparameters to thenodemodule for CoreFileObject create/update/download with SHA-1 idempotencyfile_pathorfetch_file(mutually exclusive); nodes without an HFID are looked up byfile_namederived fromfile_pathobject_file_fetchmodule for fetching file content by UUID or HFID with optional local save viadestconftest.pyfor pytest collection importsTest plan
Summary by cubic
Adds CoreFileObject support to the
nodemodule and a newobject_file_fetchaction/module to download file content by UUID or HFID with optional local save. Enforces SHA-1 idempotent uploads and schema-based validation; requiresinfrahub-sdk[all] >=1.19.0and Infrahub server>=1.8for file features.New Features
node: Addfile_path(upload) andfetch_file(download); mutually exclusive and required for CoreFileObject kinds. Auto-lookup byfile_namewhen no HFID;fetch_filereturnsbinary/text.object_file_fetch: Fetch bynode_idorhfid, return base64 content and metadata; optionaldestwrites to a directory or exact path.Bug Fixes
object_file_fetch: Validate CoreFileObject via schema introspection; use sharedINFRAHUB_ARG_SPECwith env fallbacks; remove redundantno_logdocs.node/tooling: Add type annotations to fix mypy errors; excludetests/from mypy and add targeted ruff ignores; general lint/format fixes.Written for commit f19b3c5. Summary will update on new commits.