aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
2.98k stars 560 forks source link

GetParametersByPathCommand responds with an empty array rather than an error when given the name of a parameter or SSM that doesn't exist #5928

Closed fdiorazio closed 3 months ago

fdiorazio commented 3 months ago

Checkboxes for prior research

Describe the bug

While I realize that GetParametersByPathCommand requires a path, a developer was using this command to send a specific parameter name, and I spent a lot of time debugging it only to realize AWS responds with an empty array for example: [].

The same thing happens when you give it an SSM path that doesn't exist, you receive [] instead of an error.

SDK version number

@aws-sdk/client-ssm@3.478.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

20.11.1

Reproduction Steps

import { SSMClient, GetParametersByPathCommand } from "@aws-sdk/client-ssm";

export const handler = async (event) => {
  const client = new SSMClient();
  const input = {
    Path: "/foo/bar", // HERE add a name instead of a path or the path of a SSM that doesn't exist results in '[]'
    WithDecryption: false,
  };
  const command = new GetParametersByPathCommand(input);
  const response = await client.send(command);
  console.log(response);
};

Other than the example of /foo/bar you can send it paths that don't exist like /foobar/ or a partial path like /fo and you'll still receive [] instead of an error.

Observed Behavior

The full response from AWS:

{
  '$metadata': {
    httpStatusCode: 200,
    requestId: 'xxxx-xxxx-xxxx-xxxx-xxxx',
    extendedRequestId: undefined,
    cfId: undefined,
    attempts: 1,
    totalRetryDelay: 0
  },
  Parameters: []
}

Parameters returns '[]' which is an empty array rather than an error.

Expected Behavior

AWS should respond with what it usually responds with when you send the wrong value for Path which is a validation exception error:

{
  "errorType": "ValidationException",
  "errorMessage": "The parameter doesn't meet the parameter name requirements. The parameter name must end with a forward slash \"/\". It can't be prefixed with \\\"aws\\\" or \\\"ssm\\\" (case-insensitive). It must use only letters, numbers, or the following symbols: . (period), - (hyphen), _ (underscore). Special characters are not allowed. All sub-paths, if specified, must use the forward slash symbol \"/\". Valid example: /get/parameters2-/by1./path0_./",
}

I crafted the error message above from a similar error when I sent foo/bar instead of /foo/bar with GetParametersByPathCommand to show that AWS is already expecting the Path to be a certain way.

And for situations where the parameter doesn't exist, then NoSuchKey like with S3's GetObjectCommand.

Possible Solution

AWS needs to expand the validation on GetParametersByPathCommand right now you can send it /foo or /foo/ and it returns an array of all secrets that start with foo, but it throws an error on foo/.

Also send an error if the SSM path doesn't even exist.

Additional Information/Context

We had to do our own validation on the response from AWS so that if it's an empty array, throw our own error. That's not something we usually have to do since the AWS error responses are typically perfect on their own.

RanVaknin commented 3 months ago

Hi @fdiorazio ,

Thanks for reaching out.

AWS should respond with what it usually responds with when you send the wrong value for Path which is a validation exception error

Validation exception is a client side exception thrown by the SDK when a field is bound to a specific trait in the API model does not conform to its validation rule. In this case your expectation is that SSM would bind the path member to a specific regex validation rule which is not the case. In other cases it could be a request field that is marked required but not provided. Those are the kind of cases that would cause a ValidationException to be thrown by the client.

If you didn't know, all of the AWS SDKs are code generated directly from the API models of each AWS service they interact with. Because of this, adding a regex trait to the path field is something that would need to be added by the AWS service team, and not something the SDK team can do.

Additionally, the fact that the service returns an empty array rather than a service side error, and there is no corresponding server side exception for path validation hints at an intentional design choice of not failing incorrect paths and instead returning empty results, but this is just an assumption and not tied to data. We on the SDK team do not have visibility to design choices made by the various service API teams.

While I understand your concern, and agree that some validation rule might be useful here, this is not actionable by the SDK team, and you will likely have to implement your own validation logic if you need one.

All the best, Ran~

fdiorazio commented 3 months ago

@RanVaknin where does the preceding slash error come from for SSM? I don't see it in ssm.json.

{
    "errorType": "ValidationException",
    "errorMessage": "The parameter doesn't meet the parameter name requirements. The parameter name must begin with a forward slash \"/\". It can't be prefixed with \\\"aws\\\" or \\\"ssm\\\" (case-insensitive). It must use only letters, numbers, or the following symbols: . (period), - (hyphen), _ (underscore). Special characters are not allowed. All sub-paths, if specified, must use the forward slash symbol \"/\". Valid example: /get/parameters2-/by1./path0_.",
    "name": "ValidationException",
    "$fault": "client",
    "$metadata": {
        "httpStatusCode": 400,
        "requestId": "8ee7ed03-f5c3-496b-83f0-dd2cec1c2abe",
        "attempts": 1,
        "totalRetryDelay": 0
    },
    "__type": "ValidationException",
    "message": "The parameter doesn't meet the parameter name requirements. The parameter name must begin with a forward slash \"/\". It can't be prefixed with \\\"aws\\\" or \\\"ssm\\\" (case-insensitive). It must use only letters, numbers, or the following symbols: . (period), - (hyphen), _ (underscore). Special characters are not allowed. All sub-paths, if specified, must use the forward slash symbol \"/\". Valid example: /get/parameters2-/by1./path0_.",

If I use foo/ then I get the error above. So I don't see why we can't also add one for /foo or for an SSM path that doesn't exist like /fo

github-actions[bot] commented 3 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.