diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..c72ce63 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +* @reMarkable/software-cloud diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yaml similarity index 86% rename from .github/workflows/docker.yml rename to .github/workflows/docker.yaml index 6bba781..ca89d44 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yaml @@ -23,10 +23,10 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v7 - name: Log in to the Container registry - uses: docker/login-action@v2 + uses: docker/login-action@v4 with: registry: ${{ env.REGISTRY }} username: ${{ github.actor }} @@ -34,12 +34,12 @@ jobs: - name: Extract metadata for Docker id: meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v6 with: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} - name: Build and push Docker image - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v7 with: context: . file: build/Dockerfile diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml new file mode 100644 index 0000000..b64d1fb --- /dev/null +++ b/.github/workflows/release.yaml @@ -0,0 +1,35 @@ +name: Release Chart + +on: + push: + branches: + - main + paths: + - deploy/charts/** + - .github/workflows/release.yml + +jobs: + release: + runs-on: ubuntu-latest + + permissions: + contents: write + + steps: + - name: Checkout + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Configure Git + run: | + git config user.name "$GITHUB_ACTOR" + git config user.email "$GITHUB_ACTOR@users.noreply.github.com" + + - name: Run chart-releaser + uses: helm/chart-releaser-action@v1.7.0 + with: + charts_dir: deploy/charts + config: cr.yaml + env: + CR_TOKEN: "${{ secrets.GITHUB_TOKEN }}" diff --git a/.github/workflows/test-chart.yaml b/.github/workflows/test-chart.yaml new file mode 100644 index 0000000..8f4de0c --- /dev/null +++ b/.github/workflows/test-chart.yaml @@ -0,0 +1,49 @@ +name: Lint & Test chart + +on: + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Setup Helm + uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5.0.1 + with: + version: v4.2.4 + + - uses: actions/setup-python@v7 + with: + python-version: 3.14 + + - name: Setup chart-testing + uses: helm/chart-testing-action@6ec842c01de15ebb84c8627d2744a0c2f2755c9f # v2.8.0 + + - name: Run ct list-changed + id: list-changed + run: | + changed=$(ct list-changed --config ct.yaml --target-branch ${{ github.event.repository.default_branch }}) + if [[ -n "$changed" ]]; then + echo "changed=true" >> $GITHUB_OUTPUT + fi + + - name: Run chart-testing (lint) + if: steps.list-changed.outputs.changed == 'true' + run: ct lint --config ct.yaml + + - name: Create KIND Cluster + if: steps.list-changed.outputs.changed == 'true' + uses: helm/kind-action@ef37e7f390d99f746eb8b610417061a60e82a6cc # v1.14.0 + + - name: Run chart-testing (install) + if: steps.list-changed.outputs.changed == 'true' + run: | + # Pass in a random MODULES_PROXY_SECRET secret + ct install \ + --config ct.yaml \ + --helm-extra-set-args "--set extraEnvs[0].name=MODULES_PROXY_SECRET,extraEnvs[0].value=$(openssl rand -base64 32)" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yaml similarity index 69% rename from .github/workflows/test.yml rename to .github/workflows/test.yaml index 6effa22..f555488 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yaml @@ -8,12 +8,12 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v7 - name: Set up Go - uses: actions/setup-go@v4 + uses: actions/setup-go@v7 with: - go-version: "1.22" + go-version: "1.27" - name: Run tests run: go test -race -cover ./... diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..00bb3ac --- /dev/null +++ b/.gitignore @@ -0,0 +1,4 @@ +.env* +*.pem +.terraform/ +tmp/ diff --git a/README.md b/README.md index f77f61e..af5e88d 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,48 @@ # Orbit -A small proxy to turn a Github mono-repo into a Terraform module registry. +A small proxy to turn a GitHub mono-repo into a Terraform module registry. This +proxy expects releases in the same format as a golang monorepo +(`module/vX.Y.Z`), with each module having a directory on the root of the +repository. You might find [release +please](https://github.com/googleapis/release-please) useful for managing the +release process. + +## Supported ENV variables + +| Environment Variable | Type | Default | Required | Description | +| -------------------------- | -------- | ------- | -------- | -------------------------------------- | +| MODULES_PROXY_SECRET | []byte | | Yes | Secret key for proxy token encryption. | +| CACHE_ENABLED | bool | | No | Enable or disable caching. | +| CACHE_AUTH_DISABLED | bool | | No | Disable auth for cached requests. | +| CACHE_PATH | string | /tmp | No | Path to store cache files. | +| CACHE_EXPIRATION | duration | 10s | No | Cache expiration duration. | +| GITHUB_REPOSITORIES | map | | No | Allowed repositories (per org). | +| GITHUB_ORG_MAPPINGS | map | | No | Organization name mappings. | +| GITHUB_TOKEN | string | | No | GitHub API token. | +| MODULES_TOKEN_EXPIRATION | duration | 60s | No | Expiration time for proxy tokens. | +| SERVER_HOST | string | | No | Server listen host. | +| SERVER_PORT | int | 8080 | No | Server listen port. | +| SERVER_TIMEOUT_HANDLER | duration | 10s | No | HTTP handler timeout. | +| SERVER_TIMEOUT_IDLE | duration | | No | HTTP idle timeout. | +| SERVER_TIMEOUT_READ | duration | | No | HTTP read timeout. | +| SERVER_TIMEOUT_READ_HEADER | duration | 2s | No | HTTP read header timeout. | +| SERVER_TIMEOUT_SHUTDOWN | duration | 5s | No | Graceful shutdown timeout. | +| SERVER_TIMEOUT_WRITE | duration | | No | HTTP write timeout. | +| SERVER_TLS_ENABLED | bool | | No | Enable TLS for the server. | +| SERVER_TLS_CERT_FILE | string | | No | TLS certificate file path. | +| SERVER_TLS_KEY_FILE | string | | No | TLS key file path. | +| SERVER_METRICS_ENABLED | bool | false | No | Enable metrics endpoint. | +| SERVER_METRICS_PORT | int | 9090 | No | Metrics server port. | + +**Notes:** + +- Duration values (e.g., 10s, 60s) are Go duration strings (e.g., 1m, 30s). +- Prefixes like `CACHE_`, `GITHUB_`, `MODULES_`, and `SERVER_` are used for grouping related variables. +- Some variables (like maps) may require specific formatting (e.g., JSON or comma-separated values). +- MODULES_PROXY_SECRET must be a base64-encoded 16, 24 or 32-byte key for AES encryption. + +## Deployment + +Orbit can easily be deployed using Docker, or by just running the binary +provided in the release. If you want to deploy using Kubernetes, we're providing +a helm chart under `deploy/charts/orbit`. diff --git a/Taskfile.yml b/Taskfile.yml index b06ed05..ab09442 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -26,7 +26,7 @@ tasks: build:docker: desc: Build Docker container image cmds: - - docker build --file build/Dockerfile -t hedlund/orbit . + - docker build --file build/Dockerfile -t remarkable/orbit . generate:cert: desc: Generate SSL/TLS certificate for localhost diff --git a/build/Dockerfile b/build/Dockerfile index 53701cf..e378226 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -1,4 +1,4 @@ -FROM docker.io/library/golang:1.22-alpine AS build +FROM docker.io/library/golang:1.27-alpine AS build ARG entrypoint="./cmd/server" WORKDIR /app diff --git a/cmd/server/main.go b/cmd/server/main.go index 3089c7f..497738a 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -10,20 +10,21 @@ import ( "os" "time" - "github.com/hedlund/orbit/pkg/auth" - "github.com/hedlund/orbit/pkg/envconfig" - "github.com/hedlund/orbit/pkg/github" - "github.com/hedlund/orbit/pkg/mcache" - "github.com/hedlund/orbit/pkg/router" - "github.com/hedlund/orbit/pkg/server" - "github.com/hedlund/orbit/services/modules" + "github.com/reMarkable/orbit/pkg/auth" + "github.com/reMarkable/orbit/pkg/envconfig" + "github.com/reMarkable/orbit/pkg/github" + "github.com/reMarkable/orbit/pkg/mcache" + "github.com/reMarkable/orbit/pkg/router" + "github.com/reMarkable/orbit/pkg/server" + "github.com/reMarkable/orbit/services/modules" ) type config struct { Cache struct { - Enabled bool `envconfig:"ENABLED"` - Path string `envconfig:"PATH" default:"/tmp"` - Expiration time.Duration `envconfig:"EXPIRATION" default:"10s"` + Enabled bool `envconfig:"ENABLED"` + AuthDisabled bool `envconfig:"AUTH_DISABLED"` + Path string `envconfig:"PATH" default:"/tmp"` + Expiration time.Duration `envconfig:"EXPIRATION" default:"1m"` } `envconfig:"CACHE_"` Github github.Config `envconfig:"GITHUB_"` Modules modules.Config `envconfig:"MODULES_"` @@ -36,22 +37,41 @@ func main() { log := slog.New(slog.NewTextHandler(os.Stdout, nil)) + var cache modules.KeyValueStore + if cfg.Cache.Enabled { + c := mcache.New[string, []string](cfg.Cache.Expiration) + if cfg.Cache.Expiration > 0 { + // Start a cleanup loop to remove expired items from the cache periodically. + stop := mcache.StartCleanupLoop(c, cfg.Cache.Expiration) + defer stop() + } + cache = c + } + var repo modules.Repository repo = github.New(cfg.Github, &http.Client{ Timeout: 5 * time.Second, - }) + }, cache) if cfg.Cache.Enabled { - log.Info("enabling cache", "path", cfg.Cache.Path, "expiration", cfg.Cache.Expiration) + log.Info("enabling cache", "path", cfg.Cache.Path, "expiration", cfg.Cache.Expiration, "authDisabled", cfg.Cache.AuthDisabled) + if cfg.Cache.AuthDisabled { + log.Warn("auth is disabled for cached request. This is dangerous when used with private repos, or in environments with no other access control mechanisms in place") + } repo = modules.NewCache( repo, - mcache.New[string, []string](cfg.Cache.Expiration), + cache, modules.StoreInPath(cfg.Cache.Path), log, + cfg.Cache.AuthDisabled, ) } - h, err := modules.NewHTTP(cfg.Modules, log, repo) + mh, err := modules.NewMetricsHandler(log) + if err != nil { + panic(err) + } + h, err := modules.NewHTTP(cfg.Modules, log, repo, mh) if err != nil { panic(err) } @@ -64,6 +84,8 @@ func main() { r.Get("/v1/modules/:namespace/:name/:system/:version/proxy", h.ProxyDownload) r.Get("/.well-known/terraform.json", discovery) + http.DefaultServeMux.HandleFunc("GET /metrics", mh.Metrics) + if err := server.Start(cfg.Server, log, r); err != nil { panic(err) } @@ -71,5 +93,7 @@ func main() { func discovery(w http.ResponseWriter, r *http.Request) { w.Header().Add("Content-Type", "application/json") - w.Write([]byte(`{"modules.v1":"/v1/modules"}`)) + if _, err := w.Write([]byte(`{"modules.v1":"/v1/modules"}`)); err != nil { + slog.Error("failed to write response", "error", err) + } } diff --git a/cr.yaml b/cr.yaml new file mode 100644 index 0000000..2b5d203 --- /dev/null +++ b/cr.yaml @@ -0,0 +1,3 @@ +generate-release-notes: true +release-name-template: "chart-{{ .Name }}-{{ .Version }}" +mark_as_latest: false diff --git a/ct.yaml b/ct.yaml new file mode 100644 index 0000000..b96fc38 --- /dev/null +++ b/ct.yaml @@ -0,0 +1,3 @@ +chart-dirs: + - deploy/charts +validate-maintainers: false diff --git a/deploy/charts/orbit/Chart.yaml b/deploy/charts/orbit/Chart.yaml index f599fee..ff68259 100644 --- a/deploy/charts/orbit/Chart.yaml +++ b/deploy/charts/orbit/Chart.yaml @@ -1,25 +1,9 @@ apiVersion: v2 name: orbit description: A small proxy to turn a Github mono-repo into a Terraform module registry. - -# A chart can be either an 'application' or a 'library' chart. -# -# Application charts are a collection of templates that can be packaged into versioned archives -# to be deployed. -# -# Library charts provide useful utilities or functions for the chart developer. They're included as -# a dependency of application charts to inject those utilities and functions into the rendering -# pipeline. Library charts do not define any templates and therefore cannot be deployed. type: application - -# This is the chart version. This version number should be incremented each time you make changes -# to the chart and its templates, including the app version. -# Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.0.1 - -# This is the version number of the application being deployed. This version number should be -# incremented each time you make changes to the application. Versions are not expected to -# follow Semantic Versioning. They should reflect the version the application is using. -# It is recommended to use it with quotes. -# FIXME: Update version number when first version is released -appVersion: "0.0.0" +version: 0.2.2 +# renovate: image=ghcr.io/reMarkable/orbit +appVersion: "v0.2.1" +maintainers: + - name: marcusramberg diff --git a/deploy/charts/orbit/ci/metrics-values.yaml b/deploy/charts/orbit/ci/metrics-values.yaml new file mode 100644 index 0000000..db1ecae --- /dev/null +++ b/deploy/charts/orbit/ci/metrics-values.yaml @@ -0,0 +1,2 @@ +metrics: + enabled: true diff --git a/deploy/charts/orbit/ci/token-values.yaml b/deploy/charts/orbit/ci/token-values.yaml new file mode 100644 index 0000000..f976ab2 --- /dev/null +++ b/deploy/charts/orbit/ci/token-values.yaml @@ -0,0 +1,2 @@ +github: + token: c3VwZXJzZWNyZXQxMjM0IQ== diff --git a/deploy/charts/orbit/templates/deployment.yaml b/deploy/charts/orbit/templates/deployment.yaml index 462154d..055e895 100644 --- a/deploy/charts/orbit/templates/deployment.yaml +++ b/deploy/charts/orbit/templates/deployment.yaml @@ -54,25 +54,35 @@ spec: value: "{{ .Values.github.repositories }}" {{- if .Values.cache.enabled }} - name: CACHE_ENABLED - value: true + value: "true" - name: CACHE_PATH - value: {{ .Values.cache.path }} - - name: CACHE_EXPIRY - value: {{ .Values.cache.expiry }} - {{- end }} + value: "{{ .Values.cache.path }}" + - name: CACHE_EXPIRATION + value: "{{ .Values.cache.expiry }}" + {{- end }} + {{- if .Values.metrics.enabled }} + - name: METRICS_ENABLED + value: "true" + - name: METRICS_PORT + value: "{{ .Values.metrics.service.port }}" + {{- end }} {{- with .Values.extraEnvs }} {{- toYaml . | nindent 12 }} {{- end }} {{- if .Values.github.token}} envFrom: - secretRef: - name: {{ include "orbit.fullname "}} + name: {{ include "orbit.fullname" .}} {{- else if .Values.github.token_secret }} envFrom: - secretRef: name: {{ .Values.github.token_secret }} - {{- if .Values.extraSecretsMounts.enabled }} + {{- if or .Values.extraSecretsMounts.enabled .Values.cache.enabled }} volumeMounts: + {{- if .Values.cache.enabled }} + - name: cache + mountPath: {{ .Values.cache.path }} + {{- end }} {{- range .Values.extraSecretsMounts.secrets }} - name: {{ .name }} mountPath: {{ .mountPath }} @@ -80,8 +90,12 @@ spec: {{- end }} {{- end }} {{- end }} - {{- if .Values.extraSecretsMounts.enabled }} + {{- if or .Values.extraSecretsMounts.enabled .Values.cache.enabled }} volumes: + {{- if .Values.cache.enabled }} + - name: cache + emptyDir: {} + {{- end }} {{- range .Values.extraSecretsMounts.secrets }} {{- if .secretName }} - name: {{ .name }} diff --git a/deploy/charts/orbit/templates/secret.yaml b/deploy/charts/orbit/templates/secret.yaml index db51421..5728f65 100644 --- a/deploy/charts/orbit/templates/secret.yaml +++ b/deploy/charts/orbit/templates/secret.yaml @@ -7,5 +7,5 @@ metadata: {{- include "orbit.labels" . | nindent 4 }} type: Opaque data: - GITHUB_TOKEN: {{ .Values.github.token | quote }}, + GITHUB_TOKEN: {{ .Values.github.token | quote }} {{- end }} diff --git a/deploy/charts/orbit/templates/service.yaml b/deploy/charts/orbit/templates/service.yaml index 798bf9b..20f0f8e 100644 --- a/deploy/charts/orbit/templates/service.yaml +++ b/deploy/charts/orbit/templates/service.yaml @@ -13,3 +13,25 @@ spec: name: http selector: {{- include "orbit.selectorLabels" . | nindent 4 }} +{{ if .Values.metrics.enabled }} +--- +apiVersion: v1 +kind: Service +metadata: + name: {{ include "orbit.fullname" . }}-metrics + labels: + {{- include "orbit.labels" . | nindent 4 }} + {{- with .Values.metrics.service.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + type: {{ .Values.metrics.service.type }} + ports: + - port: {{ .Values.metrics.service.port }} + targetPort: {{ .Values.metrics.service.port }} + protocol: TCP + name: metrics + selector: + {{- include "orbit.selectorLabels" . | nindent 4 }} +{{ end }} diff --git a/deploy/charts/orbit/templates/servicemonitor.yaml b/deploy/charts/orbit/templates/servicemonitor.yaml new file mode 100644 index 0000000..6e72c30 --- /dev/null +++ b/deploy/charts/orbit/templates/servicemonitor.yaml @@ -0,0 +1,17 @@ +{{- if .Values.metrics.serviceMonitor.enabled }} +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: {{ .Release.Name }}-servicemonitor + namespace: {{ .Values.metrics.serviceMonitor.namespace | default .Release.Namespace }} + labels: + {{- toYaml .Values.metrics.serviceMonitor.additionalLabels | nindent 4 }} +spec: + selector: + matchLabels: + app.kubernetes.io/instance: {{ .Release.Name }} + endpoints: + - port: metrics + interval: {{ .Values.metrics.serviceMonitor.interval }} + scrapeTimeout: {{ .Values.metrics.serviceMonitor.scrapeTimeout }} +{{- end }} diff --git a/deploy/charts/orbit/templates/tests/test-connection.yaml b/deploy/charts/orbit/templates/tests/test-connection.yaml index 7cbc2ce..1b2c98b 100644 --- a/deploy/charts/orbit/templates/tests/test-connection.yaml +++ b/deploy/charts/orbit/templates/tests/test-connection.yaml @@ -8,8 +8,7 @@ metadata: "helm.sh/hook": test spec: containers: - - name: wget - image: busybox - command: ['wget'] - args: ['{{ include "orbit.fullname" . }}:{{ .Values.service.port }}'] + - name: curl + image: curlimages/curl + args: [--verbose, --fail, --silent, {{ include "orbit.fullname" . }}:{{ .Values.service.port }}/.well-known/terraform.json] restartPolicy: Never diff --git a/deploy/charts/orbit/values.yaml b/deploy/charts/orbit/values.yaml index bd91ee0..be18ee4 100644 --- a/deploy/charts/orbit/values.yaml +++ b/deploy/charts/orbit/values.yaml @@ -18,11 +18,10 @@ github: replicaCount: 1 image: - repository: ghcr.io/hedlund/orbit + repository: ghcr.io/remarkable/orbit pullPolicy: IfNotPresent # Overrides the image tag whose default is the chart appVersion. - # FIXME: change back to empty on first release - tag: main + tag: "" imagePullSecrets: [] nameOverride: "" @@ -67,6 +66,20 @@ ingress: # hosts: # - chart-example.local +metrics: + enabled: false + serviceMonitor: + enabled: false + additionalLabels: {} + namespace: "" + interval: 30s + scrapeTimeout: 10s + metricRelabelings: [] + relabelings: [] + service: + annotations: {} + type: ClusterIP + port: 9090 resources: {} autoscaling: diff --git a/go.mod b/go.mod index 651ff30..f49ae30 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ -module github.com/hedlund/orbit +module github.com/reMarkable/orbit -go 1.22 +go 1.24 diff --git a/pkg/auth/middleware_test.go b/pkg/auth/middleware_test.go new file mode 100644 index 0000000..69df04c --- /dev/null +++ b/pkg/auth/middleware_test.go @@ -0,0 +1,50 @@ +package auth + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestTokenMiddleware(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := GetToken(r.Context(), "") + if token != "test-token" { + t.Errorf("expected token 'test-token', got '%s'", token) + } + w.WriteHeader(http.StatusOK) + }) + + middleware := TokenMiddleware(handler) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("Authorization", "Bearer test-token") + rec := httptest.NewRecorder() + + middleware.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("expected status code 200, got %d", rec.Code) + } +} + +func TestTokenMiddleware_NoToken(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + token := GetToken(r.Context(), "default-token") + if token != "default-token" { + t.Errorf("expected token 'default-token', got '%s'", token) + } + w.WriteHeader(http.StatusOK) + }) + + middleware := TokenMiddleware(handler) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + + middleware.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("expected status code 200, got %d", rec.Code) + } +} diff --git a/pkg/envconfig/envconfig.go b/pkg/envconfig/envconfig.go index 5cd01ae..72cb716 100644 --- a/pkg/envconfig/envconfig.go +++ b/pkg/envconfig/envconfig.go @@ -50,7 +50,7 @@ func Process(spec any, prefix ...string) error { func processStruct(prefix string, spec reflect.Value) error { types := spec.Type() - for n := 0; n < spec.NumField(); n++ { + for n := range spec.NumField() { field := spec.Field(n) if !field.CanSet() { continue diff --git a/pkg/envconfig/envconfig_test.go b/pkg/envconfig/envconfig_test.go new file mode 100644 index 0000000..bcdf824 --- /dev/null +++ b/pkg/envconfig/envconfig_test.go @@ -0,0 +1,177 @@ +package envconfig + +import ( + "os" + "testing" +) + +type Config struct { + Port int `envconfig:"PORT" default:"8080"` + Host string `envconfig:"HOST" required:"true"` + LogLevel string `envconfig:"LOG_LEVEL" default:"info"` +} + +func TestProcess_WithDefaults(t *testing.T) { + os.Clearenv() // Ensure no environment variables are set + + if err := os.Setenv("HOST", "localhost"); err != nil { + panic(err) + } + var cfg Config + err := Process(&cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.Port != 8080 { + t.Errorf("expected Port to be 8080, got %d", cfg.Port) + } + if cfg.LogLevel != "info" { + t.Errorf("expected LogLevel to be 'info', got %s", cfg.LogLevel) + } +} + +func TestProcess_WithEnvironmentVariables(t *testing.T) { + if err := os.Setenv("PORT", "9090"); err != nil { + panic(err) + } + if err := os.Setenv("HOST", "localhost"); err != nil { + panic(err) + } + defer os.Clearenv() + + var cfg Config + err := Process(&cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.Port != 9090 { + t.Errorf("expected Port to be 9090, got %d", cfg.Port) + } + if cfg.Host != "localhost" { + t.Errorf("expected Host to be 'localhost', got %s", cfg.Host) + } +} + +func TestProcess_MissingRequiredField(t *testing.T) { + os.Clearenv() // Ensure no environment variables are set + + var cfg Config + err := Process(&cfg) + if err == nil { + t.Fatal("expected an error for missing required field, got nil") + } + + if err.Error() != "HOST: missing required value" { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestProcess_InvalidSpec(t *testing.T) { + // Test with a non-pointer value + var cfg Config + err := Process(cfg) + if err == nil || err.Error() != "not a pointer: invalid spec" { + t.Errorf("expected 'not a pointer: invalid spec' error, got %v", err) + } + + // Test with a non-struct pointer + var invalid int + err = Process(&invalid) + if err == nil || err.Error() != "not a struct: invalid spec" { + t.Errorf("expected 'not a struct: invalid spec' error, got %v", err) + } +} + +func TestProcess_UnsupportedFieldType(t *testing.T) { + type UnsupportedConfig struct { + UnsupportedField complex64 `envconfig:"UNSUPPORTED"` + } + + os.Clearenv() + if err := os.Setenv("UNSUPPORTED", "1+2i"); err != nil { + panic(err) + } + + var cfg UnsupportedConfig + err := Process(&cfg) + if err == nil || err.Error() != "UNSUPPORTED: unknown field type" { + t.Errorf("expected 'unknown field type' error, got %v", err) + } +} + +func TestProcess_DefaultsAndRequired(t *testing.T) { + type NestedConfig struct { + InnerField string `envconfig:"INNER_FIELD" default:"default-value"` + } + + type ConfigWithDefaults struct { + Nested NestedConfig `envconfig:"NESTED"` + } + + os.Clearenv() + var cfg ConfigWithDefaults + err := Process(&cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cfg.Nested.InnerField != "default-value" { + t.Errorf("expected InnerField to be 'default-value', got %q", cfg.Nested.InnerField) + } +} + +func TestProcess_SliceField(t *testing.T) { + type SliceConfig struct { + Values []string `envconfig:"VALUES"` + } + + os.Clearenv() + if err := os.Setenv("VALUES", "one,two,three"); err != nil { + panic(err) + } + + var cfg SliceConfig + err := Process(&cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := []string{"one", "two", "three"} + if len(cfg.Values) != len(expected) { + t.Fatalf("expected %d values, got %d", len(expected), len(cfg.Values)) + } + for i, v := range cfg.Values { + if v != expected[i] { + t.Errorf("expected value %q, got %q", expected[i], v) + } + } +} + +func TestProcess_MapField(t *testing.T) { + type MapConfig struct { + Values map[string]string `envconfig:"VALUES"` + } + + os.Clearenv() + if err := os.Setenv("VALUES", "key1:value1;key2:value2"); err != nil { + panic(err) + } + + var cfg MapConfig + err := Process(&cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := map[string]string{"key1": "value1", "key2": "value2"} + if len(cfg.Values) != len(expected) { + t.Fatalf("expected %d map entries, got %d", len(expected), len(cfg.Values)) + } + for k, v := range expected { + if cfg.Values[k] != v { + t.Errorf("expected key %q to have value %q, got %q", k, v, cfg.Values[k]) + } + } +} diff --git a/pkg/envconfig/setters_test.go b/pkg/envconfig/setters_test.go new file mode 100644 index 0000000..1f9485b --- /dev/null +++ b/pkg/envconfig/setters_test.go @@ -0,0 +1,87 @@ +package envconfig + +import ( + "encoding/base64" + "reflect" + "testing" + "time" +) + +func TestSetBool(t *testing.T) { + var v bool + rv := reflect.ValueOf(&v).Elem() + + err := setBool(rv, "true") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v != true { + t.Errorf("expected true, got %v", v) + } +} + +func TestSetDuration(t *testing.T) { + var v time.Duration + rv := reflect.ValueOf(&v).Elem() + + err := setDuration(rv, "5s") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v != 5*time.Second { + t.Errorf("expected 5s, got %v", v) + } +} + +func TestSetInt(t *testing.T) { + var v int + rv := reflect.ValueOf(&v).Elem() + + err := setInt(rv, "42") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v != 42 { + t.Errorf("expected 42, got %v", v) + } +} + +func TestSetUint(t *testing.T) { + var v uint + rv := reflect.ValueOf(&v).Elem() + + err := setUint(rv, "42") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v != 42 { + t.Errorf("expected 42, got %v", v) + } +} + +func TestSetFloat(t *testing.T) { + var v float64 + rv := reflect.ValueOf(&v).Elem() + + err := setFloat(rv, "3.14") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v != 3.14 { + t.Errorf("expected 3.14, got %v", v) + } +} + +func TestSetBytes(t *testing.T) { + var v []byte + rv := reflect.ValueOf(&v).Elem() + + data := base64.StdEncoding.EncodeToString([]byte("hello")) + err := setBytes(rv, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(v) != "hello" { + t.Errorf("expected 'hello', got %s", string(v)) + } +} diff --git a/pkg/expect/expect_test.go b/pkg/expect/expect_test.go new file mode 100644 index 0000000..13209bd --- /dev/null +++ b/pkg/expect/expect_test.go @@ -0,0 +1,42 @@ +package expect + +import ( + "testing" +) + +func TestValue(t *testing.T) { + v := Value(42) + if v.exp != 42 { + t.Errorf("expected exp to be 42, got %v", v.exp) + } +} + +func TestGot(t *testing.T) { + v := Value("hello") + v.Got("world") + if v.got != "world" { + t.Errorf("expected got to be 'world', got %v", v.got) + } +} + +func TestValidate_Success(t *testing.T) { + v := Value(100) + v.Got(100) + v.Validate(t, "test-value") +} + +func TestValidate_Failure(t *testing.T) { + v := Value(100) + v.Got(200) + + // Use a subtest to isolate the failure case + t.Run("failure case", func(t *testing.T) { + // Use a helper to capture test output + helper := &testing.T{} + v.Validate(helper, "test-value") + + if !helper.Failed() { + t.Errorf("expected Validate to fail, but it did not") + } + }) +} diff --git a/pkg/github/github.go b/pkg/github/github.go index c29d944..81fdd32 100644 --- a/pkg/github/github.go +++ b/pkg/github/github.go @@ -11,11 +11,14 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "net/http" "regexp" + "slices" "strings" + "time" - "github.com/hedlund/orbit/pkg/auth" + "github.com/reMarkable/orbit/pkg/auth" ) const ( @@ -34,16 +37,39 @@ type HTTPClient interface { Do(req *http.Request) (*http.Response, error) } -func New(cfg Config, c HTTPClient) *Service { +// TagCache caches the aggregated tag names of a repository, keyed by +// "owner/repo", to avoid re-paginating all tags on every request. +type TagCache interface { + Get(key string) ([]string, bool) + Set(key string, value []string, d ...time.Duration) +} + +// noopTagCache is a cache that never stores anything. It satisfies the same Get/Set +// interface as Cache and can be used to disable caching +type noopTagCache struct{} + +// Get always reports a miss. +func (noopTagCache) Get(string) ([]string, bool) { return nil, false } + +// Set discards the value. +func (noopTagCache) Set(string, []string, ...time.Duration) {} + +func New(cfg Config, c HTTPClient, cache TagCache) *Service { + if cache == nil { + cache = noopTagCache{} + } + return &Service{ cfg: cfg, client: c, + cache: cache, } } type Service struct { cfg Config client HTTPClient + cache TagCache } // https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repository-tags @@ -53,39 +79,67 @@ func (s *Service) ListVersions(ctx context.Context, system, repo, module string) return nil, err } + tags, err := s.listTags(ctx, owner, repo) + if err != nil { + return nil, err + } + + prefix := module + "/" + versions := []string{} + for _, name := range tags { + if strings.HasPrefix(name, prefix) { + versions = append(versions, strings.TrimPrefix(name, prefix)) + } + } + return versions, nil +} + +// listTags returns the names of all tags in the repository, fetching every page +// from the GitHub API. When a cache is configured, results are cached per +// repository to avoid re-paginating all tags on subsequent requests within the +// cache expiration window. +func (s *Service) listTags(ctx context.Context, owner, repo string) ([]string, error) { + key := owner + "/" + repo + if tags, ok := s.cache.Get(key); ok { + return tags, nil + } + var ( - page = 1 - prefix = module + "/" - versions = []string{} + page = 1 + tags = []string{} ) for { uri := fmt.Sprintf("repos/%s/%s/tags?per_page=%d&page=%d", owner, repo, tagsPerPage, page) - res, err := s.makeRequest(ctx, uri) + res, err := s.makeRequest(ctx, http.MethodGet, uri) if err != nil { return nil, err } - var tags []struct { + var batch []struct { Name string `json:"name"` } - err = json.NewDecoder(res).Decode(&tags) - res.Close() + err = json.NewDecoder(res).Decode(&batch) + cerr := res.Close() + if cerr != nil { + return nil, fmt.Errorf("closing response: %w", cerr) + } + if err != nil { return nil, fmt.Errorf("decoding response: %w", err) } - for _, tag := range tags { - if strings.HasPrefix(tag.Name, prefix) { - versions = append(versions, strings.TrimPrefix(tag.Name, prefix)) - } + for _, tag := range batch { + tags = append(tags, tag.Name) } - if len(tags) < tagsPerPage { + if len(batch) < tagsPerPage { break } page++ } - return versions, nil + + s.cache.Set(key, tags) + return tags, nil } func (s *Service) ProxyDownload(ctx context.Context, system, repo, module, version string, w io.Writer) error { @@ -95,24 +149,40 @@ func (s *Service) ProxyDownload(ctx context.Context, system, repo, module, versi } uri := fmt.Sprintf("repos/%s/%s/tarball/refs/tags/%s/%s", owner, repo, module, version) - body, err := s.makeRequest(ctx, uri) + body, err := s.makeRequest(ctx, http.MethodGet, uri) if err != nil { return err } - defer body.Close() + defer func() { + if err := body.Close(); err != nil { + slog.Warn("error closing response body", "err", err) + } + }() zr, err := gzip.NewReader(body) if err != nil { return fmt.Errorf("read gzip: %w", err) } - defer zr.Close() + defer func() { + if err := zr.Close(); err != nil { + slog.Warn("error closing gzip reader", "err", err) + } + }() zw := gzip.NewWriter(w) - defer zw.Close() + defer func() { + if err := zw.Close(); err != nil { + slog.Warn("error closing gzip writer:", "err", err) + } + }() tr := tar.NewReader(zr) tw := tar.NewWriter(zw) - defer tw.Close() + defer func() { + if err := tw.Close(); err != nil { + slog.Warn("error closing tar writer:", "err", err) + } + }() prefix := fmt.Sprintf("^%s-%s-[^/]+/%s/(.+)", owner, repo, module) if err := copy(prefix, tw, tr); err != nil { @@ -121,9 +191,29 @@ func (s *Service) ProxyDownload(ctx context.Context, system, repo, module, versi return nil } -func (s *Service) makeRequest(ctx context.Context, uri string) (io.ReadCloser, error) { +func (s *Service) RepoHead(ctx context.Context, system, repo string) error { + owner := s.mapOrg(system) + if err := s.validRepo(owner, repo); err != nil { + return err + } + + uri := fmt.Sprintf("repos/%s/%s", owner, repo) + res, err := s.makeRequest(ctx, http.MethodHead, uri) + if err != nil { + return err + } + + cerr := res.Close() + if cerr != nil { + return fmt.Errorf("closing response: %w", cerr) + } + + return nil +} + +func (s *Service) makeRequest(ctx context.Context, method, uri string) (io.ReadCloser, error) { url := fmt.Sprintf("https://api.github.com/%s", uri) - req, err := http.NewRequest(http.MethodGet, url, nil) + req, err := http.NewRequest(method, url, nil) if err != nil { return nil, fmt.Errorf("new request: %w", err) } @@ -163,10 +253,8 @@ func (s *Service) validRepo(owner, repo string) error { } if repos, ok := s.cfg.Repositories[owner]; ok { - for _, r := range repos { - if r == repo { - return nil - } + if slices.Contains(repos, repo) { + return nil } } @@ -197,13 +285,19 @@ func copy(prefix string, w *tar.Writer, r *tar.Reader) error { if err := w.WriteHeader(hdr); err != nil { return fmt.Errorf("writing header: %w", err) } - io.Copy(w, r) + if _, err := io.Copy(w, r); err != nil { + slog.Error("error copying tar entry", "name", hdr.Name, "err", err) + } } } } func slurp(r io.ReadCloser) string { - defer r.Close() + defer func() { + if err := r.Close(); err != nil { + slog.Warn("error closing response body", "err", err) + } + }() b, err := io.ReadAll(r) if err != nil { diff --git a/pkg/github/github_test.go b/pkg/github/github_test.go new file mode 100644 index 0000000..d93e0b0 --- /dev/null +++ b/pkg/github/github_test.go @@ -0,0 +1,226 @@ +package github + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "errors" + "fmt" + "io" + "net/http" + "testing" + + "github.com/reMarkable/orbit/pkg/mcache" +) + +type mockHTTPClient struct { + doFunc func(req *http.Request) (*http.Response, error) +} + +func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { + return m.doFunc(req) +} + +func TestService_ListVersions(t *testing.T) { + mockClient := &mockHTTPClient{ + doFunc: func(req *http.Request) (*http.Response, error) { + if req.URL.Path == "/repos/test-org/test-repo/tags" { + body := `[ + {"name": "module/v1.0.0"}, + {"name": "module/v1.1.0"}, + {"name": "other/v2.0.0"} + ]` + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(body))), + }, nil + } + return nil, errors.New("unexpected request") + }, + } + + cfg := Config{ + OrgMappings: map[string]string{"test-system": "test-org"}, + } + service := New(cfg, mockClient, mcache.New[string, []string](mcache.NoExpiration)) + + versions, err := service.ListVersions(context.Background(), "test-system", "test-repo", "module") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := []string{"v1.0.0", "v1.1.0"} + if len(versions) != len(expected) { + t.Fatalf("expected %d versions, got %d", len(expected), len(versions)) + } + for i, v := range versions { + if v != expected[i] { + t.Errorf("expected version %q, got %q", expected[i], v) + } + } +} + +func TestService_ProxyDownload(t *testing.T) { + mockClient := &mockHTTPClient{ + doFunc: func(req *http.Request) (*http.Response, error) { + if req.URL.Path == "/repos/test-org/test-repo/tarball/refs/tags/module/v1.0.0" { + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + + // Add a fake file to the tar archive with the expected structure + header := &tar.Header{ + Name: "test-org-test-repo-branch/module/testfile.txt", + Mode: 0600, + Size: int64(len("fake tarball content")), + } + if err := tw.WriteHeader(header); err != nil { + return nil, err + } + + if _, err := tw.Write([]byte("fake tarball content")); err != nil { + return nil, err + } + + // Close the tar and gzip writers + _ = tw.Close() + _ = gz.Close() + + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(&buf), + }, nil + + } + return nil, errors.New("unexpected request") + }, + } + + cfg := Config{ + OrgMappings: map[string]string{"test-system": "test-org"}, + } + service := New(cfg, mockClient, mcache.New[string, []string](mcache.NoExpiration)) + + var buf bytes.Buffer + err := service.ProxyDownload(context.Background(), "test-system", "test-repo", "module", "v1.0.0", &buf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Decompress the output to validate its content + gz, err := gzip.NewReader(&buf) + if err != nil { + t.Fatalf("failed to create gzip reader: %v", err) + } + defer func() { + if err := gz.Close(); err != nil { + panic(err) + } + }() + + var tarBuf bytes.Buffer + if _, err := io.Copy(&tarBuf, gz); err != nil { + t.Fatalf("failed to decompress gzip: %v", err) + } + + expectedContent := "fake tarball content" + if !bytes.Contains(tarBuf.Bytes(), []byte(expectedContent)) { + t.Errorf("expected tarball to contain %q, but it was %q", expectedContent, tarBuf.Bytes()) + } +} + +func TestService_ListVersions_CachesTagsPerRepo(t *testing.T) { + var calls int + mockClient := &mockHTTPClient{ + doFunc: func(req *http.Request) (*http.Response, error) { + if req.URL.Path == "/repos/test-org/test-repo/tags" { + calls++ + body := `[ + {"name": "module/v1.0.0"}, + {"name": "other/v2.0.0"} + ]` + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(body))), + }, nil + } + return nil, errors.New("unexpected request") + }, + } + + cfg := Config{ + OrgMappings: map[string]string{"test-system": "test-org"}, + } + service := New(cfg, mockClient, mcache.New[string, []string](mcache.NoExpiration)) + + // First call for "module" should hit the API. + if _, err := service.ListVersions(context.Background(), "test-system", "test-repo", "module"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Second call for a different module in the same repo should be served from + // the cache without hitting the API again. + other, err := service.ListVersions(context.Background(), "test-system", "test-repo", "other") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if calls != 1 { + t.Fatalf("expected tags endpoint to be called once, got %d", calls) + } + + if len(other) != 1 || other[0] != "v2.0.0" { + t.Errorf("expected cached tags to yield [v2.0.0], got %v", other) + } +} + +func TestService_ListVersions_Pagination(t *testing.T) { + // Build a first page with exactly tagsPerPage entries to force a second request. + var firstPage bytes.Buffer + firstPage.WriteString("[") + for i := 0; i < tagsPerPage; i++ { + if i > 0 { + firstPage.WriteString(",") + } + fmt.Fprintf(&firstPage, `{"name": "filler/v0.0.%d"}`, i) + } + firstPage.WriteString("]") + + mockClient := &mockHTTPClient{ + doFunc: func(req *http.Request) (*http.Response, error) { + if req.URL.Path != "/repos/test-org/test-repo/tags" { + return nil, errors.New("unexpected request") + } + switch req.URL.Query().Get("page") { + case "1": + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader(firstPage.Bytes())), + }, nil + case "2": + body := `[{"name": "module/v1.0.0"}]` + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(body))), + }, nil + default: + return nil, errors.New("unexpected page") + } + }, + } + + cfg := Config{ + OrgMappings: map[string]string{"test-system": "test-org"}, + } + service := New(cfg, mockClient, mcache.New[string, []string](mcache.NoExpiration)) + + versions, err := service.ListVersions(context.Background(), "test-system", "test-repo", "module") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(versions) != 1 || versions[0] != "v1.0.0" { + t.Errorf("expected [v1.0.0] across pages, got %v", versions) + } +} diff --git a/pkg/mcache/mcache.go b/pkg/mcache/mcache.go index 571a32b..74ebf49 100644 --- a/pkg/mcache/mcache.go +++ b/pkg/mcache/mcache.go @@ -13,6 +13,8 @@ const ( NoExpiration time.Duration = -1 ) +// New creates a new Cache instance with the specified expiration duration. +// If expiration is set to NoExpiration, items will not expire. func New[K comparable, V any](expiration time.Duration) *Cache[K, V] { return &Cache[K, V]{ expiry: expiration, @@ -28,9 +30,11 @@ type Cache[K comparable, V any] struct { now func() time.Time } +// Get retrieves the value associated with the given key from the cache. +// Returns the value and true if the key exists and is not expired, otherwise returns the zero value and false. func (c *Cache[K, V]) Get(key K) (V, bool) { c.mu.RLock() - defer c.mu.Unlock() + defer c.mu.RUnlock() if item, ok := c.items[key]; ok { now := c.now().UnixNano() @@ -43,6 +47,8 @@ func (c *Cache[K, V]) Get(key K) (V, bool) { return empty, false } +// Set adds a key-value pair to the cache with an optional expiration duration. +// If no duration is provided, the default cache expiration is used. func (c *Cache[K, V]) Set(key K, value V, d ...time.Duration) { expiry := c.expiry if len(d) > 0 { @@ -62,6 +68,8 @@ func (c *Cache[K, V]) Set(key K, value V, d ...time.Duration) { } } +// Delete removes the key-value pair associated with the given key from the cache. +// Returns true if the key existed and was deleted, otherwise false. func (c *Cache[K, V]) Delete(key K) bool { c.mu.Lock() defer c.mu.Unlock() @@ -73,6 +81,7 @@ func (c *Cache[K, V]) Delete(key K) bool { return false } +// Count returns the number of items currently stored in the cache. func (c *Cache[K, V]) Count() int { c.mu.RLock() defer c.mu.RUnlock() @@ -80,6 +89,8 @@ func (c *Cache[K, V]) Count() int { return len(c.items) } +// Cleanup removes all expired items from the cache. +// Returns the number of items that were removed. func (c *Cache[K, V]) Cleanup() int { c.mu.Lock() defer c.mu.Unlock() @@ -95,6 +106,7 @@ func (c *Cache[K, V]) Cleanup() int { return count } +// Flush removes all items from the cache, regardless of expiration. func (c *Cache[K, V]) Flush() { c.mu.Lock() defer c.mu.Unlock() @@ -102,23 +114,28 @@ func (c *Cache[K, V]) Flush() { c.items = make(map[K]item[V]) } +// StartCleanupLoop starts a background loop that periodically calls the Cleanup method on the cache. +// The loop runs at the specified interval and can be stopped by calling the returned stop function. func StartCleanupLoop(c interface{ Cleanup() int }, interval time.Duration) (stop func()) { var wg sync.WaitGroup sc := make(chan struct{}) - go loop(c.Cleanup, interval, sc, &wg) + + wg.Add(1) + go func() { + defer wg.Done() + loop(c.Cleanup, interval, sc) + }() + return func() { close(sc) wg.Wait() } } -func loop(cleanup func() int, d time.Duration, stop chan struct{}, wg *sync.WaitGroup) { +func loop(cleanup func() int, d time.Duration, stop chan struct{}) { ticker := time.NewTicker(d) defer ticker.Stop() - wg.Add(1) - defer wg.Done() - for { select { case <-ticker.C: diff --git a/pkg/mcache/mcache_test.go b/pkg/mcache/mcache_test.go new file mode 100644 index 0000000..40ca84b --- /dev/null +++ b/pkg/mcache/mcache_test.go @@ -0,0 +1,90 @@ +package mcache + +import ( + "testing" + "time" +) + +func TestCache_SetAndGet(t *testing.T) { + cache := New[string, string](time.Minute) + + cache.Set("key1", "value1") + value, ok := cache.Get("key1") + if !ok || value != "value1" { + t.Errorf("expected value1, got %v", value) + } +} + +func TestCache_GetExpired(t *testing.T) { + cache := New[string, string](time.Millisecond) + + cache.Set("key1", "value1") + time.Sleep(2 * time.Millisecond) + _, ok := cache.Get("key1") + if ok { + t.Error("expected key1 to be expired") + } +} + +func TestCache_Delete(t *testing.T) { + cache := New[string, string](time.Minute) + + cache.Set("key1", "value1") + deleted := cache.Delete("key1") + if !deleted { + t.Error("expected key1 to be deleted") + } + + _, ok := cache.Get("key1") + if ok { + t.Error("expected key1 to be absent") + } +} + +func TestCache_Count(t *testing.T) { + cache := New[string, string](time.Minute) + + cache.Set("key1", "value1") + cache.Set("key2", "value2") + if count := cache.Count(); count != 2 { + t.Errorf("expected count 2, got %d", count) + } +} + +func TestCache_Cleanup(t *testing.T) { + cache := New[string, string](time.Millisecond) + + cache.Set("key1", "value1") + cache.Set("key2", "value2", 2*time.Millisecond) + time.Sleep(3 * time.Millisecond) + + cleaned := cache.Cleanup() + if cleaned != 2 { + t.Errorf("expected 2 items cleaned, got %d", cleaned) + } +} + +func TestCache_Flush(t *testing.T) { + cache := New[string, string](time.Minute) + + cache.Set("key1", "value1") + cache.Set("key2", "value2") + cache.Flush() + + if count := cache.Count(); count != 0 { + t.Errorf("expected count 0 after flush, got %d", count) + } +} + +func TestStartCleanupLoop(t *testing.T) { + cache := New[string, string](time.Millisecond) + + cache.Set("key1", "value1") + stop := StartCleanupLoop(cache, time.Millisecond) + time.Sleep(3 * time.Millisecond) + stop() + + if count := cache.Count(); count != 0 { + t.Errorf("expected count 0 after cleanup loop, got %d", count) + } +} diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go new file mode 100644 index 0000000..ccd169c --- /dev/null +++ b/pkg/router/router_test.go @@ -0,0 +1,68 @@ +package router + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestRouter_Get(t *testing.T) { + r := New() + r.Get("/test", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte("test handler")); err != nil { + panic(err) + } + }) + + req := httptest.NewRequest(http.MethodGet, "/test", nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status %d, got %d", http.StatusOK, rec.Code) + } + + if rec.Body.String() != "test handler" { + t.Fatalf("expected body %q, got %q", "test handler", rec.Body.String()) + } +} + +func TestRouter_Parameter(t *testing.T) { + r := New() + r.Get("/user/:id", func(w http.ResponseWriter, r *http.Request) { + id := GetParameter(r.Context(), "id") + w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte("user id: " + id)); err != nil { + panic(err) + } + }) + + req := httptest.NewRequest(http.MethodGet, "/user/123", nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status %d, got %d", http.StatusOK, rec.Code) + } + + expectedBody := "user id: 123" + if rec.Body.String() != expectedBody { + t.Fatalf("expected body %q, got %q", expectedBody, rec.Body.String()) + } +} + +func TestRouter_NotFound(t *testing.T) { + r := New() + + req := httptest.NewRequest(http.MethodGet, "/nonexistent", nil) + rec := httptest.NewRecorder() + + r.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("expected status %d, got %d", http.StatusNotFound, rec.Code) + } +} diff --git a/pkg/server/server.go b/pkg/server/server.go index f295175..ef60810 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -16,9 +16,7 @@ import ( "time" ) -var ( - timeoutWiggleRoom = 300 * time.Millisecond -) +var timeoutWiggleRoom = 300 * time.Millisecond type Logger interface { Error(msg string, args ...any) @@ -41,12 +39,21 @@ type Config struct { CertFile string `envconfig:"TLS_CERT_FILE"` KeyFile string `envconfig:"TLS_KEY_FILE"` } + Metrics struct { + Enabled bool `envconfig:"METRICS_ENABLED" default:"false"` + Port int `envconfig:"METRICS_PORT" default:"9090"` + } } func (c *Config) ListenAddr() string { return fmt.Sprintf("%s:%d", c.Host, c.Port) } +func (c *Config) MetricsListenAddr() string { + // Metrics listener typically binds to the same host as the main server + return fmt.Sprintf("%s:%d", c.Host, c.Metrics.Port) +} + func (c *Config) ReadTimeout() time.Duration { if c.Timeout.Read > 0 { return c.Timeout.Read @@ -73,6 +80,7 @@ func Start(cfg Config, log Logger, h http.Handler, listening ...chan net.Addr) e } ln, err := net.Listen("tcp", cfg.ListenAddr()) + var metricsLn net.Listener if err != nil { log.Error("listen error", "err", err) return err @@ -91,6 +99,27 @@ func Start(cfg Config, log Logger, h http.Handler, listening ...chan net.Addr) e ReadHeaderTimeout: cfg.Timeout.ReadHeader, WriteTimeout: cfg.WriteTimeout(), } + var metricsSrv *http.Server + if cfg.Metrics.Enabled { + metricsLn, err = net.Listen("tcp", cfg.MetricsListenAddr()) + if err != nil { + // Close the main listener if metrics listener fails + if err := ln.Close(); err != nil { + log.Error("close error", "err", err) + } + log.Error("metrics listen error", "err", err) + return err + } + log.Info("metrics server listening", "addr", metricsLn.Addr().String()) + metricsSrv = &http.Server{ + Addr: cfg.MetricsListenAddr(), + Handler: http.DefaultServeMux, + IdleTimeout: cfg.Timeout.Idle, // Reuse main timeouts + ReadTimeout: cfg.ReadTimeout(), // Reuse main timeouts + ReadHeaderTimeout: cfg.Timeout.ReadHeader, // Reuse main timeouts + WriteTimeout: cfg.WriteTimeout(), // Reuse main timeouts + } + } errs := make(chan error) if cfg.TLS.Enabled { @@ -98,6 +127,9 @@ func Start(cfg Config, log Logger, h http.Handler, listening ...chan net.Addr) e } else { go serve(srv, ln, errs) } + if metricsSrv != nil { + go serve(metricsSrv, metricsLn, errs) + } done := make(chan os.Signal, 1) signal.Notify(done, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) @@ -117,6 +149,11 @@ func Start(cfg Config, log Logger, h http.Handler, listening ...chan net.Addr) e log.Error("shutdown error", "err", err) return err } + if metricsSrv != nil { + if err := metricsSrv.Shutdown(ctx); err != nil { + log.Error("metrics shutdown error", "err", err) + } + } return nil } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go new file mode 100644 index 0000000..ca2ef63 --- /dev/null +++ b/pkg/server/server_test.go @@ -0,0 +1,51 @@ +package server + +import ( + "net" + "net/http" + "testing" + "time" +) + +type mockLogger struct { + errors []string + infos []string +} + +func (m *mockLogger) Error(msg string, args ...any) { + m.errors = append(m.errors, msg) +} + +func (m *mockLogger) Info(msg string, args ...any) { + m.infos = append(m.infos, msg) +} + +func TestStartServer(t *testing.T) { + cfg := Config{ + Host: "127.0.0.1", + Port: 8081, + Timeout: Config{}.Timeout, + } + + logger := &mockLogger{} + handler := http.NewServeMux() + handler.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + listening := make(chan net.Addr, 1) + go func() { + if err := Start(cfg, logger, handler, listening); err != nil { + t.Errorf("server failed to start: %v", err) + } + }() + + select { + case addr := <-listening: + if addr == nil { + t.Error("server did not start listening") + } + case <-time.After(2 * time.Second): + t.Error("timeout waiting for server to start") + } +} diff --git a/renovate.json b/renovate.json new file mode 100644 index 0000000..492c5fd --- /dev/null +++ b/renovate.json @@ -0,0 +1,61 @@ +{ + "$schema": "https://docs.renovatebot.com/renovate-schema.json", + "extends": [ + "config:recommended", + "group:allNonMajor", + "customManagers:dockerfileVersions", + "customManagers:githubActionsVersions", + "customManagers:helmChartYamlAppVersions" + ], + "vulnerabilityAlerts": { + "groupName": "vulnerabilityAlerts", + "enabled": true, + "schedule": [], + "prCreation": "immediate", + "labels": [ + "renovate" + ] + }, + "osvVulnerabilityAlerts": true, + "configMigration": true, + "packageRules": [ + { + "description": "Update Docker dependencies, including base images", + "matchDatasources": [ + "docker" + ], + "groupName": "docker dependencies" + }, + { + "description": "Update golang docker base images daily", + "matchDatasources": [ + "docker" + ], + "matchPackageNames": [ + "golang" + ], + "groupName": "golang dockerfile base images", + "schedule": [ + "before 07:00" + ] + }, + { + "description": "Update orbit image version right away", + "matchPackageNames": [ + "ghcr.io/reMarkable/orbit" + ], + "schedule": ["at any time"] + } + ], + "postUpdateOptions": [ + "gomodTidy", + "gomodUpdateImportPaths" + ], + "pre-commit": { + "enabled": true + }, + "schedule": [ + "before 07:00 on the first day of the month" + ], + "timezone": "Europe/Oslo" +} diff --git a/services/modules/cache.go b/services/modules/cache.go index 7588952..410abcc 100644 --- a/services/modules/cache.go +++ b/services/modules/cache.go @@ -9,6 +9,8 @@ import ( "fmt" "io" "os" + "path/filepath" + "sync" "time" ) @@ -19,23 +21,62 @@ type KeyValueStore interface { type FileStorage interface { Open(filename string) (io.ReadCloser, error) - Create(filename string) (io.WriteCloser, error) + Create(filename string) (CacheWriter, error) } -func NewCache(r Repository, s KeyValueStore, f FileStorage, l Logger) *Cache { - return &Cache{f, l, r, s} +// CacheWriter is a handle to a cache entry that is being written. The written +// content only becomes visible to Open after a successful Commit; closing +// without committing discards it, so readers never observe partial writes. +type CacheWriter interface { + io.Writer + // Commit atomically publishes the written content to readers. + Commit() error + // Close releases resources, discarding any uncommitted content. + io.Closer +} + +func NewCache(r Repository, s KeyValueStore, f FileStorage, l Logger, authDisabled bool) *Cache { + return &Cache{ + files: f, + log: l, + repo: r, + store: s, + authDisabled: authDisabled, + locks: make(map[string]*keyLock), + } } type Cache struct { - files FileStorage - log Logger - repo Repository - store KeyValueStore + files FileStorage + log Logger + repo Repository + store KeyValueStore + authDisabled bool + + // mu guards locks, the set of per-filename locks used to coalesce + // concurrent downloads of the same artifact. + mu sync.Mutex + locks map[string]*keyLock +} + +// RepoHead is used to check if we can access the repository in a cheap way. +func (c *Cache) RepoHead(ctx context.Context, owner, repo string) error { + if c.authDisabled { + return nil + } + + return c.repo.RepoHead(ctx, owner, repo) } func (c *Cache) ListVersions(ctx context.Context, owner, repo, module string) ([]string, error) { key := fmt.Sprintf("%s-%s-%s", owner, repo, module) if v, ok := c.store.Get(key); ok { + + err := c.RepoHead(ctx, owner, repo) + if err != nil { + return nil, err + } + return v, nil } @@ -50,31 +91,112 @@ func (c *Cache) ListVersions(ctx context.Context, owner, repo, module string) ([ func (c *Cache) ProxyDownload(ctx context.Context, owner, repo, module, version string, w io.Writer) error { filename := fmt.Sprintf("%s-%s-%s-%s.tar.gz", owner, repo, module, version) - if r, err := c.files.Open(filename); err != nil { - // If we just fail to open the cached file, we'll just log the error and - // then re-download it from the repository as usual. - c.log.Error("failed to open cached file", "err", err) - } else if _, err := io.Copy(w, r); err != nil { - // Since the copy operation failed, we may have partially copied the - // file, so there's no point in trying to read the original. - c.log.Error("failed to copy cached file", "err", err) + + // Verify repository access before writing any bytes, so we never leak + // cached content to callers who do not have access to the repository. + if err := c.RepoHead(ctx, owner, repo); err != nil { return err - } else { - // At this point we have copied the cached file, so we are done. - return nil } - if cw, err := c.files.Create(filename); err != nil { - // If we fail to create a cache file, we'll just proxy download directly - // from the repository without caching. - c.log.Error("failed to create cached file", "err", err) - } else { - defer cw.Close() - w = io.MultiWriter(w, cw) + // Fast path: serve directly from the cache when the file already exists. + if served, err := c.serveCached(filename, w); served { + return err + } + + // Cache miss: take a per-filename lock so that only one goroutine downloads + // and writes a given artifact at a time. Concurrent callers block here and + // then serve the freshly cached file below, avoiding duplicate downloads + // and interleaved writes to the same file. + unlock := c.lockKey(filename) + defer unlock() + + // Another goroutine may have populated the cache while we waited. + if served, err := c.serveCached(filename, w); served { + return err } + + if err := c.cacheDownload(ctx, filename, owner, repo, module, version); err != nil { + return err + } + + if served, err := c.serveCached(filename, w); served { + return err + } + + // Caching is unavailable (e.g. we failed to create the file); fall back to + // proxying the download directly from the repository. return c.repo.ProxyDownload(ctx, owner, repo, module, version, w) } +// serveCached copies a cached file to w when it exists. It returns true if the +// file was found (and thus handled), along with any error from copying. +// Callers must have verified repository access before calling. +func (c *Cache) serveCached(filename string, w io.Writer) (bool, error) { + r, err := c.files.Open(filename) + if err != nil { + return false, nil + } + defer closer(r, c.log, "failed to close read cached file") + + if _, err := io.Copy(w, r); err != nil { + c.log.Error("failed to copy cached file", "err", err) + return true, err + } + + return true, nil +} + +// cacheDownload downloads the artifact from the repository into the cache. The +// content is written to a temporary file and only published on success, so a +// failed or partial download never becomes visible to readers. A failure to +// create the cache file is not fatal: it is logged and nil is returned so the +// caller can fall back to a direct proxy download. +func (c *Cache) cacheDownload(ctx context.Context, filename, owner, repo, module, version string) error { + cw, err := c.files.Create(filename) + if err != nil { + c.log.Error("failed to create cached file", "err", err) + return nil + } + defer closer(cw, c.log, "failed to close created cached file") + + if err := c.repo.ProxyDownload(ctx, owner, repo, module, version, cw); err != nil { + return err + } + return cw.Commit() +} + +// keyLock is a mutex with a reference count, allowing unused entries to be +// removed from the Cache lock map once no goroutine holds or waits on them. +type keyLock struct { + mu sync.Mutex + ref int +} + +// lockKey acquires the lock associated with key, creating it if necessary, and +// returns a function that releases it and cleans up the entry when idle. +func (c *Cache) lockKey(key string) func() { + c.mu.Lock() + kl, ok := c.locks[key] + if !ok { + kl = &keyLock{} + c.locks[key] = kl + } + kl.ref++ + c.mu.Unlock() + + kl.mu.Lock() + return func() { + kl.mu.Unlock() + + c.mu.Lock() + kl.ref-- + if kl.ref == 0 { + delete(c.locks, key) + } + c.mu.Unlock() + } +} + // StoreInPath implements the FileStorage interface by storing files locally on // the file-system at the specified path. It's a bare minimum implementation, // and doesn't create any folders. @@ -84,10 +206,63 @@ func (s StoreInPath) Open(filename string) (io.ReadCloser, error) { return os.Open(s.path(filename)) } -func (s StoreInPath) Create(filename string) (io.WriteCloser, error) { - return os.Create(s.path(filename)) +func (s StoreInPath) Create(filename string) (CacheWriter, error) { + final := s.path(filename) + f, err := os.CreateTemp(string(s), filepath.Base(filename)+".tmp-*") + if err != nil { + return nil, err + } + + return &atomicFile{file: f, finalPath: final}, nil } func (s StoreInPath) path(filename string) string { return fmt.Sprintf("%s/%s", s, filename) } + +// atomicFile writes to a temporary file and only moves it into its final +// location on Commit, so concurrent readers never observe a partial file. +// Closing before Commit discards the temporary file. +type atomicFile struct { + file *os.File + finalPath string + committed bool +} + +func (a *atomicFile) Write(p []byte) (int, error) { + return a.file.Write(p) +} + +func (a *atomicFile) Commit() error { + if err := a.file.Sync(); err != nil { + return err + } + + if err := a.file.Close(); err != nil { + return err + } + + if err := os.Rename(a.file.Name(), a.finalPath); err != nil { + return err + } + + a.committed = true + return nil +} + +func (a *atomicFile) Close() error { + if a.committed { + return nil + } + + // Discard the temporary file when it was never committed. + _ = a.file.Close() + return os.Remove(a.file.Name()) +} + +// closer simply closes the closer and logs any errors. +func closer(c io.Closer, log Logger, msg string) { + if err := c.Close(); err != nil { + log.Error(msg, "err", err) + } +} diff --git a/services/modules/cache_test.go b/services/modules/cache_test.go new file mode 100644 index 0000000..af6d116 --- /dev/null +++ b/services/modules/cache_test.go @@ -0,0 +1,448 @@ +package modules + +import ( + "bytes" + "context" + "errors" + "io" + "os" + "sync" + "sync/atomic" + "testing" + "time" +) + +type mockKeyValueStore struct { + data map[string][]string +} + +func (m *mockKeyValueStore) Get(key string) ([]string, bool) { + v, ok := m.data[key] + return v, ok +} + +func (m *mockKeyValueStore) Set(key string, value []string, d ...time.Duration) { + m.data[key] = value +} + +type mockFileStorage struct { + mu sync.Mutex + files map[string][]byte + createErr error + created atomic.Int32 +} + +func (m *mockFileStorage) Open(filename string) (io.ReadCloser, error) { + m.mu.Lock() + defer m.mu.Unlock() + data, ok := m.files[filename] + if !ok { + return nil, errors.New("file not found") + } + return io.NopCloser(bytes.NewReader(data)), nil +} + +func (m *mockFileStorage) Create(filename string) (CacheWriter, error) { + m.created.Add(1) + if m.createErr != nil { + return nil, m.createErr + } + return &mockCacheWriter{fs: m, name: filename}, nil +} + +func (m *mockFileStorage) get(filename string) ([]byte, bool) { + m.mu.Lock() + defer m.mu.Unlock() + v, ok := m.files[filename] + return v, ok +} + +// mockCacheWriter mimics atomic writes: content is only published to the store +// on Commit, so an uncommitted (partial) write is never visible to Open. +type mockCacheWriter struct { + bytes.Buffer + fs *mockFileStorage + name string +} + +func (w *mockCacheWriter) Commit() error { + w.fs.mu.Lock() + defer w.fs.mu.Unlock() + w.fs.files[w.name] = append([]byte(nil), w.Buffer.Bytes()...) + return nil +} + +func (w *mockCacheWriter) Close() error { return nil } + +type mockCacheRepository struct { + versions map[string][]string + repoHeadSet bool + repoHeadErr error + + proxyFunc func(w io.Writer) error + proxyCalls atomic.Int32 +} + +func (m *mockCacheRepository) ListVersions(ctx context.Context, owner, repo, module string) ([]string, error) { + key := owner + "/" + repo + "/" + module + v, ok := m.versions[key] + if !ok { + return nil, errors.New("not found") + } + return v, nil +} + +func (m *mockCacheRepository) ProxyDownload(ctx context.Context, owner, repo, module, version string, w io.Writer) error { + m.proxyCalls.Add(1) + if m.proxyFunc != nil { + return m.proxyFunc(w) + } + _, err := w.Write([]byte("fake tarball content")) + return err +} + +func (m *mockCacheRepository) RepoHead(ctx context.Context, owner, repo string) error { + if m.repoHeadSet { + return m.repoHeadErr + } + prefix := owner + "/" + repo + "/" + for k := range m.versions { + if len(k) >= len(prefix) && k[:len(prefix)] == prefix { + return nil + } + } + return errors.New("not found") +} + +type mockLogger struct{} + +func (m *mockLogger) Error(msg string, keysAndValues ...any) {} +func (m *mockLogger) Info(msg string, keysAndValues ...any) {} + +func TestCache_ListVersions(t *testing.T) { + store := &mockKeyValueStore{data: make(map[string][]string)} + repo := &mockCacheRepository{versions: map[string][]string{ + "owner/repo/module": {"v1.0.0", "v1.1.0"}, + }} + cache := NewCache(repo, store, nil, nil, false) + + versions, err := cache.ListVersions(context.Background(), "owner", "repo", "module") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expected := []string{"v1.0.0", "v1.1.0"} + if len(versions) != len(expected) { + t.Fatalf("expected %d versions, got %d", len(expected), len(versions)) + } + for i, v := range versions { + if v != expected[i] { + t.Errorf("expected version %q, got %q", expected[i], v) + } + } +} + +func TestCache_ProxyDownload(t *testing.T) { + store := &mockKeyValueStore{data: make(map[string][]string)} + files := &mockFileStorage{files: make(map[string][]byte)} + repo := &mockCacheRepository{repoHeadSet: true} + logger := &mockLogger{} + cache := NewCache(repo, store, files, logger, false) + + var buf bytes.Buffer + err := cache.ProxyDownload(context.Background(), "owner", "repo", "module", "v1.0.0", &buf) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedContent := "fake tarball content" + if buf.String() != expectedContent { + t.Errorf("expected content %q, got %q", expectedContent, buf.String()) + } +} + +func TestCache_ListVersions_CacheHit(t *testing.T) { + wantErr := errors.New("forbidden") + tests := map[string]struct { + repoHeadErr error + authDisabled bool + wantErr error + wantVersions []string + }{ + "RepoHeadSucceeds": { + repoHeadErr: nil, + wantVersions: []string{"v1.0.0", "v1.1.0"}, + }, + "RepoHeadFails": { + repoHeadErr: wantErr, + wantErr: wantErr, + }, + "AuthDisabled": { + // RepoHead would fail, but auth is disabled so it must not be consulted. + repoHeadErr: wantErr, + authDisabled: true, + wantVersions: []string{"v1.0.0", "v1.1.0"}, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + store := &mockKeyValueStore{data: map[string][]string{ + "owner-repo-module": {"v1.0.0", "v1.1.0"}, + }} + repo := &mockCacheRepository{repoHeadSet: true, repoHeadErr: tc.repoHeadErr} + cache := NewCache(repo, store, nil, &mockLogger{}, tc.authDisabled) + + versions, err := cache.ListVersions(context.Background(), "owner", "repo", "module") + if !errors.Is(err, tc.wantErr) { + t.Fatalf("expected error %v, got %v", tc.wantErr, err) + } + if len(versions) != len(tc.wantVersions) { + t.Fatalf("expected %d versions, got %d", len(tc.wantVersions), len(versions)) + } + for i, v := range versions { + if v != tc.wantVersions[i] { + t.Errorf("expected version %q, got %q", tc.wantVersions[i], v) + } + } + }) + } +} + +func TestCache_ProxyDownload_CacheHit(t *testing.T) { + const cached = "cached tarball content" + wantErr := errors.New("forbidden") + tests := map[string]struct { + repoHeadErr error + wantErr error + wantContent string + }{ + "RepoHeadSucceeds": { + repoHeadErr: nil, + wantContent: cached, + }, + "RepoHeadFails": { + repoHeadErr: wantErr, + wantErr: wantErr, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + store := &mockKeyValueStore{data: make(map[string][]string)} + files := &mockFileStorage{files: map[string][]byte{ + "owner-repo-module-v1.0.0.tar.gz": []byte(cached), + }} + repo := &mockCacheRepository{repoHeadSet: true, repoHeadErr: tc.repoHeadErr} + cache := NewCache(repo, store, files, &mockLogger{}, false) + + var buf bytes.Buffer + err := cache.ProxyDownload(context.Background(), "owner", "repo", "module", "v1.0.0", &buf) + if !errors.Is(err, tc.wantErr) { + t.Fatalf("expected error %v, got %v", tc.wantErr, err) + } + if buf.String() != tc.wantContent { + t.Errorf("expected content %q, got %q", tc.wantContent, buf.String()) + } + }) + } +} + +// A cache miss must download once, publish the file to the cache via Commit, +// and serve the same content to the caller. +func TestCache_ProxyDownload_CachesAfterMiss(t *testing.T) { + const filename = "owner-repo-module-v1.0.0.tar.gz" + store := &mockKeyValueStore{data: make(map[string][]string)} + files := &mockFileStorage{files: make(map[string][]byte)} + repo := &mockCacheRepository{repoHeadSet: true} + cache := NewCache(repo, store, files, &mockLogger{}, false) + + var buf bytes.Buffer + if err := cache.ProxyDownload(context.Background(), "owner", "repo", "module", "v1.0.0", &buf); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := buf.String(); got != "fake tarball content" { + t.Errorf("expected content %q, got %q", "fake tarball content", got) + } + if got := files.created.Load(); got != 1 { + t.Errorf("expected 1 cache file created, got %d", got) + } + if data, ok := files.get(filename); !ok { + t.Error("expected file to be committed to the cache") + } else if string(data) != "fake tarball content" { + t.Errorf("expected cached content %q, got %q", "fake tarball content", string(data)) + } +} + +// A failed (partial) download must not be committed to the cache, and no +// partial content must reach the caller. +func TestCache_ProxyDownload_DownloadFailureNotCached(t *testing.T) { + const filename = "owner-repo-module-v1.0.0.tar.gz" + wantErr := errors.New("boom") + store := &mockKeyValueStore{data: make(map[string][]string)} + files := &mockFileStorage{files: make(map[string][]byte)} + repo := &mockCacheRepository{ + repoHeadSet: true, + proxyFunc: func(w io.Writer) error { + _, _ = w.Write([]byte("partial")) + return wantErr + }, + } + cache := NewCache(repo, store, files, &mockLogger{}, false) + + var buf bytes.Buffer + err := cache.ProxyDownload(context.Background(), "owner", "repo", "module", "v1.0.0", &buf) + if !errors.Is(err, wantErr) { + t.Fatalf("expected error %v, got %v", wantErr, err) + } + if buf.Len() != 0 { + t.Errorf("expected no content served, got %q", buf.String()) + } + if _, ok := files.get(filename); ok { + t.Error("expected no file to be committed after a failed download") + } +} + +// When the cache file cannot be created, the download must be proxied directly +// from the repository without caching. +func TestCache_ProxyDownload_CreateFailureFallsBack(t *testing.T) { + const filename = "owner-repo-module-v1.0.0.tar.gz" + store := &mockKeyValueStore{data: make(map[string][]string)} + files := &mockFileStorage{files: make(map[string][]byte), createErr: errors.New("disk full")} + repo := &mockCacheRepository{repoHeadSet: true} + cache := NewCache(repo, store, files, &mockLogger{}, false) + + var buf bytes.Buffer + if err := cache.ProxyDownload(context.Background(), "owner", "repo", "module", "v1.0.0", &buf); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := buf.String(); got != "fake tarball content" { + t.Errorf("expected content %q, got %q", "fake tarball content", got) + } + if _, ok := files.get(filename); ok { + t.Error("expected no file to be committed when create fails") + } + if got := repo.proxyCalls.Load(); got != 1 { + t.Errorf("expected 1 proxy download, got %d", got) + } +} + +// Concurrent requests for the same artifact must be coalesced: the repository +// is downloaded exactly once and every caller receives the full content. +func TestCache_ProxyDownload_ConcurrentCoalesced(t *testing.T) { + const goroutines = 20 + store := &mockKeyValueStore{data: make(map[string][]string)} + files := &mockFileStorage{files: make(map[string][]byte)} + repo := &mockCacheRepository{ + repoHeadSet: true, + proxyFunc: func(w io.Writer) error { + // Hold the lock briefly so callers genuinely overlap. + time.Sleep(10 * time.Millisecond) + _, err := w.Write([]byte("fake tarball content")) + return err + }, + } + cache := NewCache(repo, store, files, &mockLogger{}, false) + + var start sync.WaitGroup + var done sync.WaitGroup + start.Add(1) + bufs := make([]bytes.Buffer, goroutines) + errs := make([]error, goroutines) + for i := range goroutines { + done.Add(1) + go func(i int) { + defer done.Done() + start.Wait() + errs[i] = cache.ProxyDownload(context.Background(), "owner", "repo", "module", "v1.0.0", &bufs[i]) + }(i) + } + start.Done() + done.Wait() + + for i := range goroutines { + if errs[i] != nil { + t.Errorf("goroutine %d: unexpected error: %v", i, errs[i]) + } + if got := bufs[i].String(); got != "fake tarball content" { + t.Errorf("goroutine %d: expected content %q, got %q", i, "fake tarball content", got) + } + } + if got := repo.proxyCalls.Load(); got != 1 { + t.Errorf("expected exactly 1 download, got %d", got) + } + if got := files.created.Load(); got != 1 { + t.Errorf("expected exactly 1 cache file created, got %d", got) + } +} + +// StoreInPath must only make content visible after Commit, so readers never +// observe a partial file. +func TestStoreInPath_AtomicCommit(t *testing.T) { + dir := t.TempDir() + store := StoreInPath(dir) + + cw, err := store.Create("module.tar.gz") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, err := cw.Write([]byte("hello world")); err != nil { + t.Fatalf("unexpected write error: %v", err) + } + + // Before Commit the file must not be readable at its final location. + if _, err := store.Open("module.tar.gz"); err == nil { + t.Error("expected Open to fail before Commit") + } + + if err := cw.Commit(); err != nil { + t.Fatalf("unexpected commit error: %v", err) + } + if err := cw.Close(); err != nil { + t.Fatalf("unexpected close error: %v", err) + } + + r, err := store.Open("module.tar.gz") + if err != nil { + t.Fatalf("expected Open to succeed after Commit: %v", err) + } + defer r.Close() + data, err := io.ReadAll(r) + if err != nil { + t.Fatalf("unexpected read error: %v", err) + } + if string(data) != "hello world" { + t.Errorf("expected %q, got %q", "hello world", string(data)) + } +} + +// Closing without committing must discard the content and leave no temp files. +func TestStoreInPath_CloseDiscardsUncommitted(t *testing.T) { + dir := t.TempDir() + store := StoreInPath(dir) + + cw, err := store.Create("module.tar.gz") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, err := cw.Write([]byte("partial")); err != nil { + t.Fatalf("unexpected write error: %v", err) + } + if err := cw.Close(); err != nil { + t.Fatalf("unexpected close error: %v", err) + } + + if _, err := store.Open("module.tar.gz"); err == nil { + t.Error("expected Open to fail for uncommitted file") + } + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("unexpected error reading dir: %v", err) + } + if len(entries) != 0 { + t.Errorf("expected no leftover files, got %d", len(entries)) + } +} diff --git a/services/modules/exporter.go b/services/modules/exporter.go new file mode 100644 index 0000000..08a4a8e --- /dev/null +++ b/services/modules/exporter.go @@ -0,0 +1,92 @@ +package modules + +import ( + "fmt" + "log/slog" + "maps" + "net/http" + "slices" + "strings" +) + +type ( + Metrics struct { + requestCount map[string]int + downloadCount map[string]int + } + MetricsHandler struct { + logger Logger + metrics Metrics + } +) + +type MetricType int + +const ( + MetricTypeCounter MetricType = iota + MetricTypeGauge + MetricTypeHistogram + MetricTypeSummary + MetricTypeUntyped +) + +var metricTypeName = map[MetricType]string{ + MetricTypeCounter: "counter", + MetricTypeGauge: "gauge", + MetricTypeHistogram: "histogram", + MetricTypeSummary: "summary", + MetricTypeUntyped: "untyped", +} + +func (mt MetricType) String() string { + return metricTypeName[mt] +} + +func NewMetricsHandler(log Logger) (*MetricsHandler, error) { + m := Metrics{ + requestCount: make(map[string]int), + downloadCount: make(map[string]int), + } + return &MetricsHandler{logger: log, metrics: m}, nil +} + +func (h *MetricsHandler) Metrics(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; version=0.0.4") + w.WriteHeader(http.StatusOK) + h.writeMeta(w, MetricTypeCounter, "Total number of requests received", "request_count") + for endpoint, count := range h.metrics.requestCount { + h.writeMetrics(w, "request_count", map[string]string{"endpoint": endpoint}, count) + } + h.writeMeta(w, MetricTypeCounter, "Total number of downloads", "download_count") + for module, count := range h.metrics.downloadCount { + meta := strings.Split(module, "/") + h.writeMetrics(w, "download_count", map[string]string{"module": meta[1], "namespace": meta[0], "version": meta[2]}, count) + } +} + +func (h *MetricsHandler) writeMeta(w http.ResponseWriter, metricType MetricType, help string, metric string) { + meta := fmt.Sprintf("# HELP %s %s\n# TYPE %s %s\n", metric, help, metric, metricType) + if _, err := w.Write([]byte(meta)); err != nil { + slog.Error("write error", "err", err) + } +} + +func (h *MetricsHandler) writeMetrics(w http.ResponseWriter, metric string, labels map[string]string, value int) { + meta := fmt.Sprintf("%s{", metric) + for _, k := range slices.Sorted(maps.Keys(labels)) { + meta += fmt.Sprintf("%s=\"%s\",", k, labels[k]) + } + meta = meta[:len(meta)-1] + fmt.Sprintf("} %d\n", value) + _, err := w.Write([]byte(meta)) + if err != nil { + slog.Error("write error", "err", err) + } +} + +func (h *MetricsHandler) IncrementRequestCount(endpoint string) { + h.metrics.requestCount[endpoint]++ +} + +func (h *MetricsHandler) IncrementDownloadCount(namespace string, name string, version string) { + h.metrics.downloadCount[namespace+"/"+name+"/"+version]++ +} diff --git a/services/modules/exporter_test.go b/services/modules/exporter_test.go new file mode 100644 index 0000000..988a5c3 --- /dev/null +++ b/services/modules/exporter_test.go @@ -0,0 +1,86 @@ +package modules + +import ( + "fmt" + "maps" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" +) + +type MockLogger struct{} + +func (m MockLogger) Error(msg string, args ...any) {} +func (m MockLogger) Info(msg string, args ...any) {} + +func TestNewMetricsHandler(t *testing.T) { + logger := MockLogger{} + handler, err := NewMetricsHandler(logger) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if handler == nil { + t.Fatal("expected non-nil MetricsHandler") + } +} + +func TestMetricsHandler_Metrics(t *testing.T) { + logger := MockLogger{} + handler, _ := NewMetricsHandler(logger) + + handler.IncrementRequestCount("/test") + handler.IncrementDownloadCount("namespace", "module", "0.1") + + req := httptest.NewRequest(http.MethodGet, "/metrics", nil) + w := httptest.NewRecorder() + + handler.Metrics(w, req) + + resp := w.Result() + if resp.StatusCode != http.StatusOK { + t.Errorf("expected status 200, got %d", resp.StatusCode) + } + + body := w.Body.String() + requestCountExpected := formatMetric("request_count", map[string]string{"endpoint": "/test"}, 1) + if !strings.Contains(body, requestCountExpected) { + t.Errorf("expected '%s' for request_count metric for /test, got: %s", requestCountExpected, body) + } + downloadCountExpected := formatMetric("download_count", map[string]string{"namespace": "namespace", "module": "module", "version": "0.1"}, 1) + if !strings.Contains(body, downloadCountExpected) { + t.Errorf("expected %s for namespace/module in response body, got: %s", downloadCountExpected, body) + } +} + +func formatMetric(metric string, labels map[string]string, value int) string { + var labelParts []string + for _, k := range slices.Sorted(maps.Keys(labels)) { + v := labels[k] + labelParts = append(labelParts, fmt.Sprintf("%s=\"%s\"", k, v)) + } + labelStr := strings.Join(labelParts, ",") + return metric + "{" + labelStr + "} " + fmt.Sprintf("%d", value) +} + +func TestMetricsHandler_Metrics_Type(t *testing.T) { + logger := MockLogger{} + handler, _ := NewMetricsHandler(logger) + + handler.IncrementRequestCount("/test") + handler.IncrementDownloadCount("namespace", "module", "0.1") + + req := httptest.NewRequest(http.MethodGet, "/metrics", nil) + w := httptest.NewRecorder() + + handler.Metrics(w, req) + + body := w.Body.String() + if !strings.Contains(body, "# TYPE request_count counter") { + t.Error("expected metric type 'counter' for request_count") + } + if !strings.Contains(body, "# TYPE download_count counter") { + t.Error("expected metric type 'counter' for download_count") + } +} diff --git a/services/modules/http.go b/services/modules/http.go index 2194b78..f28db11 100644 --- a/services/modules/http.go +++ b/services/modules/http.go @@ -17,8 +17,8 @@ import ( "net/http" "time" - "github.com/hedlund/orbit/pkg/auth" - "github.com/hedlund/orbit/pkg/router" + "github.com/reMarkable/orbit/pkg/auth" + "github.com/reMarkable/orbit/pkg/router" ) var ( @@ -45,9 +45,10 @@ type Logger interface { type Repository interface { ListVersions(ctx context.Context, owner, repo, module string) ([]string, error) ProxyDownload(ctx context.Context, owner, repo, module, version string, w io.Writer) error + RepoHead(ctx context.Context, owner, repo string) error } -func NewHTTP(cfg Config, log Logger, r Repository) (*Handler, error) { +func NewHTTP(cfg Config, log Logger, r Repository, mh *MetricsHandler) (*Handler, error) { c, err := aes.NewCipher(cfg.ProxySecret) if err != nil { return nil, err @@ -62,6 +63,7 @@ func NewHTTP(cfg Config, log Logger, r Repository) (*Handler, error) { cipher: gcm, log: log, now: time.Now, + mh: mh, repo: r, }, nil } @@ -70,11 +72,15 @@ type Handler struct { cfg Config cipher Cipher log Logger + mh *MetricsHandler now func() time.Time repo Repository } func (h *Handler) ListVersions(w http.ResponseWriter, r *http.Request) { + if h.mh != nil { + h.mh.IncrementRequestCount("ListVersions") + } var ( ctx = r.Context() namespace = router.GetParameter(ctx, "namespace") @@ -98,6 +104,9 @@ func (h *Handler) ListVersions(w http.ResponseWriter, r *http.Request) { } func (h *Handler) DownloadURL(w http.ResponseWriter, r *http.Request) { + if h.mh != nil { + h.mh.IncrementRequestCount("DownloadURL") + } downloadURL := "./proxy?archive=tar.gz" if token := auth.GetToken(r.Context(), ""); token != "" { encoded, err := h.encodeToken(token) @@ -122,6 +131,7 @@ func (h *Handler) ProxyDownload(w http.ResponseWriter, r *http.Request) { token = r.URL.Query().Get("token") ) + h.mh.IncrementRequestCount("ProxyDownload") if token != "" { var err error token, err = h.decodeToken(token) @@ -132,6 +142,9 @@ func (h *Handler) ProxyDownload(w http.ResponseWriter, r *http.Request) { } ctx = auth.WithToken(ctx, token) } + if h.mh != nil { + h.mh.IncrementDownloadCount(namespace, name, version) + } // w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s-%s-%s-%s.tar.gz", owner, repo, module, version)) if err := h.repo.ProxyDownload(ctx, system, namespace, name, version, w); err != nil { diff --git a/services/modules/http_test.go b/services/modules/http_test.go index 30f0e67..2a821c1 100644 --- a/services/modules/http_test.go +++ b/services/modules/http_test.go @@ -10,8 +10,8 @@ import ( "strings" "testing" - "github.com/hedlund/orbit/pkg/expect" - "github.com/hedlund/orbit/pkg/router" + "github.com/reMarkable/orbit/pkg/expect" + "github.com/reMarkable/orbit/pkg/router" ) func TestListVersions(t *testing.T) { @@ -71,7 +71,11 @@ func TestListVersions(t *testing.T) { h.ServeHTTP(rr, tt.req) res := rr.Result() - defer res.Body.Close() + defer func() { + if err := res.Body.Close(); err != nil { + panic(err) + } + }() b, err := io.ReadAll(res.Body) if err != nil { @@ -139,6 +143,12 @@ func (m *mockRepository) ProxyDownload(ctx context.Context, owner, repo, module, return m.err } +func (m *mockRepository) RepoHead(ctx context.Context, owner, repo string) error { + m.owner.Got(owner) + m.repo.Got(repo) + return m.err +} + func (m *mockRepository) validate(t *testing.T) { t.Helper()