Fix the health test (#30)

We were previously using the `sumas` problem for health tests. The
problem is that this is not always created. Furthermore, this makes it
possible for an exploit to mess with a production problem! So now we are
using the problem called `:testproblem` (with a colon at the beginning),
which is clearly not a valid problem name. This means that the very
first healthcheck request will create this problem, giving the extra
advantage of also testing for writability.

cmd/omegaup-gitserver/auth.go

@@ -384,9 +384,10 @@ func (a *omegaupAuthorization) authorize(
 	requestContext := request.FromContext(ctx)
 	requestContext.Request.ProblemName = problem
 	requestContext.Request.Username = username
-	if username == "omegaup:health" && isLocalRequest {
-		// This is a legit health check, so we grant reading privileges.
+	if username == "omegaup:health" && requestContext.Request.ProblemName == ":testproblem" && isLocalRequest {
+		// This is a legit health check, so we grant privileges to the test problem.
 		requestContext.Request.CanView = true
+		requestContext.Request.CanEdit = true
 	} else if username == "omegaup:system" || *insecureSkipAuthorization {
 		// This is the frontend, and we trust it completely.
 		requestContext.Request.IsSystem = true

cmd/omegaup-gitserver/health_test.go

@@ -1,15 +1,13 @@
 package main
 
 import (
-	"context"
 	"errors"
 	"io"
 	"io/ioutil"
 	"net"
+	"net/http"
 	"net/http/httptest"
 	"os"
-	"path"
-	"strconv"
 	"testing"
 
 	"github.com/omegaup/githttp/v2"
@@ -33,38 +31,30 @@ func TestHealth(t *testing.T) {
 		defer os.RemoveAll(tmpDir)
 	}
 
-	ctx := context.Background()
-
 	log, err := log15.New("info", false)
 	if err != nil {
 		t.Fatalf("Failed to create log: %v", err)
 	}
-	ts := httptest.NewUnstartedServer(nil)
-	_, portString, err := net.SplitHostPort(ts.Listener.Addr().String())
-	if err != nil {
-		t.Fatalf("port: %v", err)
-	}
-	port, err := strconv.ParseInt(portString, 10, 32)
-	if err != nil {
-		t.Fatalf("parse port: %v", err)
+	config := &Config{
+		Gitserver: GitserverConfig{
+			AllowDirectPushToMaster: true,
+		},
 	}
-
-	config := DefaultConfig()
-	config.Gitserver.Port = uint16(port)
-	authorize, err := createAuthorizationCallback(&config, log)
-	if err != nil {
-		t.Fatalf("authorization callback: %v", err)
+	authCallback := omegaupAuthorization{
+		log:    log,
+		config: config,
 	}
-
+	ts := httptest.NewUnstartedServer(nil)
 	ts.Config.Handler = muxHandler(
 		nil,
-		config.Gitserver.Port,
+		uint16(ts.Listener.Addr().(*net.TCPAddr).Port),
 		tmpDir,
 		gitserver.NewGitProtocol(gitserver.GitProtocolOpts{
 			GitProtocolOpts: githttp.GitProtocolOpts{
-				AuthCallback: authorize,
+				AuthCallback: authCallback.authorize,
 				Log:          log,
 			},
+			AllowDirectPushToMaster:     config.Gitserver.AllowDirectPushToMaster,
 			HardOverallWallTimeLimit:    gitserver.OverallWallTimeHardLimit,
 			InteractiveSettingsCompiler: fakeInteractiveSettingsCompiler,
 		}),
@@ -73,27 +63,28 @@ func TestHealth(t *testing.T) {
 	ts.Start()
 	defer ts.Close()
 
-	problemAlias := "sumas"
-
-	repo, err := gitserver.InitRepository(ctx, path.Join(tmpDir, problemAlias))
+	res, err := ts.Client().Get(ts.URL + "/health/live")
 	if err != nil {
-		t.Fatalf("Failed to initialize git repository: %v", err)
+		t.Fatalf("Failed to create pre-pull request: %v", err)
+	}
+	res.Body.Close()
+	if res.StatusCode != http.StatusOK {
+		t.Fatalf("/health/live failed. status: %v, headers: %v", res.StatusCode, res.Header)
 	}
-	defer repo.Free()
-
-	for _, path := range []string{"/health/live", "/health/ready"} {
-		path := path
-		t.Run(path, func(t *testing.T) {
-			res, err := ts.Client().Get(ts.URL + path)
-			if err != nil {
-				t.Fatalf("get: %v", err)
-			}
-			defer res.Body.Close()
 
-			if res.StatusCode != 200 {
-				body, _ := io.ReadAll(res.Body)
-				t.Fatalf("health check failure: HTTP %d: %v", res.StatusCode, string(body))
-			}
-		})
+	// Run this twice because the first attempt will create the problem.
+	for i := 0; i < 2; i++ {
+		res, err = ts.Client().Get(ts.URL + "/health/ready")
+		if err != nil {
+			t.Fatalf("Failed to create pre-pull request: %v", err)
+		}
+		contents, err := io.ReadAll(res.Body)
+		if err != nil {
+			t.Fatalf("Failed to read response: %v", err)
+		}
+		res.Body.Close()
+		if res.StatusCode != http.StatusOK {
+			t.Fatalf("/health/ready failed. status: %v, headers: %v, body: %v", res.StatusCode, res.Header, string(contents))
+		}
 	}
 }

cmd/omegaup-gitserver/main.go

@@ -99,7 +99,10 @@ func muxHandler(
 		Log:      log,
 		Tracing:  tracing,
 	}))
-	_, wrappedHealthHandler := tracing.WrapHandle("/health", gitserver.HealthHandler(port))
+	_, wrappedHealthHandler := tracing.WrapHandle("/health", gitserver.HealthHandler(
+		rootPath,
+		port,
+	))
 	_, wrappedMetricsHandler := tracing.WrapHandle("/metrics", metricsHandler)
 	return &muxGitHandler{
 		log:            log,

health.go

@@ -1,17 +1,25 @@
 package gitserver
 
 import (
+	"bytes"
 	"context"
+	_ "embed" // Required by the `go:embed` comment below.
 	"fmt"
+	"io"
 	"net/http"
+	"os"
+	"path"
 	"time"
 )
 
+//go:embed testproblem.zip
+var testproblemZip []byte
+
 // HealthHandler is an implementation of k8s readiness and liveness checks. The
 // liveness check just returns HTTP 200 (because the server is reachable), but
 // the readiness check tries to interact with the service by requesting the
 // settings.json file of the sumas problem. Getting a 404 is fine.
-func HealthHandler(port uint16) http.Handler {
+func HealthHandler(rootPath string, port uint16) http.Handler {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/health/live", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
@@ -20,7 +28,39 @@ func HealthHandler(port uint16) http.Handler {
 	mux.HandleFunc("/health/ready", func(w http.ResponseWriter, r *http.Request) {
 		ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second)
 		defer cancel()
-		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://localhost:%d/sumas/+/master/settings.json", port), nil)
+
+		if _, err := os.Stat(path.Join(rootPath, ":testproblem")); os.IsNotExist(err) {
+			// Creating problem!
+			req, err := http.NewRequestWithContext(ctx, "POST", fmt.Sprintf("http://localhost:%d/:testproblem/git-upload-zip?create=true&message=Test&acceptsSubmissions=true&updatePublished=true&mergeStrategy=theirs", port), nil)
+			if err != nil {
+				w.WriteHeader(http.StatusServiceUnavailable)
+				w.Write([]byte(fmt.Sprintf("git-upload-zip request: %v", err)))
+				return
+			}
+
+			req.Header.Add("Authorization", "omegaup:health")
+			req.Header.Add("Content-Type", "application/zip")
+			req.Body = io.NopCloser(bytes.NewReader(testproblemZip))
+			res, err := http.DefaultClient.Do(req)
+			if err != nil {
+				w.WriteHeader(http.StatusServiceUnavailable)
+				w.Write([]byte(fmt.Sprintf("git-upload-zip request: %v", err)))
+				return
+			}
+			contents, readErr := io.ReadAll(res.Body)
+			if readErr != nil {
+				contents = append(contents, []byte(fmt.Sprintf(", error=%v", readErr))...)
+			}
+			res.Body.Close()
+
+			if res.StatusCode != 200 {
+				w.WriteHeader(http.StatusServiceUnavailable)
+				w.Write([]byte(fmt.Sprintf("git-upload-zip HTTP/%d. headers: %v, body: %v", res.StatusCode, res.Header, string(contents))))
+				return
+			}
+		}
+
+		req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("http://localhost:%d/:testproblem/+/private/settings.json", port), nil)
 		if err != nil {
 			w.WriteHeader(http.StatusServiceUnavailable)
 			w.Write([]byte(fmt.Sprintf("request: %v", err)))
@@ -34,9 +74,9 @@ func HealthHandler(port uint16) http.Handler {
 			w.Write([]byte(fmt.Sprintf("request: %v", err)))
 			return
 		}
-		defer res.Body.Close()
+		res.Body.Close()
 
-		if res.StatusCode != 200 && res.StatusCode != 404 {
+		if res.StatusCode != 200 {
 			w.WriteHeader(http.StatusServiceUnavailable)
 			w.Write([]byte(fmt.Sprintf("HTTP/%d", res.StatusCode)))
 			return