Skip to content

Fix WithReference to only use IResourceWithEndpoints#14254

Open
Falco20019 wants to merge 6 commits intomicrosoft:mainfrom
Falco20019:patch-1
Open

Fix WithReference to only use IResourceWithEndpoints#14254
Falco20019 wants to merge 6 commits intomicrosoft:mainfrom
Falco20019:patch-1

Conversation

@Falco20019
Copy link
Copy Markdown

@Falco20019 Falco20019 commented Jan 30, 2026

Description

This allows to use ContainerResource with WithReference. Currently the interface enforces IResourceWithServiceDiscovery while internally only using/needing IResourceWithEndpoints. For backwards compatibility I will leave the original ones although redundant.

Fixes #10286

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 30, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14254

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14254"

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 30, 2026
@davidfowl
Copy link
Copy Markdown
Contributor

Breaking change no?

@Falco20019
Copy link
Copy Markdown
Author

As the other interface inherits it, I wouldn't think so. I can't see the unit tests here in GitHub Actions right now and won't be near a PC before Monday. If they still work, it shouldn't be breaking.

@Falco20019
Copy link
Copy Markdown
Author

Breaking change no?

Ok, now I understand what you meant. It's not clashing with IResourceWithServiceDiscovery but with IResourceWithConnectionString. I will rename it to WithEndpoints to avoid issues but still give users some way to use it for simple cases.

@Falco20019
Copy link
Copy Markdown
Author

Falco20019 commented Feb 2, 2026

I sadly don't get it to compile locally on macOS with Docker Desktop following https://github.com/dotnet/aspire/blob/main/docs/machine-requirements.md

Neither through DevContainers nor natively as protoc always gives an error 139. So have to rely on the CI to see if it's working now.

I think offering WithEndpoints should be a sufficient workaround until the next breaking change, but I would love to have WithReference be adjusted to only use IResourceWithEndpoints and have the check to offer the ReferenceEnvironmentInjectionFlags.ServiceDiscovery done by pattern matching against IResourceWithServiceDiscovery (as already done). I therefore prepared it in a way, that allows for adjusting the existing WithReference method on 14.x :)

https://dev.azure.com/dnceng-public/public/_build/results?buildId=1276039&view=results seems to hint that no tests have been executed by the CI, is that normal?

@Falco20019 Falco20019 requested a review from eerhardt February 17, 2026 08:30
@Falco20019
Copy link
Copy Markdown
Author

@davidfowl Should be non-breaking now.

@davidfowl davidfowl closed this Mar 4, 2026
@davidfowl davidfowl reopened this Mar 4, 2026
@dotnet-policy-service dotnet-policy-service bot added this to the 13.3 milestone Mar 4, 2026
@davidfowl davidfowl requested a review from sebastienros March 4, 2026 09:11
@davidfowl davidfowl closed this Mar 27, 2026
@davidfowl davidfowl reopened this Mar 27, 2026
@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented Mar 27, 2026

For backwards compatibility I will leave the original ones although redundant.

Should they be made obsolete with a note that they'll be removed in the future?

@Falco20019
Copy link
Copy Markdown
Author

@JamesNK I think this would then show obsolete messages all over the place in all cases where the more specific interface is used, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add IResourceWithServiceDiscovery to ContainerResource

4 participants