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.

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

Print warning when AddJsonFile is called with "local.settings.json" #613

Open
anthonychu opened this issue Sep 3, 2021 · 6 comments · May be fixed by #2777
Open

Print warning when AddJsonFile is called with "local.settings.json" #613

anthonychu opened this issue Sep 3, 2021 · 6 comments · May be fixed by #2777
Labels
area: configuration good first issue Good for newcomers

Comments

@anthonychu
Copy link
Member

Is it possible to somehow warn if someone tries to load local.settings.json like this? Maybe an analyzer?

        public static void Main()
        {
            var host = new HostBuilder()
                .ConfigureFunctionsWorkerDefaults()
                .ConfigureAppConfiguration((context, config) =>
                {
                    config.AddJsonFile("local.settings.json", optional: true);
                })
                .Build();

            host.Run();
        }

There's no reason to do this. Locally, Core Tools already loads the settings from that file into the environment. In Azure, we don't want customers using local.settings.json. They should be using App Settings.
@SeanFeldman
Copy link
Contributor

Thinking out loud here - since the Functions SDK provides the config, could it detect that one of the Configuration providers is local.settings.json and throw?

@fabiocav
Copy link
Member

@SeanFeldman we could.

@anthonychu is the main concern here cases where customers may be unknowingly loading development settings when running in prod?

@ghost ghost added the no-recent-activity label Sep 18, 2021
@ghost
Copy link

ghost commented Sep 18, 2021

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

@ghost ghost removed the no-recent-activity label Sep 21, 2021
@fabiocav
Copy link
Member

We'll leave this open to track adding a analyzer, but this is low priority at the moment.

@fabiocav fabiocav added the good first issue Good for newcomers label Sep 22, 2021
@jeffputz
Copy link

This is an old issue, but I'm going to share a pretty strong opinion... forcing any kind of file name here is bad because it breaks from the conventions of dotnet a whole. Furthermore, using a cascade of json files to override things is a common and correct convention among teams.

The example I come across, and something we use in web apps, is that we commit an appsettings.json to source control, but we ignore appsettings.development.json. The latter is typically used by developers to set their own secrets, connection strings, or arbitrary settings like numberOfItemsToDisplay. Because you're forced into this unconventional situation, developers end up accidentally checking in local.settings.json.

Can they set an environment variable, sure, but that's not as convenient as saving a text file you already have open.

@BogdanYarotsky
Copy link

Hello, I would like to make a contribution by adding an analyzer which will warn users. Is this task still relevant?

@BogdanYarotsky BogdanYarotsky linked a pull request Oct 13, 2024 that will close this issue
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: configuration good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
6 participants
@SeanFeldman @jeffputz @fabiocav @anthonychu @mattchenderson @BogdanYarotsky