diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2022-06-28 00:01:58 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2022-07-15 11:18:26 +0300 |
commit | 861730acb7af2a0575f384952d08715031803f20 (patch) | |
tree | fe6f25c569c3666eb23a506145bbf6260d337f8c | |
parent | d23c111cb82f464582e89443040d15d6f435b971 (diff) |
Pack auxiliary binaries in main Gitaly binary
Gitaly needs to carefully maintain backwards compatibility with its
auxiliary commands to avoid issues during upgrades. Omnibus and source
deployments upgrade the binaries in place which can cause a running
Gitaly to invoke a binary from the newer release. This has been the
root cause in multiple production issues. To avoid having to maintain
backwards compatibility between Gitaly and its auxiliary binaries, we
can pin the binaries in a manner that Gitaly only ever invokes the
auxiliary binaries from the same build as the main binary. To do so,
we can pack the auxiliary binaries into the main Gitaly binary and
unpack them into a temporary directory when starting up. This way the
upgrades happen atomically, as replacing the gitaly binary is an
atomic operation and it contains the auxiliary binaries as well.
This commit adds the functionality to pack and unpack binaries in the
main Gitaly binary. When building the Gitaly binary, the auxiliary
binaries are taken from _build/bin and embedded in the main binary.
UnpackAuxiliaryBinaries can then be called on starting Gitaly to unpack
the binaries into a temporary directory where they can be invoked from.
In order to embed the binaries in Gitaly during the build, they have to
be built prior to building Gitaly. Makefile is updated to set the packed
binaries as dependencies of Gitaly's build. Gitaly depends on the fully
built binaries so the GNU build-id remains intact in the packed binaries.
The packed binaries are set also as dependencies for the prepare-test target
so the binaries exist for the tests that exercise the packing related code.
In similar fashion, the lint target requires the packed binaries to be
present as otherwise the linter will fail as it will find build failures
in Gitaly due to the embedding failing due to missing binaries. The packed
executables are added as a dependency to the lint target. Same goes for the
notice target which needs to have the packed binaries in place so the build
succeeds. Packed binaries are also added as dependency for the notice target.
-rw-r--r-- | Makefile | 15 | ||||
-rw-r--r-- | packed_binaries.go | 67 | ||||
-rw-r--r-- | packed_binaries_test.go | 44 |
3 files changed, 120 insertions, 6 deletions
@@ -241,10 +241,12 @@ TEST_REPO_MIRROR := ${TEST_REPO_DIR}/gitlab-test-mirror.git TEST_REPO_GIT := ${TEST_REPO_DIR}/gitlab-git-test.git BENCHMARK_REPO := ${TEST_REPO_DIR}/benchmark.git -# All executables provided by Gitaly -GITALY_EXECUTABLES = $(addprefix ${BUILD_DIR}/bin/,$(notdir $(shell find ${SOURCE_DIR}/cmd -mindepth 1 -maxdepth 1 -type d -print)) gitaly-git2go-v14) +# All executables provided by Gitaly. +GITALY_EXECUTABLES = $(addprefix ${BUILD_DIR}/bin/,$(notdir $(shell find ${SOURCE_DIR}/cmd -mindepth 1 -maxdepth 1 -type d -print)) gitaly-git2go-v14) +# All executables packed inside the Gitaly binary. +GITALY_PACKED_EXECUTABLES = $(filter %gitaly-hooks %gitaly-git2go-v15 %gitaly-ssh %gitaly-lfs-smudge, ${GITALY_EXECUTABLES}) # Find all Go source files. -find_go_sources = $(shell find ${SOURCE_DIR} -type d \( -name ruby -o -name vendor -o -name testdata -o -name '_*' -o -path '*/proto/go/gitalypb' \) -prune -o -type f -name '*.go' -not -name '*.pb.go' -print | sort -u) +find_go_sources = $(shell find ${SOURCE_DIR} -type d \( -name ruby -o -name vendor -o -name testdata -o -name '_*' -o -path '*/proto/go/gitalypb' \) -prune -o -type f -name '*.go' -not -name '*.pb.go' -print | sort -u) # run_go_tests will execute Go tests with all required parameters. Its # behaviour can be modified via the following variables: @@ -350,7 +352,7 @@ export GITALY_TESTING_GIT_BINARY ?= ${GIT_PREFIX}/bin/git endif .PHONY: prepare-tests -prepare-tests: libgit2 prepare-test-repos ${SOURCE_DIR}/.ruby-bundle ${GOTESTSUM} +prepare-tests: libgit2 prepare-test-repos ${SOURCE_DIR}/.ruby-bundle ${GOTESTSUM} ${GITALY_PACKED_EXECUTABLES} ${Q}mkdir -p "$(dir ${TEST_REPORT})" .PHONY: prepare-debug @@ -422,7 +424,7 @@ check-mod-tidy: .PHONY: lint ## Run Go linter. -lint: ${GOLANGCI_LINT} libgit2 +lint: ${GOLANGCI_LINT} libgit2 ${GITALY_PACKED_EXECUTABLES} ${Q}${GOLANGCI_LINT} run --build-tags "${SERVER_BUILD_TAGS},${GIT2GO_BUILD_TAGS}" --out-format tab --config ${GOLANGCI_LINT_CONFIG} ${GOLANGCI_LINT_OPTIONS} .PHONY: format @@ -529,7 +531,7 @@ ${SOURCE_DIR}/.ruby-bundle: ${GITALY_RUBY_DIR}/Gemfile.lock ${GITALY_RUBY_DIR}/G ${SOURCE_DIR}/NOTICE: ${BUILD_DIR}/NOTICE ${Q}mv $< $@ -${BUILD_DIR}/NOTICE: ${GO_LICENSES} clean-ruby-vendor-go +${BUILD_DIR}/NOTICE: ${GO_LICENSES} clean-ruby-vendor-go ${GITALY_PACKED_EXECUTABLES} ${Q}rm -rf ${BUILD_DIR}/licenses ${Q}GOOS=linux GOFLAGS="-tags=${SERVER_BUILD_TAGS},${GIT2GO_BUILD_TAGS}" ${GO_LICENSES} save ${SOURCE_DIR}/... --save_path=${BUILD_DIR}/licenses ${Q}go run ${SOURCE_DIR}/tools/noticegen/noticegen.go -source ${BUILD_DIR}/licenses -template ${SOURCE_DIR}/tools/noticegen/notice.template > ${BUILD_DIR}/NOTICE @@ -584,6 +586,7 @@ ${BUILD_DIR}/bin/%: ${BUILD_DIR}/intermediate/% | ${BUILD_DIR}/bin fi ${BUILD_DIR}/intermediate/gitaly: GO_BUILD_TAGS = ${SERVER_BUILD_TAGS} +${BUILD_DIR}/intermediate/gitaly: ${GITALY_PACKED_EXECUTABLES} ${BUILD_DIR}/intermediate/praefect: GO_BUILD_TAGS = ${SERVER_BUILD_TAGS} ${BUILD_DIR}/intermediate/gitaly-git2go-v15: GO_BUILD_TAGS = ${GIT2GO_BUILD_TAGS} ${BUILD_DIR}/intermediate/gitaly-git2go-v15: libgit2 diff --git a/packed_binaries.go b/packed_binaries.go new file mode 100644 index 000000000..1eafb3639 --- /dev/null +++ b/packed_binaries.go @@ -0,0 +1,67 @@ +package gitaly + +import ( + "embed" + "fmt" + "io" + "os" + "path/filepath" +) + +// buildDir is the directory path where our build target places the built binaries. +const buildDir = "_build/bin" + +//go:embed _build/bin/gitaly-hooks _build/bin/gitaly-ssh _build/bin/gitaly-git2go-v15 _build/bin/gitaly-lfs-smudge +// +// packedBinariesFS contains embedded binaries. If you modify the above embeddings, you must also update +// GITALY_PACKED_EXECUTABLES in Makefile. +var packedBinariesFS embed.FS + +// UnpackAuxiliaryBinaries unpacks the packed auxiliary binaries of Gitaly into destination directory. +// +// Gitaly invoking auxiliary binaries across different releases is a source of backwards compatibility issues. +// The calling protocol may change and cause issues if we don't carefully maintain the compatibility. Major version +// changing the module path also causes problems for gob encoding as it effectively changes the name of every type. +// To avoid having to maintain backwards compatibility between the different Gitaly binaries, we want to pin a given +// gitaly binary to only ever call the auxiliary binaries of the same build. We achieve this by packing the auxiliary +// binaries in the main gitaly binary and unpacking them on start to a temporary directory we can call them from. This +// way updating the gitaly binaries on the disk is atomic and a running gitaly can't call auxiliary binaries from a +// different version. +func UnpackAuxiliaryBinaries(destinationDir string) error { + entries, err := packedBinariesFS.ReadDir(buildDir) + if err != nil { + return fmt.Errorf("list packed binaries: %w", err) + } + + for _, entry := range entries { + if err := func() error { + packedPath := filepath.Join(buildDir, entry.Name()) + packedFile, err := packedBinariesFS.Open(packedPath) + if err != nil { + return fmt.Errorf("open packed binary %q: %w", packedPath, err) + } + defer packedFile.Close() + + unpackedPath := filepath.Join(destinationDir, entry.Name()) + unpackedFile, err := os.OpenFile(unpackedPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o700) + if err != nil { + return err + } + defer unpackedFile.Close() + + if _, err := io.Copy(unpackedFile, packedFile); err != nil { + return fmt.Errorf("unpack %q: %w", unpackedPath, err) + } + + if err := unpackedFile.Close(); err != nil { + return fmt.Errorf("close %q: %w", unpackedPath, err) + } + + return nil + }(); err != nil { + return err + } + } + + return nil +} diff --git a/packed_binaries_test.go b/packed_binaries_test.go new file mode 100644 index 000000000..d904ab28b --- /dev/null +++ b/packed_binaries_test.go @@ -0,0 +1,44 @@ +package gitaly + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestUnpackAuxiliaryBinaries_success(t *testing.T) { + destinationDir := t.TempDir() + require.NoError(t, UnpackAuxiliaryBinaries(destinationDir)) + + entries, err := os.ReadDir(destinationDir) + require.NoError(t, err) + + require.Greater(t, len(entries), 1, "expected multiple packed binaries present") + + for _, entry := range entries { + fileInfo, err := entry.Info() + require.NoError(t, err) + require.Equal(t, fileInfo.Mode(), os.FileMode(0o700), "expected the owner to have rwx permissions on the unpacked binary") + + sourceBinary, err := os.ReadFile(filepath.Join(buildDir, fileInfo.Name())) + require.NoError(t, err) + + unpackedBinary, err := os.ReadFile(filepath.Join(destinationDir, fileInfo.Name())) + require.NoError(t, err) + + require.Equal(t, sourceBinary, unpackedBinary, "unpacked binary does not match the source binary") + } +} + +func TestUnpackAuxiliaryBinaries_alreadyExists(t *testing.T) { + destinationDir := t.TempDir() + + existingFile := filepath.Join(destinationDir, "gitaly-hooks") + require.NoError(t, os.WriteFile(existingFile, []byte("existing file"), os.ModePerm)) + + err := UnpackAuxiliaryBinaries(destinationDir) + require.EqualError(t, err, fmt.Sprintf(`open %s: file exists`, existingFile), "expected unpacking to fail if destination binary already existed") +} |