-
Notifications
You must be signed in to change notification settings - Fork 3
Support load configuration settings from Azure Front Door #223
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
base: preview
Are you sure you want to change the base?
Changes from all commits
a0a551e
d0ecd09
8d06357
3310171
6878b64
9af1749
f97ae3d
264113d
58d1f2e
725dd63
665af4c
9d63b7c
5087206
339b3fa
b2b76ed
1577bef
87f952b
4480562
d3c5f96
414a8c8
bebf549
564f16a
28d37c2
ca056b9
b5f26c4
5b09aee
dcf5814
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { PipelinePolicy } from "@azure/core-rest-pipeline"; | ||
|
|
||
| /** | ||
| * The pipeline policy that remove the authorization header from the request to allow anonymous access to the Azure Front Door. | ||
| * @remarks | ||
| * The policy position should be perRetry, since it should be executed after the "Sign" phase: https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/core/core-client/src/serviceClient.ts | ||
| */ | ||
| export class AnonymousRequestPipelinePolicy implements PipelinePolicy { | ||
| name: string = "AppConfigurationAnonymousRequestPolicy"; | ||
|
|
||
| async sendRequest(request, next) { | ||
| if (request.headers.has("authorization")) { | ||
avanigupta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| request.headers.delete("authorization"); | ||
| } | ||
| return next(request); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The pipeline policy that remove the "sync-token" header from the request. | ||
| * The policy position should be perRetry. It should be executed after the SyncTokenPolicy in @azure/app-configuration, which is executed after retry phase: https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/appconfiguration/app-configuration/src/appConfigurationClient.ts#L198 | ||
| */ | ||
| export class RemoveSyncTokenPipelinePolicy implements PipelinePolicy { | ||
| name: string = "AppConfigurationRemoveSyncTokenPolicy"; | ||
|
|
||
| async sendRequest(request, next) { | ||
| if (request.headers.has("sync-token")) { | ||
| request.headers.delete("sync-token"); | ||
| } | ||
| return next(request); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| export const X_MS_DATE_HEADER = "x-ms-date"; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,9 @@ export const enum ErrorMessages { | |||||
| INVALID_LABEL_FILTER = "The characters '*' and ',' are not supported in label filters.", | ||||||
| INVALID_TAG_FILTER = "Tag filter must follow the format 'tagName=tagValue'", | ||||||
| CONNECTION_STRING_OR_ENDPOINT_MISSED = "A connection string or an endpoint with credential must be specified to create a client.", | ||||||
| REPLICA_DISCOVERY_NOT_SUPPORTED = "Replica discovery is not supported when loading from Azure Front Door.", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This error message is too long. And I am not a fan of putting the doc link here. Instead of pointing to AFD doc, I think the link should be pointed to one doc maintained by ourselves.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a concept owned by AFD, we dont have any doc on this topic. When we choose not to support it ourselves, it’s important to provide users with any available alternatives. I agree its long, but its necessary in this case. "Replica discovery isn’t supported via Azure Front Door. To use AppConfig replicas, add multiple replicas to your Front Door origin group. Learn more: https://learn.microsoft.com/en-us/azure/frontdoor/origin"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @jimmyca15 If you agree, we should update the error message in .net provider too.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I mean we can a doc that teach people how to use AFD with app config or a section of FAQ. Our doc can reference this AFD doc. We should have the full control of the doc.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will eventually add a doc, but it will not be available when this library is released. So for now, we need to add AFD doc link.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommendations can change, exception message lives forever in this version of the sdk. So it's a balance. In this case, I'd say this particular recommendation has to live in a doc. Sounds too much like something subject to change. In contrast to something like "to use this feature, you must set xyz flag to true"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this version of SDK, we have turned off replica discovery and load balancing. I think there should be some indication of why or what are the alternatives. I'm ok if we dont want to put the doc link at all, but we should at least hint at the origin groups concept. In this case, it's not just a recommendation - it's the only way someone can achieve resiliency using replicas.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mind putting a link, but it should be a link to our docs. Perhaps our geo-replication doc, and we add a blurb about when consuming through AFD, origin groups should be set up. Something like "For guidance on how to take advantage of geo-replication when Azure Front Door is used, visit https://aka.ms/appconfig/geo-replication-with-afd"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. The only downside of using our doc is that provider will be released before we can make any doc updates. But it's not that bad since we will eventually update our docs after ignite. |
||||||
| LOAD_BALANCING_NOT_SUPPORTED = "Load balancing is not supported when loading from Azure Front Door.", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, I can make my suggestion shorter: "Load balancing isn’t supported via Azure Front Door. To load balance across AppConfig replicas, add multiple replicas to your Front Door origin group. Learn more: https://learn.microsoft.com/en-us/azure/frontdoor/origin" |
||||||
| WATCHED_SETTINGS_NOT_SUPPORTED = "Specifying watched settings is not supported when loading from Azure Front Door. If refresh is enabled, all loaded configuration settings will be watched automatically." | ||||||
| } | ||||||
|
|
||||||
| export const enum KeyVaultReferenceErrorMessages { | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.