storj/DEVELOPING.md
Mya 9153f37055 Makefile: run lint locally in docker
Our linting process depends heavily on custom tools and linting
configuration. To help improve this process for running on local
developer machines, we can run the various tasks in our existing CI
container.

Change-Id: I60407686ce9233cc4f16e3724c5e8d44367aa200
2022-05-25 12:30:15 -05:00

430 lines
16 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Purpose
To provide useful information for developers and maintainers of Storj.
# Tooling
## Development
[`storj-up`](https://github.com/storj/up) provides a convenient way to configure and spin up a cluster locally for
active development. In addition to `storj-up`, you will need the following tools:
- TODO
## Version Control and Code Review
All of our source code is hosted on [GitHub](https://github.com/storj) and most of our reviews are done in
[Gerrit](https://review.dev.storj.io). Reviews require 2 `Code Review +2` and a passing build in order to be merged.
## Continuous Integration
Currently, builds are performed by a [Jenkins](https://build.dev.storj.io) cluster hosted in Google Cloud. Because of
issues with how Google Cloud connects their disk to your virtual machine, we've disabled debug logging in our tests for
the time being. If a build fails and the failure message is insufficient for troubleshooting the test failure, rerunning
the test locally will output debug logs to aid in the troubleshooting.
# Practices
## Git Workflow
Storj uses a [Gerrit][gerrit-link] based git workflow. While pull requests can be made against the public GitHub
repository, many engineers at Storj prefer Gerrit for code reviews. For an overview of how this works, you can read
the [Intro to Gerrit Walkthrough][gerrit-walkthrough-link]. Be sure to [sign in][] to Gerrit before attempting to
contribute any changes. You'll likely need to upload an SSH key in order to push any changes.
[sign in]: https://review.dev.storj.io
[gerrit-link]: https://review.dev.storj.io/Documentation/index.html
[gerrit-walkthrough-link]: https://review.dev.storj.io/Documentation/intro-gerrit-walkthrough-github.html
Below, you'll find several common workflows that I've used when contributing code.
### Cloning a project
When a project uses Gerrit, there is some additional work that needs to be done before you can start contributing.
1. Clone the repository.
```shell
git clone git@github.com:storj/storj.git && cd storj
```
2. Setup Gerrit commit hook and aliases.
```shell
curl -L storj.io/clone | sh
```
That should be it. At this point, the repository should be setup properly for Gerrit.
### Starting a new change set
When starting a new change, I find it useful to start a branch from the latest main (just like you would with any other
GitHub project). Unlike GitHub, we will not be pushing this branch remotely.
```shell
git checkout main
git pull
git checkout -b branch-name
```
This is where you'll work on and commit your code. When you commit your changes, your commit message should not only
describe what is changing, but why the change is being made. Storj uses the following convention for commit messages:
```
{scope}: {message}
{detail}
Change-Id: {changeID}
```
- `{scope}` refers to path within the repo that's changing. For example `satellite/metainfo` or `web/storagenode`.
Multiple scopes _can_ be provided, but should be minimized.
- `{message}` should provide a clear and succinct message about what is changing. Using words like `add`, `remove`,
`reuse`, or `refactor` are great here.
- `{detail}` provides additional information about the change. This is a good place to example why the change is being
made. If you're making performance related changes, then this should also include a performance report comparing the
before and after ([example][performance-example-link]).
- `{changeID}` refers to the automatically generated change id.
[performance-example-link]: https://github.com/storj/picobuf/commit/1d3412eb3ac13a476e56fa0e732552ed2ee89ecf
To produce this commit message, I find it easiest to start with the scope and message and then amend the commit to add
the detail. This allows the commit hook to automatically add the change id before filling in the longer detail of the
commit message. In addition to that, amending the commit provides some text guides that can be a useful reference when
determining where to wrap your detail text.
```shell
git commit -m "{scope}: {message}"
git commit --amend
```
Once a change has been commit, you can use `git push-wip` to push a work-in-progress change or `git push-review` to push
a new review. A work-in-progress change will allow you to push code without running tests or notifying the team of a new
review. All active reviews will get pulled into our review notification system and run our CI pipeline.
### Updating an existing change set
Updating an existing change set is a fairly common task. This can happen while iterating on a change or even updating it
with the latest set of changes from main. In the former case (updating while iterating on a change), you simply need to
amend your existing commit with your latest changes.
```shell
git add <updates>
git commit --amend --no-edit
```
The `--amend` flag tells git that you want the current change set to be added to the last git commit on your current
branch. The `--no-edit` flag instructs git to leave the commit message as is, and not to prompt the end user for
modifications to the message.
In the later case (when updating with the latest set of changes from main), you simply need to rebase your branch on the
updates from main.
```shell
git checkout main
git pull
git checkout branch-name
git rebase main
```
After any kind of change, you simply need to `push-wip` or `push-review` again.
### Managing relation chains
In Gerrit, multiple changes can be linked together for larger features. This is done by adding multiple commits to the
same branch. Consider the `oidc/implementation` branch below.
```
commit 533b6a2624f2a2eab45f0d68a6dd6e4fd1ec1124 (HEAD -> oidc/implementation)
Author: Mya <redacted>
Date: Mon Feb 14 14:06:35 2022 -0600
web/satellite: add consent screen for oauth
When an application wants to interact with resources on behalf of
an end-user, it needs to be granted access. In OAuth, this is done
when a user submits the consent screen.
Change-Id: Id838772f76999f63f5c9dbdda0995697b41c123a
commit ed9b75b11ecde87649621f6a02d127408828152e
Author: Mya <redacted>
Date: Tue Feb 8 15:28:11 2022 -0600
satellite/oidc: add integration test
This change adds an integration test that performs an OAuth
workflow and verifies the OIDC endpoints are functioning as
expected.
Change-Id: I18a8968b4f0385a1e4de6784dee68e1b51df86f7
commit 187aa654ff18387d0011b3741cca62270b95d6e6
Author: Mya <redacted>
Date: Thu Feb 3 14:49:38 2022 -0600
satellite/console: added oidc endpoints
This change adds endpoints for supporting OpenID Connect (OIDC) and
OAuth requests. This allows application developers to easily
develop apps with Storj using common mechanisms for authentication
and authorization.
Change-Id: I2a76d48bd1241367aa2d1e3309f6f65d6d6ea4dc
```
These commits result in 3 distinct Gerrit changes, each linked together. If the last change in the link is merged, then
all other changes are merged along with it.
#### Editing or dropping a commit
In many cases, we need to go back and amend a previous change because of some feedback, a failing test, etc. If you're
editing an existing change or dropping and old one then you can use an interactive rebase to `edit` or `drop` a previous
commit. The command below will start an interactive rebase with the 3 most recent commits. (You should adjust this
number as needed.)
```shell
git rebase -i HEAD~3
```
This should bring up a screen where you can choose which commits you want to `edit` and which ones you want to `drop`.
The code block below shows how this screen looks for the associated changes above.
```
pick 187aa654f satellite/console: added oidc endpoints
pick ed9b75b11 satellite/oidc: add integration test
pick 533b6a262 web/satellite: add consent screen for oauth
# Rebase d4cf2013e..533b6a262 onto d4cf2013e (3 commands)
#
# Commands:
# p, pick <commit> = use commit
# r, reword <commit> = use commit, but edit the commit message
# e, edit <commit> = use commit, but stop for amending
# s, squash <commit> = use commit, but meld into previous commit
# f, fixup [-C | -c] <commit> = like "squash" but keep only the previous
# commit's log message, unless -C is used, in which case
# keep only this commit's message; -c is same as -C but
# opens the editor
# x, exec <command> = run command (the rest of the line) using shell
# b, break = stop here (continue rebase later with 'git rebase --continue')
# d, drop <commit> = remove commit
# l, label <label> = label current HEAD with a name
# t, reset <label> = reset HEAD to a label
# m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
# . create a merge commit using the original merge commit's
# . message (or the oneline, if no original merge commit was
# . specified); use -c <commit> to reword the commit message
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
```
#### Adding a new commit
To add a new commit to the relation chain, `edit` the commit just before where you want to add one, then make or unstash
your modifications, and commit as you would [above](#starting-a-new-change-set). Continuing the rebase will re-apply
your later changes on top of your newly created commit.
#### Splitting a commit
Splitting an existing change into multiple changes can be significantly more difficult. You need to consider not only
how a change needs to be split, but the order in which the commits must be applied. In this case, it's preferred that
one of the split commits preserve the existing change id as it's likely your change has already accumulated some
feedback that we want to preserve. When splitting a commit, I prefer to create a new branch before rebasing the change.
```shell
git checkout oidc/implementation
git checkout -b oidc/rework
git rebase -i HEAD~3
...
```
This way, if I make a mistake or need to start over, I can go back to a known working version by aborting the rebase,
going back to the old branch, and deleting the rework.
```shell
git rebase --abort
git checkout oidc/implementation
git branch -D oidc/rework
git branch -b oidc/rework
git rebase -i HEAD~3
...
```
## Linting locally
Our code linting process requires several customized and company specific tools. These are all packaged and provided as
part of our CI container. When running the lint target, we will spin up our CI container locally, and execute the
various linters from inside the container.
```sh
make lint
```
By default, the linter runs on the entire code base but can be limited to specific packages. It is worth noting that
some linters cannot run on specific packages and will therefore be unaffected by the provided packages.
```sh
make lint LINT_TARGET="./satellite/oidc/..."
```
## Executing tests locally
The Storj project has an extensive suite of integration tests. Many of these tests require several infrastructure
dependencies. These dependencies are defined and managed by the `docker-compose.tests.yaml` file. Tests can be executed
against Postgres (`test/postgres`), or CockroachDB (`test/cockroach`), or against both (`test`). By default, the full
suite of tests is run, but can be limited using the `TEST_TARGET` make variable.
```sh
# run only against Postgres
make test/postgres TEST_TARGET="./satellite/oidc/..."
```
You can also provide multiple targets for the test harness to run in case your change spans across several packages.
```sh
# run against both Postgres and Cockroach
make test TEST_TARGET="./satellite/oidc/... ./satellite/satellitedb/..."
```
_**Note:** While you can run the full suite of tests locally, you will likely be waiting around for them to complete.
By starting with the tests from packages you have modified, you can build a great deal of confidence in your changes
before pushing them up for review._
## Developing locally with `storj-up`
Following the instructions in the `storj-up` project `README`, the following will deploy a copy of the stack.
```shell
storj-up init
docker compose up -d
```
Outside the automation `storj-up` provides, there are a handful of manual changes that can be made to support testing
and developing against different portions of the stack. In the following sections, we demonstrate how this can be done
using the satellite process as an example, but the same process should work with many of our other processes.
### Testing backend changes
To test local backend changes, all you need to do is tell `storj-up` to mount a local binary for those containers.
Before mounting the local binary, you'll need to ensure your local binary is up-to-date.
```shell
# on Linux
go install ./cmd/satellite
storj-up local-bin satellite-core satellite-admin satellite-api
# on OSX
GOOS=linux GOARCH=amd64 go install ./cmd/satellite
storj-up local-bin -s linux_amd64 satellite-core satellite-admin satellite-api
```
Once `storj-up` completes, you'll need to redeploy the containers to pick up the new configuration.
```shell
docker compose up -d
```
From here on out, all you'll need to do is recompile the binary and restart the container to pick up your latest
changes.
```shell
# on Linux
go install ./cmd/satellite
docker restart storj-satellite-core-1 storj-satellite-api-1 storj-satellite-admin-1
# on OSX
GOOS=linux GOARCH=amd64 go install ./cmd/satellite
docker restart storj-satellite-core-1 storj-satellite-api-1 storj-satellite-admin-1
```
_Aside_ - A few more notes here on OSX:
- Ensure your `GOPATH` is mountable by Docker. On OSX, only certain folders are mounted into the VM so you'll need to
make sure it falls under one of those. Otherwise, `storj-up` will attempt to mount a directory under `GOBIN` that is
unavailable on the VM.
- Even if you're on a Mac M1 (like me), you should target an `amd64` architecture. OSX provides some supporting tools
that allows you to run `amd64` workloads on `arm64` processors.
### Testing frontend changes
In order to support mounting local web builds into the container, manual changes needed to be made to the
`satellite-api` volumes block of the `docker-compose.yaml` file. The following block adds a bind mount for the web
assets. Be sure to replace `${SOURCE_DIR}` with the path to the storj source repository (`pwd` on OSX and Linux).
```yaml
- type: bind
source: ${SOURCE_DIR}/web/satellite/dist/
target: /var/lib/storj/storj/web/satellite/dist/
bind:
create_host_path: true
```
Youll need to restart the container to pick up the new volume.
```shell
docker compose up -d
```
Now, you just need to rebuild the web assets, and theyll automatically be picked up by the `satellite-api` deployment.
```shell
cd web/satellite
npm run build
```
#### Testing changes to static files (including wasm)
Any changes to the `static` directory (including wasm) will require an additional bind mount for the `satellite-api`.
```yaml
- type: bind
source: ${SOURCE_DIR}/web/satellite/static/
target: /var/lib/storj/storj/web/satellite/static/
bind:
create_host_path: true
```
Similarly, youll need to restart the container to pick up the new volume.
```shell
docker compose up -d
```
Now, any changes to the `static` directory will automatically be picked up by the backend. If youre iterating on the
wasm module, you can invoke the `wasm` or `wasm-dev` npm targets.
```shell
cd web/satellite
npm run wasm
```
# Specific Contribution Workflows
<!-- translations? -->
## Security patches
If you're submitting a patch related to a core vulnerability of the product, the review should be submit as a
work-in-progress, marked private, and the following users should be added.
- JT Olio
- Kaloyan Raev
- Egon Elibre
- Jeff Wendling
- Márton Elek
- Mya Pitzeruse
Together, we'll work through how to best roll the changes out to our network.