From 2e3cbe3e27a2db90a1b99080af3c7847f0f0d930 Mon Sep 17 00:00:00 2001 From: chenminjian Date: Thu, 13 Sep 2018 16:39:28 +0800 Subject: [PATCH] feat: add dry-run flag for config profile apply command License: MIT Signed-off-by: chenminjian <727180553@qq.com> --- core/commands/config.go | 74 +++++++++++++++++++++++++++++------ package.json | 6 +++ test/sharness/t0021-config.sh | 42 ++++++++++++++++++++ 3 files changed, 111 insertions(+), 11 deletions(-) diff --git a/core/commands/config.go b/core/commands/config.go index e0aef0bbe..ee6dfd5ee 100644 --- a/core/commands/config.go +++ b/core/commands/config.go @@ -16,10 +16,17 @@ import ( repo "github.com/ipfs/go-ipfs/repo" fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo" + "gx/ipfs/QmP2i47tnU23ijdshrZtuvrSkQPtf9HhsMb9fwGVe8owj2/jsondiff" config "gx/ipfs/QmSoYrBMibm2T3LupaLuez7LPGnyrJwdRxvTfPUyCp691u/go-ipfs-config" "gx/ipfs/Qmde5VP1qUkyQXKCfmEUA7bP64V2HAptbJ7phuPp7jXWwg/go-ipfs-cmdkit" ) +// ConfigUpdateOutput is config profile apply command's output +type ConfigUpdateOutput struct { + Old config.Config + New config.Config +} + type ConfigField struct { Key string Value interface{} @@ -333,6 +340,9 @@ var configProfileApplyCmd = &cmds.Command{ Helptext: cmdkit.HelpText{ Tagline: "Apply profile to config.", }, + Options: []cmdkit.Option{ + cmdkit.BoolOption("dry-run", "print difference between the current config and the config that would be generated"), + }, Arguments: []cmdkit.Argument{ cmdkit.StringArg("profile", true, false, "The profile to apply to the config."), }, @@ -343,13 +353,40 @@ var configProfileApplyCmd = &cmds.Command{ return } - err := transformConfig(req.InvocContext().ConfigRoot, req.Arguments()[0], profile.Transform) + dryRun, _, _ := req.Option("dry-run").Bool() + oldCfg, newCfg, err := transformConfig(req.InvocContext().ConfigRoot, req.Arguments()[0], profile.Transform, dryRun) if err != nil { res.SetError(err, cmdkit.ErrNormal) return } - res.SetOutput(nil) + res.SetOutput(&ConfigUpdateOutput{ + Old: *oldCfg, + New: *newCfg, + }) }, + Marshalers: cmds.MarshalerMap{ + cmds.Text: func(res cmds.Response) (io.Reader, error) { + if res.Error() != nil { + return nil, res.Error() + } + + v, err := unwrapOutput(res.Output()) + if err != nil { + return nil, err + } + + apply, ok := v.(*ConfigUpdateOutput) + if !ok { + return nil, e.TypeErr(apply, v) + } + + diff := jsondiff.Compare(apply.Old, apply.New) + buf := jsondiff.Format(diff) + + return strings.NewReader(string(buf)), nil + }, + }, + Type: ConfigUpdateOutput{}, } func buildProfileHelp() string { @@ -367,29 +404,44 @@ func buildProfileHelp() string { return out } -func transformConfig(configRoot string, configName string, transformer config.Transformer) error { +// transformConfig returns old config and new config instead of difference between they, +// because apply command can provide stable API through this way. +// If dryRun is true, repo's config should not be updated and persisted +// to storage. Otherwise, repo's config should be updated and persisted +// to storage. +func transformConfig(configRoot string, configName string, transformer config.Transformer, dryRun bool) (*config.Config, *config.Config, error) { r, err := fsrepo.Open(configRoot) if err != nil { - return err + return nil, nil, err } defer r.Close() cfg, err := r.Config() if err != nil { - return err + return nil, nil, err } - err = transformer(cfg) + // make a copy to avoid updating repo's config unintentionally + oldCfg := *cfg + newCfg := oldCfg + err = transformer(&newCfg) if err != nil { - return err + return nil, nil, err } - _, err = r.BackupConfig("pre-" + configName + "-") - if err != nil { - return err + if !dryRun { + _, err = r.BackupConfig("pre-" + configName + "-") + if err != nil { + return nil, nil, err + } + + err = r.SetConfig(&newCfg) + if err != nil { + return nil, nil, err + } } - return r.SetConfig(cfg) + return &oldCfg, &newCfg, nil } func getConfig(r repo.Repo, key string) (*ConfigField, error) { diff --git a/package.json b/package.json index 1e004aff8..2ec69e060 100644 --- a/package.json +++ b/package.json @@ -552,6 +552,12 @@ "name": "go-multiaddr-dns", "version": "0.2.4" }, + { + "author": "elgris", + "hash": "QmP2i47tnU23ijdshrZtuvrSkQPtf9HhsMb9fwGVe8owj2", + "name": "jsondiff", + "version": "0.0.0" + }, { "author": "marten-seemann", "hash": "QmNtLLJMc7TDMSaNuJVFipxLji9NL56QyMnBBPk2X58Mno", diff --git a/test/sharness/t0021-config.sh b/test/sharness/t0021-config.sh index 3afe1cb43..ec7ba7038 100755 --- a/test/sharness/t0021-config.sh +++ b/test/sharness/t0021-config.sh @@ -75,6 +75,17 @@ test_profile_apply_revert() { ' } +test_profile_apply_dry_run_not_alter() { + profile=$1 + + test_expect_success "'ipfs config profile apply ${profile} --dry-run' doesn't alter config" ' + cat "$IPFS_PATH/config" >expected && + ipfs config profile apply '${profile}' --dry-run && + cat "$IPFS_PATH/config" >actual && + test_cmp expected actual + ' +} + test_config_cmd() { test_config_cmd_set "beep" "boop" test_config_cmd_set "beep1" "boop2" @@ -220,6 +231,37 @@ test_config_cmd() { # need to do this in reverse as the test profile is already applied in sharness test_profile_apply_revert default-networking test + test_profile_apply_dry_run_not_alter server + + test_profile_apply_dry_run_not_alter local-discovery + + test_profile_apply_dry_run_not_alter test + + test_expect_success "'ipfs config profile apply local-discovery --dry-run' looks good with different profile info" ' + ipfs config profile apply local-discovery --dry-run > diff_info && + test `grep "DisableNatPortMap" diff_info | wc -l` = 2 + ' + + test_expect_success "'ipfs config profile apply server --dry-run' looks good with same profile info" ' + ipfs config profile apply server --dry-run > diff_info && + test `grep "DisableNatPortMap" diff_info | wc -l` = 1 + ' + + test_expect_success "'ipfs config profile apply server' looks good with same profile info" ' + ipfs config profile apply server > diff_info && + test `grep "DisableNatPortMap" diff_info | wc -l` = 1 + ' + + test_expect_success "'ipfs config profile apply local-discovery' looks good with different profile info" ' + ipfs config profile apply local-discovery > diff_info && + test `grep "DisableNatPortMap" diff_info | wc -l` = 2 + ' + + test_expect_success "'ipfs config profile apply test' looks good with different profile info" ' + ipfs config profile apply test > diff_info && + test `grep "DisableNatPortMap" diff_info | wc -l` = 2 + ' + # won't work as it changes datastore definition, which makes ipfs not launch # without converting first # test_profile_apply_revert badgerds