Merge pull request #5455 from chenminjian/feat/flags/profile-apply

feat: add dry-run flag for config profile apply command

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 {
+	OldCfg map[string]interface{}
+	NewCfg map[string]interface{}
+}
+
 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,53 @@ 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)
+
+		oldCfgMap, err := scrubPrivKey(oldCfg)
+		if err != nil {
+			res.SetError(err, cmdkit.ErrNormal)
+			return
+		}
+
+		newCfgMap, err := scrubPrivKey(newCfg)
+		if err != nil {
+			res.SetError(err, cmdkit.ErrNormal)
+			return
+		}
+
+		res.SetOutput(&ConfigUpdateOutput{
+			OldCfg: oldCfgMap,
+			NewCfg: newCfgMap,
+		})
 	},
+	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.OldCfg, apply.NewCfg)
+			buf := jsondiff.Format(diff)
+
+			return strings.NewReader(string(buf)), nil
+		},
+	},
+	Type: ConfigUpdateOutput{},
 }
 
 func buildProfileHelp() string {
@@ -367,29 +417,59 @@ func buildProfileHelp() string {
 	return out
 }
 
-func transformConfig(configRoot string, configName string, transformer config.Transformer) error {
+// scrubPrivKey scrubs private key for security reasons.
+func scrubPrivKey(cfg *config.Config) (map[string]interface{}, error) {
+	cfgMap, err := config.ToMap(cfg)
+	if err != nil {
+		return nil, err
+	}
+
+	err = scrubValue(cfgMap, []string{config.IdentityTag, config.PrivKeyTag})
+	if err != nil {
+		return nil, err
+	}
+
+	return cfgMap, nil
+}
+
+// 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) {

package.json

@@ -553,6 +553,12 @@
       "version": "0.2.4"
     },
     {
+      "author": "elgris",
+      "hash": "QmP2i47tnU23ijdshrZtuvrSkQPtf9HhsMb9fwGVe8owj2",
+      "name": "jsondiff",
+      "version": "0.0.0"
+    },
+    {
       "author": "marten-seemann",
       "hash": "QmNtLLJMc7TDMSaNuJVFipxLji9NL56QyMnBBPk2X58Mno",
       "name": "go-libp2p-quic-transport",

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,47 @@ 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
+  '
+
+  test_expect_success "'ipfs config profile apply test --dry-run' doesn't include privkey" '
+    ipfs config profile apply test --dry-run > show_config &&
+    test_expect_code 1 grep PrivKey show_config
+  '
+
+  test_expect_success "'ipfs config profile apply test' doesn't include privkey" '
+    ipfs config profile apply test > show_config &&
+    test_expect_code 1 grep PrivKey show_config
+  '
+
   # won't work as it changes datastore definition, which makes ipfs not launch
   # without converting first
   # test_profile_apply_revert badgerds