aws / aws-sdk

Landing page for the AWS SDKs on GitHub
https://aws.amazon.com/tools/
Other
68 stars 12 forks source link

StartJobCommand on Amplify previews build with main branch HEAD instead of preview branch HEAD #653

Closed dkandlyk closed 2 months ago

dkandlyk commented 7 months ago

Checkboxes for prior research

Describe the bug

When calling StartJobCommand from a preview branch and with a specific branch the rebuild pulls the main branch HEAD instead of preview branch latest commit. Example of our data (the branchName is pulled from the ListBranchesCommand):

{
    "appId": "***",
    "branchName": "pr-844",
    "jobType": "RELEASE"
}

The preview now has the changes from the Main branch instead of the specified branch

SDK version number

@aws-sdk/client-amplify @version, 3.445.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

Node version 18.12.0

Reproduction Steps

  1. Create an Amplify app and add previews.
  2. In a branch that is different from the main branch run
    
    const amplifyClient = new AmplifyClient({ region: AWS_REGION });
    const listBranchesParams = {
    appId: appId,
    maxResults: 50,
    };
    const branchList = await amplifyClient.send(new ListBranchesCommand(listBranchesParams));
    Select any of the branches (not main)

const chosenBranch = responseAmplify.branches[1].branchName

const startJobParams: StartJobCommandInput = { appId: appId, branchName: chosenBranch, jobType: JobType.RELEASE, }; await amplifyClient.send(new StartJobCommand(startJobParams));

3. In the Amplify console go to --> previews --> pick the preview branch which changed --> build history --> see the build is from main branch HEAD instead of preview branch latest commit
It can also be seen from the response await amplifyClient.send(new StartJobCommand(startJobParams));
example response:

{ "$metadata": { "httpStatusCode": 200, "requestId": "", "cfId": "", "attempts": 1, "totalRetryDelay": 0 }, "jobSummary": { "commitId": "HEAD", "commitTime": "2023-11-09T09:35:13.081Z", "jobArn": "***/pr-844/jobs/0000000003", "jobId": "3", "status": "PENDING" } }



### Observed Behavior

When calling StartJobCommand through a preview branch with the JobType.Release. The preview branch build pulls the head of the main branch instead of the preview branch head

### Expected Behavior

It should build with the preview branch head

### Possible Solution

_No response_

### Additional Information/Context

_No response_
RanVaknin commented 7 months ago

Hi @dkandlyk ,

Thanks for reaching out.

When calling StartJobCommand through a preview branch with the JobType.Release. The preview branch build pulls the head of the main branch instead of the preview branch head

From your logs I can see that it pulls "HEAD" from the branch named pr-844 as shown in your response.

"jobSummary": {
    "commitId": "HEAD",
    "commitTime": "2023-11-09T09:35:13.081Z",
    "jobArn": "***/pr-844/jobs/0000000003", <-- this shows the branch name as pr-8444
    "jobId": "3",
    "status": "PENDING"
  }

Are you sure "HEAD" does not point to your latest commit? It usually does.

When reproducing this myself it works as expected:

$ git log
commit 29a51cee6ad2d3d3d37a6521396f65e3008b8fec (HEAD -> some-foo-branch)
Author: Ran Vaknin <username@users.noreply.github.com>
Date:   Tue Nov 14 15:54:17 2023 -0800

    asdasda

commit 487ab836217891ad2229c03209fe3a425a9350c3 (origin/some-foo-branch)
Author: Ran Vaknin <username@users.noreply.github.com>
Date:   Tue Nov 14 15:42:39 2023 -0800

    foo
{
  "jobSummary": {
    "commitId": "HEAD",
    "commitTime": "2023-11-14T23:54:33.752Z",
    "jobArn": "arn:aws:amplify:us-east-1:REDACTED:apps/REDACTED/branches/some-foo-branch/jobs/0000000004",
    "jobId": "4",
    "status": "PENDING"
  }
}

Thanks, Ran~

dkandlyk commented 7 months ago

Hi @RanVaknin ! Thank you for testing this! I have tested it by adding git log to amplify.yml on my preview branch and here is the output from the commit (this is build 4):

                                 # Starting phase: preBuild
2023-11-15T07:58:33.085Z [INFO]: # Executing command: git log
2023-11-15T07:58:33.087Z [INFO]: commit dc23b9b2c36a4712172657bdbe40733f23c789b1
                                 Author: Anders Lykkegaard <——>
                                 Date:   Wed Nov 15 08:52:12 2023 +0100
                                 fix: adding changes to amplify

When I run the reproduction steps, it has removed the git log command and runs the amplify.yml from main/HEAD (this is build 5):

                                 # Starting phase: preBuild
                                 # Executing command: nvm use $VERSION_NODE_18
2023-11-15T08:27:06.747Z [INFO]: Now using node v18.13.0 (npm v8.19.3)

Furthermore, I added some test UI elements and they were removed after the build and now I have changes from main/HEAD which is not merged into my preview branch yet.

My expectation would be that the git log should be the same as before (unless for the time ofc) because there are no code changes only a redeploy.

Lastly, in the build history, the commitId is head and the link goes to main/head instead of preview branch head when clicked

image
RanVaknin commented 7 months ago

Hi @dkandlyk ,

A couple of things I'd like to point out:

  1. I would make sure that chosenBranch is in fact the branch you want to target. You can verify that by logging it before you pass it to StartJobCommand.

  2. It would be really helpful if you can enable request and response logs and share it with me here:

const amplifyClient = new AmplifyClient({ region: "us-east-1" });

amplifyClient.middlewareStack.add(next => async (args) => {
  console.log(args); 
  const response = await next(args);
  console.log(response)
  return response; 
}, { step: 'finalizeRequest' });

I'm not sure what the purpose here of amplify.yml as Im not very familiar with Amplify. I can really only speak to how the SDK should work. By inspecting those logs we can see that if the outbound request specified a branch/commit "X/latest" and instead the upstream resource returned "main/HEAD" then we should really be investigating this with the Amplify service team itself.

Thanks again, Ran~

dkandlyk commented 7 months ago

Hi @RanVaknin ! 1: As you can see in the logs I'm targeting a specific branch which is not main :) 2: I have enabled the middelware and this is the args:

{
  middlewareStack: {
    add: [Function: add],
    addRelativeTo: [Function: addRelativeTo],
    clone: [Function: clone],
    use: [Function: use],
    remove: [Function: remove],
    removeByTag: [Function: removeByTag],
    concat: [Function: concat],
    applyToStack: [Function: cloneTo],
    identify: [Function: identify],
    identifyOnResolve: [Function: identifyOnResolve],
    resolve: [Function: resolve]
  },
  input: { appId: 'secret', branchName: 'pr-844', jobType: 'RELEASE' },
  request: _HttpRequest {
    method: 'POST',
    hostname: 'amplify.eu-west-1.amazonaws.com',
    port: undefined,
    query: {},
    headers: {
      'content-type': 'application/json',
      'content-length': '21',
      host: 'amplify.eu-west-1.amazonaws.com',
      'X-Amzn-Trace-Id': 'Root=1-6557b1a4-761fe38d1ef45a966de0c7fb;Parent=2e5ef30b7b86871e;Sampled=0;Lineage=2e0fd1ff:0',
      'x-amz-user-agent': 'aws-sdk-js/3.445.0',
      'user-agent': 'aws-sdk-js/3.445.0 ua/2.0 os/linux#5.10.196-205.748.amzn2.x86_64 lang/js md/nodejs#18.18.0 api/amplify#3.445.0 exec-env/AWS_Lambda_nodejs18.x',
      'amz-sdk-invocation-id': '3f4c08d9-3ebc-4592-b289-4ce24d065d4f',
      'amz-sdk-request': 'attempt=1; max=3',
      'x-amz-date': '20231117T183206Z',
      'x-amz-security-token': '',
      'x-amz-content-sha256': '',
      authorization: ''
    },
    body: '{"jobType":"RELEASE"}',
    protocol: 'https:',
    path: '/apps/secret/branches/pr-844/jobs',
    username: undefined,
    password: undefined,
    fragment: undefined
  }
}

And here are the response:

{
  response: HttpResponse {
    statusCode: 200,
    reason: 'OK',
    headers: {
      'content-type': 'application/json',
      'content-length': '264',
      connection: 'keep-alive',
      date: 'Fri, 17 Nov 2023 18:32:06 GMT',
      'x-amzn-requestid': '86456e96-82b2-4c77-8310-3f42c27eb180',
      'access-control-allow-origin': '*',
      'access-control-allow-headers': 'Content-Type,X-Amz-Date,Authorization,X-Api-Key,X-Amz-Security-Token',
      'x-amz-apigw-id': 'OjiyCF3DjoEEYpg=',
      'access-control-allow-methods': 'GET,OPTIONS,POST',
      'access-control-expose-headers': 'x-amzn-RequestId,x-amzn-ErrorType',
      'x-amzn-trace-id': 'Root=1-6557b1a4-761fe38d1ef45a966de0c7fb',
      'x-cache': 'Miss from cloudfront',
      via: '1.1 .cloudfront.net (CloudFront)',
      'x-amz-cf-pop': 'DUB56-P1',
      'x-amz-cf-id': ''
    },
    body: IncomingMessage {
      _readableState: [ReadableState],
      _events: [Object: null prototype],
      _eventsCount: 2,
      _maxListeners: undefined,
      socket: null,
      httpVersionMajor: 1,
      httpVersionMinor: 1,
      httpVersion: '1.1',
      complete: true,
      rawHeaders: [Array],
      rawTrailers: [],
      joinDuplicateHeaders: undefined,
      aborted: false,
      upgrade: false,
      url: '',
      method: null,
      statusCode: 200,
      statusMessage: 'OK',
      client: [TLSSocket],
      _consuming: false,
      _dumped: false,
      req: [ClientRequest],
      [Symbol(kCapture)]: false,
      [Symbol(kHeaders)]: [Object],
      [Symbol(kHeadersCount)]: 30,
      [Symbol(kTrailers)]: null,
      [Symbol(kTrailersCount)]: 0
    }
  },
  output: {
    '$metadata': {
      httpStatusCode: 200,
      requestId: '86456e96-82b2-4c77-8310-3f42c27eb180',
      extendedRequestId: undefined,
      cfId: 'vrJZQ96TMvnfp44JAC7zw3_mjwU26iRuPRjcPrZSPv1Z-t3Jc0PX5w=='
    },
    jobSummary: {
      commitId: 'HEAD',
      commitTime: 2023-11-17T18:32:06.390Z,
      jobArn: 'arn:aws:amplify:eu-west-1:secret:apps/secret/branches/pr-844/jobs/0000000014',
      jobId: '14',
      status: 'PENDING'
    }
  }
}

My point was that it pulls a different file than on my target branch :)

RanVaknin commented 7 months ago

Hi @dkandlyk ,

Thanks for the logs.

My point was that it pulls a different file than on my target branch :)

From the request and response I can see that you are requesting the build to be run on branch pr-844 and indeed the response's jobSummary specifies that the jobArn is targeting branch pr-844.

I assume what you meant is that it pulls a file from a different commit rather than a different branch?

In git, HEAD is usually pointing to the latest commit. So when the amplify build tool looks at the git log history, its looking for the commit that HEAD is pointing to. Since the git log output you had there is a bit truncated, I cannot see a reference to HEAD. Based on the git log output you provided, I assume that the commit id you are trying to target is dc23b9b2c36a4712172657bdbe40733f23c789b1. In other words, if head is not pointing to that commit ID, it will not use that commit and instead use HEAD.

This is my understanding of how this works based on what we see in the response.

A speculation: Since I did not use Amplify.yml to list my git log output, I assume there might be a discrepancy between your local git commit history, the upstream git history, and whatever Amplify.yml logs. Compare the output between running locally git log to the history presented on the output from Amplify.yml. And also inspect the upstream repo's (on github or gitlab or whichever tool you have integrated to Amplify) commit history. My guess is that there might be some mis-alignment.

A workaround: When you are running startJob, you can specify the desired commitID. So if you see that the service is pulling HEAD instead of the desired commit ID you can explicitly specify that. I recognize that this is not the desired behavior, but it might unblock you for your particular use case.

As far as continuing to troubleshoot this goes, I think this could be routed to the Amplify service team itself. By inspecting the raw request and response logs we can see that the SDK behaves correctly in that it serializes the request correctly and the unexpected part is the targeted commit which might point to a service side issue, or at least a behavior that the Amplify team itself can better explain.

If the above investigation did not help, I can create an internal ticket on your behalf with the Amplify team and ask them for some more information.

Thanks again, Ran~

dkandlyk commented 7 months ago

Hi @RanVaknin ! Thank you for the thoroughly investigation and guidance :) The problem with the workaround is that we do not have a way to access the Git history and we can't get the information from GetBranchCommand. I would like you to create an internal ticket so the Amplify team can provide me with some extra guidance :) Again thank you for your help! :) Anders

RanVaknin commented 7 months ago

Hi @dkandlyk ,

I have created an internal ticket with the Amplify team. Will update you when I have a response.

Thanks, Ran~

P107055715

RanVaknin commented 6 months ago

Hi @dkandlyk ,

I've received feedback from the Amplify team regarding the issue you're experiencing with the StartJob operation in conjunction with PR previews. Here's the key takeaway:

The Amplify team has acknowledged a bug where using the StartJob command along with enabled PR previews leads to builds being incorrectly based off the target branch. However, they also noted that the simultaneous use of PR previews and manual StartJob hooks is not a recommended practice.

In typical scenarios, if PR previews are enabled and a PR is open, each new commit to that branch should automatically trigger its own deployment, making the manual invocation of StartJob unnecessary. The Amplify team is interested in understanding if there is a specific reason or unique use case behind your approach of using both features together.

If you have particular use case for this setup, please let us know. This information will help the Amplify team understand the context and explore possible solutions or workarounds for your situation.

Thanks, Ran~

dkandlyk commented 6 months ago

Hi @RanVaknin! :) Sorry, for the late response, I have been on Christmas holiday. :) Our use case: We have a mono repo with a Nextjs application. We have created a page where team members can switch environment values to support switching API's and secrets on preview branches. When they change environment values it triggers the StartJob because Nextjs needs the environment values on build time.

So a scenario for us could be a team member who has made changes to a dev API and creates a new preview branch. To test it he switches the environment value to look at the dev API instead of the prod which triggers the StartJob so Nextjs can do a rebuild with the dev API environment value.

So in these cases we do not make a commit we only need a rebuild of the application. :)

Hope it makes sense :) -Anders

RanVaknin commented 2 months ago

I have not heard back from the API team, but I can see that this is on their backlog.

Since this is not actionable by the SDK team, I'm going to go ahead and close this. I would keep an eye on your environment to see if this is being solved automatically (should be a service side fix, so no SDK update will be needed).

If you have access to support via the AWS console, feel free to refer to the ticket ID I created with that team for updates (P107055715).

Thanks again, Ran~

github-actions[bot] commented 2 months ago

This issue is now closed.

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.