Refactored most of the CLI tests to run in-process as opposed to using sub-processes (#1059)

* cli: fixed remaining testability indirections for output and logging

* cli: added cli.RunSubcommand() which is used in testing to execute a subcommand in the same process

* tests: refactored most e2e tests to invoke kopia subcommands in-process

* Makefile: enable code coverage for cli/ and internal/

* testing: pass 'testing' tag to unit tests which uses much faster (insecure) password hashing scheme

* Makefile: push coverage from PRs again

* tests: disable buffer management to reduce memory usage on ARM

* cli: fixed misaligned atomic field on ARMHF

also temporarily fixed statup-time benign race condition when setting
default on the timeZone variable, which is the last global variable.
This commit is contained in:
Jarek Kowalski authored and GitHub committed 2021-05-11 22:26:28 -07:00
1 parent 41931f21ce
commit fcd507a56d
54 files changed
+565 -235

No files matched your search

+23 -11
View File
@@ -79,6 +79,7 @@ type appServices interface {
repositoryWriterAction(act func(ctx context.Context, rep repo.RepositoryWriter) error) func(ctx *kingpin.ParseContext) error
maybeRepositoryAction(act func(ctx context.Context, rep repo.Repository) error, mode repositoryAccessMode) func(ctx *kingpin.ParseContext) error
advancedCommand(ctx context.Context)
repositoryConfigFileName() string
getProgress() *cliProgress
@@ -99,6 +100,8 @@ type advancedAppServices interface {
passwordPersistenceStrategy() passwordpersist.Strategy
getPasswordFromFlags(ctx context.Context, isNew, allowPersistent bool) (string, error)
optionsFromFlags(ctx context.Context) *repo.Options
rootContext() context.Context
}
// App contains per-invocation flags and state of Kopia CLI.
@@ -116,6 +119,7 @@ type App struct {
metricsListenAddr string
keyRingEnabled bool
persistCredentials bool
AdvancedCommands string
// subcommands
blob commandBlob
@@ -140,6 +144,7 @@ type App struct {
osExit func(int) // allows replacing os.Exit() with custom code
stdoutWriter io.Writer
stderrWriter io.Writer
rootctx context.Context
}
func (c *App) getProgress() *cliProgress {
@@ -185,9 +190,10 @@ func (c *App) setup(app *kingpin.Application) {
app.Flag("config-file", "Specify the config file to use.").Default(defaultConfigFileName()).Envar("KOPIA_CONFIG_PATH").StringVar(&c.configPath)
app.Flag("trace-storage", "Enables tracing of storage operations.").Default("true").Hidden().BoolVar(&c.traceStorage)
app.Flag("metrics-listen-addr", "Expose Prometheus metrics on a given host:port").Hidden().StringVar(&c.metricsListenAddr)
app.Flag("timezone", "Format time according to specified time zone (local, utc, original or time zone name)").Default("local").Hidden().StringVar(&timeZone)
app.Flag("timezone", "Format time according to specified time zone (local, utc, original or time zone name)").Hidden().StringVar(&timeZone)
app.Flag("password", "Repository password.").Envar("KOPIA_PASSWORD").Short('p').StringVar(&c.password)
app.Flag("persist-credentials", "Persist credentials").Default("true").Envar("KOPIA_PERSIST_CREDENTIALS_ON_CONNECT").BoolVar(&c.persistCredentials)
app.Flag("advanced-commands", "Enable advanced (and potentially dangerous) commands.").Hidden().Envar("KOPIA_ADVANCED_COMMANDS").StringVar(&c.AdvancedCommands)
c.setupOSSpecificKeychainFlags(app)
@@ -235,6 +241,7 @@ func NewApp() *App {
osExit: os.Exit,
stdoutWriter: os.Stdout,
stderrWriter: os.Stderr,
rootctx: context.Background(),
}
}
@@ -268,7 +275,7 @@ func safetyFlagVar(cmd *kingpin.CmdClause, result *maintenance.SafetyParameters)
func (c *App) noRepositoryAction(act func(ctx context.Context) error) func(ctx *kingpin.ParseContext) error {
return func(_ *kingpin.ParseContext) error {
return act(rootContext())
return act(c.rootContext())
}
}
@@ -284,7 +291,7 @@ func (c *App) serverAction(sf *serverClientFlags, act func(ctx context.Context,
return errors.Wrap(err, "unable to create API client")
}
return act(rootContext(), apiClient)
return act(c.rootContext(), apiClient)
}
}
@@ -348,8 +355,8 @@ func (c *App) repositoryWriterAction(act func(ctx context.Context, rep repo.Repo
})
}
func rootContext() context.Context {
return context.Background()
func (c *App) rootContext() context.Context {
return c.rootctx
}
type repositoryAccessMode struct {
@@ -359,7 +366,7 @@ type repositoryAccessMode struct {
func (c *App) maybeRepositoryAction(act func(ctx context.Context, rep repo.Repository) error, mode repositoryAccessMode) func(ctx *kingpin.ParseContext) error {
return func(kpc *kingpin.ParseContext) error {
ctx := rootContext()
ctx := c.rootContext()
if err := withProfiling(func() error {
c.mt.startMemoryTracking(ctx)
@@ -398,7 +405,7 @@ func (c *App) maybeRepositoryAction(act func(ctx context.Context, rep repo.Repos
}); err != nil {
// print error in red
log(ctx).Errorf("ERROR: %v", err.Error())
os.Exit(1)
c.osExit(1)
}
return nil
@@ -436,15 +443,20 @@ func (c *App) maybeRunMaintenance(ctx context.Context, rep repo.Repository) erro
return errors.Wrap(err, "error running maintenance")
}
func advancedCommand(ctx context.Context) {
if os.Getenv("KOPIA_ADVANCED_COMMANDS") != "enabled" {
log(ctx).Errorf(`
func (c *App) advancedCommand(ctx context.Context) {
if c.AdvancedCommands != "enabled" {
_, _ = errorColor.Fprintf(c.stderrWriter, `
This command could be dangerous or lead to repository corruption when used improperly.
Running this command is not needed for using Kopia. Instead, most users should rely on periodic repository maintenance. See https://kopia.io/docs/advanced/maintenance/ for more information.
To run this command despite the warning, set KOPIA_ADVANCED_COMMANDS=enabled
`)
os.Exit(1)
c.osExit(1)
}
}
func init() {
kingpin.EnableFileExpansion = false
}
+5 -1
View File
@@ -11,16 +11,20 @@ import (
type commandBlobDelete struct {
blobIDs []string
svc appServices
}
func (c *commandBlobDelete) setup(svc appServices, parent commandParent) {
cmd := parent.Command("delete", "Delete blobs by ID").Alias("remove").Alias("rm")
cmd.Arg("blobIDs", "Blob IDs").Required().StringsVar(&c.blobIDs)
cmd.Action(svc.directRepositoryWriteAction(c.run))
c.svc = svc
}
func (c *commandBlobDelete) run(ctx context.Context, rep repo.DirectRepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
for _, b := range c.blobIDs {
err := rep.BlobStorage().DeleteBlob(ctx, blob.ID(b))
+5 -1
View File
@@ -15,6 +15,8 @@ type commandBlobGC struct {
parallel int
prefix string
safety maintenance.SafetyParameters
svc appServices
}
func (c *commandBlobGC) setup(svc appServices, parent commandParent) {
@@ -24,10 +26,12 @@ func (c *commandBlobGC) setup(svc appServices, parent commandParent) {
cmd.Flag("prefix", "Only GC blobs with given prefix").StringVar(&c.prefix)
safetyFlagVar(cmd, &c.safety)
cmd.Action(svc.directRepositoryWriteAction(c.run))
c.svc = svc
}
func (c *commandBlobGC) run(ctx context.Context, rep repo.DirectRepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
opts := maintenance.DeleteUnreferencedBlobsOptions{
DryRun: c.delete != "yes",
+5 -2
View File
@@ -5,7 +5,6 @@ import (
"context"
"encoding/json"
"io"
"os"
"github.com/pkg/errors"
@@ -17,6 +16,8 @@ import (
type commandBlobShow struct {
blobShowDecrypt bool
blobShowIDs []string
out textOutput
}
func (c *commandBlobShow) setup(svc appServices, parent commandParent) {
@@ -24,11 +25,13 @@ func (c *commandBlobShow) setup(svc appServices, parent commandParent) {
cmd.Flag("decrypt", "Decrypt blob if possible").BoolVar(&c.blobShowDecrypt)
cmd.Arg("blobID", "Blob IDs").Required().StringsVar(&c.blobShowIDs)
cmd.Action(svc.directRepositoryReadAction(c.run))
c.out.setup(svc)
}
func (c *commandBlobShow) run(ctx context.Context, rep repo.DirectRepository) error {
for _, blobID := range c.blobShowIDs {
if err := c.maybeDecryptBlob(ctx, os.Stdout, rep, blob.ID(blobID)); err != nil {
if err := c.maybeDecryptBlob(ctx, c.out.stdout(), rep, blob.ID(blobID)); err != nil {
return errors.Wrap(err, "error presenting blob")
}
}
+5 -1
View File
@@ -10,16 +10,20 @@ import (
type commandContentDelete struct {
ids []string
svc appServices
}
func (c *commandContentDelete) setup(svc appServices, parent commandParent) {
cmd := parent.Command("delete", "Remove content").Alias("remove").Alias("rm")
cmd.Arg("id", "IDs of content to remove").Required().StringsVar(&c.ids)
cmd.Action(svc.directRepositoryWriteAction(c.run))
c.svc = svc
}
func (c *commandContentDelete) run(ctx context.Context, rep repo.DirectRepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
for _, contentID := range toContentIDs(c.ids) {
if err := rep.ContentManager().DeleteContent(ctx, contentID); err != nil {
+4 -1
View File
@@ -19,6 +19,7 @@ type commandContentRewrite struct {
contentRewriteSafety maintenance.SafetyParameters
contentRange contentRangeFlags
svc appServices
}
func (c *commandContentRewrite) setup(svc appServices, parent commandParent) {
@@ -33,10 +34,12 @@ func (c *commandContentRewrite) setup(svc appServices, parent commandParent) {
c.contentRange.setup(cmd)
safetyFlagVar(cmd, &c.contentRewriteSafety)
cmd.Action(svc.directRepositoryWriteAction(c.runContentRewriteCommand))
c.svc = svc
}
func (c *commandContentRewrite) runContentRewriteCommand(ctx context.Context, rep repo.DirectRepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
return maintenance.RewriteContents(ctx, rep, &maintenance.RewriteContentsOptions{
ContentIDRange: c.contentRange.contentIDRange(),
+5 -1
View File
@@ -14,6 +14,8 @@ type commandContentShow struct {
ids []string
indentJSON bool
decompress bool
out textOutput
}
func (c *commandContentShow) setup(svc appServices, parent commandParent) {
@@ -23,6 +25,8 @@ func (c *commandContentShow) setup(svc appServices, parent commandParent) {
cmd.Flag("json", "Pretty-print JSON content").Short('j').BoolVar(&c.indentJSON)
cmd.Flag("unzip", "Transparently decompress the content").Short('z').BoolVar(&c.decompress)
cmd.Action(svc.directRepositoryReadAction(c.run))
c.out.setup(svc)
}
func (c *commandContentShow) run(ctx context.Context, rep repo.DirectRepository) error {
@@ -41,5 +45,5 @@ func (c *commandContentShow) contentShow(ctx context.Context, r repo.DirectRepos
return errors.Wrapf(err, "error getting content %v", contentID)
}
return showContentWithFlags(bytes.NewReader(data), c.decompress, c.indentJSON)
return showContentWithFlags(c.out.stdout(), bytes.NewReader(data), c.decompress, c.indentJSON)
}
+5 -2
View File
@@ -2,7 +2,6 @@ package cli
import (
"context"
"os"
"strings"
"github.com/pkg/errors"
@@ -18,6 +17,8 @@ type commandDiff struct {
diffSecondObjectPath string
diffCompareFiles bool
diffCommandCommand string
out textOutput
}
func (c *commandDiff) setup(svc appServices, parent commandParent) {
@@ -27,6 +28,8 @@ func (c *commandDiff) setup(svc appServices, parent commandParent) {
cmd.Flag("files", "Compare files by launching diff command for all pairs of (old,new)").Short('f').BoolVar(&c.diffCompareFiles)
cmd.Flag("diff-command", "Displays differences between two repository objects (files or directories)").Default(defaultDiffCommand()).Envar("KOPIA_DIFF").StringVar(&c.diffCommandCommand)
cmd.Action(svc.repositoryReaderAction(c.run))
c.out.setup(svc)
}
func (c *commandDiff) run(ctx context.Context, rep repo.Repository) error {
@@ -47,7 +50,7 @@ func (c *commandDiff) run(ctx context.Context, rep repo.Repository) error {
return errors.New("arguments do diff must both be directories or both non-directories")
}
d, err := diff.NewComparer(os.Stdout)
d, err := diff.NewComparer(c.out.stdout())
if err != nil {
return errors.Wrap(err, "error creating comparer")
}
+5 -1
View File
@@ -13,6 +13,8 @@ type commandIndexOptimize struct {
optimizeDropDeletedOlderThan time.Duration
optimizeDropContents []string
optimizeAllIndexes bool
svc appServices
}
func (c *commandIndexOptimize) setup(svc appServices, parent commandParent) {
@@ -22,10 +24,12 @@ func (c *commandIndexOptimize) setup(svc appServices, parent commandParent) {
cmd.Flag("drop-contents", "Drop contents with given IDs").StringsVar(&c.optimizeDropContents)
cmd.Flag("all", "Optimize all indexes, even those above maximum size.").BoolVar(&c.optimizeAllIndexes)
cmd.Action(svc.directRepositoryWriteAction(c.runOptimizeCommand))
c.svc = svc
}
func (c *commandIndexOptimize) runOptimizeCommand(ctx context.Context, rep repo.DirectRepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
opt := content.CompactOptions{
MaxSmallBlobs: c.optimizeMaxSmallBlobs,
+5 -1
View File
@@ -13,6 +13,8 @@ import (
type commandIndexRecover struct {
blobIDs []string
commit bool
svc appServices
}
func (c *commandIndexRecover) setup(svc appServices, parent commandParent) {
@@ -20,10 +22,12 @@ func (c *commandIndexRecover) setup(svc appServices, parent commandParent) {
cmd.Flag("blobs", "Names of pack blobs to recover from (default=all packs)").StringsVar(&c.blobIDs)
cmd.Flag("commit", "Commit recovered content").BoolVar(&c.commit)
cmd.Action(svc.directRepositoryWriteAction(c.run))
c.svc = svc
}
func (c *commandIndexRecover) run(ctx context.Context, rep repo.DirectRepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
var totalCount int
+1 -1
View File
@@ -113,7 +113,7 @@ func (c *commandList) printDirectoryEntry(ctx context.Context, e fs.Entry, prefi
info = fmt.Sprintf("%v%v", c.nameToDisplay(prefix, e), errorSummary)
}
col.Println(info) //nolint:errcheck
col.Fprintln(c.out.stdout(), info) //nolint:errcheck
if c.recursive {
if subdir, ok := e.(fs.Directory); ok {
+5 -1
View File
@@ -10,16 +10,20 @@ import (
type commandManifestDelete struct {
manifestRemoveItems []string
svc appServices
}
func (c *commandManifestDelete) setup(svc appServices, parent commandParent) {
cmd := parent.Command("delete", "Remove manifest items").Alias("remove").Alias("rm")
cmd.Arg("item", "Items to remove").Required().StringsVar(&c.manifestRemoveItems)
cmd.Action(svc.repositoryWriterAction(c.run))
c.svc = svc
}
func (c *commandManifestDelete) run(ctx context.Context, rep repo.RepositoryWriter) error {
advancedCommand(ctx)
c.svc.advancedCommand(ctx)
for _, it := range toManifestIDs(c.manifestRemoveItems) {
if err := rep.DeleteManifest(ctx, it); err != nil {
+1 -1
View File
@@ -51,7 +51,7 @@ func (c *commandManifestShow) showManifestItems(ctx context.Context, rep repo.Re
c.out.printStdout("// label %v:%v\n", k, v)
}
if showerr := showContentWithFlags(bytes.NewReader(b), false, true); showerr != nil {
if showerr := showContentWithFlags(c.out.stdout(), bytes.NewReader(b), false, true); showerr != nil {
return showerr
}
}
+1 -1
View File
@@ -31,7 +31,7 @@ func (c *commandRepositoryConnect) setup(svc advancedAppServices, parent command
cc := cmd.Command(prov.name, "Connect to repository in "+prov.description)
f.setup(svc, cc)
cc.Action(func(_ *kingpin.ParseContext) error {
ctx := rootContext()
ctx := svc.rootContext()
st, err := f.connect(ctx, false)
if err != nil {
return errors.Wrap(err, "can't connect to storage")
+1 -1
View File
@@ -49,7 +49,7 @@ func (c *commandRepositoryCreate) setup(svc advancedAppServices, parent commandP
cc := cmd.Command(prov.name, "Create repository in "+prov.description)
f.setup(svc, cc)
cc.Action(func(_ *kingpin.ParseContext) error {
ctx := rootContext()
ctx := svc.rootContext()
st, err := f.connect(ctx, true)
if err != nil {
return errors.Wrap(err, "can't connect to storage")
+1 -1
View File
@@ -30,7 +30,7 @@ func (c *commandRepositoryRepair) setup(svc advancedAppServices, parent commandP
cc := cmd.Command(prov.name, "Repair repository in "+prov.description)
f.setup(svc, cc)
cc.Action(func(_ *kingpin.ParseContext) error {
ctx := rootContext()
ctx := svc.rootContext()
st, err := f.connect(ctx, false)
if err != nil {
return errors.Wrap(err, "can't connect to storage")
+6 -2
View File
@@ -21,6 +21,8 @@ import (
)
type commandRepositorySyncTo struct {
nextSyncOutputTime *timetrack.Throttle
repositorySyncUpdate bool
repositorySyncDelete bool
repositorySyncDryRun bool
@@ -30,7 +32,6 @@ type commandRepositorySyncTo struct {
lastSyncProgress string
syncProgressMutex sync.Mutex
nextSyncOutputTime timetrack.Throttle
setTimeUnsupportedOnce sync.Once
out textOutput
@@ -47,13 +48,16 @@ func (c *commandRepositorySyncTo) setup(svc advancedAppServices, parent commandP
c.out.setup(svc)
// needs to be 64-bit aligned on ARM
c.nextSyncOutputTime = new(timetrack.Throttle)
for _, prov := range storageProviders {
// Set up 'sync-to' subcommand
f := prov.newFlags()
cc := cmd.Command(prov.name, "Synchronize repository data to another repository in "+prov.description)
f.setup(svc, cc)
cc.Action(func(_ *kingpin.ParseContext) error {
ctx := rootContext()
ctx := svc.rootContext()
st, err := f.connect(ctx, false)
if err != nil {
return errors.Wrap(err, "can't connect to storage")
+5 -2
View File
@@ -2,7 +2,6 @@ package cli
import (
"context"
"os"
"github.com/pkg/errors"
@@ -13,12 +12,16 @@ import (
type commandShow struct {
path string
out textOutput
}
func (c *commandShow) setup(svc appServices, parent commandParent) {
cmd := parent.Command("show", "Displays contents of a repository object.").Alias("cat")
cmd.Arg("object-path", "Path").Required().StringVar(&c.path)
cmd.Action(svc.repositoryReaderAction(c.run))
c.out.setup(svc)
}
func (c *commandShow) run(ctx context.Context, rep repo.Repository) error {
@@ -34,7 +37,7 @@ func (c *commandShow) run(ctx context.Context, rep repo.Repository) error {
defer r.Close() //nolint:errcheck
_, err = iocopy.Copy(os.Stdout, r)
_, err = iocopy.Copy(c.out.stdout(), r)
return errors.Wrap(err, "unable to copy data")
}
+1 -1
View File
@@ -252,7 +252,7 @@ func (c *commandSnapshotList) outputManifestFromSingleSource(ctx context.Context
elidedCount = 0
previousOID = oid
col.Print(fmt.Sprintf(" %v %v %v\n", formatTimestamp(m.StartTime), oid, strings.Join(bits, " "))) //nolint:errcheck
col.Fprint(c.out.stdout(), fmt.Sprintf(" %v %v %v\n", formatTimestamp(m.StartTime), oid, strings.Join(bits, " "))) //nolint:errcheck
count++
+55
View File
@@ -0,0 +1,55 @@
package cli
import (
"context"
"io"
"github.com/alecthomas/kingpin"
"github.com/pkg/errors"
"github.com/kopia/kopia/repo/logging"
)
// RunSubcommand executes the subcommand asynchronously in current process
// with flags in an isolated CLI environment and returns standard output and standard error.
func (c *App) RunSubcommand(ctx context.Context, argsAndFlags []string) (stdout, stderr io.Reader, wait func() error, kill func()) {
kpapp := kingpin.New("test", "test")
stdoutReader, stdoutWriter := io.Pipe()
stderrReader, stderrWriter := io.Pipe()
c.stdoutWriter = stdoutWriter
c.stderrWriter = stderrWriter
c.rootctx = logging.WithLogger(ctx, logging.Writer(stderrWriter))
c.Attach(kpapp)
var exitCode int
resultErr := make(chan error, 1)
c.osExit = func(ec int) {
exitCode = ec
}
go func() {
defer close(resultErr)
defer stderrWriter.Close() //nolint:errcheck
defer stdoutWriter.Close() //nolint:errcheck
_, err := kpapp.Parse(argsAndFlags)
if err != nil {
resultErr <- err
return
}
if exitCode != 0 {
resultErr <- errors.Errorf("exit code %v", exitCode)
return
}
}()
return stdoutReader, stderrReader, func() error {
return <-resultErr
}, func() {}
}
+3 -4
View File
@@ -7,7 +7,6 @@ import (
"fmt"
"io"
"io/ioutil"
"os"
"time"
"github.com/pkg/errors"
@@ -17,9 +16,9 @@ import (
)
// TODO - remove this global.
var timeZone string
var timeZone = "local"
func showContentWithFlags(rd io.Reader, unzip, indentJSON bool) error {
func showContentWithFlags(w io.Writer, rd io.Reader, unzip, indentJSON bool) error {
if unzip {
gz, err := gzip.NewReader(rd)
if err != nil {
@@ -43,7 +42,7 @@ func showContentWithFlags(rd io.Reader, unzip, indentJSON bool) error {
rd = ioutil.NopCloser(&buf2)
}
if _, err := iocopy.Copy(os.Stdout, rd); err != nil {
if _, err := iocopy.Copy(w, rd); err != nil {
return errors.Wrap(err, "error copying data")
}