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

ci(dev-canary): publish a dev NPM dist tag on every push #4120

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

michaelfig
Copy link
Member

@michaelfig michaelfig commented Nov 24, 2021

refs: #4117, #3857

Description

Add a dev-canary CI job to publish dev snapshots to NPM for changes merged to master.

The benefit is that our SDK will be published on Github (freshest), NPM (master only), and Docker Hub (nightly only).

Security Considerations

Consider the risk/benefit of publishing to NPM:

From the Agoric side, we're only using the NPM_TOKEN to publish as constrained by the Github Actions present on master. This ensures the packages are marked as prerelease (version is dev-${commitHash}.N) and never match the latest or beta tags, only dev.

From the NPM side, the NPM agoricbot account should be added to the agoric organization, so that that Github secret NPM_TOKEN for that bot doesn't have access to publish to other NPM organizations outside agoric. I'm counting on @warner to enable that if he approves.

Documentation Considerations

Testing Considerations

@warner
Copy link
Member

warner commented Nov 30, 2021

Some questions:

  • Is NPM cool with us publishing releases as such a high rate? Is this standard practice for other projects?
  • I assume yarn release does the right thing, but to double check, does the dev-(HASH).N version sort properly (N is more significant than the hash)?
  • Unless NPM's auth language makes it possible to scope a token to e.g. only dev- releases, the proposed NPM_TOKEN is scary powerful. Assuming we don't already have one configured, adding this would enable some attacks (buggy GH Action code revealing or misusing the token, non-org-member PRs changing .github/actions/ files) that weren't previously enabled. I think we already provide a GITHUB_TOKEN, so maybe it isn't a big delta.
  • Would it be possible/appropriate to put this into a separate Actions file? I kinda want to have one file for tests, and a separate file for maintenance things. Given the use of the build cache, I wouldn't be surprised if it must indeed share a file.

@michaelfig michaelfig force-pushed the mfig-npm-cicd branch 2 times, most recently from feb9523 to a24aea7 Compare December 1, 2021 04:55
@michaelfig
Copy link
Member Author

michaelfig commented Dec 1, 2021

  • Is NPM cool with us publishing releases as such a high rate? Is this standard practice for other projects?

I think there's only one way to find out. I've seen this kind of "nightly build" for other projects, but I don't know what their rates are. NPM is now owned by GitHub, so I'd think the acceptable rates are more than they used to be. Do note that the only dev packages published are roughly the ones that actually changed.

  • I assume yarn release does the right thing, but to double check, does the dev-(HASH).N version sort properly (N is more significant than the hash)?

The dev-(HASH).N does not sort. You need to request the release explicitly or you won't get it via semver. That's why the dev dist-tag is necessary to make them useful enough for reasonable devex.

  • Unless NPM's auth language makes it possible to scope a token to e.g. only dev- releases, the proposed NPM_TOKEN is scary powerful. Assuming we don't already have one configured, adding this would enable some attacks
    (buggy GH Action code revealing or misusing the token,

Yes, that's possible.

non-org-member PRs changing .github/actions/ files) that weren't previously enabled.

They only get executed after being enabled by one of the team members, so that can only happen if we're not vigilant.

I think we already provide a GITHUB_TOKEN, so maybe it isn't a big delta.

That's what I'm thinking. And also some DOCKER_* credentials.

I personally take our Github to be our source of truth, and these other services need to reflect what we have there or we run into the problem of users consuming stale code.

  • Would it be possible/appropriate to put this into a separate Actions file? I kinda want to have one file for tests, and a separate file for maintenance things. Given the use of the build cache, I wouldn't be surprised if it must indeed share a file.

I've tried to separate these things in general. I have verified that the cache can be used in any file. However, separate files run in parallel and so race for the cache. That may end up running more Actions minutes than we expect. I'd be happy to revisit this (splitting the workflow files) some other time.

@mhofman
Copy link
Member

mhofman commented Dec 1, 2021

  • I assume yarn release does the right thing, but to double check, does the dev-(HASH).N version sort properly (N is more significant than the hash)?

The dev-(HASH).N does not sort. You need to request the release explicitly or you won't get it via semver. That's why the dev dist-tag is necessary to make them useful enough for reasonable devex.

Doesn't publishing a package require the package.json version to be updated? I guess it could be done on a dirty worktree and not committed, but that kinda goes against the idea of having source code available for everything published?

Would all the packages be pushed or only the ones impacted by a change (which does bring the question of updating dependencies if changing the versions, so that a specific tag pulls in the right dependencies).

I suppose an alternative is to have CI automatically add a commit which bumps the package versions appropriately before merging?

@warner
Copy link
Member

warner commented Dec 1, 2021

Doesn't publishing a package require the package.json version to be updated? I guess it could be done on a dirty worktree and not committed, but that kinda goes against the idea of having source code available for everything published?

I think I'm ok with there being slight differences between the tarball and the source tree that the tarball's HASH name refers to, if the differences are just metadata. It shouldn't impact anybody's ability to figure out what real source code went into the dev release.

I suppose an alternative is to have CI automatically add a commit which bumps the package versions appropriately before merging?

That sounds.. eww, like two commits per commit. I'm not ok with high-rate automatic commits made by bots. I can see being ok with it like once per real release, as part of some automated release management stuff, but I don't want an extra commit or extra tag per real commit.

@jessysaurusrex
Copy link
Contributor

The NPM_token management here sounds reasonable.

I have some questions about code distribution as NPM introduces complexity and an opportunity for things to break... especially in terms of version-mismatch issues and nightly releases.

How do we authoritatively release things now?

Ideally, how would we like to authoritatively release things in the future? ;)

Where do we want our customers to get our code from? If the answer is "Github", there is arguably a world where we can set up a process for our latest release to be pushed to our NPM package. I'm not sure that it is appropriate to use NPM/Docker for nightly distribution. There doesn't seem to be a good reason for nightly code to be immutably available in third party package registries to developers, especially if the known broken code will just hang out around there forever. Also, in my experience, projects with very active nightly release cycles tend to require developers to get that code directly from them. (e.g. Homebrew doesn't provide Rust nightlies, only major releases. Rust provides a custom toolchain for installing nightlies.)

Are there any plans to distribute and sign our software outside of the NPM universe? There is a case where someone who trusts Agoric more than NPM may want to come to us directly for software, and would like to be able to check that software against a cryptographic identity, e.g. a validator with a mature approach to dependency management.

@michaelfig
Copy link
Member Author

I have some questions about code distribution as NPM introduces complexity and an opportunity for things to break... especially in terms of version-mismatch issues and nightly releases.

How do we authoritatively release things now?

agoric-sdk/MAINTAINERS.md are the notes I've collected on this process thus far. It describes the process I follow to tag and publish to Docker and NPM as latest whenever I feel the urge. I want to codify and automate "whenever I feel the urge" to be consistent and not reliant on my urges.

dev releases are intended to be for contract developers wanting to test their code against nightly, not for regular consumption. They also exercise the release process to make sure we don't accidentally break it (which happens frequently unless I'm doing the equivalent process manually).

I tag beta releases manually when spinning up a new devnet once there's some consensus around Agoric engg that we need to update what people get by default when "trying our beta" as described in https://agoric.com/documentation.

Ideally, how would we like to authoritatively release things in the future? ;)

Just formalise the things we're already doing to make it more consistent, with an eye on automation to reduce the maintenance burden.

Where do we want our customers to get our code from? If the answer is "Github", there is arguably a world where we can set up a process for our latest release to be pushed to our NPM package. I'm not sure that it is appropriate to use NPM/Docker for nightly distribution.

We have multiple audiences, any of which may want to use a Git agoric-sdk checkout for bleeding-edge changes or their own modifications.

  • SDK developers (Git checkout)
  • dapp authors (NPM or Git checkout)
  • validators (Docker or Git checkout)

The reason I'm advocating dev NPM and Docker releases is so the dapp authors and validators don't have to use a completely different workflow just to see if their problem is fixed on latest master, or if it existed as of a specific commit.

There doesn't seem to be a good reason for nightly code to be immutably available in third party package registries to developers, especially if the known broken code will just hang out around there forever. Also, in my experience, projects with very active nightly release cycles tend to require developers to get that code directly from them. (e.g. Homebrew doesn't provide Rust nightlies, only major releases. Rust provides a custom toolchain for installing nightlies.)

I would agree, once our smart contract platform isn't in so much flux. Testing the release process is one reason I want to do it. Also, I want to make it possible for standard JS or devops tools (NPM+Docker) to be used consistently rather than having to rely on a "custom toolchain", which has been the bane of my existence. Ideally, dapp developers could consistently use their own package management tools, not just Yarn and an SDK checkout. That's why I'm trying to go in this direction.

Being able to say: "So you have a problem with the Docker beta release (or NPM beta release)? Please give dev-aidceh.1 a try" is powerful.

Are there any plans to distribute and sign our software outside of the NPM universe?

I imagine that Exo will be a signed multiplatform application.

There is a case where someone who trusts Agoric more than NPM may want to come to us directly for software, and would like to be able to check that software against a cryptographic identity, e.g. a validator with a mature approach to dependency management.

That's why all these release-generation-from-Git-checkout tools within agoric-sdk are useful. A third party could reproduce our release process, and use the artefacts locally.

@dckc
Copy link
Member

dckc commented Dec 1, 2021

...

I want to make it possible for standard JS or devops tools (NPM+Docker) to be used consistently rather than having to rely on a "custom toolchain", which has been the bane of my existence.

👍
as noted in the description, see #3857

Are there any plans to distribute and sign our software outside of the NPM universe?

I imagine that Exo will be a signed multiplatform application.

see also #2541

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

ok, this seems reasonable. maybe add a comment about the omitted PR type for future readers.

push:
branches: [ $default-branch ]
pull_request:
types: [opened, closed, reopened, synchronize]
Copy link
Member

Choose a reason for hiding this comment

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

what type of PR does this omit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, adds closed (which is not triggered by default) to the default list. I'll add a comment referring to Github documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants