Skip to content

Implement richer extensibility mechanisms for routes and clusters in … #2682

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ayrloong
Copy link

This PR is based on the Route Extensibility Design Document and primarily addresses the extensibility issues mentioned in the related issue: #1709.

I have carefully reviewed and studied the above document and related links, and I believe this functionality is highly beneficial for the extensibility of YARP, especially in supporting custom route and cluster configurations. After extensive preparation and development, I have successfully completed the implementation of this PR.

If you are already working on this feature or have a better implementation plan, I would be more than happy to close this PR and look forward to contributing ideas or providing assistance for the final implementation.

Thank you for your time and review, and I look forward to your feedback!

@ayrloong
Copy link
Author

@microsoft-github-policy-service agree

@MihaZupan MihaZupan requested a review from samsp-msft January 7, 2025 05:06
@ayrloong
Copy link
Author

ayrloong commented Feb 8, 2025

Any news?

@MihaZupan
Copy link
Member

@samsp-msft did you get a chance to look at this one?

@ayrloong
Copy link
Author

ayrloong commented Mar 30, 2025

Hi @MihaZupan
Given that Sam has not yet responded, I am hesitant to close this PR. I have taken note of the warning flagged in your latest design document, and I’d like to discuss the design plan with you via this PR to move forward with the feature.
Thank you so much

@samsp-msft
Copy link
Contributor

Sorry, been busy with another project. I trust Miha's judgement on this issue.

@MihaZupan MihaZupan self-assigned this Apr 1, 2025
@ayrloong
Copy link
Author

ayrloong commented Apr 1, 2025

@dotnet-policy-service agree

@ayrloong ayrloong reopened this Apr 2, 2025
@ayrloong
Copy link
Author

@MihaZupan Hi
Could you help move this forward when you have time?

@ayrloong
Copy link
Author

I'm not entirely happy with this implementation. I'd like to take some time to redo this PR – I'd really value your input

@MihaZupan
Copy link
Member

Sorry about the delay here.
I should be able to get to it next week, I want to talk about it with the team first.

@ayrloong
Copy link
Author

ayrloong commented Jun 5, 2025

This is my refactored code. I’m quite happy with the current structure, but I’d love to hear any suggestions or feedback you might have. Thank you so much for taking the time to review it!

  /// <summary>
   /// Factory delegate for creating route extensions.
   /// </summary>
   /// <typeparam name="T">The type of extension to create.</typeparam>
   /// <param name="section">The configuration section for this extension.</param>
   /// <param name="routeConfig">The route configuration being extended.</param>
   /// <param name="existingExtension">The existing extension instance if available (during updates).</param>
   /// <returns>The created or updated extension instance.</returns>
   public delegate T RouteExtensionFactory<T>(IConfigurationSection section, RouteConfig routeConfig, T existingExtension);

   /// <summary>
   /// Interface for providers that can create route extensions from configuration.
   /// </summary>
   public interface IRouteExtensionProvider
   {
       /// <summary>
       /// Gets the configuration section name this provider handles.
       /// </summary>
       string SectionName { get; }

       /// <summary>
       /// Creates an extension instance from the given configuration section.
       /// </summary>
       /// <param name="section">The configuration section.</param>
       /// <param name="routeConfig">The route configuration.</param>
       /// <param name="extensions">Existing extensions dictionary.</param>
       /// <returns>True if an extension was created, false otherwise.</returns>
       bool CreateExtension(IConfigurationSection section, RouteConfig routeConfig, IDictionary<Type, object> extensions);
   }


   /// <summary>
   /// A provider that creates route extensions of a specific type.
   /// </summary>
   /// <typeparam name="T">The type of extension to create.</typeparam>
   internal sealed class RouteExtensionProvider<T> : IRouteExtensionProvider
   {
       private readonly RouteExtensionFactory<T> _factory;

       public RouteExtensionProvider(string sectionName, RouteExtensionFactory<T> factory)
       {
           SectionName = sectionName ?? throw new ArgumentNullException(nameof(sectionName));
           _factory = factory ?? throw new ArgumentNullException(nameof(factory));
       }

       /// <inheritdoc/>
       public string SectionName { get; }

       /// <inheritdoc/>
       public bool CreateExtension(IConfigurationSection section, RouteConfig routeConfig, IDictionary<Type, object> extensions)
       {
           if (section is null || routeConfig is null || extensions is null)
           {
               return false;
           }

           var extensionSection = section.GetSection(SectionName);
           if (!extensionSection.Exists())
           {
               return false;
           }

           T existingExtension = default;
           extensions.TryGetValue(typeof(T), out var existingObject);
           if (existingObject is T existing)
           {
               existingExtension = existing;
           }

           var extension = _factory(extensionSection, routeConfig, existingExtension);
           if (extension != null)
           {
               extensions[typeof(T)] = extension;
               return true;
           }

           return false;
       }
   }

  public static class ReverseProxyBuilderExtensions
   {
       /// <summary>
       /// Adds a route extension factory that will be used to create extension instances from configuration.
       /// </summary>
       /// <typeparam name="T">The type of extension to create.</typeparam>
       /// <param name="builder">The <see cref="IReverseProxyBuilder"/>.</param>
       /// <param name="sectionName">The configuration section name for this extension.</param>
       /// <param name="factory">The factory function to create the extension.</param>
       /// <returns>The <see cref="IReverseProxyBuilder"/>.</returns>
       public static IReverseProxyBuilder AddRouteExtension<T>(
           this IReverseProxyBuilder builder,
           string sectionName,
           RouteExtensionFactory<T> factory)
       {
           if (builder == null)
           {
               throw new ArgumentNullException(nameof(builder));
           }

           if (string.IsNullOrEmpty(sectionName))
           {
               throw new ArgumentException("Section name cannot be null or empty", nameof(sectionName));
           }

           if (factory == null)
           {
               throw new ArgumentNullException(nameof(factory));
           }

           builder.Services.AddSingleton<IRouteExtensionProvider>(
               new RouteExtensionProvider<T>(sectionName, factory));

           return builder;
       }
   }

internal class ConfigurationConfigProvider : IProxyConfigProvider
   {
       private readonly IEnumerable<IRouteExtensionProvider> _routeExtensionProviders;

       // ... other members ...

       private void ProcessRouteExtensions(IConfigurationSection section, RouteConfig route)
       {
           var extensions = new Dictionary<Type, object>();
           var extensionsSection = section.GetSection("Extensions");

           if (extensionsSection.Exists())
           {
               // Apply all registered route extension providers
               foreach (var provider in _routeExtensionProviders)
               {
                   provider.CreateExtension(extensionsSection, route, extensions);
               }
           }

           route.Extensions = extensions;
       }

       // ... other methods ...
   }

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 this pull request may close these issues.

3 participants