-
Notifications
You must be signed in to change notification settings - Fork 80
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
Proposal: authenticated publish endpoint #672
Comments
Ooh! As someone who is maybe interested in making her own package manager someday, this seems like a pretty good solution! And it can also be improved on later if there are ideas on how to make it better/smoother also, for example for first publishes |
While this may at first seem like a good idea, it's exactly the kind of situation that we're running away from with this whole registry thing. The main problem with out-of-band payloads is that if you ever need to process all the packages in a standard way (as we are doing here for example, we might need to do it again), then you don't have that information in the source - as it's the case so far with the Bowerfiles, old spago files, and so on - so it's easy to lose these payloads. I don't think it was a stretch to standardise the The |
What about generating a purs.json from the manifest on publish, and adding it to the tarball? This could be done with spago.yaml packages also. That way, every package in the registry would have a purs.json in its published source. Or is there a problem with that approach? The reason i suggest this, is that as a package manager author, it would be an annoying barrier to have to be "blessed" by the registry in order to publish packages without maintaining a purs.json |
This is where things stand at the moment, and the package manager is responsible to make sure the Though, as Thomas noted in the first message, it's easy to have the two formats (the custom one for the package manager and the purs.json) get out of sync, which can be inconvenient. So we ended up special casing the For these special-cased packages we always include a |
Re this:
The registry has a whole set of very annoying checks for getting packages in - having to merge a decoder for your config file in the registry codebase is a pretty mild one, and it's not even compulsory (since you could always generate a |
An additional note on the proposal:
We are dealing with tags only temporarily until we fix/bypass |
Oh, sorry, i wasn't clear. My suggestion was that, any package manager can send a manifest with the payload, and the registry would generate a purs.json from that, and put it in the tarball. The source on github or whatever would not have a |
This also implies that the decoders for the config format are running in the Registry, which is exactly what I am proposing, with the additional requirement that the manifest is included in the source instead of the API payload. I don't quite understand why passing things out-of-band seems so appealing - if we want to pass things out-of-band we might as well just allow people to upload tarballs themselves. |
It's the opposite: passing a
I don't think this is quite fair; passing a manifest is much different than accepting an arbitrary tarball for upload. We still run exhaustive checks on the source code, prune files, detect licenses, and so on. The appeal for passing a manifest in particular is to avoid special-casing package managers. I can envision scenarios where it's not convenient to write a decoder — even spago.yaml requires the library to bring in a JS yaml library we wouldn't otherwise use, and other package managers can use formats that are even harder for the registry to deal with, whether it's a TOML file or Dhall or something Nix-based. Or where a registry decoder is insufficient to produce an accurate manifest, like the many problems we've had working with spago.dhall files. We could remove the special-casing for spago.yaml, even.
I don't think of a Instead of the registry having to know how to decode everyone's custom formats, I think we should have package managers have to work with the registry format. Instead of "you have to include a purs.json in the source or the registry has implemented a decoder for your format" we say "you have to include a purs.json in the source or send it to the registry in a signed payload." Then the registry is only committed to supporting the I've spent more time on the legacy importer than anything else in the registry and honestly it's terrible dealing with the quirks of the few formats we're parsing into
I think this is a good criticism of this proposal — if we can't derive a manifest from the package source, then things like the legacy importer don't work anymore. You would have to publish that package yourself. However, I don't think it's a deal breaker for a few reasons:
|
I understand that this effort was hard and painful and a lot of work, but this sounds like we want to make it easier for the registry codebase at the expense of everyone else, which I do not support. The perspective that I have is that the Erlang ecosystem is quite fragmented when it comes to build systems, and trying to put together a custom build for your dependencies is a truly horrendous task (you can tell I've been there 😄), since packages could have a rebar3 config, or an erlang.mk config, or a bazel build file, or just a makefile, or whatever else really. None of these are clearly specced so any attempt to decode them is painful at best. I would like the PureScript ecosystem to look less like the Erlang ecosystem and a bit more like the JS ecosystem, where there's a common config format - the |
OK, I think I was misunderstanding your objections above. It sounds like you're specifically concerned that package source code won't have a standard manifest file, so if you're not getting the packages from the registry then you aren't guaranteed you'll be able to read a manifest from them. You want the registry to require a standard manifest in the source code as an encouragement to the ecosystem to use them in general. I'm sympathetic to this, but I don't think it's possible. Before I describe why, here are some considerations I have in mind:
Two other points:
The only way the registry can unify the ecosystem behind a single format is to standardize on a single format. That format cannot be purs.json — it has to be, like package.json, sufficient for any package manager to use. We long ago decided not to do this and to introduce purs.json as a minimal shared format for specifying at least your dependencies, and to leave others free to design richer formats for their package managers (like spago.yaml), and to leave the design space open to introduce package managers that do things like install native dependencies along with PureScript ones. The fight to have a standard manifest format feels like it has already been lost. We already have no guarantee that even a purs.json file will be present in package source code. But we have always guaranteed that registry tarballs have a purs.json file in them that all package managers can use, and I think this is a sufficient guarantee for the registry to make. This authenticated endpoint proposal just lifts the source code restriction for spago.yaml / purs.json for publishing to the registry. You are always guaranteed a purs.json file in the resulting tarball. |
I was against explicitly making an exception for spago.yaml files - if that's not good anymore I'm happy to only accept purs.json files in the registry, or explicitly welcome integrations with other formats inside the registry
I am happy to standardize on a
I disagree - if the package manager automates this it's not a headache. Stack does this with cabal files.
I am proposing that we either make the
This does not solve the problem I mentioned above of package managers that bypass our tarball storage.
It provides a spec (in the form of code that you can run) of all of the formats that we accept in the registry. We don't need any commitment, any additional spec is the responsibility of the package manager authors (in the same way that I provided the decoder for the
My point here is that the tarballs are not the upstream. They are an artifact that we produce as a product of the publishing process. |
OK. It sounds like we have some different priorities, where:
Does my paraphrase of your comments sound right to you?
Point taken, that's true.
I'll have to think more about this, but perhaps it's a nice middle ground.
I would be happy with this too, except that I don't feel confident specifying it 😆 |
It sounds like this is the crux of Thomas' concern, and it sounds like there are two issues. First, that it's a "headache" to generate/commit a separate file. Second, that the resulting file goes out-of-sync with the main package manager's config file (e.g. So, first, why is this a headache? AFAICT, the only headache I've found is Second, there's an alternative way to store
When the Registry runs, it just looks for the |
Storing the |
Looping back around to our earlier conversation — I am OK with supporting multiple package manager config formats by implementing decoders for them in the registry, but only if we reserve the right to consider some formats too much work and not provide an integration for them. (Those package managers can still generate and commit a purs.json file.) I still think it would be easier for the registry to just accept a manifest in source or over the API but I understand the objection about manifests potentially not existing in source code.
I've reflected more on why generating and committing a specific file for publishing is so annoying, and your comments here, and I agree it's not as big a deal as I've been making it out to be. The spago/bower combo is terrible because nothing checks the bower file, but of course when generating and committing a purs.json file the registry will reject it if it's invalid — there's some validation in place that the file isn't bogus. And spago provides plenty of pre-publish checks as well that Bower never did. Perhaps it's not such a headache after all. That said — I still don't like having to generate and commit a second essentially duplicated manifest, so I would rather implement integrations in the registry for package manager config formats rather than require everyone to commit a purs.json file. |
We can continue discussing this, but since the authenticated publish endpoint won't be happening I'll go ahead and close it. |
The registry currently offers a single endpoint for publishing packages,
/publish
. Anyone can send a package name, location, and ref to publish the package. The registry will fetch the source, validate, it, publish, and push the docs to Pursuit.When the registry fetches the source it will also pick up the manifest file for the package. The registry supports a
purs.json
file and it also has first-class support for aspago.yaml
file. It has first-class support forspago.yaml
so that Spago users don't have to generate and commit apurs.json
file.It's important that users don't have to commit an extra file because, as we've seen with the old
spago.dhall
/bower.json
pairs, the file that is generated for publishing almost inevitably falls out of sync with the primary package manager file. In the publishing context this means either publishing with an unexpected, out-of-date manifest, or failing altogether and having to tag a new version just to fix the out-of-sync manifest.We don't want the registry to have to special case a bunch of different potential package manager file formats. It was a stretch to even special-case the spago.yaml format. But I think it's a shame that package managers other than Spago will be stuck having to generate and commit a purs.json file as part of publishing. Instead, I'd like to propose an alternative: an authenticated
/publish
endpoint which allows you to send a manifest in the publish payload, rather than requiring it exist in the source code.Authenticated Publish
The authenticated publish endpoint would be the same as the normal endpoint, with a new field:
manifest
.The purpose of this endpoint is for package managers to publish packages without having to commit a
purs.json
file and without the registry having to special-case their particular manifest format. Instead, so long as they can generate a properManifest
then the registry can accept their code.The reason the endpoint has to be authenticated is because we consider the non-authenticated API untrusted input, and we want users to have full control over the contents of their manifests. An arbitrary user shouldn't be able to just send a manifest to the registry and publish it.
Internal Changes
The new endpoint requires almost no changes to the registry. We would simply authenticate, then go through the normal
publish
workflow, except that if we already have aManifest
then we don't read one from disk. (Or we throw an exception if you sent a manifest, and have one committed, and they don't match.) Similar to how we branch forspago.yaml
files.Considerations
The major wrinkle to this proposal is that we only support a single authentication mechanism — SSH signatures, with public keys recorded in the
owners
field of the manifest and then copied to metadata. There is no way a package can be registered via the authenticated publish endpoint since we can't authenticate the publisher until we have a manifest published the ordinary way.Still, this reduces the
purs.json
burden to just your first-time publish, after which it can be deleted. Alternately, we can consider relaxingpurs.json
parsing such that we read only theowners
key in the case the authenticated publish was used, for verification, and then defer to their provided manifest otherwise. This means apurs.json
file has to be maintained still, but only for the sake of listing owners, which is a rare change. (I'm in favor of this.)For GitHub users specifically we can consider even more automatic methods; for example, all GitHub users have their public SSH keys at their username + .keys, such as mine: https://github.com/thomashoneyman.keys; for repositories owned by a particular user we could look at their public SSH keys to verify the signature without needing anything to be committed. This would strictly be in addition to other methods, not in place of them, since we are platform-agnostic.
The text was updated successfully, but these errors were encountered: