Skip to content

Apply CDN URL substitution to asset locations stored in BAR#16392

Merged
mmitche merged 6 commits intomainfrom
copilot/update-asset-locations-to-cdn
Dec 11, 2025
Merged

Apply CDN URL substitution to asset locations stored in BAR#16392
mmitche merged 6 commits intomainfrom
copilot/update-asset-locations-to-cdn

Conversation

Copy link
Contributor

Copilot AI commented Dec 9, 2025

Asset locations stored in BAR for published blobs pointed to Azure storage URLs (e.g., dotnetbuilds.blob.core.windows.net/internal) which are only accessible via CDN. This made stored locations unusable.

Changes

  • Centralized CDN mappings in PublishingConstants.AccountsWithCdns:

    • dotnetcli.blob.core.windows.netbuilds.dotnet.microsoft.com
    • dotnetbuilds.blob.core.windows.netci.dot.net
  • Enhanced TargetFeedConfig.SafeTargetURL to apply CDN substitution when generating asset locations for BAR storage using UriBuilder.Host property for clean URI manipulation

  • Simplified LatestLinksManager.ComputeLatestLinkBase to remove redundant CDN substitution logic since SafeTargetURL already handles it

Example

Publishing to https://dotnetbuilds.blob.core.windows.net/internal?sv=token now stores https://ci.dot.net/internal in BAR instead of the blob storage URL.

To double check:

Original prompt

This section details on the original issue you should resolve

<issue_title>Asset locations for newly published assets do not point to CDN locations</issue_title>
<issue_description>After publishing, the asset locations that point to dotnetbuilds are no longer accessible except via CDN. This is unfortunate. I see two options:

  • Update the target publishing locations to point to the CDN (not sure this will work) URIs
  • Have the asset location update do an X->Y substitution for known CDN locations. Note that it already does this for aka.ms targets.

For example, when an asset is published to https://dotnetbuilds.blob.core.windows.net/internal, the asset location should actually be ci.dot.net/internal.</issue_description>

<agent_instructions>Please implement the second bullet. Please take note of how existing aka.ms link configuration is handled. It already does X->Y substitution. You may want to combine those approaches</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Update asset locations to point to CDN after publishing Apply CDN URL substitution to asset locations stored in BAR Dec 9, 2025
Copilot AI requested a review from mmitche December 9, 2025 21:10
@mmitche mmitche marked this pull request as ready for review December 10, 2025 16:29
}
var authority = new Uri(feedBaseUrl).Authority;
if (AccountsWithCdns.TryGetValue(authority, out var replacementAuthority))
if (PublishingConstants.AccountsWithCdns.TryGetValue(authority, out var replacementAuthority))
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Because feedbaseUrl uses SafeTargetUrl, the additional replacement here is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right. Removed the redundant CDN substitution logic since SafeTargetURL already handles it. Fixed in 99fd736.

.nuget/nuget.exe Outdated
Copy link
Member

Choose a reason for hiding this comment

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

@copilot Remove this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 1f7f75a.

akoeplinger
akoeplinger previously approved these changes Dec 11, 2025
@mmitche mmitche enabled auto-merge (squash) December 11, 2025 22:01
@mmitche mmitche merged commit dd63118 into main Dec 11, 2025
8 of 9 checks passed
@mmitche mmitche deleted the copilot/update-asset-locations-to-cdn branch December 11, 2025 22:47
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.

Asset locations for newly published assets do not point to CDN locations

3 participants