diff options
| -rw-r--r-- | doc/development/README.md | 4 | ||||
| -rw-r--r-- | doc/development/contributing/style_guides.md | 1 | ||||
| -rw-r--r-- | doc/development/go_guide/index.md | 216 |
3 files changed, 221 insertions, 0 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index 05715274a81..d5829e31343 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -123,3 +123,7 @@ description: 'Learn how to contribute to GitLab.' ## Compliance - [Licensing](licensing.md) for ensuring license compliance + +## Go guides + +- [Go Guidelines](go_guide/index.md) diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 6f1ba5d62a5..0eedef5e14f 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -21,6 +21,7 @@ of _prohibited this user from being saved due to the following errors:_ the text should be _sorry, we could not create your account because:_ 1. Code should be written in [US English][us-english] +1. [Go](../go_guide/index.md) This is also the style used by linting tools such as [RuboCop](https://github.com/bbatsov/rubocop) and [Hound CI](https://houndci.com). diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md new file mode 100644 index 00000000000..cdc806a2d31 --- /dev/null +++ b/doc/development/go_guide/index.md @@ -0,0 +1,216 @@ +# Go standards and style guidelines + +This document describes various guidelines and best practices for GitLab +projects using the [Go language](https://golang.org). + +## Overview + +GitLab is built on top of [Ruby on Rails](https://rubyonrails.org/), but we're +also using Go for projects where it makes sense. Go is a very powerful +language, with many advantages, and is best suited for projects with a lot of +IO (disk/network access), HTTP requests, parallel processing, etc. Since we +have both Ruby on Rails and Go at GitLab, we should evaluate carefully which of +the two is best for the job. + +This page aims to define and organize our Go guidelines, based on our various +experiences. Several projects were started with different standards and they +can still have specifics. They will be described in their respective +`README.md` or `PROCESS.md` files. + +## Code Review + +We follow the common principles of +[Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments). + +Reviewers and maintainers should pay attention to: + +- `defer` functions: ensure the presence when needed, and after `err` check. +- Inject dependencies as parameters. +- Void structs when marshalling to JSON (generates `null` instead of `[]`). + +### Security + +Security is our top priority at GitLab. During code reviews, we must take care +of possible security breaches in our code: + +- XSS when using text/template +- CSRF Protection using Gorilla +- Use a Go version without known vulnerabilities +- Don't leak secret tokens +- SQL injections + +Remember to run +[SAST](https://docs.gitlab.com/ee/user/project/merge_requests/sast.html) +**[ULTIMATE]** on your project (or at least the [gosec +analyzer](https://gitlab.com/gitlab-org/security-products/analyzers/gosec)), +and to follow our [Security +requirements](../code_review.md#security-requirements). + +Web servers can take advantages of middlewares like [Secure](https://github.com/unrolled/secure). + +### Finding a reviewer + +Many of our projects are too small to have full-time maintainers. That's why we +have a shared pool of Go reviewers at GitLab. To find a reviewer, use the +[Engineering Projects](https://about.gitlab.com/handbook/engineering/projects/) +page in the handbook. "GitLab Community Edition (CE)" and "GitLab Community +Edition (EE)" both have a "Go" section with its list of reviewers. + +To add yourself to this list, add the following to your profile in the +[team.yml](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/team.yml) +file and ask your manager to review and merge. + +```yaml +projects: + gitlab-ee: reviewer go + gitlab-ce: reviewer go +``` + +## Code style and format + +- Avoid global variables, even in packages. By doing so you will introduce side + effects if the package is included multiple times. +- Use `go fmt` before committing ([Gofmt](https://golang.org/cmd/gofmt/) is a + tool that automatically formats Go source code). + +### Automatic linting + +All Go projects should include these GitLab CI/CD jobs: + +```yaml +go lint: + image: golang:1.11 + script: + - go get -u golang.org/x/lint/golint + - golint -set_exit_status +``` + +Once [recursive includes](https://gitlab.com/gitlab-org/gitlab-ce/issues/56836) +become available, you will be able to share job templates like this +[analyzer](https://gitlab.com/gitlab-org/security-products/ci-templates/raw/master/includes-dev/analyzer.yml). + +## Dependencies + +Dependencies should be kept to the minimum. The introduction of a new +dependency should be argued in the merge request, as per our [Approval +Guidelines](../code_review.html#approval-guidelines). Both [License +Management](https://docs.gitlab.com/ee/user/project/merge_requests/license_management.html) +**[ULTIMATE]** and [Dependency +Scanning](https://docs.gitlab.com/ee/user/project/merge_requests/dependency_scanning.html) +**[ULTIMATE]** should be activated on all projects to ensure new dependencies +security status and license compatibility. + +### Modules + +Since Go 1.11, a standard dependency system is available behind the name [Go +Modules](https://github.com/golang/go/wiki/Modules). It provides a way to +define and lock dependencies for reproducible builds. It should be used +whenever possible. + +There was a [bug on modules +checksums](https://github.com/golang/go/issues/29278) in Go < v1.11.4, so make +sure to use at least this version to avoid `checksum mismatch` errors. + +### ORM + +We don't use object-relational mapping libraries (ORMs) at GitLab (except +[ActiveRecord](https://guides.rubyonrails.org/active_record_basics.html) in +Ruby on Rails). Projects can be structured with services to avoid them. +[PQ](https://github.com/lib/pq) should be enough to interact with PostgreSQL +databases. + +### Migrations + +In the rare event of managing a hosted database, it's necessary to use a +migration system like ActiveRecord is providing. A simple library like +[Journey](https://github.com/db-journey/journey), designed to be used in +`postgres` containers, can be deployed as long-running pods. New versions will +deploy a new pod, migrating the data automatically. + +## Testing + +We should not use any specific library or framework for testing, as the +[standard library](https://golang.org/pkg/) provides already everything to get +started. For example, some external dependencies might be worth considering in +case we decide to use a specific library or framework: + +- [Testify](https://github.com/stretchr/testify) +- [httpexpect](https://github.com/gavv/httpexpect) + +Use [subtests](https://blog.golang.org/subtests) whenever possible to improve +code readability and test output. + +### Benchmarks + +Programs handling a lot of IO or complex operations should always include +[benchmarks](https://golang.org/pkg/testing/#hdr-Benchmarks), to ensure +performance consistency over time. + +## CLIs + +Every Go program is launched from the command line. +[cli](https://github.com/urfave/cli) is a convenient package to create command +line apps. It should be used whether the project is a daemon or a simple cli +tool. Flags can be mapped to [environment +variables](https://github.com/urfave/cli#values-from-the-environment) directly, +which documents and centralizes at the same time all the possible command line +interactions with the program. Don't use `os.GetEnv`, it hides variables deep +in the code. + +## Daemons + +### Logging + +The usage of a logging library is strongly recommended for daemons. Even though +there is a `log` package in the standard library, we generally use +[logrus](https://github.com/sirupsen/logrus). Its plugin ("hooks") system +makes it a powerful logging library, with the ability to add notifiers and +formatters at the logger level directly. + +### Tracing and Correlation + +[LabKit](https://gitlab.com/gitlab-org/labkit) is a place to keep common +libraries for Go services. Currently it's vendored into two projects: +Workhorse and Gitaly, and it exports two main (but related) pieces of +functionality: + +- [`gitlab.com/gitlab-org/labkit/correlation`](https://gitlab.com/gitlab-org/labkit/tree/master/correlation): + for propagating and extracting correlation ids between services. +- [`gitlab.com/gitlab-org/labkit/tracing`](https://gitlab.com/gitlab-org/labkit/tree/master/tracing): + for instrumenting Go libraries for distributed tracing. + +This gives us a thin abstraction over underlying implementations that is +consistent across Workhorse, Gitaly, and, in future, other Go servers. For +example, in the case of `gitlab.com/gitlab-org/labkit/tracing` we can switch +from using Opentracing directly to using Zipkin or Gokit's own tracing wrapper +without changes to the application code, while still keeping the same +consistent configuration mechanism (i.e. the `GITLAB_TRACING` environment +variable). + +### Context + +Since daemons are long-running applications, they should have mechanisms to +manage cancellations, and avoid unnecessary resources consumption (which could +lead to DDOS vulnerabilities). [Go +Context](https://github.com/golang/go/wiki/CodeReviewComments#contexts) should +be used in functions that can block and passed as the first parameter. + +## Dockerfiles + +Every project should have a `Dockerfile` at the root of their repository, to +build and run the project. Since Go program are static binaries, they should +not require any external dependency, and shells in the final image are useless. +We encourage [Multistage +builds](https://docs.docker.com/develop/develop-images/multistage-build/): + +- They let the user build the project with the right Go version and + dependencies. +- They generate a small, self-contained image, derived from `Scratch`. + +Generated docker images should have the program at their `Entrypoint` to create +portable commands. That way, anyone can run the image, and without parameters +it will display its help message (if `cli` has been used). + +--- + +[Return to Development documentation](../README.md). |