From 3f63a6035f4adadfe7f780f859bdb17b3b7087ce Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:14:57 -0700 Subject: [PATCH 1/3] Move configs-db integration tests to ./integration The integration-configs-db CI job was the only test suite that built a parallel "swap dbtest variant via build tag" pipeline outside the ./integration/ e2e framework, and as a side effect re-ran the entire pkg/configs and pkg/ruler unit-test suites against a Postgres-backed dbtest just so pkg/configs/api could exercise the real DB. See #7652. This consolidates onto the ./integration/ pattern every other Docker-dependent test in the repo uses: - New integration_configs_db-gated e2e test in integration/configs_db_test.go spins up Postgres + a cortex -target=configs container, drives the configs HTTP API end-to-end (round-trip, 401/404, multi-tenant isolation, "newest version" admin endpoint, migrations-on-startup). - New e2e helpers: NewPostgres in integration/e2e/db, Postgres image constant, NewConfigs cortex service constructor in e2ecortex, and a dedicated ConfigsClient (kept separate from Client to avoid colliding with the existing alertmanager helpers). - Drop the integration build tag from pkg/configs/db/dbtest by deleting dbtest/integration.go and renaming dbtest/unit.go to dbtest.go. The package becomes a single-variant in-memory helper. - Delete the configs-integration-test Makefile target and the integration-configs-db CI job; remove it from deploy.needs. The existing discover-tags auto-discovery picks up the new build tag and runs it in the standard integration matrix on amd64 and arm64. - Add postgres:9.6.16 to the per-tag Preload Images step. Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com> --- .github/workflows-doc.md | 3 +- .github/workflows/test-build-deploy.yml | 34 +--- Makefile | 22 +-- integration/configs_db_test.go | 184 +++++++++++++++++++ integration/e2e/db/db.go | 23 +++ integration/e2e/images/images.go | 1 + integration/e2ecortex/configs_client.go | 128 +++++++++++++ integration/e2ecortex/services.go | 21 +++ pkg/configs/db/dbtest/{unit.go => dbtest.go} | 2 - pkg/configs/db/dbtest/integration.go | 68 ------- 10 files changed, 362 insertions(+), 124 deletions(-) create mode 100644 integration/configs_db_test.go create mode 100644 integration/e2ecortex/configs_client.go rename pkg/configs/db/dbtest/{unit.go => dbtest.go} (95%) delete mode 100644 pkg/configs/db/dbtest/integration.go diff --git a/.github/workflows-doc.md b/.github/workflows-doc.md index a4f406a9627..2152edcd27e 100644 --- a/.github/workflows-doc.md +++ b/.github/workflows-doc.md @@ -17,7 +17,6 @@ test-build-deploy.yml specifies a workflow that runs all Cortex continuous integ |------------------------|-------------------------------------------------------------------------------------------------------------------------------|------| | lint | Runs linting and ensures vendor directory, protos and generated documentation are consistent. | CI | | test | Runs units tests on Cassandra testing framework. | CI | -| integration-configs-db | Integration tests for database configurations. | CI | | integration | Runs integration tests after upgrading golang, pulling necessary docker images and downloading necessary module dependencies. | CI | | Security/CodeQL | CodeQL is a semantic code analysis engine used for automating security checks. | CI | | build | Builds and saves an up-to-date Cortex image and website. | CI | @@ -62,7 +61,7 @@ As of October 2020, GitHub Actions do not persist between different jobs in the | Artifact | Stored In | Used By | Purpose of Storing Artifact | |-------------------------------|-----------|---------------------------------------------|-----------------------------| | website public | build | deploy_website | share data between jobs | -| Docker Images | build | deploy, integration, integrations-config-db | share data between jobs | +| Docker Images | build | deploy, integration | share data between jobs | *Note:* Docker Images are zipped before uploading as a workaround. The images contain characters that are illegal in the upload-artifact action. ```yaml diff --git a/.github/workflows/test-build-deploy.yml b/.github/workflows/test-build-deploy.yml index 80b1cd5ae5f..b356a553717 100644 --- a/.github/workflows/test-build-deploy.yml +++ b/.github/workflows/test-build-deploy.yml @@ -327,6 +327,8 @@ jobs: elif [ "$TEST_TAGS" = "integration_query_fuzz" ]; then retry docker pull quay.io/cortexproject/cortex:v1.20.1 retry docker pull quay.io/prometheus/prometheus:v3.8.1 + elif [ "$TEST_TAGS" = "integration_configs_db" ]; then + retry docker pull postgres:9.6.16 fi retry docker pull memcached:1.6.1 retry docker pull redis:7.0.4-alpine @@ -346,38 +348,8 @@ jobs: env: IMAGE_PREFIX: ${{ secrets.IMAGE_PREFIX }} - integration-configs-db: - needs: [build, lint] - runs-on: ${{ matrix.runner }} - timeout-minutes: 20 - strategy: - fail-fast: false - matrix: - include: - - runner: ubuntu-24.04 - arch: amd64 - - runner: ubuntu-24.04-arm - arch: arm64 - steps: - - name: Checkout Repo - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - - name: Download Docker Images Artifact - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 - with: - name: Docker Images - - name: Extract Docker Images Archive - run: tar -xvf images.tar -C / - - name: Run Integration Configs Tests - timeout-minutes: 15 - # Github Actions does not support TTY in their default runners yet - run: | - touch build-image/.uptodate - MIGRATIONS_DIR=$(pwd)/cmd/cortex/migrations - echo "Running configs integration tests on ${{ matrix.arch }}" - make BUILD_IMAGE=quay.io/cortexproject/build-image:master-5f643d518c TTY='' configs-integration-test - deploy: - needs: [build, test, lint, integration, integration-configs-db] + needs: [build, test, lint, integration] if: (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/')) && github.repository == 'cortexproject/cortex' runs-on: ubuntu-24.04 container: diff --git a/Makefile b/Makefile index d50efb8121d..0c8bf681a55 100644 --- a/Makefile +++ b/Makefile @@ -104,7 +104,6 @@ pkg/alertmanager/alertspb/alerts.pb.go: pkg/alertmanager/alertspb/alerts.proto all: $(UPTODATE_FILES) test: protos mod-check: protos -configs-integration-test: protos lint: protos build-image/$(UPTODATE): build-image/* @@ -133,22 +132,6 @@ exes $(EXES) protos $(PROTO_GOS) lint test cover shell mod-check check-protos do @echo ">>>> Entering build container: $@" @$(SUDO) time docker run --rm $(TTY) -i $(GOVOLUMES) $(BUILD_IMAGE) $@; -configs-integration-test: build-image/$(UPTODATE) - @mkdir -p $(shell pwd)/.pkg - @mkdir -p $(shell pwd)/.cache - @DB_CONTAINER="$$(docker run -d -e 'POSTGRES_DB=configs_test' postgres:9.6.16)"; \ - echo ; \ - echo ">>>> Entering build container: $@"; \ - $(SUDO) docker run --rm $(TTY) -i $(GOVOLUMES) \ - -v $(shell pwd)/cmd/cortex/migrations:/migrations:z \ - --workdir /go/src/github.com/cortexproject/cortex \ - --link "$$DB_CONTAINER":configs-db.cortex.local \ - -e DB_ADDR=configs-db.cortex.local \ - $(BUILD_IMAGE) $@; \ - status=$$?; \ - docker rm -f "$$DB_CONTAINER"; \ - exit $$status - else exes: $(EXES) @@ -175,7 +158,7 @@ lint: golangci-lint run # Ensure no blocklisted package is imported. - GOFLAGS="-tags=requires_docker,integration,integration_alertmanager,integration_backward_compatibility,integration_memberlist,integration_querier,integration_ruler,integration_query_fuzz,integration_remote_write_v2" faillint -paths "github.com/bmizerany/assert=github.com/stretchr/testify/assert,\ + GOFLAGS="-tags=requires_docker,integration,integration_alertmanager,integration_backward_compatibility,integration_configs_db,integration_memberlist,integration_querier,integration_ruler,integration_query_fuzz,integration_remote_write_v2" faillint -paths "github.com/bmizerany/assert=github.com/stretchr/testify/assert,\ golang.org/x/net/context=context,\ sync/atomic=go.uber.org/atomic,\ github.com/prometheus/client_golang/prometheus.{MultiError}=github.com/prometheus/prometheus/tsdb/errors.{NewMulti},\ @@ -229,9 +212,6 @@ cover: shell: bash -configs-integration-test: - /bin/bash -c "go test -v -tags 'netgo integration slicelabels' -timeout 10m ./pkg/configs/... ./pkg/ruler/..." - mod-check: GO111MODULE=on go mod download GO111MODULE=on go mod verify diff --git a/integration/configs_db_test.go b/integration/configs_db_test.go new file mode 100644 index 00000000000..2c808c4c481 --- /dev/null +++ b/integration/configs_db_test.go @@ -0,0 +1,184 @@ +//go:build integration_configs_db + +package integration + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/cortexproject/cortex/integration/e2e" + e2edb "github.com/cortexproject/cortex/integration/e2e/db" + "github.com/cortexproject/cortex/integration/e2ecortex" + "github.com/cortexproject/cortex/pkg/configs/userconfig" +) + +// configsDBSetup starts Postgres + a configs-target Cortex service, returning the +// scenario and the configs service. The caller can build a ConfigsClient against +// configs.HTTPEndpoint(). +func configsDBSetup(t *testing.T) (*e2e.Scenario, *e2ecortex.CortexService) { + t.Helper() + + s, err := e2e.NewScenario(networkName) + require.NoError(t, err) + t.Cleanup(s.Close) + + postgres := e2edb.NewPostgres(e2edb.PostgresHostName) + require.NoError(t, s.StartAndWaitReady(postgres)) + + configs := e2ecortex.NewConfigs("configs", map[string]string{ + "-configs.database.uri": fmt.Sprintf( + "postgres://%s@%s:%d/%s?sslmode=disable", + e2edb.PostgresUser, + e2e.NetworkContainerHost(networkName, e2edb.PostgresHostName), + e2edb.PostgresPort, + e2edb.PostgresDB, + ), + }, "") + require.NoError(t, s.StartAndWaitReady(configs)) + + return s, configs +} + +func makeUserID(prefix string, n int) string { + return fmt.Sprintf("%s-%d", prefix, n) +} + +func makeRulesConfig(payload string) userconfig.Config { + return userconfig.Config{ + RulesConfig: userconfig.RulesConfig{ + FormatVersion: userconfig.RuleFormatV2, + Files: map[string]string{ + "rules.yaml": payload, + }, + }, + } +} + +func makeAlertmanagerConfig(receiver string) userconfig.Config { + return userconfig.Config{ + AlertmanagerConfig: fmt.Sprintf(`route: + receiver: %s +receivers: +- name: %s +`, receiver, receiver), + RulesConfig: userconfig.RulesConfig{FormatVersion: userconfig.RuleFormatV2}, + } +} + +// TestConfigsDB_MigrationsRunOnStartup proves the configs service successfully +// runs the SQL migrations under cmd/cortex/migrations against an empty database. +// configsDBSetup already waits for the /ready endpoint, so reaching this point +// is the assertion. +func TestConfigsDB_MigrationsRunOnStartup(t *testing.T) { + _, _ = configsDBSetup(t) +} + +func TestConfigsDB_AnonymousReturns401(t *testing.T) { + _, configs := configsDBSetup(t) + + client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "") + _, code, err := client.GetRulesConfig(context.Background()) + require.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, code) +} + +func TestConfigsDB_GetMissingReturns404(t *testing.T) { + _, configs := configsDBSetup(t) + + client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "user-missing") + _, code, err := client.GetRulesConfig(context.Background()) + require.NoError(t, err) + assert.Equal(t, http.StatusNotFound, code) +} + +func TestConfigsDB_PostAndGetRulesConfig(t *testing.T) { + _, configs := configsDBSetup(t) + + client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "tenant-rules") + cfg := makeRulesConfig("groups: []") + + code, err := client.PostRulesConfig(context.Background(), cfg) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, code) + + view, code, err := client.GetRulesConfig(context.Background()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + require.NotNil(t, view) + assert.Equal(t, cfg.RulesConfig.Files, view.Config.RulesConfig.Files) +} + +func TestConfigsDB_PostAndGetAlertmanagerConfig(t *testing.T) { + _, configs := configsDBSetup(t) + + client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "tenant-am") + cfg := makeAlertmanagerConfig("dummy") + + code, err := client.PostAlertmanagerConfig(context.Background(), cfg) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, code) + + view, code, err := client.GetAlertmanagerConfig(context.Background()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + require.NotNil(t, view) + assert.Equal(t, cfg.AlertmanagerConfig, view.Config.AlertmanagerConfig) +} + +func TestConfigsDB_MultiTenantIsolation(t *testing.T) { + _, configs := configsDBSetup(t) + + tenants := []string{makeUserID("iso", 1), makeUserID("iso", 2)} + configs1 := makeRulesConfig("groups: [{name: t1}]") + configs2 := makeRulesConfig("groups: [{name: t2}]") + + c1 := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), tenants[0]) + c2 := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), tenants[1]) + + code, err := c1.PostRulesConfig(context.Background(), configs1) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, code) + + code, err = c2.PostRulesConfig(context.Background(), configs2) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, code) + + view1, code, err := c1.GetRulesConfig(context.Background()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + assert.Equal(t, configs1.RulesConfig.Files, view1.Config.RulesConfig.Files) + + view2, code, err := c2.GetRulesConfig(context.Background()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + assert.Equal(t, configs2.RulesConfig.Files, view2.Config.RulesConfig.Files) +} + +func TestConfigsDB_GetAllConfigsReturnsLatest(t *testing.T) { + _, configs := configsDBSetup(t) + + tenant := makeUserID("latest", 1) + client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), tenant) + + older := makeRulesConfig("groups: [{name: older}]") + newer := makeRulesConfig("groups: [{name: newer}]") + for _, cfg := range []userconfig.Config{older, older, newer} { + code, err := client.PostRulesConfig(context.Background(), cfg) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, code) + } + + admin := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "") + all, code, err := admin.GetAllRulesConfigs(context.Background()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + + view, ok := all[tenant] + require.True(t, ok, "tenant %q not present in admin response", tenant) + assert.Equal(t, newer.RulesConfig.Files, view.Config.RulesConfig.Files) +} diff --git a/integration/e2e/db/db.go b/integration/e2e/db/db.go index 81cade220cd..20cd63baab5 100644 --- a/integration/e2e/db/db.go +++ b/integration/e2e/db/db.go @@ -14,6 +14,11 @@ import ( const ( MinioAccessKey = "Cheescake" MinioSecretKey = "supersecret" + + PostgresDB = "configs_test" + PostgresUser = "postgres" + PostgresPort = 5432 + PostgresHostName = "configs-db" ) // NewMinio returns minio server, used as a local replacement for S3. @@ -69,6 +74,24 @@ func NewETCD() *e2e.HTTPService { 9000, // Metrics ) } + +// NewPostgres returns a postgres server suitable for tests that need a real database +// (e.g. the configs API). The default database, user and port match the values exposed +// as Postgres* constants in this package. +func NewPostgres(name string) *e2e.ConcreteService { + s := e2e.NewConcreteService( + name, + images.Postgres, + nil, + e2e.NewCmdReadinessProbe(e2e.NewCommand("pg_isready", "-U", PostgresUser, "-d", PostgresDB)), + PostgresPort, + ) + s.SetEnvVars(map[string]string{ + "POSTGRES_DB": PostgresDB, + "POSTGRES_HOST_AUTH_METHOD": "trust", + }) + return s +} func NewPrometheus(image string, flags map[string]string) *e2e.HTTPService { return NewPrometheusWithName("prometheus", image, flags) } diff --git a/integration/e2e/images/images.go b/integration/e2e/images/images.go index 0cf845f976d..5354ca05595 100644 --- a/integration/e2e/images/images.go +++ b/integration/e2e/images/images.go @@ -12,4 +12,5 @@ var ( Consul = "consul:1.8.4" ETCD = "quay.io/coreos/etcd:v3.5.29" Prometheus = "quay.io/prometheus/prometheus:v3.8.1" + Postgres = "postgres:9.6.16" ) diff --git a/integration/e2ecortex/configs_client.go b/integration/e2ecortex/configs_client.go new file mode 100644 index 00000000000..fb8d3002f40 --- /dev/null +++ b/integration/e2ecortex/configs_client.go @@ -0,0 +1,128 @@ +package e2ecortex + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + + "github.com/cortexproject/cortex/pkg/configs/userconfig" +) + +// ConfigsClient drives the configs API (target=configs). It is intentionally separate +// from Client because the configs service is a distinct deployment with its own URL +// and an alertmanager_config endpoint that would otherwise collide with the +// multitenant alertmanager helpers on Client. +type ConfigsClient struct { + endpoint string // host:port of the configs service HTTP API + orgID string + httpClient *http.Client +} + +// NewConfigsClient builds a client for the configs API hosted at endpoint +// (e.g. "127.0.0.1:1234"). orgID may be empty for endpoints that don't require +// tenant authentication (e.g. the private/admin routes). +func NewConfigsClient(endpoint, orgID string) *ConfigsClient { + return &ConfigsClient{ + endpoint: endpoint, + orgID: orgID, + httpClient: &http.Client{}, + } +} + +// ConfigsView mirrors api.ConfigsView for unmarshaling the /private endpoint response. +// Duplicated here so the integration package doesn't pull in pkg/configs/api just for a struct. +type ConfigsView struct { + Configs map[string]userconfig.View `json:"configs"` +} + +func (c *ConfigsClient) do(ctx context.Context, method, path string, body io.Reader) (*http.Response, []byte, error) { + req, err := http.NewRequestWithContext(ctx, method, fmt.Sprintf("http://%s%s", c.endpoint, path), body) + if err != nil { + return nil, nil, err + } + if c.orgID != "" { + req.Header.Set("X-Scope-OrgID", c.orgID) + } + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, nil, err + } + defer resp.Body.Close() + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return resp, nil, err + } + return resp, respBody, nil +} + +// GetRulesConfig fetches the rules config for the client's tenant. +func (c *ConfigsClient) GetRulesConfig(ctx context.Context) (*userconfig.View, int, error) { + return c.getConfig(ctx, "/api/prom/configs/rules") +} + +// PostRulesConfig stores a new version of the rules config for the client's tenant. +func (c *ConfigsClient) PostRulesConfig(ctx context.Context, cfg userconfig.Config) (int, error) { + return c.postConfig(ctx, "/api/prom/configs/rules", cfg) +} + +// GetAlertmanagerConfig fetches the alertmanager config for the client's tenant. +// Named to avoid colliding with Client.GetAlertmanagerConfig, which targets the +// multitenant alertmanager service rather than the configs service. +func (c *ConfigsClient) GetAlertmanagerConfig(ctx context.Context) (*userconfig.View, int, error) { + return c.getConfig(ctx, "/api/prom/configs/alertmanager") +} + +// PostAlertmanagerConfig stores a new version of the alertmanager config for the client's tenant. +func (c *ConfigsClient) PostAlertmanagerConfig(ctx context.Context, cfg userconfig.Config) (int, error) { + return c.postConfig(ctx, "/api/prom/configs/alertmanager", cfg) +} + +// GetAllRulesConfigs hits the admin endpoint that returns every tenant's newest +// rules config keyed by tenant ID. +func (c *ConfigsClient) GetAllRulesConfigs(ctx context.Context) (map[string]userconfig.View, int, error) { + resp, body, err := c.do(ctx, http.MethodGet, "/private/api/prom/configs/rules", nil) + if err != nil { + return nil, 0, err + } + if resp.StatusCode != http.StatusOK { + return nil, resp.StatusCode, nil + } + var view ConfigsView + if err := json.Unmarshal(body, &view); err != nil { + return nil, resp.StatusCode, fmt.Errorf("decoding response: %w", err) + } + return view.Configs, resp.StatusCode, nil +} + +func (c *ConfigsClient) getConfig(ctx context.Context, path string) (*userconfig.View, int, error) { + resp, body, err := c.do(ctx, http.MethodGet, path, nil) + if err != nil { + return nil, 0, err + } + if resp.StatusCode != http.StatusOK { + return nil, resp.StatusCode, nil + } + var view userconfig.View + if err := json.Unmarshal(body, &view); err != nil { + return nil, resp.StatusCode, fmt.Errorf("decoding response: %w", err) + } + return &view, resp.StatusCode, nil +} + +func (c *ConfigsClient) postConfig(ctx context.Context, path string, cfg userconfig.Config) (int, error) { + buf, err := json.Marshal(cfg) + if err != nil { + return 0, err + } + resp, _, err := c.do(ctx, http.MethodPost, path, bytes.NewReader(buf)) + if err != nil { + return 0, err + } + return resp.StatusCode, nil +} diff --git a/integration/e2ecortex/services.go b/integration/e2ecortex/services.go index fb0df2d8182..7d068145c4e 100644 --- a/integration/e2ecortex/services.go +++ b/integration/e2ecortex/services.go @@ -447,3 +447,24 @@ func NewRuler(name string, consulAddress string, flags map[string]string, image grpcPort, ) } + +func NewConfigs(name string, flags map[string]string, image string) *CortexService { + if image == "" { + image = GetDefaultImage() + } + + return NewCortexService( + name, + image, + e2e.NewCommandWithoutEntrypoint("cortex", e2e.BuildArgs(e2e.MergeFlags(map[string]string{ + "-target": "configs", + "-log.level": "warn", + // /migrations is the path the cortex Docker image copies the SQL migrations to; + // see cmd/cortex/Dockerfile. + "-configs.database.migrations-dir": "/migrations", + }, flags))...), + e2e.NewHTTPReadinessProbe(httpPort, "/ready", 200, 299), + httpPort, + grpcPort, + ) +} diff --git a/pkg/configs/db/dbtest/unit.go b/pkg/configs/db/dbtest/dbtest.go similarity index 95% rename from pkg/configs/db/dbtest/unit.go rename to pkg/configs/db/dbtest/dbtest.go index f38748225a1..828b491fc07 100644 --- a/pkg/configs/db/dbtest/unit.go +++ b/pkg/configs/db/dbtest/dbtest.go @@ -1,5 +1,3 @@ -//go:build !integration - package dbtest import ( diff --git a/pkg/configs/db/dbtest/integration.go b/pkg/configs/db/dbtest/integration.go deleted file mode 100644 index ae58d7987bd..00000000000 --- a/pkg/configs/db/dbtest/integration.go +++ /dev/null @@ -1,68 +0,0 @@ -//go:build integration - -package dbtest - -import ( - "fmt" - "os" - "testing" - - "github.com/stretchr/testify/require" - "github.com/weaveworks/common/logging" - - "github.com/cortexproject/cortex/pkg/configs/db" - "github.com/cortexproject/cortex/pkg/configs/db/postgres" -) - -var ( - done chan error - dbAddr string - migrationsDir string - errRollback = fmt.Errorf("rolling back test data") -) - -func init() { - dbAddr = os.Getenv("DB_ADDR") - if dbAddr == "" { - dbAddr = "127.0.0.1" - } - - migrationsDir = os.Getenv("MIGRATIONS_DIR") - if migrationsDir == "" { - migrationsDir = "/migrations" - } -} - -// Setup sets up stuff for testing, creating a new database -func Setup(t *testing.T) db.DB { - require.NoError(t, logging.Setup("debug")) - // Don't use db.MustNew, here so we can do a transaction around the whole test, to rollback. - pg, err := postgres.New( - fmt.Sprintf("postgres://postgres@%s/configs_test?sslmode=disable", dbAddr), - fmt.Sprintf("file:%s", migrationsDir), - ) - require.NoError(t, err) - - newDB := make(chan db.DB) - done = make(chan error) - go func() { - done <- pg.Transaction(func(tx postgres.DB) error { - // Pass out the tx so we can run the test - newDB <- tx - // Wait for the test to finish - return <-done - }) - }() - // Get the new database - return <-newDB -} - -// Cleanup cleans up after a test -func Cleanup(t *testing.T, database db.DB) { - if done != nil { - done <- errRollback - require.Equal(t, errRollback, <-done) - done = nil - } - require.NoError(t, database.Close()) -} From 991b5376a1137ed31a8f58d562c1f10371fb8c52 Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com> Date: Sat, 27 Jun 2026 18:10:23 -0700 Subject: [PATCH 2/3] configs e2e: cover GetConfigs(since), empty GetAllConfigs, and POST 401 Restore the Postgres-specific SQL paths the deleted integration-configs-db job exercised via pkg/configs/api/api_test.go that the initial e2e port missed: - TestConfigsDB_GetConfigsSince exercises the ?since= filter (GetConfigs(since) SQL), the one query path not hit by the other tests. - TestConfigsDB_GetAllConfigsEmpty asserts GetAllConfigs returns an empty set (not an error) on a freshly-migrated DB. - TestConfigsDB_PostAnonymousReturns401 covers the write-path tenant-auth check, mirroring the existing read-path 401 test. Add GetAllRulesConfigsSince to ConfigsClient to drive the since filter. The backend-agnostic api_test.go tests (validation, body-format, template parsing) are intentionally not ported: they exercise handler logic that is identical across DB backends and still run against the in-memory DB in `make test`, so running them against Postgres only re-introduces the duplication this change set removed. Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com> --- integration/configs_db_test.go | 62 +++++++++++++++++++++++++ integration/e2ecortex/configs_client.go | 12 ++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/integration/configs_db_test.go b/integration/configs_db_test.go index 2c808c4c481..142ffeb8125 100644 --- a/integration/configs_db_test.go +++ b/integration/configs_db_test.go @@ -182,3 +182,65 @@ func TestConfigsDB_GetAllConfigsReturnsLatest(t *testing.T) { require.True(t, ok, "tenant %q not present in admin response", tenant) assert.Equal(t, newer.RulesConfig.Files, view.Config.RulesConfig.Files) } + +// TestConfigsDB_GetAllConfigsEmpty proves the admin GetAllConfigs SQL returns an +// empty result set (not an error) against a freshly-migrated, empty database. +func TestConfigsDB_GetAllConfigsEmpty(t *testing.T) { + _, configs := configsDBSetup(t) + + admin := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "") + all, code, err := admin.GetAllRulesConfigs(context.Background()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + assert.Empty(t, all) +} + +// TestConfigsDB_PostAnonymousReturns401 covers the write path's tenant-auth check, +// mirroring the read-path check in TestConfigsDB_AnonymousReturns401. +func TestConfigsDB_PostAnonymousReturns401(t *testing.T) { + _, configs := configsDBSetup(t) + + client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "") + code, err := client.PostRulesConfig(context.Background(), makeRulesConfig("groups: []")) + require.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, code) +} + +// TestConfigsDB_GetConfigsSince proves the GetConfigs(since) SQL filter returns +// only configs whose version ID is greater than the supplied cursor. This is the +// one Postgres-specific query path not exercised by the other tests. +func TestConfigsDB_GetConfigsSince(t *testing.T) { + _, configs := configsDBSetup(t) + ctx := context.Background() + endpoint := configs.HTTPEndpoint() + + // Post one config per tenant, in order, capturing each assigned version ID. + post := func(tenant, payload string) userconfig.ID { + c := e2ecortex.NewConfigsClient(endpoint, tenant) + code, err := c.PostRulesConfig(ctx, makeRulesConfig(payload)) + require.NoError(t, err) + require.Equal(t, http.StatusNoContent, code) + view, code, err := c.GetRulesConfig(ctx) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + return view.ID + } + + user1, user2, user3 := makeUserID("since", 1), makeUserID("since", 2), makeUserID("since", 3) + post(user1, "groups: [{name: first}]") + id2 := post(user2, "groups: [{name: second}]") + post(user3, "groups: [{name: third}]") + + // since=id2 must exclude user1 and user2 (IDs <= id2) and include only user3. + admin := e2ecortex.NewConfigsClient(endpoint, "") + all, code, err := admin.GetAllRulesConfigsSince(ctx, id2) + require.NoError(t, err) + require.Equal(t, http.StatusOK, code) + + _, has1 := all[user1] + _, has2 := all[user2] + _, has3 := all[user3] + assert.False(t, has1, "user1 (ID < since) should be excluded") + assert.False(t, has2, "user2 (ID == since) should be excluded") + assert.True(t, has3, "user3 (ID > since) should be included") +} diff --git a/integration/e2ecortex/configs_client.go b/integration/e2ecortex/configs_client.go index fb8d3002f40..559b5aa0055 100644 --- a/integration/e2ecortex/configs_client.go +++ b/integration/e2ecortex/configs_client.go @@ -86,7 +86,17 @@ func (c *ConfigsClient) PostAlertmanagerConfig(ctx context.Context, cfg userconf // GetAllRulesConfigs hits the admin endpoint that returns every tenant's newest // rules config keyed by tenant ID. func (c *ConfigsClient) GetAllRulesConfigs(ctx context.Context) (map[string]userconfig.View, int, error) { - resp, body, err := c.do(ctx, http.MethodGet, "/private/api/prom/configs/rules", nil) + return c.getAllRulesConfigs(ctx, "/private/api/prom/configs/rules") +} + +// GetAllRulesConfigsSince hits the admin endpoint with a ?since= filter, +// returning only configs whose version ID is greater than since. +func (c *ConfigsClient) GetAllRulesConfigsSince(ctx context.Context, since userconfig.ID) (map[string]userconfig.View, int, error) { + return c.getAllRulesConfigs(ctx, fmt.Sprintf("/private/api/prom/configs/rules?since=%d", since)) +} + +func (c *ConfigsClient) getAllRulesConfigs(ctx context.Context, path string) (map[string]userconfig.View, int, error) { + resp, body, err := c.do(ctx, http.MethodGet, path, nil) if err != nil { return nil, 0, err } From da6e085a22c709bf6f5032c518e0abb1aaa542c3 Mon Sep 17 00:00:00 2001 From: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com> Date: Sat, 27 Jun 2026 18:38:01 -0700 Subject: [PATCH 3/3] configs e2e: add ConfigsClient request timeout and surface POST body - Add a 30s request timeout, matching the sibling Client: a `timeout` field set in NewConfigsClient and applied via context.WithTimeout in do(), so a hung configs service fails fast instead of blocking until the global test timeout. - Return the response body from PostRulesConfig/PostAlertmanagerConfig (and postConfig), matching the convention of Client's *Raw helpers. Success-path POST assertions now use require.Equalf to print the body on an unexpected status, so a future 400/500 is debuggable instead of a bare "expected 204, got N". Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com> --- integration/configs_db_test.go | 26 ++++++++++++------------- integration/e2ecortex/configs_client.go | 23 +++++++++++++++------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/integration/configs_db_test.go b/integration/configs_db_test.go index 142ffeb8125..8e4a3140836 100644 --- a/integration/configs_db_test.go +++ b/integration/configs_db_test.go @@ -102,9 +102,9 @@ func TestConfigsDB_PostAndGetRulesConfig(t *testing.T) { client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "tenant-rules") cfg := makeRulesConfig("groups: []") - code, err := client.PostRulesConfig(context.Background(), cfg) + code, body, err := client.PostRulesConfig(context.Background(), cfg) require.NoError(t, err) - require.Equal(t, http.StatusNoContent, code) + require.Equalf(t, http.StatusNoContent, code, "unexpected status, body: %s", body) view, code, err := client.GetRulesConfig(context.Background()) require.NoError(t, err) @@ -119,9 +119,9 @@ func TestConfigsDB_PostAndGetAlertmanagerConfig(t *testing.T) { client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "tenant-am") cfg := makeAlertmanagerConfig("dummy") - code, err := client.PostAlertmanagerConfig(context.Background(), cfg) + code, body, err := client.PostAlertmanagerConfig(context.Background(), cfg) require.NoError(t, err) - require.Equal(t, http.StatusNoContent, code) + require.Equalf(t, http.StatusNoContent, code, "unexpected status, body: %s", body) view, code, err := client.GetAlertmanagerConfig(context.Background()) require.NoError(t, err) @@ -140,13 +140,13 @@ func TestConfigsDB_MultiTenantIsolation(t *testing.T) { c1 := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), tenants[0]) c2 := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), tenants[1]) - code, err := c1.PostRulesConfig(context.Background(), configs1) + code, body, err := c1.PostRulesConfig(context.Background(), configs1) require.NoError(t, err) - require.Equal(t, http.StatusNoContent, code) + require.Equalf(t, http.StatusNoContent, code, "unexpected status, body: %s", body) - code, err = c2.PostRulesConfig(context.Background(), configs2) + code, body, err = c2.PostRulesConfig(context.Background(), configs2) require.NoError(t, err) - require.Equal(t, http.StatusNoContent, code) + require.Equalf(t, http.StatusNoContent, code, "unexpected status, body: %s", body) view1, code, err := c1.GetRulesConfig(context.Background()) require.NoError(t, err) @@ -168,9 +168,9 @@ func TestConfigsDB_GetAllConfigsReturnsLatest(t *testing.T) { older := makeRulesConfig("groups: [{name: older}]") newer := makeRulesConfig("groups: [{name: newer}]") for _, cfg := range []userconfig.Config{older, older, newer} { - code, err := client.PostRulesConfig(context.Background(), cfg) + code, body, err := client.PostRulesConfig(context.Background(), cfg) require.NoError(t, err) - require.Equal(t, http.StatusNoContent, code) + require.Equalf(t, http.StatusNoContent, code, "unexpected status, body: %s", body) } admin := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "") @@ -201,7 +201,7 @@ func TestConfigsDB_PostAnonymousReturns401(t *testing.T) { _, configs := configsDBSetup(t) client := e2ecortex.NewConfigsClient(configs.HTTPEndpoint(), "") - code, err := client.PostRulesConfig(context.Background(), makeRulesConfig("groups: []")) + code, _, err := client.PostRulesConfig(context.Background(), makeRulesConfig("groups: []")) require.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, code) } @@ -217,9 +217,9 @@ func TestConfigsDB_GetConfigsSince(t *testing.T) { // Post one config per tenant, in order, capturing each assigned version ID. post := func(tenant, payload string) userconfig.ID { c := e2ecortex.NewConfigsClient(endpoint, tenant) - code, err := c.PostRulesConfig(ctx, makeRulesConfig(payload)) + code, body, err := c.PostRulesConfig(ctx, makeRulesConfig(payload)) require.NoError(t, err) - require.Equal(t, http.StatusNoContent, code) + require.Equalf(t, http.StatusNoContent, code, "unexpected status, body: %s", body) view, code, err := c.GetRulesConfig(ctx) require.NoError(t, err) require.Equal(t, http.StatusOK, code) diff --git a/integration/e2ecortex/configs_client.go b/integration/e2ecortex/configs_client.go index 559b5aa0055..25c79da3679 100644 --- a/integration/e2ecortex/configs_client.go +++ b/integration/e2ecortex/configs_client.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "time" "github.com/cortexproject/cortex/pkg/configs/userconfig" ) @@ -18,6 +19,7 @@ import ( type ConfigsClient struct { endpoint string // host:port of the configs service HTTP API orgID string + timeout time.Duration httpClient *http.Client } @@ -28,6 +30,7 @@ func NewConfigsClient(endpoint, orgID string) *ConfigsClient { return &ConfigsClient{ endpoint: endpoint, orgID: orgID, + timeout: 30 * time.Second, httpClient: &http.Client{}, } } @@ -39,6 +42,9 @@ type ConfigsView struct { } func (c *ConfigsClient) do(ctx context.Context, method, path string, body io.Reader) (*http.Response, []byte, error) { + ctx, cancel := context.WithTimeout(ctx, c.timeout) + defer cancel() + req, err := http.NewRequestWithContext(ctx, method, fmt.Sprintf("http://%s%s", c.endpoint, path), body) if err != nil { return nil, nil, err @@ -67,7 +73,9 @@ func (c *ConfigsClient) GetRulesConfig(ctx context.Context) (*userconfig.View, i } // PostRulesConfig stores a new version of the rules config for the client's tenant. -func (c *ConfigsClient) PostRulesConfig(ctx context.Context, cfg userconfig.Config) (int, error) { +// It returns the HTTP status code and response body so callers can assert on the +// status and surface the body for debugging on unexpected responses. +func (c *ConfigsClient) PostRulesConfig(ctx context.Context, cfg userconfig.Config) (int, []byte, error) { return c.postConfig(ctx, "/api/prom/configs/rules", cfg) } @@ -79,7 +87,8 @@ func (c *ConfigsClient) GetAlertmanagerConfig(ctx context.Context) (*userconfig. } // PostAlertmanagerConfig stores a new version of the alertmanager config for the client's tenant. -func (c *ConfigsClient) PostAlertmanagerConfig(ctx context.Context, cfg userconfig.Config) (int, error) { +// It returns the HTTP status code and response body (see PostRulesConfig). +func (c *ConfigsClient) PostAlertmanagerConfig(ctx context.Context, cfg userconfig.Config) (int, []byte, error) { return c.postConfig(ctx, "/api/prom/configs/alertmanager", cfg) } @@ -125,14 +134,14 @@ func (c *ConfigsClient) getConfig(ctx context.Context, path string) (*userconfig return &view, resp.StatusCode, nil } -func (c *ConfigsClient) postConfig(ctx context.Context, path string, cfg userconfig.Config) (int, error) { +func (c *ConfigsClient) postConfig(ctx context.Context, path string, cfg userconfig.Config) (int, []byte, error) { buf, err := json.Marshal(cfg) if err != nil { - return 0, err + return 0, nil, err } - resp, _, err := c.do(ctx, http.MethodPost, path, bytes.NewReader(buf)) + resp, body, err := c.do(ctx, http.MethodPost, path, bytes.NewReader(buf)) if err != nil { - return 0, err + return 0, nil, err } - return resp.StatusCode, nil + return resp.StatusCode, body, nil }