Separate concerns of package source mapping and CPM

[JonDouglas]

NuGet Product(s) Affected

NuGet SDK

Current Behavior

Today, CPM is "tightly coupled" with package source mapping when there are multiple feeds detected and CPM is enabled.

Desired Behavior

CPM should not be affected by package source mapping.

Package source mapping should not be affected by CPM.

If behaviors are desired to drive best practice adoption, a separate property / experience should be considered that people can customize. i.e. <NuGetStrictMode> or similar.

TL;DR - You should not run into package source mapping issues when enabling CPM. And vice versa. We should remove this coupling.

We could consider keeping the warning for multiple sources regardless of CPM.

Additional Context

No response

[richlander]

I hit this issue. I enabled CPM and am now having to figure out the package feed issue (which is a bit hard) at the same time. That was unexpected, unfortunate, and stalling my PR. It's certainly not helping with CPM adoption.

dotnet/winforms#12175

[nkolev92]

This was done in #11505.

The 2 features can and function independently.

What is a thing today is that CPM encourages best practices such as PSM adoption, in the spirit of secure by default.

cc @aortiz-msft

[JonDouglas]

What is a thing today is that CPM encourages best practices such as PSM adoption, in the spirit of secure by default.

Yep, this issue is proposing that PR is reverted or the functionality is not tightly coupled to each other in this way.

[richlander]

I think the idea of "we'll turn on all the security features at once" is very much NOT in the spirit of secure by default.

[jeffkl]

Some thoughts:

1. I do not believe its a bad idea to lump together features. Obviously some features should not be lumped together, while others should. And I do not believe there's one way to do it for security related features. Each design decision can be made with the present information, some designs can follow strict guidelines, while others cannot. There is no one-size-fits-all here.
2. We spent time discussing the decision to warn users when adopting Central Package Management (CPM) if they were using multiple feeds that Package Source Mapping (PSM) was recommended. My personal opinion was that we should not introduce a new warning which could break a person's build.
3. At the time of shipping the coupling of CPM and PSM, we were very clear that CPM was under active development. We decided it was totally appropriate in this case to ship a new warning, even if people had already adopted CPM. Users were already warned that things could change and anyone who adopted CPM already would be okay with us introducing a warning, making this a viable option.
4. Since PSM with CPM is only a warning, I felt like it was low friction for new adopters to just suppress it. Providing users with an escape hatch is a great strategy and, in my opinion, this coupling is a good example where we made a decision for a default but made it easy for people to do what they wanted to do. If we had made it impossible to use CPM without PSM, I would never have agreed to the design.
5. The documentation for NU1507 (the warning that you get with multiple feeds but not using PSM with CPM) is in my opinion very well written and explains the reasoning behind why we think PSM should be used, and it provides very clear instructions on how to get unblocked. I think we did a great job in this case of making it so people would not be frustrated. They onboard to CPM, get a warning, and are given very clear guidance on how to move forward. Again, if the warning said something to the effect of "You must use package source mapping", that would have been a terrible design.

I think this is a good example of where our team had differing opinions, we had appropriate discussions, and we found an agreeable compromise. I also think we did the right thing for customers, push them towards a more secure configuration while providing them a clear path forward if they are not willing or able to use package source mapping.

[richlander]

This discussion is about warnings, less about overall UX and adoption. I came to this feature area fresh. I was confused and surprised when I needed to deal with PSM. I then looked at two use cases within our own org and found one disabled PSM based on a false premise and another used PSM but in name only (all wildcards). That's a very low sample size, but it is not a good sign when our own org is (IMO) misusing this feature.

Here is the one case I mentioned: https://github.com/dotnet/arcade/blob/7e8b8f4f321c8671aa01b53567d31aaa4950706f/Directory.Packages.props#L6

[zivkan]

I've talked to dnceng about package source mapping in the past, but the problem is that when a new version of .NET is being built, it dynamically creates new Azure Artifacts feeds, and the Arcade SDK dynamically adds those feeds to the nuget.config file of the repo being built. How to maintain correct package source mapping configuration, when feeds are programmatically added and which packages they contain appears to be non-deterministic (as far as I could tell, from my discussions with them), is something that most customers (even within Microsoft) will not experience.

So, there are technical reasons why Arcade enabled repos that take part in Maestro/.NET builds cannot (currently) use package source mapping.