-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Error: "The entity type 'DataProtectionKey' requires a primary key" when using .Net 7 previews #43187
Comments
Thanks for moving. I originally went to open this in EFCore, but then decided it was more of a aspnetcore issue. 🤣 Just to sanity check, this morning I ran |
Okay, so additional info. I've managed to workaround this by explicitly adding the key with my
With this, the app starts up correctly, as the key has been correctly added to the entity. So I guess the questions are:
Let me know if you need any more details. |
@Webreaper Sounds like this could be related to trimming. I suspect when the |
Yes, it could be feasible. I do deploy as a single-file executable, and usually have trimming etc enabled:
but I disabled trimming to see if it would have an effect (or would fix this problem). Also, I had trimming disabled for Preview 5 due to a related bug (dotnet/runtime#70758) but disabling trimming doesn't prevent this issue from happening. |
@Webreaper Are you sure |
If trimming is not enabled in the build/publish, how would it be trimmed? Or is there some magic I'm missing? |
I don't know enough about trimming to answer, but I suspect it is worth you investigating. |
Maybe @richlander might have an idea? |
Also, if this was a trimming error, it wouldn't explain why it doesn't happen before EFCore 7 preview 5. |
@Webreaper What happens if you remove this line: dpk.HasKey(x => x.Id); And replace it with: Console.WriteLine(new DataProtectionKey().Id); Or anything else that references the |
Sure. Will give it a try. |
Can you share the |
Here's the https://github.com/Webreaper/Damselfly/blob/develop/scripts/makeserver.sh#L33 It evaluates to:
I have tried removing the trimming, as follows, but it didn't prevent the error:
I also have Directory.Build.props, and here's the project file which doesn't have anything fancy in terms of properties. Let me know if I can point you at anything else. Note that this commit seemed to fix the problem per earlier comments. So it's not a huge issue, but it's somewhat confusing as I don't know why it should be needed (I would presume that |
Do you get the error just the following command? dotnet publish Damselfly.Web -r linux-x64 -f net7.0 -c Release |
Will try. Can't remember if my deployment scripts will break if it's not a single file deployment, so I might have to tweak. |
Okay, so that's confirmed. This breaks:
So it is to do with single-file deployment and/or trimming. What combination do you want me to try next? |
Okay, so adding Then adding Adding |
Perfect. That's what we wanted to find out. Let's see if we can get some help. |
I think the behavior here is correct. The application is mapping a type ( From a trimming perspective, we don't have any knowledge that the |
Makes sense. I guess the thing that confuses me is that this issue doesn't happen at with .Net 6, and (I'm pretty certain, but can confirm if you need to) that it didn't happen with the .Net 7 before preview 5. So that implies a change in behaviour that might confuse people when 7 GA is released. Perhaps as you guys are turning the screws on the trimming efficiency it's resulted in the change? But possibly you might be able to put some hints to stop it getting trimmed? After all, I'm not doing anything explicit with the Id field, I'm just using the standard EFCore Data Protection Keys functions - which made this error super confusing from my perspective at first because it's not even referenced within my app - it's something internal to Asp.Net identity. I assumed that in Aspnetcore Identity this property is already being explicitly mapped as a key.... Edit: just realised that last comment may have been directed at Rich and the identity folks, not me. 😊 |
That is likely because of this change in .NET 7. |
That was directed at you @Webreaper. |
Got it. I'm fine with this and the fix works well. I'm just wondering if there's any hints or docs you could put together because other people will get caught by this; while the fix to add the reference is straightforward, it might catch people unawares to have to add a |
Oh, and I think the "on Linux" part may be misleading; I may not see it on Mac because that's my Dev env so I don't usually publish as a single file there. I'll test it when I get 5 mins, and if it's consistent I'll change the issue title to "when publishing trimmed" so finding the issue and solution is easier for anyone who is hit by it. |
We do plan to make the experience better when trimming. For example, we could annotate EF APIs to preserve all public properties on entity CLR types. We're not sure we'll be able to do significant trimming work for 7.0, but at least some guidance/documentation does make sense. |
Additional info on the cause of this here, for anyone landing on this issue: https://devblogs.microsoft.com/dotnet/announcing-dotnet-7-preview-7/#trimming-and-nativeaot-all-assemblies-trimmed-by-default |
Transferring this back to ASP.NET since this is not an issue in EF. It's possible that |
@ajcvickers is there an issue to track the trimming support improvements that @roji mentions above? Would be good to link them here - and I'd like to track so that I can remove the |
@Webreaper Trimming improvements for EF Core are unlikely to have an impact on this, at least in short to medium term. EF Core knows nothing of |
Oh, yes - good point. Presumably it's up to the identity/aspnetcore folks to decorate it appropriately. Or something. Although since it's clearly declared as a key where it's defined, I was assuming that @roji was talking about doing some magic to automatically do that for any entity... :) |
Where is is clearly declared as a key? |
Dammit, you got me. I'd assumed it was..... But apparently not: https://github.com/dotnet/aspnetcore/blob/main/src/DataProtection/EntityFrameworkCore/src/DataProtectionKey.cs#L14 Presumably, if they explicitly decorated that with |
@Webreaper That would not help unless anything attributed with |
Ah, okay. Is there a way to make it so that something decorated with |
That's not something I can answer. |
This is all something I'm planning to explore, though I don't think keys should be handled in a special way here: basically public properties on entity types shouldn't get trimmed, we should be able to do that by annotating modelBuilder.Entity, DbSet<>, etc. |
The problem here is nothing in the DataProtection EntityFramework library references There are
Number 2 is more complicated, but it's the better fix. Because EntityFramework isn't annotated, all trimmed libraries that use EF are going to have to ensure entities aren't trimmed manually (i.e. fix number 1). Ideally, EF should be annotated to do it for you. Generally, EF entities are defined in user apps and aren't trimmed. But people who enable full trimming of all assemblies will also see problems like this. Correctly annotating EF will take a lot of time. Here is a short-term fix (number 1) for DataProtection EntityFramework - #43204. There is only one entity, so it's simple to fix. @Webreaper You should be able to remove your workaround after upgrading to .NET 7 RC1. |
Awesome. Thanks James! |
Is there an existing issue for this?
Describe the bug
I have implemented
DataProtectionKey
storage using an EFCore DB context, and it has been working fine for nearly a year.In June, I upgraded my application to .Net 7 preview 5 and EFCore 7 preview 5, and now see the following exception:
Migrations failed with exception: System.InvalidOperationException: The entity type 'DataProtectionKey' requires a primary key to be defined. If you intended to use a keyless entity type, call 'HasNoKey' in 'OnModelCreating'. For more information on keyless entity types, see https://go.microsoft.com/fwlink/?linkid=2141943.
The webserver then terminates within
IHost.Run
with the following (similar) exception:I am using the SQLite DB provider.
This only happens on Linux (deployed in an Ubuntu docker container). If I run the same code, against the same .db file, on OSX, the migrations complete without issue. I have not tried Windows.
The error suggests using the
[Keyless]
attribute to resolve this error, but I don't know enough aboutDataProtectionKey
s to know if it would cause any issues.Note that the problem persists with .Net 7 preview 6 and EFCore preview 6.
Expected Behavior
The expected behaviour is that the
db.Database.Migrate();
call completes without issue and the application starts normally.Steps To Reproduce
I declare my
DataProtectionKeys
in myDbContext
here: https://github.com/Webreaper/Damselfly/blob/develop/Damselfly.Core/Models/ImageContext.cs#L43public DbSet<DataProtectionKey> DataProtectionKeys { get; set; }
I add configure the DataProtection here: https://github.com/Webreaper/Damselfly/blob/master/Damselfly.Web/Startup.cs#L73
services.AddDataProtection().PersistKeysToDbContext<ImageContext>();
I can't find anywhere in the documentation that suggests I need to do something explicit to create a key.
The DataProtectionKeys were added in 2021, in this Migration: https://github.com/Webreaper/Damselfly/blob/master/Damselfly.Migrations.Sqlite/Migrations/20210922112630_AddDataProtectionKeys.cs:
Exceptions (if any)
.NET Version
.Net 7 preview 5 and preview 6
Anything else?
The issue only happens when running the containerised deployment on Linux. Running exactly the same code on OSX works fine.
The text was updated successfully, but these errors were encountered: