Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Publishing improvements: directory walking; prevent Vault unneeded version increment #602
Conversation
in order to avoid version increment
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
|
Looks good overall, just a few comments |
| @@ -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 | |||
autrilla
Jan 9, 2020
Collaborator
Out of curiosity, what's your use case for this?
Out of curiosity, what's your use case for this?
mmorev
Jan 10, 2020
•
Author
Contributor
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 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.
autrilla
Jan 10, 2020
Collaborator
Thanks for explaining, it's always good to see how people use sops :)
Thanks for explaining, it's always good to see how people use sops :)
| Usage: "Omit file extensions in destination path when publishing sops file to configured destinations", | ||
| }, | ||
| cli.BoolFlag{ | ||
| Name: "recurse", |
autrilla
Jan 9, 2020
Collaborator
Please change all mentions of this to recursive
Please change all mentions of this to recursive
| @@ -46,10 +49,27 @@ func Run(opts Opts) error { | |||
| if err != nil { | |||
| return err | |||
| } | |||
| if info.IsDir() { | |||
| if info.IsDir() && !opts.Recurse { | |||
autrilla
Jan 9, 2020
Collaborator
I'd nest this,
if info.IsDir() {
if !opts.Recursive {
return fmt.Errorf("can't operate on a directory")
}
err = filepath.Walk(opts.InputPat...
}
I'd nest this,
if info.IsDir() {
if !opts.Recursive {
return fmt.Errorf("can't operate on a directory")
}
err = filepath.Walk(opts.InputPat...
}| 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 { |
autrilla
Jan 9, 2020
Collaborator
You should handle the error passed into the function
You should handle the error passed into the function
| } 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 { |
autrilla
Jan 9, 2020
Collaborator
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?
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?
mmorev
Jan 10, 2020
Author
Contributor
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:
Line 253
in
3ab2d41
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:
Line 253 in 3ab2d41
fix filepath.Walk abuse; rename recursive flag; minor fixes
|
LGTM, but I have one comment because this might break some people's use case. |
| @@ -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 | |||
autrilla
Jan 10, 2020
Collaborator
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.
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.
mmorev
Jan 11, 2020
Author
Contributor
Good idea. Fixed this behavior in 01b5fb6: log warn when no read access, log info when data not changed
Good idea. Fixed this behavior in 01b5fb6: log warn when no read access, log info when data not changed
Dont fail Vault publish with write-only access; improve vault publish logging
Fix destination path on single file publish
|
LGTM! Just left one small grammar tweak comment. |
Co-Authored-By: AJ Bahnken <1144310+ajvb@users.noreply.github.com>
Recursive publish - use relative paths
|
Added another one improvement in the last commit - make use of relative paths for recursive publish, so the destination path is: |
Hello.
This PR contains 3 changes in 3 corresponding commits: