bagetter / BaGetter

A lightweight NuGet and symbol server
https://www.bagetter.com
MIT License
166 stars 38 forks source link

Add more options for AllowPackageOverwrites #69

Closed GuidoLeibbrand closed 3 months ago

GuidoLeibbrand commented 4 months ago

Problem

Currently, there is only the setting "AllowPackageOverwrites" in which you can generally set whether a package can be overwritten.

However, there is no option to allow overwriting only for pre-released packages. In our environment, we also create SNAPSHOT versions. For example: 1.1.0-SNAPSHOT. The SNAPSHOT version can be built as often as you like and contains the latest git version for the Softwareversion.

Solution 1 (simple)

Not only support true/false for the flag "AllowPackageOverwrites". An enum can be used here instead of a boolean. In my case, "true | OnlyPrerelease | false" would be sufficient.

The options mean:

Solution 2 (User role concept)

@metallkiller had also noted that a more complex scenario should perhaps be considered.

If we add user accounts, we could also pivot and set permissions for admins and different user roles. So the flag "AllowPackageOverwrites" depends on a user role an can be set to:

seriouz commented 4 months ago

For the time beeing I would go with Solution 1. Later when we have Private Feeds / User with roles we have to adapt to solution 2.

Regenhardt commented 4 months ago

For the time beeing I would go with Solution 1. Later when we have Private Feeds / User with roles we have to adapt to solution 2.

I'm ok with that.

When set to allow, can we somehow restrict it to requiring a parameter like --force to actually be overwritten? There's a good reason overwriting an existing package is not usually possible: https://learn.microsoft.com/en-us/azure/devops/artifacts/artifacts-key-concepts?view=azure-devops
Basically, nuget packages are heavily cached on multiple levels. They are meant to be immutable.

We should probably include a warning somewhere.

seriouz commented 4 months ago

I like the idea of a warning, when the package has been overwritten.

Regenhardt commented 4 months ago

When overwriting a package, I think we should reset the download count to 0 and upload date to now, as it's technically a new package and should be treated as one. Agreed?

Regenhardt commented 4 months ago

It seems there are some services that allow overwriting. They use either replace=true or AllowOverwriteExistingPackages=true on a push as a guard against accidental overwriting. I suppose that's a good idea, I'd like to add that guard to BaGetter too, what do you think?

Regenhardt commented 4 months ago

Can I use PrereleaseOnly instead? Sounds...more direct? I'm not sure, don't have proper rational arguments here, I prefer it but will go with it if you guys have another answer.

Regenhardt commented 4 months ago

It seems there are some services that allow overwriting. They use either replace=true or AllowOverwriteExistingPackages=true on a push as a guard against accidental overwriting. I suppose that's a good idea, I'd like to add that guard to BaGetter too, what do you think?

If we add this guard, people upgrading to BaGetter who currently use AllowPackageOverwrites: true would experience this as a breaking change, think there are many people who use BaGet like that? I can live with this breaking change as I don't expect too many people with this use case.

Also we could add a migration guide at some point, I'm starting to suspect that BaGet might not be developed further in the future.