Skip to content

Commit

Permalink
Fix/continuoustest allow tenant and auth (#9038)
Browse files Browse the repository at this point in the history
* fix: ensure tenant-id is set when used in combination with auth

* refactor default tenant string to constant

* Update CHANGELOG.md

* Update CHANGELOG.md

* Adjust continuoustest flags to reflect tenant-id change

* gci and gofmt

* docs: update generated continuoustest helptext

* CHANGELOG formatting

* Update CHANGELOG.md

---------

Co-authored-by: Nick Pillitteri <[email protected]>
  • Loading branch information
kressnick25 and 56quarters authored Aug 21, 2024
1 parent e58c12b commit 970fe27
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
### Mimir Continuous Test

* [CHANGE] Use test metrics that do not pass through 0 to make identifying incorrect results easier. #8630
* [CHANGE] Allowed authentication to Mimir using both Tenant ID and basic/bearer auth. #9038
* [FEATURE] Experimental support for the `-tests.send-chunks-debugging-header` boolean flag to send the `X-Mimir-Chunk-Info-Logger: series_id` header with queries. #8599
* [ENHANCEMENT] Include human-friendly timestamps in diffs logged when a test fails. #8630
* [ENHANCEMENT] Add histograms to measure latency of read and write requests. #8583
Expand Down
8 changes: 4 additions & 4 deletions cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2992,11 +2992,11 @@ Usage of ./cmd/mimir/mimir:
-tenant-federation.max-tenants int
The max number of tenant IDs that may be supplied for a federated query if enabled. 0 to disable the limit.
-tests.basic-auth-password string
The password to use for HTTP bearer authentication. (mutually exclusive with tenant-id or bearer-token flags)
The password to use for HTTP bearer authentication. (mutually exclusive with bearer-token flag)
-tests.basic-auth-user string
The username to use for HTTP bearer authentication. (mutually exclusive with tenant-id or bearer-token flags)
The username to use for HTTP bearer authentication. (mutually exclusive with bearer-token flag)
-tests.bearer-token string
The bearer token to use for HTTP bearer authentication. (mutually exclusive with tenant-id flag or basic-auth flags)
The bearer token to use for HTTP bearer authentication. (mutually exclusive with basic-auth flags)
-tests.read-endpoint string
The base endpoint on the read path. The URL should have no trailing slash. The specific API path is appended by the tool to the URL, for example /api/v1/query_range for range query API, so the configured URL must not include it.
-tests.read-timeout duration
Expand All @@ -3008,7 +3008,7 @@ Usage of ./cmd/mimir/mimir:
-tests.smoke-test
Run a smoke test, i.e. run all tests once and exit.
-tests.tenant-id string
The tenant ID to use to write and read metrics in tests. (mutually exclusive with basic-auth or bearer-token flags) (default "anonymous")
The tenant ID to use to write and read metrics in tests. (default "anonymous")
-tests.write-batch-size int
The maximum number of series to write in a single request. (default 1000)
-tests.write-endpoint string
Expand Down
8 changes: 4 additions & 4 deletions cmd/mimir/help.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -800,11 +800,11 @@ Usage of ./cmd/mimir/mimir:
-tenant-federation.max-tenants int
The max number of tenant IDs that may be supplied for a federated query if enabled. 0 to disable the limit.
-tests.basic-auth-password string
The password to use for HTTP bearer authentication. (mutually exclusive with tenant-id or bearer-token flags)
The password to use for HTTP bearer authentication. (mutually exclusive with bearer-token flag)
-tests.basic-auth-user string
The username to use for HTTP bearer authentication. (mutually exclusive with tenant-id or bearer-token flags)
The username to use for HTTP bearer authentication. (mutually exclusive with bearer-token flag)
-tests.bearer-token string
The bearer token to use for HTTP bearer authentication. (mutually exclusive with tenant-id flag or basic-auth flags)
The bearer token to use for HTTP bearer authentication. (mutually exclusive with basic-auth flags)
-tests.read-endpoint string
The base endpoint on the read path. The URL should have no trailing slash. The specific API path is appended by the tool to the URL, for example /api/v1/query_range for range query API, so the configured URL must not include it.
-tests.read-timeout duration
Expand All @@ -816,7 +816,7 @@ Usage of ./cmd/mimir/mimir:
-tests.smoke-test
Run a smoke test, i.e. run all tests once and exit.
-tests.tenant-id string
The tenant ID to use to write and read metrics in tests. (mutually exclusive with basic-auth or bearer-token flags) (default "anonymous")
The tenant ID to use to write and read metrics in tests. (default "anonymous")
-tests.write-batch-size int
The maximum number of series to write in a single request. (default 1000)
-tests.write-endpoint string
Expand Down
22 changes: 15 additions & 7 deletions pkg/continuoustest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (
)

const (
maxErrMsgLen = 256
maxErrMsgLen = 256
defaultTenant = "anonymous"
)

// MimirClient is the interface implemented by a client used to interact with Mimir.
Expand Down Expand Up @@ -58,10 +59,10 @@ type ClientConfig struct {
}

func (cfg *ClientConfig) RegisterFlags(f *flag.FlagSet) {
f.StringVar(&cfg.TenantID, "tests.tenant-id", "anonymous", "The tenant ID to use to write and read metrics in tests. (mutually exclusive with basic-auth or bearer-token flags)")
f.StringVar(&cfg.BasicAuthUser, "tests.basic-auth-user", "", "The username to use for HTTP bearer authentication. (mutually exclusive with tenant-id or bearer-token flags)")
f.StringVar(&cfg.BasicAuthPassword, "tests.basic-auth-password", "", "The password to use for HTTP bearer authentication. (mutually exclusive with tenant-id or bearer-token flags)")
f.StringVar(&cfg.BearerToken, "tests.bearer-token", "", "The bearer token to use for HTTP bearer authentication. (mutually exclusive with tenant-id flag or basic-auth flags)")
f.StringVar(&cfg.TenantID, "tests.tenant-id", defaultTenant, "The tenant ID to use to write and read metrics in tests.")
f.StringVar(&cfg.BasicAuthUser, "tests.basic-auth-user", "", "The username to use for HTTP bearer authentication. (mutually exclusive with bearer-token flag)")
f.StringVar(&cfg.BasicAuthPassword, "tests.basic-auth-password", "", "The password to use for HTTP bearer authentication. (mutually exclusive with bearer-token flag)")
f.StringVar(&cfg.BearerToken, "tests.bearer-token", "", "The bearer token to use for HTTP bearer authentication. (mutually exclusive with basic-auth flags)")

f.Var(&cfg.WriteBaseEndpoint, "tests.write-endpoint", "The base endpoint on the write path. The URL should have no trailing slash. The specific API path is appended by the tool to the URL, for example /api/v1/push for the remote write API endpoint, so the configured URL must not include it.")
f.IntVar(&cfg.WriteBatchSize, "tests.write-batch-size", 1000, "The maximum number of series to write in a single request.")
Expand Down Expand Up @@ -104,9 +105,10 @@ func NewClient(cfg ClientConfig, logger log.Logger) (*Client, error) {
if cfg.WriteProtocol != "prometheus" && cfg.WriteProtocol != "otlp-http" {
return nil, fmt.Errorf("the only supported write protocols are \"prometheus\" or \"otlp-http\"")
}
// Ensure not both tenant-id and basic-auth are used at the same time
// Ensure not multiple auth methods set at the same time
// Allow tenantID and auth to be defined at the same time for tenant testing
// anonymous is the default value for TenantID.
if (cfg.TenantID != "anonymous" && cfg.BasicAuthUser != "" && cfg.BasicAuthPassword != "" && cfg.BearerToken != "") || // all authentication at once
if (cfg.TenantID != defaultTenant && cfg.BasicAuthUser != "" && cfg.BasicAuthPassword != "" && cfg.BearerToken != "") || // all authentication at once
(cfg.BasicAuthUser != "" && cfg.BasicAuthPassword != "" && cfg.BearerToken != "") { // basic auth and bearer token
return nil, errors.New("either set tests.tenant-id or tests.basic-auth-user/tests.basic-auth-password or tests.bearer-token")
}
Expand Down Expand Up @@ -272,8 +274,14 @@ func (rt *clientRoundTripper) RoundTrip(req *http.Request) (*http.Response, erro

if rt.bearerToken != "" {
req.Header.Set("Authorization", "Bearer "+rt.bearerToken)
if rt.tenantID != defaultTenant {
req.Header.Set("X-Scope-OrgID", rt.tenantID)
}
} else if rt.basicAuthUser != "" && rt.basicAuthPassword != "" {
req.SetBasicAuth(rt.basicAuthUser, rt.basicAuthPassword)
if rt.tenantID != defaultTenant {
req.Header.Set("X-Scope-OrgID", rt.tenantID)
}
} else {
req.Header.Set("X-Scope-OrgID", rt.tenantID)
}
Expand Down
124 changes: 124 additions & 0 deletions pkg/continuoustest/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,130 @@ func TestClient_Query(t *testing.T) {
})
}

func TestClient_QueryHeaders(t *testing.T) {
var (
receivedRequests []*http.Request
)

server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
receivedRequests = append(receivedRequests, request)

// Read requests must go through strong read consistency
require.Equal(t, api.ReadConsistencyStrong, request.Header.Get(api.ReadConsistencyHeader))

writer.WriteHeader(http.StatusOK)
_, err := writer.Write([]byte(`{"status":"success","data":{"resultType":"vector","result":[]}}`))
require.NoError(t, err)
}))
t.Cleanup(server.Close)

cfg := ClientConfig{}
flagext.DefaultValues(&cfg)
require.NoError(t, cfg.WriteBaseEndpoint.Set(server.URL))
require.NoError(t, cfg.ReadBaseEndpoint.Set(server.URL))

c, err := NewClient(cfg, log.NewNopLogger())
require.NoError(t, err)

ctx := context.Background()

t.Run("default tenant header is used without auth", func(t *testing.T) {
receivedRequests = nil

_, err := c.Query(ctx, "up", time.Unix(0, 0))
require.NoError(t, err)

require.Len(t, receivedRequests, 1)
assert.Equal(t, "anonymous", receivedRequests[0].Header.Get("X-Scope-OrgID"))
assert.Empty(t, receivedRequests[0].Header.Get("Authorization"))
})

t.Run("tenant header is not used when basic auth is used", func(t *testing.T) {
cfg = ClientConfig{}
flagext.DefaultValues(&cfg)
require.NoError(t, cfg.WriteBaseEndpoint.Set(server.URL))
require.NoError(t, cfg.ReadBaseEndpoint.Set(server.URL))
cfg.BasicAuthUser = "mimir-user"
cfg.BasicAuthPassword = "guest"

c, err := NewClient(cfg, log.NewNopLogger())
require.NoError(t, err)
ctx := context.Background()
receivedRequests = nil

_, err = c.Query(ctx, "up", time.Unix(0, 0))
require.NoError(t, err)

require.Len(t, receivedRequests, 1)
assert.Empty(t, receivedRequests[0].Header.Get("X-Scope-OrgID"))
assert.NotEmpty(t, receivedRequests[0].Header.Get("Authorization"))
})

t.Run("tenant header is not used when bearer token used", func(t *testing.T) {
cfg = ClientConfig{}
flagext.DefaultValues(&cfg)
require.NoError(t, cfg.WriteBaseEndpoint.Set(server.URL))
require.NoError(t, cfg.ReadBaseEndpoint.Set(server.URL))
cfg.BearerToken = "mimir-token"

c, err := NewClient(cfg, log.NewNopLogger())
require.NoError(t, err)
ctx := context.Background()
receivedRequests = nil

_, err = c.Query(ctx, "up", time.Unix(0, 0))
require.NoError(t, err)

require.Len(t, receivedRequests, 1)
assert.Empty(t, receivedRequests[0].Header.Get("X-Scope-OrgID"))
assert.NotEmpty(t, receivedRequests[0].Header.Get("Authorization"))
})

t.Run("tenant header can be used as well as basic auth", func(t *testing.T) {
cfg = ClientConfig{}
flagext.DefaultValues(&cfg)
require.NoError(t, cfg.WriteBaseEndpoint.Set(server.URL))
require.NoError(t, cfg.ReadBaseEndpoint.Set(server.URL))
cfg.BasicAuthUser = "mimir-user"
cfg.BasicAuthPassword = "guest"
cfg.TenantID = "tenant1"

c, err := NewClient(cfg, log.NewNopLogger())
require.NoError(t, err)
ctx := context.Background()
receivedRequests = nil

_, err = c.Query(ctx, "up", time.Unix(0, 0))
require.NoError(t, err)

require.Len(t, receivedRequests, 1)
assert.Equal(t, "tenant1", receivedRequests[0].Header.Get("X-Scope-OrgID"))
assert.NotEmpty(t, receivedRequests[0].Header.Get("Authorization"))
})

t.Run("tenant header can be used as well as bearer token", func(t *testing.T) {
cfg = ClientConfig{}
flagext.DefaultValues(&cfg)
require.NoError(t, cfg.WriteBaseEndpoint.Set(server.URL))
require.NoError(t, cfg.ReadBaseEndpoint.Set(server.URL))
cfg.BearerToken = "mimir-token"
cfg.TenantID = "tenant1"

c, err := NewClient(cfg, log.NewNopLogger())
require.NoError(t, err)
ctx := context.Background()
receivedRequests = nil

_, err = c.Query(ctx, "up", time.Unix(0, 0))
require.NoError(t, err)

require.Len(t, receivedRequests, 1)
assert.Equal(t, "tenant1", receivedRequests[0].Header.Get("X-Scope-OrgID"))
assert.NotEmpty(t, receivedRequests[0].Header.Get("Authorization"))
})

}

// ClientMock mocks MimirClient.
type ClientMock struct {
mock.Mock
Expand Down

0 comments on commit 970fe27

Please sign in to comment.