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

[updatekeys] Fix input-type cli flag being ignored #1694

Closed

Conversation

kleinschrader
Copy link
Contributor

At my workplace we use sops, but we use yaml files and end them in .yaml.enc instead of just .yaml.
We recently wanted to add a new key and used sops updatekeys --input-type=yaml <file> however it would throw json decode errors.

I do not know go and have never worked with it so this is the best i could conjure up and i didnt mange to get tests to run so i can only say that --input-type now works with updatekeys.

Any feedback is welcome and i am happy to make changes.

@kleinschrader kleinschrader force-pushed the fix-input-type-updatekeys branch from 4a95c6b to 6e16eeb Compare December 3, 2024 07:59
@felixfontein
Copy link
Contributor

Sorry, I see that I didn't reply so far. I think your description makes sense; I didn't have time yet to take a closer look at your implementation and I didn't have time to think about whether this is more a bugfix or more a new feature, but I hope I'll have time soon to do so.

@felixfontein
Copy link
Contributor

This actually used to work, it got accidentally disabled in e9e2346, which is part of #1273, and got included in 3.9.0. So this is definitely a bugfix.

@@ -45,12 +46,23 @@ func updateFile(opts Opts) error {
if err != nil {
return err
}
store := common.DefaultStoreForPath(sc, opts.InputPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should simply be changed to store := common.DefaultStoreForPathOrFormat(sc, opts.InputPath, opts.InputType). That function takes care of everything.

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

Successfully merging this pull request may close these issues.

2 participants