Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable indentation for yaml output in .sops.yaml #900

Closed
cdobbyn opened this issue Jul 6, 2021 · 19 comments · Fixed by #1273
Closed

Configurable indentation for yaml output in .sops.yaml #900

cdobbyn opened this issue Jul 6, 2021 · 19 comments · Fixed by #1273

Comments

@cdobbyn
Copy link

cdobbyn commented Jul 6, 2021

It would be great if we could configure our indentation for yaml in the .sops.yaml configuration file. Right now it's just hardcoded to set the indent to 4 all the time.

https://github.com/mozilla/sops/blob/66043e71a81787d6513bc2e5505a29aac67dc6f1/stores/yaml/store.go#L322

Yaml spec does not dictate 4 spaces.
https://yaml.org/spec/1.2/spec.html#id2777534

In YAML block styles, structure is determined by indentation. In general, indentation is defined as a zero or more space characters at the start of a line.
@jamesgoodhouse
Copy link

I did a quick dive into the code and due to how the code is structured, there doesn't appear to be any quick or easy way to implement this or otherwise make the YAML store receive configuration parameters. What would likely need to be done is a larger refactor that would allow configurations to be passed down to all of the stores, but that appeared to be a large amount of effort.

@maxstreese
Copy link

I would like to add to this one idea, which is: Could things be implemented in such a way that EditorConfig is applied?

It looks to me like an up-to-date parser library for it does exist in Go, so it should be possible to integrate it into SOPS. Though I have no idea if people actually like this idea. Personally I would love it.

Let me know if this makes sense, I'd be happy to try implementing something with some guidance (it's been a while since I have written something in Go, but hey... gotta start contributing something at some point).

@jamesgoodhouse
Copy link

I started a draft PR (#930) but have not had time to completely finish it. It does incorporate EditorConfig nor do I personally want to take that on.

@maxstreese
Copy link

But would you be open to having that as a feature or do you generally not like the idea? In case of the former maybe I could try what I can come up with.

@felixfontein
Copy link
Contributor

I personally don't like the idea of using something like EditorConfig. IMO such formatting details should be specified in .sops.yaml or on command line, but not in arbitrary 3rd party configs that are otherwise unrelated to sops.

@jamesgoodhouse
Copy link

I agree with @felixfontein. SOPS shouldn't have knowledge of or attempt to handle anything third party related.

@budimanjojo
Copy link

Or there shouldn't be a hardcoded indentation set and use the default from go-yaml? As far as I know it defaults to 2 spaces, is there any reason why should that SetIndent(4) method call exist?

@felixfontein
Copy link
Contributor

is there any reason why should that SetIndent(4) method call exist?

Two words: backwards compatibility. :)

@budimanjojo
Copy link

@felixfontein but it causes a problem that a lot of us notice, yamllint always complain about inconsistent indentation.

@felixfontein
Copy link
Contributor

@budimanjojo I do not think anyone is opposed to making indentation configurable. The only PR we have in this direction right now, #930, is unfortunately not finished and nobody wanted to pick it up and continue with it (#900 (comment)).

If your main concern is yamllint, please note that yamllint can be configured: https://yamllint.readthedocs.io/en/stable/configuration.html#ignoring-paths You can use that to ignore all *.sops.yaml files, for example.

@budimanjojo
Copy link

Understood, it's still a valid yaml nonetheless. Only a bit annoying to my eyes 😢
I might look at the code base one day and see if I can continue on that PR.

@drduker
Copy link

drduker commented Jul 27, 2022

I hate 4 spaces. This would be great to have!

@EvertonSA
Copy link

4 spaces sucks, its so bad, this feature is the best

@SISheogorath
Copy link

If you don't have to add anything to the issue, and apparently didn't bother to write a PR to fix the issue, please don't bother everyone subscribed to this topic with your whimsies regarding 4 spaces.

The problem is known and clear to everyone involved, so all that is needed is someone to go ahead and fix it. If it bothers you so much, you are the right person to do it. It probably takes as much time as complaining here.

@teaglebuilt
Copy link

If you don't have to add anything to the issue, and apparently didn't bother to write a PR to fix the issue, please don't bother everyone subscribed to this topic with your whimsies regarding 4 spaces.

The problem is known and clear to everyone involved, so all that is needed is someone to go ahead and fix it. If it bothers you so much, you are the right person to do it. It probably takes as much time as complaining here.

bahaha!

@teaglebuilt
Copy link

i know its tedious but i do agree, it would be nice. Although, if its important enough then create a pr for it. Plus, its easy enough just to ignore any sops encrypted files in your linter if that is what the hype is all about

@llbbl
Copy link

llbbl commented Mar 24, 2023

Hello, fellow sops enjoyers 🥇

I got a workaround until one of us feels like refactoring a bunch of Go code. 👀 🍭

YQ to resuce https://github.com/mikefarah/yq

use yq eval plus pretty print to output to new fixed file

yq e -P somefile.yml > somefile.fixed.yml

use yq eval plus --inplace (-i) to update that badboy without creating new file

yq e -Pi somefile.yml

🎭

@anutator
Copy link

anutator commented May 2, 2023

yq doesn't help. It changes only encrypted file. When I open secrets.yml in vim or VSCode it has 4 spaces again!

@Ph0tonic
Copy link
Contributor

Ph0tonic commented Sep 8, 2023

Hi,
I tried to finish PR #930 made by @jamesgoodhouse see #1273.
Let me know what you think about it, so maybe we can move forward with this issue. 👍

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 a pull request may close this issue.