Publishing improvements: directory walking; prevent Vault unneeded version increment #602

mmorev · 2020-01-09T05:10:05Z

Hello.
This PR contains 3 changes in 3 corresponding commits:

implement -recurse cmd option for directory walking for publish command, so you can publish a bunch of files, preserving structure;
implement -omit-extensions cmd option to omit file extension in destination secret path. Can be specified in .sops.yaml;
implement pre-check for already existing Vault secret, to avoid version increment if data not changed.

in order to avoid version increment

codecov-io · 2020-01-09T05:16:33Z

Codecov Report

Merging #602 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #602      +/-   ##
===========================================
+ Coverage    37.11%   37.13%   +0.02%     
===========================================
  Files           21       21              
  Lines         2891     2892       +1     
===========================================
+ Hits          1073     1074       +1     
  Misses        1724     1724              
  Partials        94       94

Impacted Files	Coverage Δ
config/config.go	`71.64% <100%> (+0.21%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53c6470...0c26330. Read the comment docs.

autrilla

Looks good overall, just a few comments

README.rst

autrilla · 2020-01-09T18:59:22Z

README.rst

@@ -968,6 +968,7 @@ This command requires a ``.sops.yaml`` configuration file. Below is an example:
        vault_kv_mount_name: "secret/" # default
        vault_kv_version: 2 # default
        path_regex: vault/*
+        omit_extensions: true


Out of curiosity, what's your use case for this?

Thanks for this question!
The goal is to use Vault as secrets source for several applications in different environments. As soon as it is a team work, we want to view diffs and approve/reject changes just like code changes in any Git platform (github, bitbucket) we do.
Vault has no option to stage changes like this (the only option in Enterprise edition is to approve the fact of write access, without seeing what data will be modified) and no diffs between versions or something. So we decided to store some info in Git/Sops and publish it to Vault in a batch triggered by repository change, and then lock down Vault to read-only mode by policies.
So in Vault we have some secrets schema, extensions are not needed there.

Thanks for explaining, it's always good to see how people use sops :)

autrilla · 2020-01-09T19:00:06Z

cmd/sops/main.go

+					Usage: "Omit file extensions in destination path when publishing sops file to configured destinations",
+				},
+				cli.BoolFlag{
+					Name:  "recurse",


Please change all mentions of this to recursive

fixed in 3ab2d41

autrilla · 2020-01-09T19:09:08Z

cmd/sops/subcommand/publish/publish.go

@@ -46,10 +49,27 @@ func Run(opts Opts) error {
 	if err != nil {
 		return err
 	}
-	if info.IsDir() {
+	if info.IsDir() && !opts.Recurse {


I'd nest this,

if info.IsDir() { if !opts.Recursive { return fmt.Errorf("can't operate on a directory") } err = filepath.Walk(opts.InputPat... }

Made some rework in 3ab2d41. See below.

autrilla · 2020-01-09T19:46:29Z

cmd/sops/subcommand/publish/publish.go

 		return fmt.Errorf("can't operate on a directory")
+	} else if info.IsDir() && opts.Recurse {
+		err = filepath.Walk(opts.InputPath, func(subPath string, info os.FileInfo, err error) error {


You should handle the error passed into the function

fixed in 3ab2d41

autrilla · 2020-01-09T19:49:13Z

cmd/sops/subcommand/publish/publish.go

+	} else if info.IsDir() && opts.Recurse {
+		err = filepath.Walk(opts.InputPath, func(subPath string, info os.FileInfo, err error) error {
+			subAbsPath, _ := filepath.Abs(subPath)
+			if !info.IsDir() && subAbsPath != path {


Could subAbsPath != path ever be false? path is the original input path, and by the mere fact that we got to this code path, we've already established it's a directory, so !info.,IsDir() would be false anyway and it would short-circuit. Am I missing something?

Thanks for this comment. I just found out I had abused filepath.Walk function by using additional recursion inside it. So i moved out Walk call, IsDir check and store type detection to main.go: https://github.com/mozilla/sops/blob/3ab2d41c2f596e1fe5fa59c72a559514bcc91080/cmd/sops/main.go#L253

cmd/sops/subcommand/publish/publish.go

fix filepath.Walk abuse; rename recursive flag; minor fixes

autrilla

LGTM, but I have one comment because this might break some people's use case.

autrilla · 2020-01-10T18:43:06Z

publish/vault.go

@@ -65,6 +66,17 @@ func (vaultd *VaultDestination) UploadUnencrypted(data map[string]interface{}, f
 		}
 	}

+	existingSecret, err := client.Logical().Read(vaultd.secretsPath(fileName))
+	if err != nil {
+		return err


I think it might be good to keep going regardless (after printing a warning) if there's an error reading the secret. I'm imagining a situation where someone has given SOPS permission to only write to Vault.

Good idea. Fixed this behavior in 01b5fb6: log warn when no read access, log info when data not changed

… logging

Dont fail Vault publish with write-only access; improve vault publish logging

Fix destination path on single file publish

ajvb

LGTM! Just left one small grammar tweak comment.

cmd/sops/main.go

Co-Authored-By: AJ Bahnken <1144310+ajvb@users.noreply.github.com>

Recursive publish - use relative paths

mmorev · 2020-01-14T12:41:34Z

Added another one improvement in the last commit - make use of relative paths for recursive publish, so the destination path is: vault_kv_mount_name + vault_path + path under the directory passed to sops publish --recursive

MOREV Mikhail added 3 commits January 9, 2020 11:07

Add -recurse option for publish subcommand

8a216e9

Add -omit-extensions option for publish subcommand

ef68940

Skip publish to Vault if secret is up-to-date

10ef21c

in order to avoid version increment

update docs

4254322

ajvb requested review from ajvb and autrilla January 9, 2020 17:12

autrilla reviewed Jan 9, 2020

View reviewed changes

MOREV Mikhail and others added 2 commits January 10, 2020 09:57

fix filepath.Walk abuse; rename recursive flag; minor fixes

3ab2d41

Merge pull request #1 from mmorev/fix-recursive

02b0437

fix filepath.Walk abuse; rename recursive flag; minor fixes

autrilla approved these changes Jan 10, 2020

View reviewed changes

MOREV Mikhail and others added 4 commits January 11, 2020 10:49

Dont fail Vault publish with write-only access; improve vault publish…

01b5fb6

… logging

Merge pull request #2 from mmorev/fix-recursive

ed31727

Dont fail Vault publish with write-only access; improve vault publish logging

Fix destination path on single file publish

3ccc7e4

Merge pull request #3 from mmorev/fix-singlefile

67f1654

Fix destination path on single file publish

ajvb approved these changes Jan 13, 2020

View reviewed changes

cmd/sops/main.go Outdated Show resolved Hide resolved

mmorev and others added 3 commits January 14, 2020 00:45

Update cmd/sops/main.go

3db9c71

Co-Authored-By: AJ Bahnken <1144310+ajvb@users.noreply.github.com>

Recursive publish - use relative paths

0c6558b

Merge pull request #4 from mmorev/fix-relativepaths

0c26330

Recursive publish - use relative paths

ajvb merged commit df39dca into getsops:develop Jan 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publishing improvements: directory walking; prevent Vault unneeded version increment #602

Publishing improvements: directory walking; prevent Vault unneeded version increment #602

mmorev commented Jan 9, 2020

codecov-io commented Jan 9, 2020 •

edited

autrilla left a comment

autrilla Jan 9, 2020

mmorev Jan 10, 2020 •

edited

autrilla Jan 10, 2020

autrilla Jan 9, 2020

mmorev Jan 10, 2020

autrilla Jan 9, 2020

mmorev Jan 10, 2020

autrilla Jan 9, 2020

mmorev Jan 10, 2020

autrilla Jan 9, 2020

mmorev Jan 10, 2020

autrilla left a comment

autrilla Jan 10, 2020

mmorev Jan 11, 2020

ajvb left a comment

mmorev commented Jan 14, 2020 •

edited

Publishing improvements: directory walking; prevent Vault unneeded version increment #602

Publishing improvements: directory walking; prevent Vault unneeded version increment #602

Conversation

mmorev commented Jan 9, 2020

codecov-io commented Jan 9, 2020 • edited

Codecov Report

autrilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmorev Jan 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

autrilla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajvb left a comment

Choose a reason for hiding this comment

mmorev commented Jan 14, 2020 • edited

codecov-io commented Jan 9, 2020 •

edited

mmorev Jan 10, 2020 •

edited

mmorev commented Jan 14, 2020 •

edited