Skip to content

Added support for passing values read from a file using --file#556

Merged
tehsis merged 14 commits intomainfrom
pterradillos/from-file
Jul 28, 2025
Merged

Added support for passing values read from a file using --file#556
tehsis merged 14 commits intomainfrom
pterradillos/from-file

Conversation

@tehsis
Copy link
Collaborator

@tehsis tehsis commented Jul 16, 2025

Description

This PR adds supports for --file <path> modifier in esc env set.
This modifier will read the file in and, if it's a valid string content, will use that to set the value of the specified key.

Usage

$ esc env set --file /path/to/file.txt myorg/myproj/myenv "mykey"

the --file flag also supports passing "-" to read from stdin so its consistent with the behavior of --file flag in esc env edit

$ cat /path/to/file.txt | esc env set --file - myorg/myproj/myenv

The flag can be combined with other flags. For example to store a file as a rawstring content and secret:

$ esc env set --secret --string --file /path/to/file.pem myorg/myproj/myenv "mykey"

To simplify reading file content, we fixed a bug where a new line is always being added at the end of the value when using --value string in esc env open and esc env get.

Users will be able to get the file content from esc.

eg.

$ esc env set --secret --string --file /path/to/file.pem myorg/myproj/myenv "mypemfile"
$ esc env get --show-secrets --value string myorg/myproj/myenv "mypemfile" >> /path/to/file_1.pem
$ diff /path/to/file_1.pem /path/to/file.pem
$

Known Issues

--file does not currently supports binary files (or any other file that does not contain valid utf-8 characters). These files require special treatment and might be included in another change (see #483)

@tehsis tehsis changed the title Added support for passing values read from a file using --from-file Added support for passing values read from a file using --file Jul 19, 2025
@tehsis tehsis marked this pull request as ready for review July 19, 2025 13:04
@tehsis tehsis requested review from Copilot and pgavlin July 19, 2025 13:04
Copy link

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 adds support for a --file flag to the esc env set command, allowing users to read values from files or stdin instead of providing them as command-line arguments. Additionally, it fixes a bug where an unnecessary newline was being appended to string values in esc env open and esc env get commands.

  • Adds --file parameter to esc env set command with support for reading from files or stdin (using "-")
  • Modifies argument validation to allow fewer arguments when --file flag is used
  • Fixes newline handling in string output format for env open and env get commands

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
cmd/esc/cli/env_set.go Implements --file flag functionality with UTF-8 validation and argument handling changes
cmd/esc/cli/env_open.go Fixes string output format to avoid duplicate newlines and updates help text
cmd/esc/cli/env_get.go Updates help text to include 'string' format option
CHANGELOG_PENDING.md Documents the new --file parameter feature

@tehsis tehsis requested review from rgharris and seanyeh July 21, 2025 19:41
Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

These changes need tests. We have a pretty robust test harness for the CLI's code--you can use the tests for env set and the tests for env edit -f for reference. I'd recommend adding tests in a new testdata file (e.g. env-set-file.yaml).

@tehsis tehsis requested a review from pgavlin July 22, 2025 12:52
Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

Could use one more test, but LGTM otherwise

@tehsis tehsis requested review from pgavlin and rgharris July 23, 2025 20:59
Copy link
Contributor

@rgharris rgharris left a comment

Choose a reason for hiding this comment

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

LGTM other than there may still be a leading newline issue.

@tehsis
Copy link
Collaborator Author

tehsis commented Jul 25, 2025

I was able to reproduce the multiline issue that Robert mentioned. Will take a look into that

@tehsis tehsis force-pushed the pterradillos/from-file branch from ac42243 to 5c48797 Compare July 28, 2025 12:58
@tehsis tehsis merged commit cfc297d into main Jul 28, 2025
8 checks passed
@tehsis tehsis deleted the pterradillos/from-file branch July 28, 2025 17:39
@pulumi-bot
Copy link

This PR has been shipped in release v0.15.0.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants