Skip to content

Commit dff25e3

Browse files
bogdan-stbogdan-st
committed
sugestions from dsabsay
Signed-off-by: Bogdan Stancu <[email protected]>
1 parent 61632a1 commit dff25e3

File tree

4 files changed

+168
-57
lines changed

4 files changed

+168
-57
lines changed

‎integration/overrides_test.go‎

Lines changed: 110 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,7 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
9999
require.NoError(t, err)
100100
defer resp.Body.Close()
101101

102-
assert.Equal(t, http.StatusOK, resp.StatusCode)
103-
104-
var overrides map[string]interface{}
105-
err = json.NewDecoder(resp.Body).Decode(&overrides)
106-
require.NoError(t, err)
107-
108-
assert.Empty(t, overrides)
102+
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
109103
})
110104

111105
t.Run("POST overrides for new user", func(t *testing.T) {
@@ -196,13 +190,119 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
196190
require.NoError(t, err)
197191
defer resp.Body.Close()
198192

193+
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
194+
})
195+
196+
require.NoError(t, s.Stop(cortexSvc))
197+
}
198+
199+
func TestOverridesAPIHardLimits(t *testing.T) {
200+
s, err := e2e.NewScenario(networkName)
201+
require.NoError(t, err)
202+
defer s.Close()
203+
204+
minio := e2edb.NewMinio(9001, "cortex")
205+
require.NoError(t, s.StartAndWaitReady(minio))
206+
207+
// Runtime config with hard limits
208+
runtimeConfig := map[string]interface{}{
209+
"overrides": map[string]interface{}{},
210+
"hard_overrides": map[string]interface{}{
211+
"user1": map[string]interface{}{
212+
"ingestion_rate": 10000,
213+
"max_global_series_per_user": 50000,
214+
},
215+
},
216+
"api_allowed_limits": []string{
217+
"ingestion_rate",
218+
"max_global_series_per_user",
219+
},
220+
}
221+
runtimeConfigData, err := yaml.Marshal(runtimeConfig)
222+
require.NoError(t, err)
223+
224+
s3Client, err := s3.NewBucketWithConfig(nil, s3.Config{
225+
Endpoint: minio.HTTPEndpoint(),
226+
Insecure: true,
227+
Bucket: "cortex",
228+
AccessKey: e2edb.MinioAccessKey,
229+
SecretKey: e2edb.MinioSecretKey,
230+
}, "overrides-test-hard-limits", nil)
231+
require.NoError(t, err)
232+
233+
require.NoError(t, s3Client.Upload(context.Background(), "runtime.yaml", bytes.NewReader(runtimeConfigData)))
234+
235+
flags := map[string]string{
236+
"-target": "overrides",
237+
238+
"-runtime-config.file": "runtime.yaml",
239+
"-runtime-config.backend": "s3",
240+
"-runtime-config.s3.access-key-id": e2edb.MinioAccessKey,
241+
"-runtime-config.s3.secret-access-key": e2edb.MinioSecretKey,
242+
"-runtime-config.s3.bucket-name": "cortex",
243+
"-runtime-config.s3.endpoint": minio.NetworkHTTPEndpoint(),
244+
"-runtime-config.s3.insecure": "true",
245+
}
246+
247+
cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides-hard-limits", flags, "")
248+
require.NoError(t, s.StartAndWaitReady(cortexSvc))
249+
250+
t.Run("POST overrides within hard limits", func(t *testing.T) {
251+
overrides := map[string]interface{}{
252+
"ingestion_rate": 5000, // Within hard limit of 10000
253+
"max_global_series_per_user": 25000, // Within hard limit of 50000
254+
}
255+
requestBody, err := json.Marshal(overrides)
256+
require.NoError(t, err)
257+
258+
req, err := http.NewRequest("POST", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader(requestBody))
259+
require.NoError(t, err)
260+
req.Header.Set("X-Scope-OrgID", "user1")
261+
req.Header.Set("Content-Type", "application/json")
262+
263+
resp, err := http.DefaultClient.Do(req)
264+
require.NoError(t, err)
265+
defer resp.Body.Close()
266+
199267
assert.Equal(t, http.StatusOK, resp.StatusCode)
268+
})
200269

201-
var overrides map[string]interface{}
202-
err = json.NewDecoder(resp.Body).Decode(&overrides)
270+
t.Run("POST overrides exceeding hard limits", func(t *testing.T) {
271+
overrides := map[string]interface{}{
272+
"ingestion_rate": 15000, // Exceeds hard limit of 10000
273+
}
274+
requestBody, err := json.Marshal(overrides)
275+
require.NoError(t, err)
276+
277+
req, err := http.NewRequest("POST", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader(requestBody))
278+
require.NoError(t, err)
279+
req.Header.Set("X-Scope-OrgID", "user1")
280+
req.Header.Set("Content-Type", "application/json")
281+
282+
resp, err := http.DefaultClient.Do(req)
283+
require.NoError(t, err)
284+
defer resp.Body.Close()
285+
286+
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
287+
})
288+
289+
t.Run("POST overrides for user without hard limits", func(t *testing.T) {
290+
overrides := map[string]interface{}{
291+
"ingestion_rate": 20000, // No hard limits for user2
292+
}
293+
requestBody, err := json.Marshal(overrides)
294+
require.NoError(t, err)
295+
296+
req, err := http.NewRequest("POST", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader(requestBody))
297+
require.NoError(t, err)
298+
req.Header.Set("X-Scope-OrgID", "user2")
299+
req.Header.Set("Content-Type", "application/json")
300+
301+
resp, err := http.DefaultClient.Do(req)
203302
require.NoError(t, err)
303+
defer resp.Body.Close()
204304

205-
assert.Empty(t, overrides)
305+
assert.Equal(t, http.StatusOK, resp.StatusCode)
206306
})
207307

208308
require.NoError(t, s.Stop(cortexSvc))

‎pkg/overrides/api.go‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,9 @@ import (
1616
)
1717

1818
const (
19-
// HTTP status codes
20-
StatusOK = 200
21-
StatusBadRequest = 400
22-
StatusUnauthorized = 401
23-
StatusInternalServerError = 500
24-
2519
// Error messages
26-
ErrInvalidJSON = "Invalid JSON"
20+
ErrInvalidJSON = "Invalid JSON"
21+
ErrUserNotFound = "User not found"
2722

2823
// Runtime config errors
2924
ErrRuntimeConfig = "runtime config read error"
@@ -39,7 +34,8 @@ func (a *API) getAllowedLimitsFromBucket(ctx context.Context) ([]string, error)
3934

4035
var config runtimeconfig.RuntimeConfigValues
4136
if err := yaml.NewDecoder(reader).Decode(&config); err != nil {
42-
return []string{}, nil // No allowed limits if config can't be decoded
37+
level.Error(a.logger).Log("msg", "failed to decode runtime config", "err", err)
38+
return []string{}, fmt.Errorf("failed to decode runtime config")
4339
}
4440

4541
return config.APIAllowedLimits, nil
@@ -49,14 +45,19 @@ func (a *API) getAllowedLimitsFromBucket(ctx context.Context) ([]string, error)
4945
func (a *API) GetOverrides(w http.ResponseWriter, r *http.Request) {
5046
userID, _, err := tenant.ExtractTenantIDFromHTTPRequest(r)
5147
if err != nil {
52-
http.Error(w, err.Error(), StatusUnauthorized)
48+
http.Error(w, err.Error(), http.StatusUnauthorized)
5349
return
5450
}
5551

5652
// Read overrides from bucket storage
5753
overrides, err := a.getOverridesFromBucket(r.Context(), userID)
5854
if err != nil {
59-
http.Error(w, err.Error(), StatusInternalServerError)
55+
if err.Error() == ErrUserNotFound {
56+
http.Error(w, "User not found", http.StatusBadRequest)
57+
} else {
58+
level.Error(a.logger).Log("msg", "failed to get overrides from bucket", "userID", userID, "err", err)
59+
http.Error(w, "Internal server error", http.StatusInternalServerError)
60+
}
6061
return
6162
}
6263

@@ -72,65 +73,70 @@ func (a *API) GetOverrides(w http.ResponseWriter, r *http.Request) {
7273
func (a *API) SetOverrides(w http.ResponseWriter, r *http.Request) {
7374
userID, _, err := tenant.ExtractTenantIDFromHTTPRequest(r)
7475
if err != nil {
75-
http.Error(w, err.Error(), StatusUnauthorized)
76+
http.Error(w, err.Error(), http.StatusUnauthorized)
7677
return
7778
}
7879

7980
var overrides map[string]interface{}
8081
if err := json.NewDecoder(r.Body).Decode(&overrides); err != nil {
81-
http.Error(w, ErrInvalidJSON, StatusBadRequest)
82+
http.Error(w, ErrInvalidJSON, http.StatusBadRequest)
8283
return
8384
}
8485

8586
// Get allowed limits from runtime config
8687
allowedLimits, err := a.getAllowedLimitsFromBucket(r.Context())
8788
if err != nil {
88-
http.Error(w, "Failed to read allowed limits", StatusInternalServerError)
89+
level.Error(a.logger).Log("msg", "failed to get allowed limits from bucket", "userID", userID, "err", err)
90+
http.Error(w, "Internal server error", http.StatusInternalServerError)
8991
return
9092
}
9193

9294
// Validate that only allowed limits are being changed
9395
if err := ValidateOverrides(overrides, allowedLimits); err != nil {
94-
http.Error(w, err.Error(), StatusBadRequest)
96+
level.Error(a.logger).Log("msg", "invalid overrides validation", "userID", userID, "err", err)
97+
http.Error(w, "Invalid overrides", http.StatusBadRequest)
9598
return
9699
}
97100

98101
// Validate that values don't exceed hard limits from runtime config
99102
if err := a.validateHardLimits(overrides, userID); err != nil {
100-
http.Error(w, err.Error(), StatusBadRequest)
103+
level.Error(a.logger).Log("msg", "hard limits validation failed", "userID", userID, "err", err)
104+
http.Error(w, "Invalid overrides", http.StatusBadRequest)
101105
return
102106
}
103107

104108
// Write overrides to bucket storage
105109
if err := a.setOverridesToBucket(r.Context(), userID, overrides); err != nil {
106-
http.Error(w, err.Error(), StatusInternalServerError)
110+
level.Error(a.logger).Log("msg", "failed to set overrides to bucket", "userID", userID, "err", err)
111+
http.Error(w, "Internal server error", http.StatusInternalServerError)
107112
return
108113
}
109114

110-
w.WriteHeader(StatusOK)
115+
w.WriteHeader(http.StatusOK)
111116
}
112117

113118
// DeleteOverrides removes tenant-specific overrides
114119
func (a *API) DeleteOverrides(w http.ResponseWriter, r *http.Request) {
115120
userID, _, err := tenant.ExtractTenantIDFromHTTPRequest(r)
116121
if err != nil {
117-
http.Error(w, err.Error(), StatusUnauthorized)
122+
http.Error(w, err.Error(), http.StatusUnauthorized)
118123
return
119124
}
120125

121126
if err := a.deleteOverridesFromBucket(r.Context(), userID); err != nil {
122-
http.Error(w, err.Error(), StatusInternalServerError)
127+
level.Error(a.logger).Log("msg", "failed to delete overrides from bucket", "userID", userID, "err", err)
128+
http.Error(w, "Internal server error", http.StatusInternalServerError)
123129
return
124130
}
125131

126-
w.WriteHeader(StatusOK)
132+
w.WriteHeader(http.StatusOK)
127133
}
128134

129135
// getOverridesFromBucket reads overrides for a specific tenant from the runtime config file
130136
func (a *API) getOverridesFromBucket(ctx context.Context, userID string) (map[string]interface{}, error) {
131137
reader, err := a.bucketClient.Get(ctx, a.runtimeConfigPath)
132138
if err != nil {
133-
return map[string]interface{}{}, nil
139+
return nil, fmt.Errorf("failed to get runtime config: %w", err)
134140
}
135141
defer reader.Close()
136142

@@ -155,8 +161,11 @@ func (a *API) getOverridesFromBucket(ctx context.Context, userID string) (map[st
155161

156162
return result, nil
157163
}
164+
// User does not exist in config - return error
165+
return nil, fmt.Errorf(ErrUserNotFound)
158166
}
159167

168+
// No tenant limits configured - return empty map (no overrides)
160169
return map[string]interface{}{}, nil
161170
}
162171

‎pkg/overrides/limits.go‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ package overrides
33
import (
44
"context"
55
"fmt"
6+
"slices"
67
"strconv"
78
"strings"
89

10+
"github.com/go-kit/log/level"
911
"gopkg.in/yaml.v3"
1012

1113
"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
@@ -23,7 +25,7 @@ func ValidateOverrides(overrides map[string]interface{}, allowedLimits []string)
2325
var invalidLimits []string
2426

2527
for limitName := range overrides {
26-
if !IsLimitAllowed(limitName, allowedLimits) {
28+
if !slices.Contains(allowedLimits, limitName) {
2729
invalidLimits = append(invalidLimits, limitName)
2830
}
2931
}
@@ -35,39 +37,23 @@ func ValidateOverrides(overrides map[string]interface{}, allowedLimits []string)
3537
return nil
3638
}
3739

38-
// GetAllowedLimits returns the allowed limits from runtime config
39-
// If no allowed limits are configured, returns empty slice (no limits allowed)
40-
func GetAllowedLimits(allowedLimits []string) []string {
41-
return allowedLimits
42-
}
43-
44-
// IsLimitAllowed checks if a specific limit can be modified
45-
func IsLimitAllowed(limitName string, allowedLimits []string) bool {
46-
for _, allowed := range allowedLimits {
47-
if allowed == limitName {
48-
return true
49-
}
50-
}
51-
return false
52-
}
53-
5440
// validateHardLimits checks if the provided overrides exceed any hard limits from the runtime config
5541
func (a *API) validateHardLimits(overrides map[string]interface{}, userID string) error {
5642
// Read the runtime config to get hard limits
5743
reader, err := a.bucketClient.Get(context.Background(), a.runtimeConfigPath)
5844
if err != nil {
59-
// If we can't read the config, skip hard limit validation
60-
return nil
45+
level.Error(a.logger).Log("msg", "failed to read hard limits configuration", "userID", userID, "err", err)
46+
return fmt.Errorf("failed to validate hard limits")
6147
}
6248
defer reader.Close()
6349

6450
var config runtimeconfig.RuntimeConfigValues
6551
if err := yaml.NewDecoder(reader).Decode(&config); err != nil {
66-
// If we can't decode the config, skip hard limit validation
67-
return nil
52+
level.Error(a.logger).Log("msg", "failed to decode hard limits configuration", "userID", userID, "err", err)
53+
return fmt.Errorf("failed to validate hard limits")
6854
}
6955

70-
// If no hard overrides are defined, skip validation
56+
// If no hard overrides are defined, allow the request
7157
if config.HardTenantLimits == nil {
7258
return nil
7359
}
@@ -80,12 +66,14 @@ func (a *API) validateHardLimits(overrides map[string]interface{}, userID string
8066

8167
yamlData, err := yaml.Marshal(userHardLimits)
8268
if err != nil {
83-
return nil // Skip validation if we can't marshal
69+
level.Error(a.logger).Log("msg", "failed to marshal hard limits", "userID", userID, "err", err)
70+
return fmt.Errorf("failed to validate hard limits")
8471
}
8572

8673
var hardLimitsMap map[string]interface{}
8774
if err := yaml.Unmarshal(yamlData, &hardLimitsMap); err != nil {
88-
return nil // Skip validation if we can't unmarshal
75+
level.Error(a.logger).Log("msg", "failed to unmarshal hard limits", "userID", userID, "err", err)
76+
return fmt.Errorf("failed to validate hard limits")
8977
}
9078

9179
// Validate each override against the user's hard limits

0 commit comments

Comments
 (0)