Skip to content

Warn user about .yml config file extension #1798

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Warn user about .yml config file extension #1798

wants to merge 2 commits into from

Conversation

onnimonni
Copy link

@onnimonni onnimonni commented Mar 13, 2025 •
edited
Loading

Hey,

I spent way too much time to debug my environment and it turned out that the reason was that I had added my config into .sops.yml instead of .sops.yaml.

# This is not working
$ sops secrets/server.yaml
config file not found, or has no creation rules, and no keys provided through command line options

# fix the problem
$ mv .sops.yml .sops.yaml                                                                                                                                                 
renamed '.sops.yml' -> '.sops.yaml'

# It works now
$ sops secrets/server.yaml

This is my first contribution into sops and I'm happy to hear feedback and improve this. Thanks in advance 🙇

@onnimonni
Add support for .yml file extension in config file
4619e54 
Signed-off-by: Onni Hakala <onni@flaky.build>
@onnimonni onnimonni force-pushed the allow-config-file-yml-extension branch from 1d1ce9d to 4619e54 Compare March 13, 2025 16:53
@felixfontein
Copy link
Contributor

Thanks for your contribution!

Personally I'm against this change. Having different config file names is usually more a source of confusion.

There's already a note in the documentation, though it's a bit hidden since there's no central configuration documentation so far: https://github.com/getsops/sops?tab=readme-ov-file#213using-sopsyaml-conf-to-select-kms-pgp-and-age-for-new-files

@felixfontein felixfontein requested a review from a team March 16, 2025 12:10
@onnimonni
Copy link
Author

onnimonni commented Mar 16, 2025 •
edited
Loading

Personally I'm against this change. Having different config file names is usually more a source of confusion.

I think this is a good point and similarly as we have linting rules in the code it's fine if you and the rest of the team want to keep this convention in sops.

How about logging a warning instead if .sops.yml variant is found? Or throwing an error?

The warning would make it more obvious for the user that their config was found but .yml is not supported.

AFAIK most other software will allow both variants and I assume I will not be the last one to stumble for this.

EDIT: Clarified weird sentences 👍

@felixfontein
Copy link
Contributor

How about logging a warning instead if .sops.yml variant is found? Or throwing an error?

I think it would be best to report a warning if SOPS needs a config file and doesn't find one, but does find a .sops.yml while searching for .sops.yaml. I think that helps already a lot and improves the failure message you'll get.

@onnimonni
Remove support for .yml config extension and just log warning instead
f0ea3ac 
Signed-off-by: Onni Hakala <onni@flaky.build>
@onnimonni onnimonni force-pushed the allow-config-file-yml-extension branch from f958052 to f0ea3ac Compare March 18, 2025 20:59
@onnimonni onnimonni changed the title Add support for .yml file extension in config file Warn user about .yml config file extension Mar 18, 2025
@onnimonni
Copy link
Author

Sorry that it took so long and I'm definitely not golang expert but I think this should do it 👍

@felixfontein felixfontein mentioned this pull request Mar 30, 2025
Merged
@felixfontein
Copy link
Contributor

I created a new PR (#1820) which comes closer to what I'd prefer the warning to behave like, and which also compiles :) Thanks for your contribution though, I think this warning is really helpful!

@onnimonni
Copy link
Author

Thanks @felixfontein 🙇! I'm very happy with the new error messages 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@onnimonni @felixfontein