These guidelines for writing Go code are WIP
This document covers common coding styles and guidelines for all ForgeRock products.
Within the FRaaS codebases we are not currently adding licence headers to individual source files. This practice diverges from the standard practice of doing so across other projects.
The ForgeRock Go coding style
- Don't use os.Exit (or logging.Fatal) anywhere except the main function - this stops any 'defer' statements from running and also prints no message when it exits, which can cause confusion when trying to debug why a program suddenly stopped. If something has failed in such a way that it can't recover, panic instead. If you can't do anything with the error, just return the error from the function you are in (and handle this case further up).
- If you are returning an error from another package (or a standard library function), wrap it with errors.Wrap from github.com/pkg/errors which will add context to the error to make it easier to understand where it comes from. If you are creating a new error, use errors.New (or errors.Errorf instead of fmt.Errorf) from the same library which provide the same utility.
- Each Go project should include one or more README.md files detailing
- An overview of the project and its purpose
- How to build, test, deploy and configure the executable
- All public constants, structs, fields, interfaces and functions must have Go doc
- All packages must have Go doc - Packages with more than one source file should consider providing package documentation using a doc.go file
Source code layout
In addition to good documentation, having a consistent approach to organising code across directories and within a given source file makes it easier for engineers to move between projects and get up to speed quickly.
- Each Go project should use a standard layout
- Each Go source file should, as far as possible, be readable top-to-bottom with public / high level functions at the top of source files and private / helper functions lower down:
- public functions
- private functions
Where possible, agreed standards relating to source files should be enforced by linting during continuous integration. The linting rules currently in use by the FRaaS team are:
- Avoid logging directly at the site where an error is produced or returned. Instead let the entry point to processing do the logging. If the returned error message is insufficient (often they are already sufficient) use
errors.Wrapto add context to the returned error.
- Use the
github.com/pkg/errorspackage instead of
errors, and user
errors.WithStack(err)wherever an error is produced or returned from an external package.
WithStackproduces a stack trace pointing to the line of code which produced the error, which also prevents us from having to add our own custom error message so that we can correlate the error message to the line of code which produced it.This can also be done wherever there's an
Any new packages that need to log things should use the common logger specified in common/pkg/logging/logging.go. This can be used by importing in your package (in package.go, or another file)
Programs should only exit in a 'main' function, and should log which error caused the program to fail at the 'Panic' level. This will make it obvious when looking at logs where and why the program exited.
In addition to the points raised above, we should endeavour to write idiomatic Go. Guidance for what these idioms are and how to follow them can be found in:
What not to do
- Do not write a 'custom' mock - eg, write a struct which implements the interface you want to mock. If somebody needs to change the interface further down the line, they will then need to modify your mock as well. When using an autogenerated mock, they can just run 'go generate' to recreate it.
- Do not use a mock for an interface in the same package. Mocks should provide a description of the interface for something in the package - the package itself should not need to test the mock.
When adding a new interface that will require a mock there are a few simple steps to follow
- Follow the instructions to install mockery
- Add a line either above the interface you've added or in a package.go file alongside the interface with a 'go generate' comment
- Run 'go generate ./...' in the relevant folder - this will run 'mockery' and generate a mock for your interface under the 'mocks' folder
Your interface should look like this:
If you need to generate a mock for an interface but you also need to use that mock in the same package, this will cause import cycles. To get around this, add '--inpackage' to the list of arguments to 'mockery' in the generate comment.
If you want to use one of these mocks in one of your tests, there is a small utility function in go/common/pkg/testutil/mockhelper.go which can be used to do some common setup and mock assertion. An example of how to use this:
The testify documentation will have more information about what methods these mocks will have and how to use them.
Assertions in tests
We use https://github.com/stretchr/testify for writing assertions in tests. This has 2 common operations
- 'assert' functions will fail the test if the assertion is incorrect, but the test will continue to run.
- 'require' functions will fail the test and immediately stop the test if the assertion is incorrect.
'require' should be used when it's impossible to continue the test, such as if an unrecoverable error happens during test setup or if a particular object used is not valid:
If a test can continue after something fails, use 'assert' instead.
Adding 'options' to a constructor
This is a fairly standard pattern to add extra options to a constructor for a struct. Say you have a struct with a constructor like this
If you want to extend this struct, the obvious way to do it is to add extra things to the constructor:
In future people might want to add even more things to this structure, and this approach does not scale:
- People might not care about any other fields in the struct other than the ones they care about
- Every time people instantiate a struct, they need to pass a large number of arguments
- Every time this signature changes, every other place in the code that uses this constructor needs changing as well
If you find yourself adding a lot of fields to a struct like this, use 'options' instead.
- Set sensible defaults for the fields in the contructor
- Specify a private interface controlling how to modify these fields (so that people can't define their own options, to keep the implementation in one package)
- Add any options as types which implement this interface
Now anybody who wants to create one of these structs can pass options defining only what they care about rather than every single field in the struct.
If you are writing a test where there are multiple 'sub tests' (such as checking the behaviour of a certain endpoint with multiple different data inputs), Use t.Run() to separate out these sub-tests.
If you have a test setup like this:
Then as soon as any test runs into an error, it will implicitly fail all other tests as well. Instead, use sub-tests like this:
Creating GCP API clients
When creating an API client (for example, a logging client that is used to read logs from the GCP API) and you only ever want to read data, pass a 'WithScope' option to the constructor to limit what the client can do:
Commonly used libraries
- (Pending review and approval from others) Validator validates struct data to ensure input matches what's required. github.com/go-playground/validator
- Test assertions with nice output https://github.com/stretchr/testify
- Try to use the cloud.google.com/go/... client libraries rather than google.golang.org/... if one exists
- github.com/spf13/cobra and github.com/spf13/viper for command line utilities
- https://pkg.go.dev/golang.org/x/sync/errgroup for a more intuitive way to do a group of work in parallel
- When setting up logging, use the common logger in go/common/pkg/logging/logging.go (which under the hood uses https://github.com/sirupsen/logrus)
- github.com/gin-gonic/gin for HTTP servers
- github.com/hashicorp/go-multierror for handling a case where multiple errors might be accumulated in a loop