VAULT-25848 replace mholt/archiver with native go calls #27228
Conversation
…bug caught by TestDebugCommand_PartialPermissions
github-actions
bot
added
the
hashicorp-contributed-pr
command/debug_test.go
Outdated
| } | ||
|
|
||
| // We set ignoreUnexpectedHeaders to true as replication_status.json is only sometimes | ||
| // produced. Relying on it being or not being there wuld be racy. |
There was a problem hiding this comment.
Note that this was true both before and after my change. In other words, if you add the new test code with the old production code, it still exhibits this behaviour. I'm not entirely sure why, but I'm glad it's consistent, at least. Sometimes it's there, sometimes it's not.
|
CI Results: |
VioletHynes
added
backport/1.16.x
backport/ent/1.15.x+ent
| @@ -374,7 +375,7 @@ func (c *DebugCommand) generateIndex() error { | |||
| } | |||
|
|
|||
| // Write out file | |||
| if err := ioutil.WriteFile(filepath.Join(c.flagOutput, "index.json"), bytes, 0o600); err != nil { | |||
| if err := os.WriteFile(filepath.Join(c.flagOutput, "index.json"), bytes, 0o600); err != nil { | |||
command/debug.go
Outdated
| } | ||
|
|
||
| // addFileToTar takes a file at filePath and adds it to the tar | ||
| // being written to by tarWriter, alongside its header. T |
There was a problem hiding this comment.
| // being written to by tarWriter, alongside its header. T | |
| // being written to by tarWriter, alongside its header. |
command/debug.go
Outdated
| func addFileToTar(sourceDir, filePath string, tarWriter *tar.Writer) error { | ||
| file, err := os.Open(filePath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to open file %q: %s", filePath, err) |
There was a problem hiding this comment.
Is it worth using %w for the err in these fmt.Errorf calls?
There was a problem hiding this comment.
From what I can tell, %v and %s should be equivalent for errs -- is there a reason you think %v might be preferred?
There was a problem hiding this comment.
Sorry, I mean't %w but went and confusingly typed %v.
There was a problem hiding this comment.
Oh, good shout. I always forget about that one since it only sometimes works (only for Errorf). Definitely should be used here. Will update!
|
Build Results: |
* VAULT-25848 update product code to remove mholt/archiver dependency * VAULT-25848 replace tests, still WIP while I figure out if there's a bug caught by TestDebugCommand_PartialPermissions * VAULT-25848 actually remove the dep * VAULT-25848 add headers for directories, improve test * Comment cleanup * Typo * Use %w * Typo
) * VAULT-25848 update product code to remove mholt/archiver dependency * VAULT-25848 replace tests, still WIP while I figure out if there's a bug caught by TestDebugCommand_PartialPermissions * VAULT-25848 actually remove the dep * VAULT-25848 add headers for directories, improve test * Comment cleanup * Typo * Use %w * Typo
This removes the mholt/archiver dependency, and replaces it in the only place where it's used (debug) with native Go calls instead. This passed with the old and new tests.
As far as I can tell, everything works the same. I've added the backport labels as I think I'll need to backport this due to this dependency being flagged as having a vulnerability.