[#589] Add LimitExceeded error #593

r.loginov · 2024-12-23T02:36:50Z

r.loginov commented

2024-12-23 02:36:50 +00:00

close #589

As part of this PR, an error was added when access was denied due to exceeding the quota. Examples of how it used to be and how it is now:

(In a situation where we receive an error exceeding the quota from the node)

before:

$ aws s3api put-object --bucket cats --key cat1 --body testf --endpoint http://localhost:8084

An error occurred (AccessDenied) when calling the PutObject operation: Access Denied.

now:

$ aws s3api put-object --bucket cats --key cat1 --body testf --endpoint http://localhost:8084

An error occurred (LimitExceeded) when calling the PutObject operation: You have reached the quota limit.

close #589 As part of this PR, an error was added when access was denied due to exceeding the quota. Examples of how it used to be and how it is now: (In a situation where we receive an error exceeding the quota from the node) before: ``` $ aws s3api put-object --bucket cats --key cat1 --body testf --endpoint http://localhost:8084 An error occurred (AccessDenied) when calling the PutObject operation: Access Denied. ``` now: ``` $ aws s3api put-object --bucket cats --key cat1 --body testf --endpoint http://localhost:8084 An error occurred (LimitExceeded) when calling the PutObject operation: You have reached the quota limit. ```

r.loginov self-assigned this 2024-12-23 02:36:50 +00:00

r.loginov added 1 commit 2024-12-23 02:36:51 +00:00

[#589 ] Add LimitExceeded error

/ DCO (pull_request) Successful in 2m9s

Details

/ Vulncheck (pull_request) Successful in 2m53s

Details

/ Builds (pull_request) Successful in 3m16s

Details

/ Lint (pull_request) Successful in 3m52s

Details

/ Tests (pull_request) Successful in 3m24s

Details

c58334a378

The Access Denied status may be received
from APE due to exceeding the quota. In
this situation, you need to return the
appropriate error.

Signed-off-by: Roman Loginov <r.loginov@yadro.com>

r.loginov requested review from storage-services-developers 2024-12-23 02:36:52 +00:00

r.loginov requested review from storage-services-committers 2024-12-23 02:36:52 +00:00

nzinkevich reviewed 2024-12-23 06:24:12 +00:00

api/errors/errors.go Outdated

					
				@ -1766,0 +1770,4 @@

						ErrCode:        ErrLimitExceeded,

						Code:           "LimitExceeded",

						Description:    "You have reached the quota limit.",

						HTTPStatusCode: http.StatusConflict,

nzinkevich commented

2024-12-23 06:21:33 +00:00

It seems to me that the StatusConflict is not very suitable for quota limit errors. Maybe we should use another response code, such as TooManyRequests or Forbidden?

r.loginov commented

2024-12-23 08:23:42 +00:00

That's a good point. I agree that at first glance, the Conflict status is not quite appropriate
I used it for two reasons:

The same status is used in the aws error on the basis of which this error was made.
Based on RFC 9110, it seems that this status is appropriate. If you look at the phrase

This code is used in situations where the user might be able to resolve the conflict and resubmit the request. The server SHOULD generate content that includes enough information for a user to recognize the source of the conflict.

This is exactly the situation we are in. Upon receiving a message that the quota has been exceeded, the user can remove this quota himself by deleting related resources. And resend the request, which will work successfully.

Using the Too Many Requests status doesn't seem quite right, because based on RFC 6585, this status is used in communications when a user has sent too many requests in a given period of time. And in our situation, we are talking specifically about the resource quota (the amount of space occupied, the number of containers, the number of objects).

Using the Forbidden status can be considered for use in this situation. However, I am a little confused that if you follow RFC 9110, then this status is used in situations where a user with certain credentials does not have access to this resource. On the one hand, this is true, but only create/put operations are not available to the user. The user may well request and delete the resource. Moreover, it turns out that the user can delete the data in the resource himself, thereby removing his quota. It doesn't seem to be the behavior that the current status implies.

However, I don't have a clear opinion on whether to use Conflict or Forbidden. Let's also wait for the opinions of other reviewers.

That's a good point. I agree that at first glance, the `Conflict` status is not quite appropriate I used it for two reasons: 1. The same status is used in the [aws error](https://docs.aws.amazon.com/IAM/latest/APIReference/API_CreateUser.html#API_CreateUser_Errors) on the basis of which this error was made. 2. Based on [RFC 9110](https://datatracker.ietf.org/doc/html/rfc9110#name-409-conflict), it seems that this status is appropriate. If you look at the phrase > This code is used in situations where the user might be able to resolve the conflict and resubmit the request. The server SHOULD generate content that includes enough information for a user to recognize the source of the conflict. This is exactly the situation we are in. Upon receiving a message that the quota has been exceeded, the user can remove this quota himself by deleting related resources. And resend the request, which will work successfully. Using the Too Many Requests status doesn't seem quite right, because based on [RFC 6585](https://datatracker.ietf.org/doc/html/rfc6585#section-4), this status is used in communications when a user has sent too many requests in a given period of time. And in our situation, we are talking specifically about the resource quota (the amount of space occupied, the number of containers, the number of objects). Using the Forbidden status can be considered for use in this situation. However, I am a little confused that if you follow [RFC 9110](https://datatracker.ietf.org/doc/html/rfc9110#name-403-forbidden), then this status is used in situations where a user with certain credentials does not have access to this resource. On the one hand, this is true, but only create/put operations are not available to the user. The user may well request and delete the resource. Moreover, it turns out that the user can delete the data in the resource himself, thereby removing his quota. It doesn't seem to be the behavior that the current status implies. However, I don't have a clear opinion on whether to use Conflict or Forbidden. Let's also wait for the opinions of other reviewers.

👍 1

r.loginov commented

2024-12-24 05:21:22 +00:00

We have agreed that we will use the 409 Conflict

r.loginov marked this conversation as resolved

r.loginov changed title from ~~[#589] Add LimitExceeded error~~ to WIP: [#589] Add LimitExceeded error

2024-12-23 11:58:02 +00:00

dkirillov reviewed 2024-12-23 11:59:45 +00:00

internal/frostfs/frostfs.go Outdated

					
				@ -477,6 +477,9 @@ func handleObjectError(msg string, err error) error {

					}

					if reason, ok := frosterr.IsErrObjectAccessDenied(err); ok {

						if strings.Contains(reason, "Quota limit reached") {