Skip to content

add support for connecting to Azure Front Door with Azure App Configuration store as an origin #601

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 72 commits into
base: preview
Choose a base branch
from

Conversation

samsadsam
Copy link
Contributor

@samsadsam samsadsam commented Oct 22, 2024

Usage Patterns

Collection Monitoring Pattern (Recommended)

builder.AddAzureAppConfiguration(options =>
{
    options.ConnectAzureFrontDoor(new Uri("https://your-cdn-endpoint.azurefd.net"))
           .Select("app:*")
           .ConfigureRefresh(refresh =>
           {
               // Monitor entire collection for changes
               refresh.RegisterAll()
                      .SetRefreshInterval(TimeSpan.FromSeconds(30));
           });
});

Sentinel Key Pattern

builder.AddAzureAppConfiguration(options =>
{
    options.ConnectAzureFrontDoor(new Uri("https://your-cdn-endpoint.azurefd.net"))
           .Select("app:*")
           .ConfigureRefresh(refresh =>
           {
               // Register specific keys as sentinels
               refresh.Register("app:sentinel", refreshAll: true)
                      .SetRefreshInterval(TimeSpan.FromSeconds(30));
           });
});

How do we guarantee fresh config when refresh all is triggered?

In the case of when a refresh all is triggered, we add a new token param to the query that essentially triggers CDN to fetch from origin (Azure App Configuration) so that we get the latest configuration. This token is the first change etag we detected, the one that triggered a "refresh all" operation.
It is either a page new etag (Collection Monitoring Pattern) or sentinel key new etag (Sentinel Key Pattern). We treat feature flags the same as collection monitoring.
We keep using this token until a new change happens and a new etag is detected.

samsadsam and others added 3 commits October 22, 2024 16:39
…zureAppConfigurationOptions.cs

Co-authored-by: Avani Gupta <avanigupta@users.noreply.github.com>
@avanigupta
Copy link
Member

Note: Correlation-Context header should contain a special tag when ConnectCdn is used.

@samsadsam samsadsam changed the base branch from main to preview January 23, 2025 13:10
@samsadsam samsadsam changed the base branch from preview to main January 23, 2025 13:41
@samsadsam samsadsam changed the base branch from main to preview January 23, 2025 13:42
@jimmyca15
Copy link
Member

@samsadsam, @avanigupta led a portal design review recently in which it came up that we should use Azure Front Door as API terminology instead of CDN. There were a couple of reasons.

  • Azure CDN is being retired
  • We don't support generic CDN, just Azure Front Door

So instead of ConnectCdn as the public method, I believe this will need to become ConnectAzureFrontDoor. Any summaries/supporting APIs would need an update as well.

@zhenlan
Copy link
Contributor

zhenlan commented Jun 11, 2025

So instead of ConnectCdn as the public method, I believe this will need to become ConnectAzureFrontDoor. Any summaries/supporting APIs would need an update as well.

Any preference between ConnectAzureFrontDoor and ConnectAzureFrontDoorCdn? AFD has many other things than CDN.

@jimmyca15
Copy link
Member

So instead of ConnectCdn as the public method, I believe this will need to become ConnectAzureFrontDoor. Any summaries/supporting APIs would need an update as well.

Any preference between ConnectAzureFrontDoor and ConnectAzureFrontDoorCdn? AFD has many other things than CDN.

According to their docs, it looks like the CDN part is implied because "Azure Front Door is Microsoft's advanced cloud Content Delivery Network (CDN)"

image

In that case I'd say we go with ConnectAzureFrontDoor

@samsadsam samsadsam changed the title add ConnectCdn API add support for connecting to Azure Front Door with Azure App Configuration store as an origin Jun 12, 2025
@avanigupta
Copy link
Member

Looks good, the only remaining work is to rename cdn->afd everywhere.

@samsadsam
Copy link
Contributor Author

Looks good, the only remaining work is to rename cdn->afd everywhere.

I don’t think we need to do this for internal classes, I think cdn is better than afd as we can support other CDN providers in the future with same internal classes.

My understanding is that we only need to do this for the public apis

cc @jimmyca15

@jimmyca15
Copy link
Member

Looks good, the only remaining work is to rename cdn->afd everywhere.

I don’t think we need to do this for internal classes, I think cdn is better than afd as we can support other CDN providers in the future with same internal classes.

My understanding is that we only need to do this for the public apis

cc @jimmyca15

If it's easier to understand by linking the two with afd, then I'm fine with it.

@avanigupta
Copy link
Member

Looks good, the only remaining work is to rename cdn->afd everywhere.

I don’t think we need to do this for internal classes, I think cdn is better than afd as we can support other CDN providers in the future with same internal classes.
My understanding is that we only need to do this for the public apis
cc @jimmyca15

If it's easier to understand by linking the two with afd, then I'm fine with it.

My thinking is that if we were starting from scratch today, knowing we’re adding a new API called ConnectAzureFrontDoor, and commit to supporting AFD (not generic CDN) - would we still name classes/interfaces with CDN?

@samsadsam
Copy link
Contributor Author

@avanigupta @jimmyca15 does it look good now?

}

[Fact]
public async Task AfdTests_ParallelAppsHaveSameAfdTokenSequence()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of this testcase?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we really care about is whether both apps can eventually get the latest configuration from the cdn. I understand what this testcase is doing, but I don't think it is meaningful.

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.

8 participants