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

Richer Extensibility for routes and clusters #1709

Open
samsp-msft opened this issue May 10, 2022 · 6 comments
Open

Richer Extensibility for routes and clusters #1709

samsp-msft opened this issue May 10, 2022 · 6 comments
Assignees
Labels
Type: Idea This issue is a high-level idea for discussion.
Milestone

Comments

@samsp-msft
Copy link
Contributor

samsp-msft commented May 10, 2022

Problem Statement

Today if you want to extend the route or clusters, you can only do it through the metadata property on each, which is a Dictionary<string, string>. If you want to be able to have structured data its not possible without you forcing it into a string and then parsing it when needed. There are scenarios like A/B testing, or authenticating with back end servers (not pass thru) where you want to be able to store a structure of data in config, and have it available at runtime on the route/cluster objects.

Design for this feature has been moved to #1796

@samsp-msft
Copy link
Contributor Author

Two things we should consider from #87:

  • What happens to state objects that are not directly derived from config. For example load balancing counters. You wouldn't want them to be reset any time configuration changes. So there probably needs to be a way for them to persist, and have a notification that either they have been orphaned as config has changed, or automatically move them to the new state objects, and have notification that they may want to pickup the new configuration, along with a pointer to the config data.
  • Thread safety is a concern for these objects. Its probably mostly a documentation problem rather than anything we can directly fix. However the collection that contains them needs to be thread safe.

@samsp-msft samsp-msft moved this to 📋 Backlog in YARP 2.x Jun 9, 2022
@samsp-msft samsp-msft moved this from 📋 Backlog to 🔖 Ready in YARP 2.x Jun 9, 2022
@karelz
Copy link
Member

karelz commented Jun 16, 2022

Triage: Pretty open ended. We need agreement first on the design.

@karelz karelz moved this from 🔖 Ready to Design needed in YARP 2.x Jun 16, 2022
@karelz karelz modified the milestones: YARP 2.0.0, Backlog Jan 9, 2023
@ayrloong
Copy link

ayrloong commented Sep 11, 2024

@karelz Hi I have seen this design https://github.com/microsoft/reverse-proxy/blob/main/docs/designs/route-extensibility.md
I think this function is very meaningful. I have implemented this part of the function. Can I submit a PR?

@karelz
Copy link
Member

karelz commented Oct 21, 2024

@ayrloong I don't think the linked doc covers this issue, does it?

@ayrloong
Copy link

@karelz Thanks for your reply Here is my initial implementation. when getting Extensions, I can't link like above. I need to use As

{
    "ReverseProxy": {
        "Routes": {
            "route1": {
                "ClusterId": "cluster1",
                "Match": {
                    "Path": "{**catch-all}"
                },
                "Extensions": {
                    "UserModel": {
                        "Name": "lq1"
                    },
                    "ABTest": {
                        "CE": 3,
                        "Names": [
                            "1",
                            "2",
                            "3"
                        ]
                    }
                }
            }
        },
        "Clusters": {
            "cluster1": {
                "Destinations": {
                    "destination1": {
                        "Address": "https://example.com/"
                    }
                }
            }
        }
    }
}

Startup.cs

builder.Services.AddReverseProxy()
    .LoadFromConfig(builder.Configuration.GetSection("ReverseProxy"), [typeof(UserModel), typeof(ABTest)]);

app.UseEndpoints(endpoints =>
{
    // We can customize the proxy pipeline and add/remove/replace steps
    endpoints.MapReverseProxy(proxyPipeline =>
    {
        // Use a custom proxy middleware, defined below
        proxyPipeline.Use((context, next) =>
        {
            var proxyFeature = context.Features.Get<IReverseProxyFeature>();
            var user = proxyFeature.Route.Config.Extensions[typeof(UserModel)] as UserModel;
            
            var abTest = proxyFeature.Route.Config.Extensions[typeof(ABTest)] as ABTest;

            Console.WriteLine(abTest.CE);
            return next();
        });
        proxyPipeline.UseSessionAffinity();
        proxyPipeline.UseLoadBalancing();
    });
});

ReverseProxyServiceCollectionExtensions.cs

    public static IReverseProxyBuilder LoadFromConfig(this IReverseProxyBuilder builder, IConfiguration config,
        Type[] extensionsTypes)
    {
        if (config is null)
        {
            throw new ArgumentNullException(nameof(config));
        }

        builder.Services.AddSingleton<IProxyConfigProvider>(sp =>
        {
            // This is required because we're capturing the configuration via a closure
            return new ConfigurationConfigProvider(sp.GetRequiredService<ILogger<ConfigurationConfigProvider>>(),
                config, extensionsTypes);
        });

        return builder;
    }

RouteModel.cs

  public IReadOnlyDictionary<Type, object>? Extensions { get; init; }

ConfigurationConfigProvider.cs

private static IReadOnlyDictionary<Type, object>? CreateExtensions(IConfigurationSection section)
    {
        if (section.GetChildren() is var children && !children.Any())
        {
            return null;
        }


        var mutable = new Dictionary<Type, object>();
        
        foreach (var extensions in extensionsTypes)
        {
            var instance = section.GetSection(extensions.Name).Get(extensions);
            if (instance != null)
            {
                mutable[extensions] = instance;
            }
        }

        return new ReadOnlyDictionary<Type, object>(mutable);
    }

@ayrloong
Copy link

Hello, I have spent some time working on this and implemented this part, submitting a draft PR for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Idea This issue is a high-level idea for discussion.
Projects
Status: Spec wishlist
Development

No branches or pull requests

3 participants