Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add filename to exec-file #761

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

cgroschupp
Copy link
Contributor

@cgroschupp cgroschupp commented Oct 23, 2020

Some tools (e.g. Terraform) require the right file extension to recognize the file type.

see more details for terraform at https://www.terraform.io/docs/configuration/variables.html#variable-definitions-tfvars-files

i get this error

~/.go/bin/sops exec-file test.json "terraform plan -var-file={}"

Error: Argument or block definition required

  on /tmp/.sops640510710/tmp-file line 1:
   1: {

An argument or block definition is required here.

exit status 1

works for me:

~/.go/bin/sops exec-file --filename test.json test.json "terraform plan -var-file={}"

Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.
...

@cgroschupp cgroschupp force-pushed the feature/exec-filename branch from df51e2f to 1a810e1 Compare October 23, 2020 13:52
@cgroschupp cgroschupp changed the base branch from master to develop October 30, 2020 08:13
@cgroschupp
Copy link
Contributor Author

@ajvb @autrilla can you please review?

@autrilla
Copy link
Contributor

Before we add a new feature that we'll have to maintain forever, can you explain why this is useful for you? The file name gets passed to the command anyway, and there's no way for the command to know what it'll be ahead of time because the directory will have a random name.

@cgroschupp
Copy link
Contributor Author

@autrilla I have updated the description.

Copy link
Contributor

@autrilla autrilla left a comment

Choose a reason for hiding this comment

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

I'm OK with adding the feature, but other than my code comment, can you please:

  1. Document this in README.md (similar to how e.g. --user is documented).
  2. Modify the description with a more concrete example of a tool (actual commands, terraform works for me) that needs this to work properly? It would be useful for other people searching for a solution when they encounter the same problem you did.

cmd/sops/main.go Show resolved Hide resolved
@cgroschupp cgroschupp force-pushed the feature/exec-filename branch from 1a810e1 to 513f793 Compare December 29, 2020 09:52
@autrilla autrilla self-requested a review December 29, 2020 09:58
Copy link
Contributor

@autrilla autrilla left a comment

Choose a reason for hiding this comment

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

Thanks! We have some CI issues, but I'll make sure this PR will make it into the next release.

@renannprado
Copy link

Looking forward to use this feature 😄

@autrilla
Copy link
Contributor

autrilla commented Feb 8, 2021

Can you please rebase so this branch picks up the CI fixed that are now in develop?

@cgroschupp cgroschupp force-pushed the feature/exec-filename branch from 513f793 to 4a5ceca Compare February 9, 2021 07:15
@cgroschupp
Copy link
Contributor Author

@autrilla done

@autrilla
Copy link
Contributor

autrilla commented Feb 9, 2021

Thanks!

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.

3 participants