Skip to content

Conversation

ilia-kats
Copy link

@ilia-kats ilia-kats commented Sep 9, 2025

this matches the behavior of the builtin file lookup

SUMMARY

The binary_file plugin now automatically unvaults vaulted files, matching the behavior of the built-in file plugin. I realize that DataLoader._get_file_contents is technically not public due to the underscore in front, but the alternative would be to either access DataLoader._vault, which is also not public, or using DataLoader.get_real_file, which creates an unnecessary temporary file.

Fixes #10799.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

binary_file lookup

ADDITIONAL INFORMATION

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug lookup lookup plugin new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels Sep 9, 2025
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

@ilia-kats thanks for your contribution.

result.append(base64.b64encode(f.read()).decode("utf-8"))
except Exception as exc:
raise AnsibleLookupError(f"Error while reading {path}: {exc}")
result.append(base64.b64encode(self._loader._get_file_contents(path)[0]).decode("utf-8"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bcoca @sivel is DataLoader._get_file_contents() safe to use in collections, or is it considered private?

If it is considered private, how should collections interact with Vault-encrypted files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I gathered on Matrix, it doesn't look like there's a public API for this. The only API there is decrypts the file to another file on disk, which is something we really want to avoid.

Also DataLoader._get_file_contents() doesn't set the SourceWasEncrypted tag (and the functionality to set it is definitely private), so I guess there's no way to properly achieve this right now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I created ansible/ansible#85852 for this.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-11 Automatically create a backport for the stable-10 branch labels Sep 9, 2025
@ansibullbot ansibullbot added feature This issue/PR relates to a feature request has_issue labels Sep 12, 2025
@russoz russoz changed the title make binary_file automatically unvault vaulted files (closes #10799) binary_file lookp: automatically unvault vaulted files (closes #10799) Sep 15, 2025
@russoz russoz changed the title binary_file lookp: automatically unvault vaulted files (closes #10799) binary_file lookup: automatically unvault vaulted files (closes #10799) Sep 15, 2025
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-11 Automatically create a backport for the stable-10 branch bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. feature This issue/PR relates to a feature request has_issue lookup lookup plugin new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review stale_ci CI is older than 7 days, rerun before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

binary_file should automatically unvault vaulted files
3 participants