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

Uploaded file must be a non-empty zip is a problem for many recent versions #1017

Open
SamStephens opened this issue Jan 8, 2025 · 7 comments
Labels
bug Something isn't working needs-investigation

Comments

@SamStephens
Copy link
Contributor

This issue was discussed extensively via #478. The issue was closed because "Later versions should work. We are planning on replacing the current approach with something else.".

However, there is no clear timeframe for what will replace this, and in the meantime, this issue is getting worse. I just tried versions 3.1.1 to 3.1.4, and all of them suffered the issue.

The root cause of this issue appears to be the logic that allows a prebuilt version of the Lambda to be downloaded from Github instead of built locally, and indeed these versions work when I set the environment variable NO_PREBUILT_LAMBDA=1.

My suggestion is still to rip out the prebuilt lambda functionality entirely, or at least disable it by default. I think that recommendation is even stronger if this package is to be replaced, as clearly the underlying issues with the prebuilt lambda will not be fixed.

@mrgrain
Copy link
Contributor

mrgrain commented Jan 8, 2025

Thanks for reporting!

I just tried versions 3.1.1 to 3.1.4, and all of them suffered the issue.

Hmm, that's interesting and annoying. I can see that all versions from 3.1.1 to 3.1.4 have the bootstrap files published. This indicates to me a different cause than the previous issue(s). I also cannot replicate it, whereas with the old root cause I could.

Could you check two more versions for me?

I'd like to rule out/in if the new feature in 3.1.0 could be the culprit.

Also can you post the exact error logs. Assuming they are similar to the previously posted logs, there might still be subtle differences that give us a hint.

My suggestion is still to rip out the prebuilt lambda functionality entirely, or at least disable it by default. I think that recommendation is even stronger if this package is to be replaced, as clearly the underlying issues with the prebuilt lambda will not be fixed.

Yes. This is the same as #222. We don't currently have a timeline for this.

@mrgrain mrgrain added bug Something isn't working needs-investigation labels Jan 8, 2025
@SamStephens
Copy link
Contributor Author

@mrgrain I've just tried that version and 3.0.154 is okay and 3.1.0 is not, so it looks like your new 3.1.0 feature could be the issue.

Here's my deploy logs:

LibreofficeImageStack: deploying... [1/1]
LibreofficeImageStack: creating CloudFormation changeset...
LibreofficeImageStack | 0/4 | 11:22:33 AM | UPDATE_IN_PROGRESS   | AWS::CloudFormation::Stack  | LibreofficeImageStack User Initiated
LibreofficeImageStack | 0/4 | 11:22:37 AM | UPDATE_IN_PROGRESS   | AWS::Lambda::Function       | Custom::CDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB (CustomCDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB28EAD8E4)
LibreofficeImageStack | 0/4 | 11:22:38 AM | UPDATE_FAILED        | AWS::Lambda::Function       | Custom::CDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB (CustomCDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB28EAD8E4) Resource handler returned message: "Uploaded file must be a non-empty zip (Service: Lambda, Status Code: 400, Request ID: c4031487-301b-4699-87bd-eca04b58228c)" (RequestToken: 84f35085-f417-b949-1d31-2ed615c660b1, HandlerErrorCode: InvalidRequest)
LibreofficeImageStack | 0/4 | 11:22:38 AM | UPDATE_ROLLBACK_IN_P | AWS::CloudFormation::Stack  | LibreofficeImageStack The following resource(s) failed to update: [CustomCDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB28EAD8E4].
LibreofficeImageStack | 0/4 | 11:22:41 AM | UPDATE_IN_PROGRESS   | AWS::Lambda::Function       | Custom::CDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB (CustomCDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB28EAD8E4)
LibreofficeImageStack | 1/4 | 11:22:54 AM | UPDATE_COMPLETE      | AWS::Lambda::Function       | Custom::CDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB (CustomCDKECRDeploymentbd07c930edb94112a20f03f096f53666512MiB28EAD8E4)
LibreofficeImageStack | 2/4 | 11:22:54 AM | UPDATE_ROLLBACK_COMP | AWS::CloudFormation::Stack  | LibreofficeImageStack
LibreofficeImageStack | 3/4 | 11:22:55 AM | UPDATE_ROLLBACK_COMP | AWS::CloudFormation::Stack  | LibreofficeImageStack

@SamStephens
Copy link
Contributor Author

@mrgrain

My suggestion is still to rip out the prebuilt lambda functionality entirely, or at least disable it by default. I think that recommendation is even stronger if this package is to be replaced, as clearly the underlying issues with the prebuilt lambda will not be fixed.

Yes. This is the same as #222. We don't currently have a timeline for this.

Not quite the same, I think. What I'm suggesting is a smaller task, simply taking out all the pre-built functionality, and making the build work the way it does today when NO_PREBUILT_LAMBDA=1 is set. #222 is a more substantial rethink of how this repository deals with that lambda, to address how long builds with NO_PREBUILT_LAMBDA=1 take. If #222 were implemented, it would supersede what I'm suggesting. But I think there's value in having this issue open too, as a more tactical option. Apart from anything else, simply ripping out the prebuilt functionality is something a consumer of this library could contribute if the pain got too great, whereas #222 takes more design and would be harder to implement without AWS involvement.

When you say "We are planning on replacing the current approach with something else." in #478 (comment), are you referring to #222, or to this library being replaced entirely?

@mrgrain
Copy link
Contributor

mrgrain commented Jan 13, 2025

When you say "We are planning on replacing the current approach with something else." in #478 (comment), are you referring to #222, or to this library being replaced entirely?

I was referring to us being aware that the pre-built lambdas are a pain, i.e. #222

Long-term, we believe publishing an image directly to a named ECR (instead of a asset ECR first and then copying the image) is something that is worth exploring for core AWS CDK. However that wouldn't necessarily capture all use cases of this package anyway.

@mrgrain
Copy link
Contributor

mrgrain commented Jan 13, 2025

@mrgrain I've just tried that version and 3.0.154 is okay and 3.1.0 is not, so it looks like your new 3.1.0 feature could be the issue.

Thanks for confirming. This line is the only thing on the PR that vaguely has anything to do with it. But I don't really understand yet how and why this would effect anything. 🤔

https://github.com/cdklabs/cdk-ecr-deployment/pull/961/files#diff-bc37d034bad564583790a46f19d807abfe519c5671395fd494d8cce506c42947R35https://github.com/cdklabs/cdk-ecr-deployment/pull/961/files#diff-bc37d034bad564583790a46f19d807abfe519c5671395fd494d8cce506c42947R35

@SamStephens
Copy link
Contributor Author

@mrgrain I don't understand either. And actually, it looks like something a little different to #478 may be going on, because if you look at the releases, they include bootstrap and bootstrap.sha256, whereas the broken releases #478 refers to do not include those files (which is why they were broken).

@knapp42
Copy link

knapp42 commented Jan 22, 2025

I am having the same issue. I will stick with 3.0.154 for now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-investigation
Projects
None yet
Development

No branches or pull requests

3 participants