summaryrefslogtreecommitdiff
path: root/doc/development/go_guide/index.md
blob: cdc806a2d3165a517f7cfb9ba4cbee9e0f13621b (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
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).