diff --git a/cmd/sops/main.go b/cmd/sops/main.go index f682204e7..1700523d7 100644 --- a/cmd/sops/main.go +++ b/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, diff --git a/cmd/sops/set.go b/cmd/sops/set.go index a6e8ed357..7a7da298d 100644 --- a/cmd/sops/set.go +++ b/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 } diff --git a/functional-tests/src/lib.rs b/functional-tests/src/lib.rs index eb0282c4e..7d1793bb9 100644 --- a/functional-tests/src/lib.rs +++ b/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 = diff --git a/sops.go b/sops.go index 718f51bf8..01c103d45 100644 --- a/sops.go +++ b/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) { diff --git a/sops_test.go b/sops_test.go index fe1f44e2c..0115024a0 100644 --- a/sops_test.go +++ b/sops_test.go @@ -1023,10 +1023,33 @@ func TestSetNewKey(t *testing.T) { }, }, } - set := branch.Set([]interface{}{"foo", "bar", "foo"}, "hello") + set, changed := branch.Set([]interface{}{"foo", "bar", "foo"}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, "hello", set[0].Value.(TreeBranch)[0].Value.(TreeBranch)[1].Value) } +func TestSetNewKeyUnchanged(t *testing.T) { + branch := TreeBranch{ + TreeItem{ + Key: "foo", + Value: TreeBranch{ + TreeItem{ + Key: "bar", + Value: TreeBranch{ + TreeItem{ + Key: "baz", + Value: "foobar", + }, + }, + }, + }, + }, + } + set, changed := branch.Set([]interface{}{"foo", "bar", "baz"}, "foobar") + assert.Equal(t, false, changed) + assert.Equal(t, "foobar", set[0].Value.(TreeBranch)[0].Value.(TreeBranch)[0].Value) +} + func TestSetNewBranch(t *testing.T) { branch := TreeBranch{ TreeItem{ @@ -1034,7 +1057,8 @@ func TestSetNewBranch(t *testing.T) { Value: "value", }, } - set := branch.Set([]interface{}{"foo", "bar", "baz"}, "hello") + set, changed := branch.Set([]interface{}{"foo", "bar", "baz"}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, TreeBranch{ TreeItem{ Key: "key", @@ -1067,7 +1091,8 @@ func TestSetArrayDeepNew(t *testing.T) { }, }, } - set := branch.Set([]interface{}{"foo", 2, "bar"}, "hello") + set, changed := branch.Set([]interface{}{"foo", 2, "bar"}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, "hello", set[0].Value.([]interface{})[2].(TreeBranch)[0].Value) } @@ -1078,13 +1103,15 @@ func TestSetNewKeyDeep(t *testing.T) { Value: "bar", }, } - set := branch.Set([]interface{}{"foo", "bar", "baz"}, "hello") + set, changed := branch.Set([]interface{}{"foo", "bar", "baz"}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, "hello", set[0].Value.(TreeBranch)[0].Value.(TreeBranch)[0].Value) } func TestSetNewKeyOnEmptyBranch(t *testing.T) { branch := TreeBranch{} - set := branch.Set([]interface{}{"foo", "bar", "baz"}, "hello") + set, changed := branch.Set([]interface{}{"foo", "bar", "baz"}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, "hello", set[0].Value.(TreeBranch)[0].Value.(TreeBranch)[0].Value) } @@ -1099,13 +1126,15 @@ func TestSetArray(t *testing.T) { }, }, } - set := branch.Set([]interface{}{"foo", 0}, "uno") + set, changed := branch.Set([]interface{}{"foo", 0}, "uno") + assert.Equal(t, true, changed) assert.Equal(t, "uno", set[0].Value.([]interface{})[0]) } func TestSetArrayNew(t *testing.T) { branch := TreeBranch{} - set := branch.Set([]interface{}{"foo", 0, 0}, "uno") + set, changed := branch.Set([]interface{}{"foo", 0, 0}, "uno") + assert.Equal(t, true, changed) assert.Equal(t, "uno", set[0].Value.([]interface{})[0].([]interface{})[0]) } @@ -1116,7 +1145,8 @@ func TestSetExisting(t *testing.T) { Value: "foobar", }, } - set := branch.Set([]interface{}{"foo"}, "bar") + set, changed := branch.Set([]interface{}{"foo"}, "bar") + assert.Equal(t, true, changed) assert.Equal(t, "bar", set[0].Value) } @@ -1127,7 +1157,8 @@ func TestSetArrayLeafNewItem(t *testing.T) { Value: []interface{}{}, }, } - set := branch.Set([]interface{}{"array", 2}, "hello") + set, changed := branch.Set([]interface{}{"array", 2}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, TreeBranch{ TreeItem{ Key: "array", @@ -1147,7 +1178,8 @@ func TestSetArrayNonLeaf(t *testing.T) { }, }, } - set := branch.Set([]interface{}{"array", 0, "hello"}, "hello") + set, changed := branch.Set([]interface{}{"array", 0, "hello"}, "hello") + assert.Equal(t, true, changed) assert.Equal(t, TreeBranch{ TreeItem{ Key: "array",