Skip to content

Conversation

@Harleenijjar
Copy link
Contributor

@Harleenijjar Harleenijjar commented Nov 17, 2022

Issue #, if available:
Bug: sam build --use-container doesn't correctly build golang containers #3894
Unable to build go files using --use-container when go files are in different directory than the main file because it only mounts one directory which is the codeUri to the docker container. This implies that only the files in that directory are found when using a container. Works for non container builds because no mounting is required.

Description of changes:
Added a method to build go files while using a container when go files are in a different directory than the main files. Method is called when sam build is unable to find the files which will imply that the files are in a different directory. It will run this method and be able to find the main.go files.Therefore, Mounting the parent directory with all the files. This implies the CodeUri must be the parent directory and handler must be the path to the main.go file. This way the build command will be able to find the go files in the parent directory plus follow the path to the main.go file.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Harleenijjar Harleenijjar requested a review from a team as a code owner November 17, 2022 19:14
@github-actions github-actions bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. area/workflow/go_modules labels Nov 17, 2022
Copy link
Contributor

@mildaniel mildaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Looks good overall. I left a few nit-picky comments and a question.

Can we also just add an integration test case which covers the customer's specific use-case to make sure we don't do anything to break that in the future.

@moelasmar
Copy link
Contributor

moelasmar commented Nov 18, 2022

Thanks @Harleenijjar for the contribution.
I think there is some issue with this approach. In the linked issue, the customer did not add the go.mod in the same directory as the main.go file, but in a parent directory.

so, when customer executes sam build, SAM CLI will execute go build command in the code_uri directory, and go command will find the go.mod file in the parent directory. But, when customer executes sam build --use-container, SAM CLI will mount only the code_uri to the created container, so when SAM CLI executes go build command, go will not find the go.mod file anywhere, as SAM CLI does not mount the parent directories.

so I think using the Handler as a location to run go build command will not solve the problem, as actually the Handler is not a location or a directory name, it can be any name the customer choose. The handler is the name of the generated executable file created by go build command.

I think the solution is we need the customer to provide a base directory to be mounted to the container, and then we can normally run the go build command from the code_uri path. We can suggest adding a new metadata info to be added to the Lambda function to refer to the base directory where the source code is defined, and also the module files.

Copy link
Contributor

@moelasmar moelasmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left my comment in the conversation

Copy link
Contributor

@mndeveci mndeveci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I would recommend running make pr to find the errors for the CI jobs that is failing now.

@mildaniel
Copy link
Contributor

This PR also addresses this issue: aws/aws-sam-cli#2844

@Harleenijjar Harleenijjar requested review from mildaniel and mndeveci and removed request for mildaniel December 7, 2022 18:11
Copy link
Contributor

@mildaniel mildaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates!

@mndeveci mndeveci changed the title Builds for Go files in a different directory feat: builds for Go files in a different directory Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/workflow/go_modules pr/internal stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants