Merge pull request #1754 from felixfontein/set-delete-idempotent

set subcommand: add --idempotent flag that will not write the file if no change happened

Author: felixfontein

cmd/sops/main.go

@@ -1356,6 +1356,10 @@ func main() {
 					Usage:  "comma separated list of decryption key types",
 					EnvVar: "SOPS_DECRYPTION_ORDER",
 				},
+				cli.BoolFlag{
+					Name:  "idempotent",
+					Usage: "do nothing if the given index already has the given value",
+				},
 			}, keyserviceFlags...),
 			Action: func(c *cli.Context) error {
 				if c.Bool("verbose") {
@@ -1393,7 +1397,7 @@ func main() {
 				if err != nil {
 					return toExitError(err)
 				}
-				output, err := set(setOpts{
+				output, changed, err := set(setOpts{
 					OutputStore:     outputStore,
 					InputStore:      inputStore,
 					InputPath:       fileName,
@@ -1408,6 +1412,11 @@ func main() {
 					return toExitError(err)
 				}
 
+				if !changed && c.Bool("idempotent") {
+					log.Info("File not written due to no change")
+					return nil
+				}
+
 				// We open the file *after* the operations on the tree have been
 				// executed to avoid truncating it when there's errors
 				file, err := os.Create(fileName)
@@ -1845,7 +1854,7 @@ func main() {
 			if err != nil {
 				return toExitError(err)
 			}
-			output, err = set(setOpts{
+			output, _, err = set(setOpts{
 				OutputStore:     outputStore,
 				InputStore:      inputStore,
 				InputPath:       fileName,

cmd/sops/set.go

@@ -21,7 +21,7 @@ type setOpts struct {
 	DecryptionOrder []string
 }
 
-func set(opts setOpts) ([]byte, error) {
+func set(opts setOpts) ([]byte, bool, error) {
 	// Load the file
 	// TODO: Issue #173: if the file does not exist, create it with the contents passed in as opts.Value
 	tree, err := common.LoadEncryptedFileWithBugFixes(common.GenericDecryptOpts{
@@ -32,7 +32,7 @@ func set(opts setOpts) ([]byte, error) {
 		KeyServices: opts.KeyServices,
 	})
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	// Decrypt the file
@@ -44,22 +44,23 @@ func set(opts setOpts) ([]byte, error) {
 		DecryptionOrder: opts.DecryptionOrder,
 	})
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	// Set the value
-	tree.Branches[0] = tree.Branches[0].Set(opts.TreePath, opts.Value)
+	var changed bool
+	tree.Branches[0], changed = tree.Branches[0].Set(opts.TreePath, opts.Value)
 
 	err = common.EncryptTree(common.EncryptTreeOpts{
 		DataKey: dataKey, Tree: tree, Cipher: opts.Cipher,
 	})
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 
 	encryptedFile, err := opts.OutputStore.EmitEncryptedFile(*tree)
 	if err != nil {
-		return nil, common.NewExitError(fmt.Sprintf("Could not marshal tree: %s", err), codes.ErrorDumpingTree)
+		return nil, false, common.NewExitError(fmt.Sprintf("Could not marshal tree: %s", err), codes.ErrorDumpingTree)
 	}
-	return encryptedFile, err
+	return encryptedFile, changed, err
 }

functional-tests/src/lib.rs

@@ -296,6 +296,115 @@ bar: baz",
         panic!("Output JSON does not have the expected structure");
     }
 
+    #[test]
+    fn set_json_file_update_idempotent_write() {
+        let file_path = prepare_temp_file(
+            "test_set_update_idempotent_write.json",
+            r#"{"a": 2, "b": "ba"}"#.as_bytes(),
+        );
+        assert!(
+            Command::new(SOPS_BINARY_PATH)
+                .arg("encrypt")
+                .arg("-i")
+                .arg(file_path.clone())
+                .output()
+                .expect("Error running sops")
+                .status
+                .success(),
+            "sops didn't exit successfully"
+        );
+        let mut before = String::new();
+        File::open(file_path.clone())
+            .unwrap()
+            .read_to_string(&mut before)
+            .unwrap();
+        let output = Command::new(SOPS_BINARY_PATH)
+            .arg("set")
+            .arg("--output-type")
+            .arg("yaml")
+            .arg(file_path.clone())
+            .arg(r#"["b"]"#)
+            .arg(r#""ba""#)
+            .output()
+            .expect("Error running sops");
+        println!(
+            "stdout: {}, stderr: {}",
+            String::from_utf8_lossy(&output.stdout),
+            String::from_utf8_lossy(&output.stderr)
+        );
+        assert!(output.status.success(), "sops didn't exit successfully");
+        let mut after = String::new();
+        File::open(file_path.clone())
+            .unwrap()
+            .read_to_string(&mut after)
+            .unwrap();
+        assert!(before != after);
+        assert!(after.starts_with("a: "));
+        let output = Command::new(SOPS_BINARY_PATH)
+            .arg("decrypt")
+            .arg("--input-type")
+            .arg("yaml")
+            .arg("--output-type")
+            .arg("yaml")
+            .arg(file_path.clone())
+            .output()
+            .expect("Error running sops");
+        println!(
+            "stdout: {}, stderr: {}",
+            String::from_utf8_lossy(&output.stdout),
+            String::from_utf8_lossy(&output.stderr)
+        );
+        let data = &String::from_utf8_lossy(&output.stdout);
+        assert!(data == "a: 2\nb: ba\n");
+    }
+
+    #[test]
+    fn set_json_file_update_idempotent_nowrite() {
+        let file_path = prepare_temp_file(
+            "test_set_update_idempotent_nowrite.json",
+            r#"{"a": 2, "b": "ba"}"#.as_bytes(),
+        );
+        assert!(
+            Command::new(SOPS_BINARY_PATH)
+                .arg("encrypt")
+                .arg("-i")
+                .arg(file_path.clone())
+                .output()
+                .expect("Error running sops")
+                .status
+                .success(),
+            "sops didn't exit successfully"
+        );
+        let mut before = String::new();
+        File::open(file_path.clone())
+            .unwrap()
+            .read_to_string(&mut before)
+            .unwrap();
+        let output = Command::new(SOPS_BINARY_PATH)
+            .arg("set")
+            .arg("--output-type")
+            .arg("yaml")
+            .arg("--idempotent")
+            .arg(file_path.clone())
+            .arg(r#"["b"]"#)
+            .arg(r#""ba""#)
+            .output()
+            .expect("Error running sops");
+        println!(
+            "stdout: {}, stderr: {}",
+            String::from_utf8_lossy(&output.stdout),
+            String::from_utf8_lossy(&output.stderr)
+        );
+        assert!(output.status.success(), "sops didn't exit successfully");
+        let mut after = String::new();
+        File::open(file_path.clone())
+            .unwrap()
+            .read_to_string(&mut after)
+            .unwrap();
+        println!("before: {}\nafter: {}", &before, &after,);
+        assert!(before == after);
+    }
+
     #[test]
     fn set_json_file_insert() {
         let file_path =

sops.go

@@ -127,6 +127,48 @@ type TreeBranch []TreeItem
 // Trees usually have more than one branch
 type TreeBranches []TreeBranch
 
+func equals(oneBranch interface{}, otherBranch interface{}) bool {
+	switch oneBranch := oneBranch.(type) {
+	case TreeBranch:
+		otherBranch, ok := otherBranch.(TreeBranch)
+		if !ok || len(oneBranch) != len(otherBranch) {
+			return false
+		}
+		for i, item := range oneBranch {
+			otherItem := otherBranch[i]
+			if !equals(item.Key, otherItem.Key) || !equals(item.Value, otherItem.Value) {
+				return false
+			}
+		}
+		return true
+	case []interface{}:
+		otherBranch, ok := otherBranch.([]interface{})
+		if !ok || len(oneBranch) != len(otherBranch) {
+			return false
+		}
+		for i, item := range oneBranch {
+			if !equals(item, otherBranch[i]) {
+				return false
+			}
+		}
+		return true
+	case Comment:
+		otherBranch, ok := otherBranch.(Comment)
+		if !ok {
+			return false
+		}
+		return oneBranch.Value == otherBranch.Value
+	default:
+		// Unexpected type
+		return oneBranch == otherBranch
+	}
+}
+
+// Compare a branch with another one
+func (branch TreeBranch) Equals(other TreeBranch) bool {
+	return equals(branch, other)
+}
+
 func valueFromPathAndLeaf(path []interface{}, leaf interface{}) interface{} {
 	switch component := path[0].(type) {
 	case int:
@@ -156,47 +198,55 @@ func valueFromPathAndLeaf(path []interface{}, leaf interface{}) interface{} {
 	}
 }
 
-func set(branch interface{}, path []interface{}, value interface{}) interface{} {
+func set(branch interface{}, path []interface{}, value interface{}) (interface{}, bool) {
 	switch branch := branch.(type) {
 	case TreeBranch:
 		for i, item := range branch {
 			if item.Key == path[0] {
+				var changed bool
 				if len(path) == 1 {
+					changed = !equals(branch[i].Value, value)
 					branch[i].Value = value
 				} else {
-					branch[i].Value = set(item.Value, path[1:], value)
+					branch[i].Value, changed = set(item.Value, path[1:], value)
 				}
-				return branch
+				return branch, changed
 			}
 		}
 		// Not found, need to add the next path entry to the branch
 		value := valueFromPathAndLeaf(path, value)
 		if newBranch, ok := value.(TreeBranch); ok && len(newBranch) > 0 {
-			return append(branch, newBranch[0])
+			return append(branch, newBranch[0]), true
 		}
-		return branch
+		return branch, true
 	case []interface{}:
 		position := path[0].(int)
+		var changed bool
 		if len(path) == 1 {
 			if position >= len(branch) {
-				return append(branch, value)
+				return append(branch, value), true
 			}
+			changed = !equals(branch[position], value)
 			branch[position] = value
 		} else {
 			if position >= len(branch) {
 				branch = append(branch, valueFromPathAndLeaf(path[1:], value))
+				changed = true
+			} else {
+				branch[position], changed = set(branch[position], path[1:], value)
 			}
-			branch[position] = set(branch[position], path[1:], value)
 		}
-		return branch
+		return branch, changed
 	default:
-		return valueFromPathAndLeaf(path, value)
+		newValue := valueFromPathAndLeaf(path, value)
+		return newValue, !equals(branch, newValue)
 	}
 }
 
 // Set sets a value on a given tree for the specified path
-func (branch TreeBranch) Set(path []interface{}, value interface{}) TreeBranch {
-	return set(branch, path, value).(TreeBranch)
+func (branch TreeBranch) Set(path []interface{}, value interface{}) (TreeBranch, bool) {
+	v, changed := set(branch, path, value)
+	return v.(TreeBranch), changed
 }
 
 func unset(branch interface{}, path []interface{}) (interface{}, error) {