Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seems like the clients require ClientRequestToken to be set #4640

Closed
3 tasks done
aldex32 opened this issue Apr 18, 2023 · 10 comments
Closed
3 tasks done

Seems like the clients require ClientRequestToken to be set #4640

aldex32 opened this issue Apr 18, 2023 · 10 comments
Assignees
Labels
bug This issue is a bug. needs-review This issue/pr needs review from an internal developer. p0 This issue is the highest priority

Comments

@aldex32
Copy link

aldex32 commented Apr 18, 2023

Checkboxes for prior research

Describe the bug

Updating sdk clients to latest version v3.315.0 started to fail with, but doesn't fail with v3.310.0 at least.
The error below is coming from @aws-sdk/client-secrets-manager, but might happen also for other clients.

InvalidRequestException: You must provide a ClientRequestToken value. We recommend a UUID-type value.
    at de_InvalidRequestExceptionRes (/home/runner/work/*******/node_modules/@aws-sdk/client-secrets-manager/dist-cjs/protocols/Aws_json1_1.js:1183:23)
    at de_PutSecretValueCommandError (/home/runner/work/*******/node_modules/@aws-sdk/client-secrets-manager/dist-cjs/protocols/Aws_json1_1.js:678:25)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-serde/dist-cjs/deserializerMiddleware.js:7:24
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-signing/dist-cjs/middleware.js:14:20
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:27:46
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26
    at async upsertSecretObject (/home/runner/work/*******/misc/scripts/bootstrap-common.js:46:9)
    at async Promise.all (index 4)
    at async module.exports (/home/runner/work/*******/misc/scripts/bootstrap-pr-it-broad-environment.js:28:13) {
  '$fault': 'client',
  '$metadata': {
    httpStatusCode: 400,
    requestId: '1f36d279-0731-4fc5-a519-79e86ce27e49',
    extendedRequestId: undefined,
    cfId: undefined,
    attempts: 1,
    totalRetryDelay: 0
  },
  __type: 'InvalidRequestException'
}

SDK version number

@aws-sdk/client-secrets-manager@3.315.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.12.1

Reproduction Steps

const client = new SecretsManager({ region: 'eu-west-1' });
await client.createSecret({ Name: 'secret-name', SecretString: JSON.stringify({secretValue: 'secret-value'}) })

Observed Behavior

InvalidRequestException: You must provide a ClientRequestToken value. We recommend a UUID-type value.
    at de_InvalidRequestExceptionRes (/home/runner/work/*******/node_modules/@aws-sdk/client-secrets-manager/dist-cjs/protocols/Aws_json1_1.js:1183:23)
    at de_PutSecretValueCommandError (/home/runner/work/*******/node_modules/@aws-sdk/client-secrets-manager/dist-cjs/protocols/Aws_json1_1.js:678:25)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-serde/dist-cjs/deserializerMiddleware.js:7:24
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-signing/dist-cjs/middleware.js:14:20
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-retry/dist-cjs/retryMiddleware.js:27:46
    at async /home/runner/work/*******/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26
    at async upsertSecretObject (/home/runner/work/*******/misc/scripts/bootstrap-common.js:46:9)
    at async Promise.all (index 4)
    at async module.exports (/home/runner/work/*******/misc/scripts/bootstrap-pr-it-broad-environment.js:28:13) {
  '$fault': 'client',
  '$metadata': {
    httpStatusCode: 400,
    requestId: '1f36d279-0731-4fc5-a519-79e86ce27e49',
    extendedRequestId: undefined,
    cfId: undefined,
    attempts: 1,
    totalRetryDelay: 0
  },
  __type: 'InvalidRequestException'
}

Expected Behavior

Expected behaviour is that ClientRequestToken is set by the sdk itself if is omitted.

Possible Solution

No response

Additional Information/Context

No response

@aldex32 aldex32 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 18, 2023
@aukevanleeuwen
Copy link

aukevanleeuwen commented Apr 18, 2023

I can't exactly get my head around it, but it seems something is going wrong in the 'new' function take from this PR: #4616.

This is from a debugging session, so it's not the typescript reference, but the dist, so it's a bit messy, but essentially what I see is the following

// node_modules/@aws-sdk/client-secrets-manager/dist-cjs/protocols/Aws_json1_1.js
// L1256
const se_PutSecretValueRequest = (input, context) => {
    return (0, smithy_client_1.take)(input, {
        ClientRequestToken: (_) => _ ?? (0, uuid_1.v4)(),
        SecretBinary: context.base64Encoder,
        SecretId: [],
        SecretString: [],
        VersionStages: smithy_client_1._json,
    });
};

This is called while running the serializer middleware of the SDK. The intended use if I understand the MR that I linked above, is to put mapping functions in here. So ClientRequestToken has the mapping function <explicitly-set-value> ?? uuid.v4(), which makes sense, this is what the documentation states.

However the 'instructions' given to the take() functions aren't applied for fields that aren't defined in the input? So essentially it comes out unchanged in the case of not putting in a ClientRequestToken.

At least that's exactly what this test of the #take() function specifies:

https://github.com/kuhe/aws-sdk-js-v3/blob/73f539d21ee5c1cc600602cb00cf778a14632f53/packages/smithy-client/src/object-mapping.spec.ts#L133-L160

So I'm not entirely sure what's going on here, but it's not working as intended. This also makes me quite suspicious of a lot of other cases where this structure for defaulting stuff is used.

@birtles
Copy link

birtles commented Apr 19, 2023

I am seeing this with the Athena client from v3.315.0 where StartQueryExecutionCommand seems to fail to generate a ClientRequestToken automatically.

I wonder if #4625 is to blame since that appears to be where take from #4616 is actually put to use.

@aman-codes1
Copy link

aman-codes1 commented Apr 19, 2023

My Jenkins Test Cases are failing but Test Cases are passing locally! Using version (https://www.npmjs.com/package/@aws-sdk/client-secrets-manager/v/3.310.0)
image

@aukevanleeuwen
Copy link

My Jenkins Test Cases are failing but Test Cases are passing locally! Using version (npmjs.com/package/@aws-sdk/client-secrets-manager/v/3.310.0)

@aman-codes1: this issue is about something that seems like a regression somewhere between v3.310.0 and v3.315.0. Since you are using v3.310.0, your problem is (very likely) not related to this issue. Thanks for keeping this issue clean of further comments related to the above. Below is a possible solution to your problem though.

Possible solution to your problem

I have a hunch what your problem is: the error describes that it doesn't understand the ?? operator. The Nullish coalescing operator was added in Node 14, which is required for the AWS SDK v3.201.0+.

So if it works locally, you are probably running Node 14+. Make sure that you are also using Node 14+ in your Jenkins instance as well.

@teohjk
Copy link

teohjk commented Apr 19, 2023

I have the same error on Node v18. Using @aws-sdk/client-secrets-manager@^3.315.0

@aman-codes1
Copy link

Thanks @aukevanleeuwen for the quick solution. It started working. The issue is resolved now.

@RanVaknin RanVaknin self-assigned this Apr 19, 2023
@RanVaknin RanVaknin added investigating Issue is being investigated and/or work is in progress to resolve the issue. needs-review This issue/pr needs review from an internal developer. p0 This issue is the highest priority and removed needs-triage This issue or PR still needs to be triaged. labels Apr 19, 2023
@RanVaknin
Copy link
Contributor

Hi everyone,

Thanks for letting us know. I've assigned this issue the highest priority and we will start investigating right away.

Thanks,
Ran~

@kuhe
Copy link
Contributor

kuhe commented Apr 19, 2023

hi, I am preparing a fix for this issue now.

For affected versions, please use this workaround importing a uuid generator:

import { SecretsManager } from "@aws-sdk/client-secrets-manager";
import { v4 } from 'uuid';

const client = new SecretsManager({ region: "...." });
await client.createSecret({
  Name: "secret-name",
  ClientRequestToken: v4(),
  SecretString: JSON.stringify({ secretValue: "secret-value" }),
});

@kuhe kuhe self-assigned this Apr 19, 2023
@kuhe kuhe added pending-release This issue will be fixed by an approved PR that hasn't been released yet. and removed investigating Issue is being investigated and/or work is in progress to resolve the issue. labels Apr 19, 2023
@kuhe
Copy link
Contributor

kuhe commented Apr 19, 2023

a fix was released in https://www.npmjs.com/package/@aws-sdk/client-secrets-manager/v/3.316.0

@kuhe kuhe closed this as completed Apr 19, 2023
@kuhe kuhe removed the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Apr 19, 2023
stekern added a commit to capralifecycle/cals-cli that referenced this issue Apr 21, 2023
@github-actions
Copy link

github-actions bot commented May 4, 2023

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. needs-review This issue/pr needs review from an internal developer. p0 This issue is the highest priority
Projects
None yet
Development

No branches or pull requests

7 participants