integ-runner: Something is not right about integs

[IkeNefcy]

Describe the bug

Lately I've been seeing a reoccurring issue with integs.

example

[~] Custom::CognitoUserPassword UserSetUserPasswordAD2F2A64
 └─ [~] Create
     └─ [~] .Fn::Join:
         └─ @@ -12,6 +12,6 @@
            [ ]         "User.Username"
            [ ]       ]
            [ ]     },
            [-]     "\",\"Password\":\"TestUser@123\",\"Permanent\":true},\"physicalResourceId\":{\"id\":\"SetUserPassword\"},\"logApiResponseData\":true}"
            [+]     "\",\"Password\":\"TestUser@123\",\"Permanent\":true},\"physicalResourceId\":{\"id\":\"SetUserPassword\"}}"
            [ ]   ]
            [ ] ]

Some Fn::Join commands will add this tag at the end, ,\"logApiResponseData\":true, This comes from the actual synth when you use --update-on-failed, so every time you update the integ, this is added, then when you actually run the integ command to check, you get this error, somehow the integ synth is not adding this and therefore erroring.

Every time that I've seen this, I just manually remove ,\"logApiResponseData\":true, and it's fixed.
But I noticed this a lot in many examples when I did my lambda commit which involved a ton of integ updates;
#33291

As mentioned to fix I just removed them for now. I don't have a list of examples but I am working on a PR right now and the error above came from .../aws-elasticloadbalancingv2/test/integ.alb.oidc.js

Opening this as a general discussion, I haven't actually done any deep dive on this yet, maybe after I finish this PR I will if no one has looked yet

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

I'm expecting that --update-on-failed and yarn integ create compatible results. In this case where yarn integ would show ,\"logApiResponseData\":true, in it's synth.

Current Behavior

,\"logApiResponseData\":true, is not showing in the yarn integ synth and erroring.

Reproduction Steps

This example is a pain in the butt because certificates.
I'll find a simpler one.
Also you would have to make a small change for it to detect that it's changed, or I think there may be a way to force it, can't remember
yarn integ test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js --update-on-failed (force?)
then
yarn integ test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js
and see the error

Possible Solution

No idea, once I get a chance to look at what integ-runner is doing maybe I'll have something. Or if anyone has anything to add / this is known.

Additional Information/Context

No response

CDK CLI Version

2.178.2

Framework Version

No response

Node.js Version

18.18.0

OS

MacOS Sequioa 15.3

Language

TypeScript

Language Version

No response

Other information

No response

[IkeNefcy]

Update; I wanted to verify everything I was doing cleanly, so I redid the same integ
.../aws-elasticloadbalancingv2/test/integ.alb.oidc.js
but instead of env vars I hard coded the keys for hosted zone id etc.
At first it wasn't taking because of what I assume was app.synth(), somehow the synth was just ignoring my build and locally would synth correctly, but would deploy a whole other template.
So I upgraded the test to integ-tests-alpha. And retried.
This worked and here is my workflow and results:

yarn integ <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js
Before upgrade this would show

  CHANGED    aws-elasticloadbalancingv2/test/integ.alb.oidc 1.122s
      Resources
[~] AWS::CertificateManager::Certificate Certificate4E7ABB08 replace
 ├─ [~] DomainName (requires replacement)
 │   ├─ [-] *.example.com
 │   └─ [+] test.mine.com
 └─ [~] DomainValidationOptions (requires replacement)
     └─ @@ -1,6 +1,6 @@
        [ ] [
        [ ]   {
        [-]     "DomainName": "*.example.com",
        [-]     "HostedZoneId": "Z23ABC4XYZL05B"
        [+]     "DomainName": "test.mine.com",
        [+]     "HostedZoneId": "ZUCCHINI"
        [ ]   }
        [ ] ]
[~] AWS::Cognito::UserPool UserPool6BA7E5F2
 └─ [+] UserPoolAddOns
     └─ {"AdvancedSecurityAdditionalFlows":{},"AdvancedSecurityMode":"OFF"} //This is the change I'm working on, ignore
[~] AWS::Cognito::UserPoolDomain UserPoolDomainD0EA232A replace
 └─ [~] Domain (requires replacement)
     ├─ [-] z23abc4xyzl05b
     └─ [+] zucchini
[~] AWS::Cognito::UserPoolClient UserPoolUserPoolClient40176907
 └─ [~] CallbackURLs
     └─ @@ -1,3 +1,3 @@
        [ ] [
        [-]   "https://*.example.com/oauth2/idpresponse"
        [+]   "https://test.mine.com/oauth2/idpresponse"
        [ ] ]
[~] AWS::Route53::RecordSet ARecordE7B57761 replace
 ├─ [~] HostedZoneId (requires replacement)
 │   ├─ [-] Z23ABC4XYZL05B
 │   └─ [+] ZUCCHINI
 └─ [~] Name (requires replacement)
     ├─ [-] example.com.
     └─ [+] mine.com.
[~] AWS::Lambda::Function Signin352C80E6
 └─ [~] Environment
     └─ [~] .Variables:
         └─ [~] .TEST_URL:
             ├─ [-] https://*.example.com
             └─ [+] https://test.mine.com

After the upgrade;

  CHANGED    aws-elasticloadbalancingv2/test/integ.alb.oidc 0.945s
      IntegAlbOidc exists in snapshot, but not in actual
  CHANGED    aws-elasticloadbalancingv2/test/integ.alb.oidc 0.946s
      integ-alb-oidc does not exist in snapshot, but does in actual

Next
yarn integ <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js --update-on-failed

This deployed and returned success

Snapshot Results: 

Tests:    1 failed, 1 total
Failed: <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js

Running integration tests for failed tests...

Running in parallel across regions: us-east-1, us-east-2, us-west-2
Running test <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js in us-east-1
  SUCCESS    aws-elasticloadbalancingv2/test/integ.alb.oidc-IntegTest/DefaultTest 487.553s
       NO ASSERTIONS

Test Results: 

Tests:    1 passed, 1 total
✨  Done in 489.50s.

So we are done right???
Well just to verify
yarn integ <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js

And the result?

yarn run v1.22.22
$ integ-runner --language javascript <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js

Verifying integration test snapshots...

  CHANGED    aws-elasticloadbalancingv2/test/integ.alb.oidc 1.08s
      Resources
[~] Custom::CognitoUser UserFDDCDD17
 └─ [~] Create
     └─ [~] .Fn::Join:
         └─ @@ -5,6 +5,6 @@
            [ ]     {
            [ ]       "Fn::ImportValue": "integalboidcIntegAlbOidc690E4965:ExportsOutputRefUserPool6BA7E5F296FD7236"
            [ ]     },
            [-]     "\",\"Username\":\"[email protected]\",\"UserAttributes\":[{\"Name\":\"email\",\"Value\":\"[email protected]\"},{\"Name\":\"email_verified\",\"Value\":\"true\"}],\"MessageAction\":\"SUPPRESS\"},\"physicalResourceId\":{\"id\":\"User\"},\"logApiResponseData\":true}"
            [+]     "\",\"Username\":\"[email protected]\",\"UserAttributes\":[{\"Name\":\"email\",\"Value\":\"[email protected]\"},{\"Name\":\"email_verified\",\"Value\":\"true\"}],\"MessageAction\":\"SUPPRESS\"},\"physicalResourceId\":{\"id\":\"User\"}}"
            [ ]   ]
            [ ] ]
[~] Custom::CognitoUserPassword UserSetUserPasswordAD2F2A64
 └─ [~] Create
     └─ [~] .Fn::Join:
         └─ @@ -12,6 +12,6 @@
            [ ]         "User.Username"
            [ ]       ]
            [ ]     },
            [-]     "\",\"Password\":\"TestUser@123\",\"Permanent\":true},\"physicalResourceId\":{\"id\":\"SetUserPassword\"},\"logApiResponseData\":true}"
            [+]     "\",\"Password\":\"TestUser@123\",\"Permanent\":true},\"physicalResourceId\":{\"id\":\"SetUserPassword\"}}"
            [ ]   ]
            [ ] ]



Snapshot Results: 

Tests:    1 failed, 1 total
Failed: <path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.oidc.js
Error: Some tests failed!
To re-run failed tests run: integ-runner --update-on-failed
    at main (<path>/aws-cdk-lib/aws-cdk/packages/@aws-cdk/integ-runner/lib/cli.js:192:19)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

So where did logApiResponseData come from? and is this something we want or no?
If we DO want this, then integ-runner is messed up
If we DON'T want this, then the synth that is deployed is messed up, but I think this is less likely given the success message before.

[IkeNefcy]

The specific elements flagging in the template are

const cognitoUserProps = {
  userPool: testCase.userPool,
  username: '[email protected]',
  password: 'TestUser@123',
};
const testUser = new CognitoUser(stack, 'User', cognitoUserProps);

Which is a custom class

class CognitoUser extends Construct {
  readonly username: string;
  readonly password: string;
  constructor(scope: Construct, id: string, props: CognitoUserProps) {
    super(scope, id);
    const user = new AwsCustomResource(this, 'Resource', {
      resourceType: 'Custom::CognitoUser',
      onCreate: {
        service: 'CognitoIdentityServiceProvider',
        action: 'adminCreateUser',
        parameters: {
          UserPoolId: props.userPool.userPoolId,
          Username: props.username,
          UserAttributes: [
            {
              Name: 'email',
              Value: props.username,
            },
            {
              Name: 'email_verified',
              Value: 'true',
            },
          ],
          MessageAction: 'SUPPRESS',
        },
        physicalResourceId: PhysicalResourceId.of('User'),
      },
      policy: AwsCustomResourcePolicy.fromStatements([new iam.PolicyStatement({
        actions: ['cognito-idp:AdminCreateUser'],
        resources: [props.userPool.userPoolArn],
      })]),
    });

    new AwsCustomResource(this, 'SetUserPassword', {
      resourceType: 'Custom::CognitoUserPassword',
      onCreate: {
        service: 'CognitoIdentityServiceProvider',
        action: 'adminSetUserPassword',
        parameters: {
          UserPoolId: props.userPool.userPoolId,
          Username: user.getResponseField('User.Username'),
          Password: props.password,
          Permanent: true,
        },
        physicalResourceId: PhysicalResourceId.of('SetUserPassword'),
      },
      policy: AwsCustomResourcePolicy.fromStatements([new iam.PolicyStatement({
        actions: ['cognito-idp:AdminSetUserPassword'],
        resources: [props.userPool.userPoolArn],
      })]),
    }).node.addDependency(user);
    this.password = props.password;
    this.username = props.username;
  }
}

creating

  "UserFDDCDD17": {
   "Type": "Custom::CognitoUser",
   "Properties": {
    "ServiceToken": {
     "Fn::GetAtt": [
      "AWS679f53fac002430cb0da5b7982bd22872D164C4C",
      "Arn"
     ]
    },
    "Create": {
     "Fn::Join": [
      "",
      [
       "{\"service\":\"CognitoIdentityServiceProvider\",\"action\":\"adminCreateUser\",\"parameters\":{\"UserPoolId\":\"",
       {
        "Fn::ImportValue": "integalboidcIntegAlbOidc690E4965:ExportsOutputRefUserPool6BA7E5F296FD7236"
       },
       "\",\"Username\":\"[email protected]\",\"UserAttributes\":[{\"Name\":\"email\",\"Value\":\"[email protected]\"},{\"Name\":\"email_verified\",\"Value\":\"true\"}],\"MessageAction\":\"SUPPRESS\"},\"physicalResourceId\":{\"id\":\"User\"},\"logApiResponseData\":true}"
      ]
     ]
    },
    "InstallLatestAwsSdk": "false"
   },
   "DependsOn": [
    "UserCustomResourcePolicyC2EB5139"
   ],
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete"
  },

and

   "Type": "Custom::CognitoUserPassword",
   "Properties": {
    "ServiceToken": {
     "Fn::GetAtt": [
      "AWS679f53fac002430cb0da5b7982bd22872D164C4C",
      "Arn"
     ]
    },
    "Create": {
     "Fn::Join": [
      "",
      [
       "{\"service\":\"CognitoIdentityServiceProvider\",\"action\":\"adminSetUserPassword\",\"parameters\":{\"UserPoolId\":\"",
       {
        "Fn::ImportValue": "integalboidcIntegAlbOidc690E4965:ExportsOutputRefUserPool6BA7E5F296FD7236"
       },
       "\",\"Username\":\"",
       {
        "Fn::GetAtt": [
         "UserFDDCDD17",
         "User.Username"
        ]
       },
       "\",\"Password\":\"TestUser@123\",\"Permanent\":true},\"physicalResourceId\":{\"id\":\"SetUserPassword\"},\"logApiResponseData\":true}"
      ]
     ]
    },
    "InstallLatestAwsSdk": "false"
   },
   "DependsOn": [
    "UserCustomResourcePolicyC2EB5139",
    "UserFDDCDD17",
    "UserSetUserPasswordCustomResourcePolicy7B250C76"
   ],
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete"

[IkeNefcy]

Given how many commits we are doing per week, I can't be the only person who has seen ,\"logApiResponseData\":true
and just deleted it to pass.
Feel free to chime in with any other examples.

[pahud]

Is this specific about any PR you are working on?

Did you mean when you run yarn integ or yarn integ-runner it just add "logApiResponseData":true" which cause snapshot mismatches?

[IkeNefcy]

No, not specific to any given PR or change, just like you said it's when I run yarn integ it's adding "logApiResponseData":true when using --update-on-failed, then right after that, without the --update-on-failed flag it's saying it should not be there in the actual test

[IkeNefcy]

I've noticed it on 2 different PRs I worked on, so it's not specific to the changes, it seems like it's the integ behavior itself. Even without a code edit you can recreate this behavior by just deleting a single character in the template snapshot so it will think you need to run an update. Once you update you will see that if you try again to run, it's not actually succeeding.

[IkeNefcy]

More examples

integ.user-pool-domain-cfdist
integ.user-pool-client-explicit-props
integ.user-pool-client-secret