Skip to content

Latest commit

 

History

History
429 lines (310 loc) · 15.7 KB

DEVELOPING.md

File metadata and controls

429 lines (310 loc) · 15.7 KB
Raw

Purpose

To provide useful information for developers and maintainers of Storj.

Tooling

Development

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 and most of our reviews are done in Gerrit. Reviews require 2 Code Review +2 and a passing build in order to be merged.

Continuous Integration

Currently, builds are performed by a Jenkins 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 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. 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.

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. 
    git clone [email protected]:storj/storj.git && cd storj
  2. Setup Gerrit commit hook and aliases. 
    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.

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).
  • {changeID} refers to the automatically generated change id.

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.

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.

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.

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.)

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. 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.

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.

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.

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.

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.

# 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.

# 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.

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.

# 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.

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.

# 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).

    - type: bind
      source: ${SOURCE_DIR}/web/satellite/dist/
      target: /var/lib/storj/storj/web/satellite/dist/
      bind:
        create_host_path: true

You’ll need to restart the container to pick up the new volume.

docker compose up -d

Now, you just need to rebuild the web assets, and they’ll automatically be picked up by the satellite-api deployment.

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.

    - type: bind
      source: ${SOURCE_DIR}/web/satellite/static/
      target: /var/lib/storj/storj/web/satellite/static/
      bind:
        create_host_path: true

Similarly, you’ll need to restart the container to pick up the new volume.

docker compose up -d

Now, any changes to the static directory will automatically be picked up by the backend. If you’re iterating on the wasm module, you can invoke the wasm or wasm-dev npm targets.

cd web/satellite

npm run wasm

Specific Contribution Workflows

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.