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

New Feature - Merge configuration files #296

Closed
AaronFixer opened this issue Mar 29, 2018 · 18 comments · Fixed by #316
Closed

New Feature - Merge configuration files #296

AaronFixer opened this issue Mar 29, 2018 · 18 comments · Fixed by #316
Labels
feature A new feature help wanted Not actively being worked on. If you plan to contribute, please drop a note. medium effort Likely a few days of development effort

Comments

@AaronFixer
Copy link

Expected Behavior / New Feature

This may already be a feature but I see no mention of it in the documentation and it isn't working in my project.

It would be good to support multiple .json files, potentially with better naming conventions. I have an app with a number of services and routes, and it would be nice to separate them out into dedicated files rather then have one huge configuration.json.

For example:

  • AccountRoutes.json <- Would have all routes for the account controller/service
  • ComponentActions.json <- Would have all the routes for component actions controller/service

etc. etc.

Is this already possible? It seems that when I define multiple .AddJsonFile() it only picks up the routes in the last file it pinks up with the ReRoutes field.

@AaronFixer AaronFixer changed the title Supporting multiple *.json files? Supporting multiple json route files? Mar 29, 2018
@TomPallister TomPallister changed the title Supporting multiple json route files? New Feature - Merge configuration files Mar 29, 2018
@TomPallister TomPallister added feature A new feature medium effort Likely a few days of development effort help wanted Not actively being worked on. If you plan to contribute, please drop a note. labels Mar 29, 2018
@TomPallister
Copy link
Member

@AaronFixer thanks for your interest in the project!

This feature isn't supported yet I will but it on the backlog but if you would like to have a go yourself please feel free to submit a PR.

There are a couple of considerations here.

  1. Ocelot uses configuration.json as a database so a solution would need to take multiple files and merge them into configuration.json or have a second file that ocelot uses as a database (not sure I like this).

  2. Been thinking about this for a while completely change how configuration works. I was thinking of making it more like identity server where it is provided statically in c# / database.

  3. You could just do this yourself in ConfigureAppConfiguration with something like below and it would not need to be part of Ocelot.

.ConfigureAppConfiguration((hostingContext, config) =>
                {
                    merger.Merge();
      
                    config
                        .SetBasePath(hostingContext.HostingEnvironment.ContentRootPath)
                        .AddJsonFile("configuration.json");
                })

I think 1 and 3 are better options at the moment for me but that could change!

Anyway let me know if you want to give this a try and I will be of as much assistance as I can...otherwise I'm afraid you will have to wait a bit for the feature :(

@AaronFixer
Copy link
Author

For me this is definitely something I think I (and many others) could potentially benefit from with large projects wanting to use a Gateway!

I'll have to do this in my spare time, although I don't imagine it should take too long.

1 and 3 both work. 3 is something I could just use for the time being as a workaround. I'll see if I get a chance to work on a PR for 1. in the meantime.

Thanks for getting back to me!

@TomPallister
Copy link
Member

Yep I agree this would be a useful feature! If you don't get round to it its on my list and I expect to do it in the next months or so.

TomPallister added a commit that referenced this issue Apr 15, 2018
TomPallister pushed a commit that referenced this issue Apr 15, 2018
TomPallister pushed a commit that referenced this issue Apr 17, 2018
TomPallister added a commit that referenced this issue Apr 17, 2018
* #296 renamed configuration.json to ocelot.json in preparation

* removed things we dont need for tests

* another file we dont need

* removed some async we dont need

* refactoring to consolidate configuration code

* removed another pointless abstraction

* #296 started writing merge code

* #296 coming up with ideas for this config merging

* #296 still hacking this idea around

* #296 will now do a crappy merge on the configuration

* #296 change so tests pass on windows
@TomPallister TomPallister reopened this Apr 17, 2018
@TomPallister
Copy link
Member

@AaronFixer Ive merged a change that lets you have multiple configuration files..the docs for this are here. Can you let me know if you have any suggestions or this meets you use case before I release the change to NuGet?

@AaronFixer
Copy link
Author

@TomPallister Sorry for not doing this myself, I got caught up with the project I'm working on and didn't get a chance to look into this.

I'll definitely take a look at the changes and provide some feedback ASAP! Thank you.

TomPallister added a commit that referenced this issue Apr 18, 2018
@TomPallister
Copy link
Member

@AaronFixer no worries, Ive just released this in 5.5.3, hopefully its OK, if not let me know and re-open issue!

@CesarD
Copy link

CesarD commented Jun 21, 2019

Hello, people.
I think I'm hitting a bug with this feature.

I have ocelot.Development.json, ocelot.Testing.json and ocelot.global.json.
When I run the app on debug from VS, which should stick to the Development environment, it loads the Testing configuration in ocelot.json.

After taking a look at Ocelot's source code, I see this line:

string excludeConfigName = env?.EnvironmentName != null ? $"ocelot.{env.EnvironmentName}.json" : string.Empty;
// ...
var files = new DirectoryInfo(folder)
                .EnumerateFiles()
                .Where(fi => reg.IsMatch(fi.Name) && (fi.Name != excludeConfigName))
                .ToList();

Unless I'm getting this wrong, it's excluding my Development environment file and keeping only the Testing one, which I think goes against the purpose of this feature, isn't it? It should include Development as it's the current environment in which the app is running and exclude all other environment files so they don't get merged into a conflicting configuration.

@CesarD
Copy link

CesarD commented Jun 21, 2019

BTW, and sorry for the double post, it would be great to extend this feature into detecting file naming as the following:

ocelot.customers.{environment}.json
ocelot.users.{environment}.json
ocelot.products.{environment}.json

This way this would allow us to better isolate groups of APIs also for each environment.

@zhutoutou
Copy link

Can this method support IHostEnvironment Or How transform IHostEnvironment to IWebHostEnvironment ?

@Faisal-developer
Copy link

Faisal-developer commented Apr 29, 2020

@zhutoutou use this

   public static IHostBuilder CreateHostBuilder(string[] args) =>
                 Host.CreateDefaultBuilder(args)
                    .ConfigureWebHostDefaults(webBuilder =>
                    {
                        webBuilder.ConfigureAppConfiguration((hostingContext, config) =>
                          {
                              config.AddOcelot("Routes", hostingContext.HostingEnvironment);
                              config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath)
                                    .AddJsonFile("ocelot.json",true)
                                    .AddEnvironmentVariables();
                          })
                         .ConfigureServices(services =>
                         {
                             services.AddOcelot();
                         })
                        .Configure(app =>
                        {
                            //app.UseRouting();
                            //app.UseEndpoints(endpoints => {
                            //    endpoints.MapControllers();
                            //});
                            app.UseOcelot().Wait();

@NicoJuicy
Copy link

NicoJuicy commented Jan 28, 2021

@Faisal-developer commented on Apr 29, 2020

A bit better formatting, it wasn't clear what changed to me ( fyi it's the part with wrapping ConfigureAppConfiguration with ConfigureWebHostDefaults:

      Host.CreateDefaultBuilder(args)
                .ConfigureWebHostDefaults(webBuilder =>
                {
                    webBuilder.UseStartup<Startup>();
                })
            .ConfigureWebHostDefaults(webBuilder =>//new for .net core 3
            {
                webBuilder.UseUrls("https://*:44326")
                .ConfigureAppConfiguration((hostingContext, config) =>
                {
                    config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath)
                    //  .AddJsonFile("gateway/v1/orders.json"
                    //.AddJsonFile("gateway/v1/shipping.json") //this won't work to load multiple configs
                    .AddEnvironmentVariables()
                    .AddOcelot("gateway/v1", hostingContext.HostingEnvironment); //loads multiple configurations : https://github.com/ThreeMammals/Ocelot/issues/1047
                });  
            })  
            //old way. Gives an error in .net core 3.0
            //.ConfigureAppConfiguration((hostingContext, config) =>
            // {
            //           config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath)
            //           .AddJsonFile("gateway/v1/orders.json"
            //});

@ThreeMammals ThreeMammals deleted a comment from Flave1 Nov 10, 2023
@ThreeMammals ThreeMammals deleted a comment from zhutoutou Nov 11, 2023
@raman-m
Copy link
Member

raman-m commented Nov 11, 2023

@CesarD commented on Jun 21, 2019
@CesarD commented on Jun 21, 2019

Hi César!
Are you still with Ocelot?
FYI #1183

@CesarD
Copy link

CesarD commented Nov 11, 2023

@raman-m commented on Nov 11, 2023

No, sorry, I moved onto bla-bla since quite a while ago.

@raman-m
Copy link
Member

raman-m commented Nov 11, 2023

@CesarD LoL It is OK that you've moved to another gateway.
I found your past comments really valuable, and we can include these requirements as extra ones to #651 and #1183

@CesarD
Copy link

CesarD commented Nov 11, 2023

Yeah, sorry, is just that for a couple years there was no movement and it was difficult to justify implementing Ocelot while there was another MS-backed project doing similar stuff and being actively developed... 😅

@raman-m
Copy link
Member

raman-m commented Nov 11, 2023

@CesarD Hmm... Bla-bla gateway can merge configuration files, right? If Yes, could you share links to me plz?
This article? Multiple Configuration Sources

Don't you mind if I invite you to review the code and features of PR #1183 in a week?

@CesarD
Copy link

CesarD commented Nov 11, 2023

Well, bla-bla gateway doesn't require to load the configuration from a specific file or type of files, you can very well have its config in the appsettings.json directly and you just need to specifiy the section where you want to pull the config from... If you want to load extra files into the configuration it's up to you, as long as the files don't overlap same configurations between each other, so it provides another kind of flexibility that I got used to.
Perhaps you can adapt Ocelot likewise...

BTW, I don't mind being invited for review 😉

@raman-m
Copy link
Member

raman-m commented Nov 18, 2023

@CesarD commented on Nov 11
being actively developed...

Aha! Especially when you have a team of 3+ full time senior developers on-site at Microsoft with bla-bla salaries. 😛


@CesarD commented on Nov 11
config in the appsettings.json

Yeap! Ocelot has strange design of configuration feature based on ocelot.json file. We will discuss and plan to migrate to appsettings.json with significant refactoring of Configuration feature. Current design is ugly, and developers are confused that they must write special files to configure app instead of well-known ASP.NET approach to use appsettings.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature help wanted Not actively being worked on. If you plan to contribute, please drop a note. medium effort Likely a few days of development effort
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants