Skip to content
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

RFC: Add staging workflow for CI and human interoperability #92

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions accepted/0000-staged-packages.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# RFC: Staged Packages

## Summary

Staging workflow which allows CI to publish new package versions, while allowing
humans to be the final arbiters of general availability.

## Motivation

Anything published by CI is instantly available for general use, which isn't
ideal when a human is required to be in the loop. And even when fully automated
CI/CD is allowed, the inability to use 2FA with auth tokens is a security risk.

## Detailed Explanation

We need a minimum of four things to solve these problems:

1. Staging area on the registry invisible to most `npm` commands
2. Auth tokens that can be scoped to the staging area
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do auth tokens need to be scoped or to we need the ability to grant specific permissions to users on a per module basis? I could see it useful to have:

  • Full access - current permissions granted when a user is added to a package (combination of all other permissions)
  • Admin - can manage permissions
  • Stage - user can stage a version or reject a staged version
  • Promote - user can approve or reject a staged version
  • Publish - user can directly publish

I think this would be important because some users might maintain packages in different organizations. Say one organization I work with wants everything to go through staging but I don't want this to change the workflow for my personal packages or have to maintain multiple NPM auth tokens.

These permissions would also be very useful to situations like having CI handle the publish. In that case you'd want to restrict publish to CI only but you still need humans to have administrative privileges.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some of the conversations that happened before the initial RFC (but which I don't see reflected in the RFC here) the assumption was that either a specific token could be restricted to only doing staged publishes in general, or a user/team could be granted access to only stage in a given package.

The complexity around that part is a bit orthogonal to the rest of this, however, and so for now, I think it's best to leave it aside. For the purpose of limiting the scope of this RFC, we can assume that users who are maintainers can both stage, publish, and promote, since it's up to the server to allow or reject any given action based on ACLs and token metadata.

3. CLI flag which lets `npm` commands act on the staging area
4. CLI command that lets users promote a staged package

## Rationale and Alternatives

One alternative is to defer to third-party repository managers for staging
functionality. However, this requires that users set up and manage yet another
tool, which most won't bother to do. In addition, even if users do acquire those
tools, because staging commands aren't built into the npm CLI, they have to
learn a new interface to perform staging operations, which adds unnecessary
friction to the developer experience and again encourages the wrong sorts of
publishing behaviors.

Another alternative is to build this using our existing `dist-tag`
functionality. The appeal is that it uses a mechanism we've already built. The
downside is that we would have to actively ignore these tagged packages in the
CLI and would be eating up versions as well, not ideal.

Yet another alternative is to build a fancy release manager application,
separate from the CLI. However, this is a larger project, and we can almost
certainly implement a useful staging workflow based on tooling that already
exists. Also, even if we start by adding this to the CLI, it doesn't mean we
can't build a more complex application on top of the CLI later on.

After considering these alternatives, we belive a staging workflow built into
the npm CLI provides the shortest path to useful staging functionality that we
can iterate upon. It will allow us to test these new concepts in a tool with
which we and our community are both familiar.

## Implementation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would need a CLI command to reject a staged package (delete without publishing).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't be too hard to just make npm unpublish allowed for any version that is staged. (Ie, unpublish would remove from both versions and stagedVersions.versions, and the registry would allow removal from stagedVersions without any restriction on dependents or publish time.)


* Add a `stagedVersions` object to the packument which lists all packages that
have been staged and not promoted.
* A new `--stage` flag for the `npm publish` command that allows a package to be
published to staging.
* A new `--stage` flag for the `npm install` command that allows a package to be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth clarifying here that semver rules still apply (i.e. the staged version, if any, will only be installed if it matches the specifier in package.json)

Copy link

@wesleytodd wesleytodd Feb 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I am reading this, I feel like one other addition should be made here. Install --stage would only work with a single package and a specific version. Example:

Good:

$ npm i --stage foo@1.0.0

Bad:

$ npm i --stage
$ npm i --stage foo
$ npm i --stage foo@^1.0.0
$ npm i --stage foo@latest

Again with the goal that installing from the stage be hard and a very explicit action.

EDIT: read from top to bottom and I see @isaacs has some other ideas. I will have time later today to read them, but it looks like this comment might have been premature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would npm i --stage foo@1.0.0 do to package.json? A use case for this feature would be to stage an update from a monorepo then post a PR against something outside the monorepo to have Travis-CI test it using the staged update before I promoting.

installed from staging.
* A new `--stage` flag for the `npm view` command that allows us to retrieve data
for staged packages.
* A new `npm promote` command that allows a user to promote a staged package.
* The ability to set a `staging` scope on auth tokens so we can grant read or
publish access to staging.
* A new user permission that allows us to grant a user the ability to read or

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a little unclear on how will this work with 2FA. At the moment, the users can set their 2FA to be "auth-only" or always, but there's no in-between mode - how would that work with --stage?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the call, it was clarified that there'd be an option of a token with r/w permission without 2FA specifically for staging.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is 100% determined by the registry, there'll be a subsequent discussion/RFC around adding token permissions and TFA requirements other than login/write.

The CLI is agnostic to such things. It just says "this is who I am" and "this is what I want to do", and it's up to the registry to decide whether to allow it, prompt for a TFA token, or reject it.

For the time being, given the status quo on the registry side, promoting a package requires the same permissions as publishing. But there's no reason why it necessarily has to stay that way forever, and we all expect to change it, it's just outside the scope of this RFC.

promote packages in staging.
* These are separate from the existing read and publish permissions, since we

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this strictly necessary for the MVP?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think it is.

may want a user to have access to staging, but not production, and
vice-versa.

## Prior Art

* Pull Requests on GitHub allow humans to serve as the gatekeeper before new
code is merged into master. The "two +1s" rule helps reduce the risk of bad
code getting merged in, and having a CI system configured to build from master
means that approving a PR is conceptually similar to the gatekeeping
capabilities in the staging workflow we've defined above.
* Staging in third-party repository managers, like Nexus Repository Manager,
also meets any gatekeeping requirements.
* In Nexus 2, this was implemented via dedicated staging repositories, where
packages could be published and then promoted to a production repository,
all via a nice staging UI.
* In Nexus 3, they shifted to an API-first approach, where new packages had
staging metadata set on them, and the API could enable CI and other tooling
to stage or promote packages by updating this metadata.

## Unresolved Questions and Bikeshedding

* Currently out of scope (but up for debate):
* Webhook support
* Metadata related to fitness of staged packages
* Multiple staged packages (current spec has a limit of one, successive stages
clobber the previous staged version).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little unclear - in the context of this RFC, the "package" means essentially the "specific version of a package"?

This might need clarification, to rephrase that the limit applies to each package version, and the previous staged tarball will not be kept.