sops icon indicating copy to clipboard operation
sops copied to clipboard

exec-file, exec-env: support extracting sub-part

Open mikonieminen opened this issue 2 years ago • 9 comments

Add support for extracting sub-part of the document when running exec-file or exec-env command.

mikonieminen avatar Jul 27 '22 13:07 mikonieminen

is there a chance to get this reviewed? is the only way to use a password file with a unique secret, like ansible requires.

oneingan avatar Aug 02 '23 15:08 oneingan

This looks good to me. @mikonieminen can you please rebase and sign-off the commit?

felixfontein avatar Dec 17 '23 15:12 felixfontein

This looks good to me. @mikonieminen can you please rebase and sign-off the commit?

Done and sorry for the delay. I hope it's good now.

What do you think about the error handling and messages? Maybe I should use toExitError(err) as in some other cases or do you think the current error handling is good?

Also, I've added example part in the command description. Do you think this is good or should that go somewhere else?

How about adding tests? I'm not experienced with go and not sure where and how to add these. Maybe in a separate PR since this has been pending quite some time and some other people have been asking for the functionality?

mikonieminen avatar Dec 29 '23 11:12 mikonieminen

Done and sorry for the delay. I hope it's good now.

Thanks, I think it looks good!

What do you think about the error handling and messages? Maybe I should use toExitError(err) as in some other cases or do you think the current error handling is good?

I think it is good this way.

Also, I've added example part in the command description. Do you think this is good or should that go somewhere else?

I think it's fine (also it has been there in the 'original' --extract).

How about adding tests? I'm not experienced with go and not sure where and how to add these. Maybe in a separate PR since this has been pending quite some time and some other people have been asking for the functionality?

I think tests would be great. I think functional tests would be suited best for exec-env and exec-file, but right now there are none so there's nothing you can easily copy. I'll try to create some for the existing exec-env and exec-file functionalities, then you can take a look at that PR.

felixfontein avatar Dec 29 '23 13:12 felixfontein

How about adding tests? I'm not experienced with go and not sure where and how to add these. Maybe in a separate PR since this has been pending quite some time and some other people have been asking for the functionality?

I think tests would be great. I think functional tests would be suited best for exec-env and exec-file, but right now there are none so there's nothing you can easily copy. I'll try to create some for the existing exec-env and exec-file functionalities, then you can take a look at that PR.

That explains. Would be great, if you find the time to add tests that would be easy for me to use as an example for adding tests for these features. Happy to have a look at such PR.

Thanks.

mikonieminen avatar Dec 29 '23 14:12 mikonieminen

You can take a look at the latest commit of #1396. Right now I'm struggling with that the tests for the PR fail, while the same tests pass locally, and I have no clue why. This isn't the first time this is happening to me, and I'm still trying to figure out what happens...

felixfontein avatar Dec 29 '23 14:12 felixfontein

I finally managed to get #1396 working. I hope it'll get merged soon, then it should be easy to add a test for this :)

felixfontein avatar Feb 09 '24 21:02 felixfontein