Skip to content

Conversation

@VojtechVitek
Copy link

@VojtechVitek VojtechVitek commented Sep 16, 2019

Support TOML format via https://github.com/BurntSushi/toml (see comparison of pkgs).

Fixes #369.

example.toml:

hello = "Welcome to SOPS! Edit this file as you please!"
example_key = "example_value"
example_array = ["example_value1", "example_value2"]
example_number = 1234.56789
example_booleans = [true, false]

[this]
  [this.is]
    [this.is.a]
      [this.is.a.nested]
        value = "x"
        value_unencrypted = "z"

[[array]]
  x = 1

[[array]]
  x = 2

$ sops -e example.com

hello = "ENC[AES256_GCM,data:ruUUaseZvqhH7pqzXnEB96yTIGjh/H9k85EcKgRpfOaxs4iKV83KggwF+yAxpw==,iv:4R7NkawLmxW994T67Bliuf0CpnUCmWPX0fQ+1pdQqzM=,tag:TbuH3mo0F2PiDahRvXTyuA==,type:str]"
example_key = "ENC[AES256_GCM,data:YbDiIY1NqdOVwZG2/w==,iv:1yHgwaQTAhb23hMS0ikf5IvSP0cNIhOkv3TA5s1T/q0=,tag:dWPi4TOhn2tqPn7l8BYg5Q==,type:str]"
example_array = ["ENC[AES256_GCM,data:35Tl7tcJuheA52Sn660=,iv:rje7VOdH7e0rUBSfrTpddNoDk31HVmTFiyIVe3USHMM=,tag:CiEpdcHMMhQvvKwoSRESrg==,type:str]", "ENC[AES256_GCM,data:THo2TzzX8m1LiDjcT6o=,iv:RFYqkUOtWH4plsEywQ71Uxs7i7014R3y9S5nqDviXsE=,tag:oa76Fbm6mq1tkPSu5p696Q==,type:str]"]
example_number = "ENC[AES256_GCM,data:E+tWSaKlHX6ruA==,iv:nW5/QAfxWMx0pMOE3supVABqavBdh6L2VHdRdCfRFiE=,tag:9n8ifLzNsk8IM5YlcTZXAQ==,type:float]"
example_booleans = ["ENC[AES256_GCM,data:RyWVyA==,iv:D9hZrRP9msiU/ug6EwFjqAcgRS78TcWIQplqfCFkkQw=,tag:7PZIOONP0e6O2LuXi0H2lQ==,type:bool]", "ENC[AES256_GCM,data:Ga1MGmg=,iv:b+4EbrEwylWBJXLBktJfem1CYsCRur+0JzJ4L878Glg=,tag:jftvTeUEa13/WFbpquuEXQ==,type:bool]"]

[this]
  [this.is]
    [this.is.a]
      [this.is.a.nested]
        value = "ENC[AES256_GCM,data:6A==,iv:1qQrmVmLQrAPZb6IjMcw7aVgklBqDyYfFSF8Bkz82DY=,tag:+u3CB4xyzA9knFiS/6A06A==,type:str]"
        value_unencrypted = "z"

[[array]]
  x = "ENC[AES256_GCM,data:TQ==,iv:gBpHK+l+ov6uHdh0z38UCn2yAMbH48pbcSNdouGSuAc=,tag:NS+L69Isn40Rf/krnSEeig==,type:int64]"

[[array]]
  x = "ENC[AES256_GCM,data:pw==,iv:+Azie2Hr11zOV++gYEzNGn4kEE/uRkBfiP1Cqna3tPE=,tag:+Ct31l+1MTG79wDleV4n1Q==,type:int64]"

[sops]
  shamir_threshold = 0
  lastmodified = "2019-09-16T13:24:42Z"
  mac = "ENC[AES256_GCM,data:u3R+iOeV8WRuJYaKZfgDt1t6s3IsTlmETAZdQWaEtySYUih/2l3eoNo62celtYYs0YTaIFb67NCFre4CYdW2/Wfqh0vP0geYnW8gIxX/zszrxPUr785qnaFDoXZOTk/M6Sj2SYe1c/QpRyj6hGup/2a+tmpR4GbuQP4Ma3hE1xo=,iv:MPzM+70L/oMBerfC7sJJwiDvQv3NYa+VxnnI5Syg4cg=,tag:b7PRjKYbkUBzCn50Xi4WyA==,type:str]"
  unencrypted_suffix = "_unencrypted"
  version = "3.4.0"

  [[sops.pgp]]
    created_at = "2019-09-16T13:19:31Z"
    enc = "-----BEGIN PGP MESSAGE-----\n\nwcBMAyUpShfNkFB/AQgAP48lEqyHGit9muq1dfJ0zW7iURaHuBRxQiIagVornCwE\nyBazyRbf+vfT2DRM9gj4bz59cHoO3tRx2p5dYjc4Igj73e+3HJwREKJVp3Gaw5Bo\nlLgiczArRDJ8H5RhlmNXjeX2umwtXh/Utue1m5Yh52emSENv4pTDJ4iUM6TiWAwa\nyfX0UuO2GZdW81TePJ2MzwRqxYDEY6hXEtPk7IzdrPsWRep0GFOwTEv2RaNx2+dH\nTpVE27SwFqKnBcWfFtEXMOjLXolBcti3wquWL+Q/g4OwFi2/cGU83Vg5Ywf2AEvh\nI5iQX1qBOSX/9UJJSXW7qrzp00oaXle0CrMIVQB+j9LgAeRn2YzNJgiBFvmMh7lr\nwrPd4WPK4HzgBuGQ5uDp4iZGMNXgY+XOg6qhVpbUiE7c6ObkIOKczpKxdSRSmor2\nY6GE9wWN2eAC5JZRZmzfpOwcG8E1iBnCL1Dild/veuGitwA=\n=gLdE\n-----END PGP MESSAGE-----"
    fp = "FBC7B9E2A4F9289AC0C1D4843D16CEE4A27381B4"

@codecov-io
Copy link

codecov-io commented Sep 16, 2019

Codecov Report

Merging #533 into develop will decrease coverage by 0.13%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #533      +/-   ##
===========================================
- Coverage    36.46%   36.32%   -0.14%     
===========================================
  Files           20       20              
  Lines         2863     2874      +11     
===========================================
  Hits          1044     1044              
- Misses        1725     1736      +11     
  Partials        94       94
Impacted Files Coverage Δ
stores/stores.go 0% <ø> (ø) ⬆️
sops.go 55.78% <0%> (-0.58%) ⬇️
aes/cipher.go 64.28% <0%> (-3.01%) ⬇️
decrypt/decrypt.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8969af...37c24ef. Read the comment docs.

Copy link
Contributor

@autrilla autrilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me! Just a few minor nits.

I'd like to see some tests before merging too, ideally both unit and functional. Should be able to pretty much copy the ones from other stores.

// TOML reader that preserves original order via toml.Metadata.Keys(),
// see https://godoc.org/github.com/BurntSushi/toml#MetaData.Keys
// and converts the unoredered data (multi-level tree of
// map[string]interface{}) into sops.BranchTree.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// map[string]interface{}) into sops.BranchTree.
// map[string]interface{}) into sops.TreeBranch.

return printTreeItem(newIndenter(w, ""), item)
}

// TOML indenter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documenting why this is needed would be nice.

(not sure it's actually needed -- if it's just for better styled output, mentioning it would be good too)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, will do. It all comes down to limitations of the TOML libraries, I'll explain further in the comment.

metadataHolder := stores.SopsFile{}
err := toml.Unmarshal(in, &metadataHolder)
if err != nil {
return sops.Tree{}, fmt.Errorf("Error unmarshalling metadata: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general rule, error messages should start with a lower case letter. Can you please fix this for all your errors?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was following style of the existing codebase:
Screen Shot 2019-09-16 at 10 32 58 PM

Perhaps we could revisit all the errors at once in a separate PR and utilize the new Go 1.13's error wrapping?

case []interface{}:
return nil, fmt.Errorf("Error extracting array of %v items. Please, access an individual item.", len(v))
default:
if err := printTreeBranch(&b, sops.TreeBranch{sops.TreeItem{Key: "_delete", Value: v}}); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I feel about this. At the very least I'd add a comment describing why this is done. I imagine the alternative is harder, but people are going to wonder why this happens when reading the code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TOML library lets you encrypt structs and maps only, ie. a key is required for each value. Marshal() would fail on plain values.

This is actually a limitation of the TOML language specs. Let me document this better.

@vincentserpoul
Copy link

What's left to be done on this PR? Any help needed to merge it?

@VojtechVitek
Copy link
Author

Sorry, I lost context and moved on. However, this PR was working.

I'm busy with other work now. If you want me to finish this PR, please PM me at vojtech.vitek @golang.cz and/or consider sponsoring me.

@autrilla
Copy link
Contributor

What's left to be done on this PR? Any help needed to merge it?

Mainly tests, but there are also some outstanding review comments.

@vincentserpoul vincentserpoul mentioned this pull request Feb 3, 2021
@ajvb ajvb closed this Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants