Merge branch 'pks-makefile-spring-cleanup' into 'master'

Makefile: Spring cleanup

See merge request gitlab-org/gitaly!4409

13 files changed, 166 insertions, 68 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4a33e6d17..92bbe4302 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -218,7 +218,9 @@ test:pgbouncer:
     - *test_before_script
     - while ! psql -h "${PGHOST_PGBOUNCER}" -p "${PGPORT_PGBOUNCER}" -U "${PGUSER}" -c 'SELECT 1' > /dev/null; do echo "awaiting PgBouncer service to be ready..." && sleep 1 ; done && echo "PgBouncer service is ready!"
   script:
-    - make test-postgres
+    # We need to explicitly build all prerequisites so that we can run tests unprivileged.
+    - make build prepare-tests
+    - setpriv --reuid=9999 --regid=9999 --clear-groups --no-new-privs env HOME=/dev/null make test-with-praefect SKIP_RSPEC_BUILD=YesPlease
 
 test:nightly:
   <<: *test_definition
diff --git a/.golangci.yml b/.golangci.yml
index b1359a125..4ef4e7da7 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -77,38 +77,6 @@ issues:
     - linters:
         - errcheck
       text: "Error return value of `[^`]+.(Close|Serve)` is not checked"
-    - linters:
-        - errcheck
-      path: "cmd/gitaly-wrapper/main.go"
-      text: "Error return value of `cmd.Wait` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/praefect/nodes/local_elector.go"
-      text: "Error return value of `s.checkNodes` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/praefect/nodes/manager.go"
-      text: "Error return value of `strategy.checkNodes` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/praefect/nodes/sql_elector.go"
-      text: "Error return value of `s.checkNodes` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/middleware/limithandler/limithandler.go"
-      text: "Error return value of `limiter.Limit` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/supervisor/supervisor.go"
-      text: "Error return value of `(cmd.Process.Kill)?` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/gitaly/rubyserver/worker.go"
-      text: "Error return value of `syscall.Kill` is not checked"
-    - linters:
-        - errcheck
-      path: "internal/command/command.go"
-      text: "Error return value of `syscall.Kill` is not checked"
   # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
   max-issues-per-linter: 0
   # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
diff --git a/Makefile b/Makefile
index 8d2f9c628..b6372e551 100644
--- a/Makefile
+++ b/Makefile
@@ -71,12 +71,12 @@ GO_LDFLAGS        := -X ${GITALY_PACKAGE}/internal/version.version=${GITALY_VERS
 GO_BUILD_TAGS     := tracer_static,tracer_static_jaeger,tracer_static_stackdriver,continuous_profiler_stackdriver,static,system_libgit2
 
 # Dependency versions
-GOLANGCI_LINT_VERSION     ?= 1.43.0
+GOLANGCI_LINT_VERSION     ?= 1.44.2
 GOCOVER_COBERTURA_VERSION ?= aaee18c8195c3f2d90e5ef80ca918d265463842a
-GOFUMPT_VERSION           ?= 0.2.0
+GOFUMPT_VERSION           ?= 0.3.1
 GOIMPORTS_VERSION         ?= 2538eef75904eff384a2551359968e40c207d9d2
-GOSUMTEST_VERSION         ?= v1.7.0
-GO_LICENSES_VERSION       ?= 73411c8fa237ccc6a75af79d0a5bc021c9487aad
+GOTESTSUM_VERSION         ?= v1.7.0
+GO_LICENSES_VERSION       ?= v1.0.0
 # https://pkg.go.dev/github.com/protocolbuffers/protobuf
 PROTOC_VERSION            ?= v3.17.3
 # https://pkg.go.dev/google.golang.org/protobuf
@@ -416,11 +416,6 @@ test-with-proxies: prepare-tests
 test-with-praefect: prepare-tests
 	${Q}GITALY_TEST_WITH_PRAEFECT=YesPlease $(call run_go_tests)
 
-.PHONY: test-postgres
-## Run Go tests with Postgres.
-test-postgres: TEST_PACKAGES := gitlab.com/gitlab-org/gitaly/v14/internal/praefect/...
-test-postgres: test-go
-
 .PHONY: race-go
 ## Run Go tests with race detection enabled.
 race-go: TEST_OPTIONS := ${TEST_OPTIONS} -race
@@ -459,7 +454,6 @@ lint: ${GOLANGCI_LINT} libgit2
 format: ${GOIMPORTS} ${GOFUMPT}
 	${Q}${GOIMPORTS} -w -l $(call find_go_sources)
 	${Q}${GOFUMPT} -w $(call find_go_sources)
-	${Q}${GOIMPORTS} -w -l $(call find_go_sources)
 
 .PHONY: notice-up-to-date
 notice-up-to-date: ${BUILD_DIR}/NOTICE
@@ -478,9 +472,6 @@ clean:
 clean-ruby-vendor-go:
 	mkdir -p ${SOURCE_DIR}/ruby/vendor && find ${SOURCE_DIR}/ruby/vendor -type f -name '*.go' -delete
 
-.PHONY: check-proto
-check-proto: proto no-proto-changes lint-proto
-
 .PHONY: rubocop
 ## Run Rubocop.
 rubocop: ${SOURCE_DIR}/.ruby-bundle
@@ -490,9 +481,8 @@ rubocop: ${SOURCE_DIR}/.ruby-bundle
 ## Generate coverage report via Go tests.
 cover: TEST_OPTIONS  := ${TEST_OPTIONS} -coverprofile "${COVERAGE_DIR}/all.merged"
 cover: prepare-tests libgit2 ${GOCOVER_COBERTURA}
-	${Q}echo "NOTE: make cover does not exit 1 on failure, don't use it to check for tests success!"
+	${Q}rm -rf "${COVERAGE_DIR}"
 	${Q}mkdir -p "${COVERAGE_DIR}"
-	${Q}rm -f "${COVERAGE_DIR}/all.merged" "${COVERAGE_DIR}/all.html"
 	${Q}$(call run_go_tests)
 	${Q}go tool cover -html  "${COVERAGE_DIR}/all.merged" -o "${COVERAGE_DIR}/all.html"
 	@ # sed is used below to convert file paths to repository root relative paths. See https://gitlab.com/gitlab-org/gitlab/-/issues/217664
@@ -519,6 +509,9 @@ proto: ${PROTOC} ${PROTOC_GEN_GO} ${PROTOC_GEN_GO_GRPC} ${SOURCE_DIR}/.ruby-bund
 		${SOURCE_DIR}/internal/middleware/limithandler/testdata/test.proto
 	${PROTOC} ${SHARED_PROTOC_OPTS} -I ${SOURCE_DIR}/proto -I ${PROTOC_INSTALL_DIR}/include --go_out=${SOURCE_DIR}/proto --go-grpc_out=${SOURCE_DIR}/proto ${SOURCE_DIR}/proto/go/internal/linter/testdata/*.proto
 
+.PHONY: check-proto
+check-proto: proto no-proto-changes lint-proto
+
 .PHONY: lint-proto
 lint-proto: ${PROTOC} ${PROTOC_GEN_GITALY}
 	${Q}${PROTOC} --plugin=${PROTOC_GEN_GITALY} -I ${SOURCE_DIR}/proto -I ${PROTOC_INSTALL_DIR}/include --gitaly_out=proto_dir=${SOURCE_DIR}/proto,gitalypb_dir=${SOURCE_DIR}/proto/go/gitalypb:${SOURCE_DIR} ${SOURCE_DIR}/proto/*.proto
@@ -529,19 +522,13 @@ no-changes:
 
 .PHONY: no-proto-changes
 no-proto-changes: | ${BUILD_DIR}
-	${Q}${GIT} diff -- '*.pb.go' 'ruby/proto/gitaly' >${BUILD_DIR}/proto.diff
-	${Q}if [ -s ${BUILD_DIR}/proto.diff ]; then echo "There is a difference in generated proto files. Please take a look at ${BUILD_DIR}/proto.diff file." && exit 1; fi
+	${Q}${GIT} diff --exit-code -- '*.pb.go' 'ruby/proto/gitaly'
 
 .PHONY: dump-database-schema
 ## Dump the clean database schema of Praefect into a file.
 dump-database-schema: build
 	${Q}"${SOURCE_DIR}"/_support/generate-praefect-schema >"${SOURCE_DIR}"/_support/praefect-schema.sql
 
-.PHONY: smoke-test
-smoke-test: TEST_PACKAGES := ${SOURCE_DIR}/internal/gitaly/rubyserver
-smoke-test: all rspec
-	$(call run_go_tests)
-
 .PHONY: upgrade-module
 upgrade-module:
 	${Q}go run ${SOURCE_DIR}/_support/module-updater/main.go -dir . -from=${FROM_MODULE} -to=${TO_MODULE}
@@ -567,12 +554,7 @@ ${SOURCE_DIR}/NOTICE: ${BUILD_DIR}/NOTICE
 
 ${BUILD_DIR}/NOTICE: ${GO_LICENSES} clean-ruby-vendor-go
 	${Q}rm -rf ${BUILD_DIR}/licenses
-	${Q}GOOS=linux GOFLAGS="-tags=${GO_BUILD_TAGS}" ${GO_LICENSES} save ./... --save_path=${BUILD_DIR}/licenses
-	@ # some projects may be copied from the Go module cache
-	@ # (GOPATH/pkg/mod) and retain strict permissions (444). These
-	@ # permissions are not desirable when removing and rebuilding:
-	${Q}find ${BUILD_DIR}/licenses -type d -exec chmod 0755 {} \;
-	${Q}find ${BUILD_DIR}/licenses -type f -exec chmod 0644 {} \;
+	${Q}GOOS=linux GOFLAGS="-tags=${GO_BUILD_TAGS}" ${GO_LICENSES} save ${SOURCE_DIR}/... --save_path=${BUILD_DIR}/licenses
 	${Q}go run ${SOURCE_DIR}/_support/noticegen/noticegen.go -source ${BUILD_DIR}/licenses -template ${SOURCE_DIR}/_support/noticegen/notice.template > ${BUILD_DIR}/NOTICE
 
 ${BUILD_DIR}:
@@ -734,7 +716,7 @@ ${GOIMPORTS}:         TOOL_VERSION = ${GOIMPORTS_VERSION}
 ${GOLANGCI_LINT}:     TOOL_PACKAGE = github.com/golangci/golangci-lint/cmd/golangci-lint
 ${GOLANGCI_LINT}:     TOOL_VERSION = v${GOLANGCI_LINT_VERSION}
 ${GOTESTSUM}:         TOOL_PACKAGE = gotest.tools/gotestsum
-${GOTESTSUM}:         TOOL_VERSION = ${GOSUMTEST_VERSION}
+${GOTESTSUM}:         TOOL_VERSION = ${GOTESTSUM_VERSION}
 ${GO_LICENSES}:       TOOL_PACKAGE = github.com/google/go-licenses
 ${GO_LICENSES}:       TOOL_VERSION = ${GO_LICENSES_VERSION}
 ${PROTOC_GEN_GO}:     TOOL_PACKAGE = google.golang.org/protobuf/cmd/protoc-gen-go
diff --git a/NOTICE b/NOTICE
index 96c4113c4..14241458b 100644
--- a/NOTICE
+++ b/NOTICE
@@ -15364,6 +15364,129 @@ LICENSE - github.com/xanzy/ssh-agent
 
 
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+license_test.go - gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/service/repository
+package repository
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/rubyserver"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+)
+
+func testSuccessfulFindLicenseRequest(t *testing.T, cfg config.Cfg, client gitalypb.RepositoryServiceClient, rubySrv *rubyserver.Server) {
+	testhelper.NewFeatureSets(featureflag.GoFindLicense).Run(t, func(t *testing.T, ctx context.Context) {
+		for _, tc := range []struct {
+			desc                  string
+			nonExistentRepository bool
+			files                 map[string]string
+			expectedLicense       string
+			errorContains         string
+		}{
+			{
+				desc:                  "repository does not exist",
+				nonExistentRepository: true,
+				errorContains:         "rpc error: code = NotFound desc = GetRepoPath: not a git repository",
+			},
+			{
+				desc: "empty if no license file in repo",
+				files: map[string]string{
+					"README.md": "readme content",
+				},
+				expectedLicense: "",
+			},
+			{
+				desc: "high confidence mit result and less confident mit-0 result",
+				files: map[string]string{
+					"LICENSE": `MIT License
+
+Copyright (c) [year] [fullname]
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.`,
+				},
+				expectedLicense: "mit",
+			},
+			{
+				desc: "unknown license",
+				files: map[string]string{
+					"LICENSE.md": "this doesn't match any known license",
+				},
+				expectedLicense: "other",
+			},
+		} {
+			t.Run(tc.desc, func(t *testing.T) {
+				repo, repoPath := gittest.CreateRepository(ctx, t, cfg)
+
+				var treeEntries []gittest.TreeEntry
+				for file, content := range tc.files {
+					treeEntries = append(treeEntries, gittest.TreeEntry{
+						Mode:    "100644",
+						Path:    file,
+						Content: content,
+					})
+				}
+
+				gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"), gittest.WithTreeEntries(treeEntries...), gittest.WithParents())
+
+				if tc.nonExistentRepository {
+					require.NoError(t, os.RemoveAll(repoPath))
+				}
+
+				resp, err := client.FindLicense(ctx, &gitalypb.FindLicenseRequest{Repository: repo})
+				if tc.errorContains != "" {
+					require.Error(t, err)
+					require.Contains(t, err.Error(), tc.errorContains)
+					return
+				}
+
+				require.NoError(t, err)
+				testhelper.ProtoEqual(t, &gitalypb.FindLicenseResponse{
+					LicenseShortName: tc.expectedLicense,
+				}, resp)
+			})
+		}
+	})
+}
+
+func testFindLicenseRequestEmptyRepo(t *testing.T, cfg config.Cfg, client gitalypb.RepositoryServiceClient, rubySrv *rubyserver.Server) {
+	testhelper.NewFeatureSets(featureflag.GoFindLicense).Run(t, func(t *testing.T, ctx context.Context) {
+		repo, repoPath := gittest.InitRepo(t, cfg, cfg.Storages[0])
+		require.NoError(t, os.RemoveAll(repoPath))
+
+		_, err := client.CreateRepository(ctx, &gitalypb.CreateRepositoryRequest{Repository: repo})
+		require.NoError(t, err)
+
+		resp, err := client.FindLicense(ctx, &gitalypb.FindLicenseRequest{Repository: repo})
+		require.NoError(t, err)
+
+		require.Empty(t, resp.GetLicenseShortName())
+	})
+}
+
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 LICENSE - gitlab.com/gitlab-org/gitaly/v14/internal/middleware/panichandler
 Copyright (c) 2016 Masahiro Sano
 
diff --git a/cmd/gitaly-wrapper/main.go b/cmd/gitaly-wrapper/main.go
index 247b71f2d..2ce2037c4 100644
--- a/cmd/gitaly-wrapper/main.go
+++ b/cmd/gitaly-wrapper/main.go
@@ -58,7 +58,7 @@ func main() {
 	} else {
 		logger.Info("spawning a process")
 
-		proc, err := spawnProcess(binary, arguments)
+		proc, err := spawnProcess(logger, binary, arguments)
 		if err != nil {
 			logger.WithError(err).Fatal("spawn gitaly")
 		}
@@ -103,7 +103,7 @@ func findProcess(pidFilePath string) (*os.Process, error) {
 	return nil, nil
 }
 
-func spawnProcess(bin string, args []string) (*os.Process, error) {
+func spawnProcess(logger *logrus.Entry, bin string, args []string) (*os.Process, error) {
 	cmd := exec.Command(bin, args...)
 	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=true", bootstrap.EnvUpgradesEnabled))
 
@@ -116,7 +116,11 @@ func spawnProcess(bin string, args []string) (*os.Process, error) {
 	}
 
 	// This cmd.Wait() is crucial. Without it we cannot detect if the command we just spawned has crashed.
-	go cmd.Wait()
+	go func() {
+		if err := cmd.Wait(); err != nil {
+			logger.WithError(err).Error("waiting for supervised command")
+		}
+	}()
 
 	return cmd.Process, nil
 }
diff --git a/internal/command/command.go b/internal/command/command.go
index ccf13d291..ba964209a 100644
--- a/internal/command/command.go
+++ b/internal/command/command.go
@@ -254,6 +254,7 @@ func New(ctx context.Context, cmd *exec.Cmd, stdin io.Reader, stdout, stderr io.
 		<-ctx.Done()
 
 		if process := cmd.Process; process != nil && process.Pid > 0 {
+			//nolint:errcheck // TODO: do we want to report errors?
 			// Send SIGTERM to the process group of cmd
 			syscall.Kill(-process.Pid, syscall.SIGTERM)
 		}
diff --git a/internal/gitaly/rubyserver/worker.go b/internal/gitaly/rubyserver/worker.go
index ba2b76197..f3dd20aa0 100644
--- a/internal/gitaly/rubyserver/worker.go
+++ b/internal/gitaly/rubyserver/worker.go
@@ -217,10 +217,12 @@ func (w *worker) waitTerminate(pid int) {
 	terminationCounter.WithLabelValues(w.Name).Inc()
 
 	w.logPid(pid).Info("sending SIGTERM")
+	//nolint:errcheck // TODO: do we want to report errors?
 	syscall.Kill(pid, syscall.SIGTERM)
 
 	time.Sleep(w.gracefulRestartTimeout)
 
 	w.logPid(pid).Info("sending SIGKILL")
+	//nolint:errcheck // TODO: do we want to report errors?
 	syscall.Kill(pid, syscall.SIGKILL)
 }
diff --git a/internal/gitaly/service/smarthttp/server.go b/internal/gitaly/service/smarthttp/server.go
index 62134032a..e1a3c60d0 100644
--- a/internal/gitaly/service/smarthttp/server.go
+++ b/internal/gitaly/service/smarthttp/server.go
@@ -18,7 +18,8 @@ type server struct {
 
 // NewServer creates a new instance of a grpc SmartHTTPServer
 func NewServer(locator storage.Locator, gitCmdFactory git.CommandFactory,
-	cache cache.Streamer, serverOpts ...ServerOpt) gitalypb.SmartHTTPServiceServer {
+	cache cache.Streamer, serverOpts ...ServerOpt,
+) gitalypb.SmartHTTPServiceServer {
 	s := &server{
 		locator:       locator,
 		gitCmdFactory: gitCmdFactory,
diff --git a/internal/praefect/nodes/local_elector.go b/internal/praefect/nodes/local_elector.go
index bea1ef68b..95c2c3f35 100644
--- a/internal/praefect/nodes/local_elector.go
+++ b/internal/praefect/nodes/local_elector.go
@@ -52,7 +52,11 @@ func (s *localElector) bootstrap(d time.Duration) {
 		<-timer.C
 
 		ctx := context.TODO()
-		s.checkNodes(ctx)
+
+		if err := s.checkNodes(ctx); err != nil {
+			s.log.WithError(err).Warn("error checking nodes")
+		}
+
 		timer.Reset(d)
 	}
 }
diff --git a/internal/praefect/nodes/manager.go b/internal/praefect/nodes/manager.go
index f7578cfbf..e57abeb0b 100644
--- a/internal/praefect/nodes/manager.go
+++ b/internal/praefect/nodes/manager.go
@@ -229,6 +229,9 @@ func (n *Mgr) Stop() {
 func (n *Mgr) checkShards() {
 	for _, strategy := range n.strategies {
 		ctx := context.Background()
+
+		//nolint:errcheck // We don't care for this error. The nodes manager is only
+		// used for the deprecated SQL elector anyway.
 		strategy.checkNodes(ctx)
 	}
 }
diff --git a/internal/praefect/nodes/sql_elector.go b/internal/praefect/nodes/sql_elector.go
index fb976a3ed..b3adf2d8c 100644
--- a/internal/praefect/nodes/sql_elector.go
+++ b/internal/praefect/nodes/sql_elector.go
@@ -144,6 +144,8 @@ func (s *sqlElector) stop() {
 
 func (s *sqlElector) bootstrap(d time.Duration) {
 	ctx := context.Background()
+	//nolint:errcheck // We don't care for this error. The nodes manager is only
+	// used for the deprecated SQL elector anyway.
 	s.checkNodes(ctx)
 }
 
@@ -159,6 +161,9 @@ func (s *sqlElector) monitor(d time.Duration) {
 			return
 		case <-ticker.C:
 		}
+
+		//nolint:errcheck // We don't care for this error. The nodes manager is only
+		// used for the deprecated SQL elector anyway.
 		s.checkNodes(ctx)
 	}
 }
diff --git a/internal/praefect/router_per_repository.go b/internal/praefect/router_per_repository.go
index bd3a37f66..2685829f8 100644
--- a/internal/praefect/router_per_repository.go
+++ b/internal/praefect/router_per_repository.go
@@ -64,7 +64,8 @@ func NewPerRepositoryRouter(
 	csg datastore.ConsistentStoragesGetter,
 	ag AssignmentGetter,
 	rs datastore.RepositoryStore,
-	defaultReplicationFactors map[string]int) *PerRepositoryRouter {
+	defaultReplicationFactors map[string]int,
+) *PerRepositoryRouter {
 	return &PerRepositoryRouter{
 		conns:                     conns,
 		pg:                        pg,
diff --git a/internal/supervisor/supervisor.go b/internal/supervisor/supervisor.go
index 9c90866ae..f695b071e 100644
--- a/internal/supervisor/supervisor.go
+++ b/internal/supervisor/supervisor.go
@@ -200,6 +200,8 @@ spawnLoop:
 				break waitLoop
 			case <-p.shutdown:
 				if cmd.Process != nil {
+					//nolint:errcheck // TODO: do we want to report
+					// errors?
 					cmd.Process.Kill()
 				}
 				<-waitCh