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

fix: Updated build success message for hook packages #4351

Merged
merged 5 commits into from
Oct 28, 2022

Conversation

lucashuy
Copy link
Contributor

@lucashuy lucashuy commented Oct 27, 2022

Why is this change necessary?

Changes the build success messages to not mention commands that won't work with current hook packages.

How does it address the issue?

Checks for the hook package and changes which commands are mentioned in the message.

eg when using a hook package:

Build Succeeded

Built Artifacts  : .aws-sam/build
Built Template   : .aws-sam/build/template.yaml

Commands you can use next
=========================
[*] Invoke Function: sam local invoke
[*] Start a local API: sam local start-api --hook-package-id terraform
[*] Emulate local Lambda functions: sam local start-lambda --hook-package-id terraform

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Add input/output type hints to new functions/methods
  • Write design document if needed (Do I need to write a design document?)
  • Write/update unit tests
  • Write/update integration tests
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

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

@github-actions github-actions bot added area/build sam build command pr/internal labels Oct 27, 2022
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.

Thanks Lucas, I juts left one small comment.

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.

thanks Lucas

@moelasmar moelasmar merged commit aae29ef into aws:develop-terraform-hooks Oct 28, 2022
moelasmar pushed a commit that referenced this pull request Oct 31, 2022
* Updated build success message for hook packages

* Fix linting error

* Removed start API suggested command

* Added hook flag to invoke suggestion
mndeveci added a commit that referenced this pull request Nov 11, 2022
* feat: add terraform hook package skeleton (#3976)

* feat: add terraform hook package skeleton

* remove sample dependency

* redirect all output to stderr except hook output

* fix batch script so it actually works when invoked from anywhere

* remove scripts, and replace with entry method

* change to single hooks module with prepare entry method stub

* Hooks wrapper (#3998)

* Add Hook Wrapper

* run black reformat

* Update hook config and tests

* Update hook wrapper and tests

* Update prepare; Add docstrings

* Improve code coverage

* Update package look up logic and remove logs_path param

* Fix lint issue

* Update logic to parse metadata file loc from output in prepare method

* Update json schema

* Update json schema

* feat: add --hook-package-id option to sam local invoke, and sam local start-lambda commands (#4058)

* feat: add hook-package-id option to sam local invoke, and start-lambda commands

* run black

* apply PR comments

* fix lint issues

* fix error message

* feat: add basic prepare hook (#4050)

* test: Terraform support integration testing for SAM Local commands (#4068)

* fix the terraform hook config file to align with the Config schema

* make sure to create the iacs_metadata output directory if it does not exist

* exclude the files from the returned available hooks packages

* update resources with relative paths to be absolute paths by joining them to the terraform application root directory

* sam local commands integration testing

* fix lint issues

* install terraform cli in Appveyor

* testing install terraform on ubuntu

* fix install terraform on ubuntu

* fix install terraform on ubuntu

* remove printing sam local start-lambda command output

* enhance the _update_resources_paths docs

* use osutils.tempfile_platform_independent instead of NamedTemporaryFile to avoid windows issues

* resolve Pr comments

* resolve Pr comments

* fix comment

* feat: Adding Image PackageType lambda function mapping (#4069)

* fix the terraform hook config file to align with the Config schema

* make sure to create the iacs_metadata output directory if it does not exist

* exclude the files from the returned available hooks packages

* update resources with relative paths to be absolute paths by joining them to the terraform application root directory

* sam local commands integration testing

* fix lint issues

* install terraform cli in Appveyor

* testing install terraform on ubuntu

* fix install terraform on ubuntu

* fix install terraform on ubuntu

* remove printing sam local start-lambda command output

* image package type lambda function mapping

* enhance the _update_resources_paths docs

* use osutils.tempfile_platform_independent instead of NamedTemporaryFile to avoid windows issues

* resolve Pr comments

* resolve Pr comments

* fix comment

* apply pr comments

* guard our prepare hook against terraform plan contract changes

* feat: add experimental flag to sam local invoke, and sam local start-lambda commands  (#4070)

* fix the terraform hook config file to align with the Config schema

* make sure to create the iacs_metadata output directory if it does not exist

* exclude the files from the returned available hooks packages

* update resources with relative paths to be absolute paths by joining them to the terraform application root directory

* sam local commands integration testing

* fix lint issues

* install terraform cli in Appveyor

* testing install terraform on ubuntu

* fix install terraform on ubuntu

* fix install terraform on ubuntu

* remove printing sam local start-lambda command output

* image package type lambda function mapping

* feat: add experimental flag to sam local invoke and start-lambda commands

* running black, and fixing lint issues

* enhance the _update_resources_paths docs

* use osutils.tempfile_platform_independent instead of NamedTemporaryFile to avoid windows issues

* resolve Pr comments

* resolve Pr comments

* fix comment

* feat: add experimental flag to sam local invoke and start-lambda commands

* fix merging issue

* fix merging issue

* apply black and lint issues

* handle PR comments

* handle PR comments

* feat: Add custom Makefile path, and custom project root directory to custom builder (#4105)

* feat: Add custom working directory to custom builder (#4112)

* feat: Add custom working directory to custom builder

* apply black

* remove the test cases that depend on the new lambda builders version.

* Add hook_package_id option to sam build (#4136)

* Add hook_package_id option to sam build

* Fix type hint

* feat: script for copying terraform built artifacts (#4117)

* feat: script for copying terraform built artifacts

* Built with the idea that it should work with python2.7+.
* Has a pylint ignore on it because it showcase exceptions for python3
  ways to do things.
* Uses argparse for parsing command line options that are passed to it.
* Extremely basic way in which resolve the specified expression.

* fix: add scripts directory to the pyinstaller hooks.

* tests: add integration tests for copy_terraform_built_artifacts.py
script

* fix: windows subprocess fix

* convert list of commands passed to subprocess to be each converted to
  `str`

* fix: custom `copytree` method

* fix: only join with terraform project root if relative extracted path.

* fix: address comments

* feat: make sure that the output directory, and project root directory paths are absolute paths (#4151)

* feat: make sure that the output directory, and project root directory paths are absolute paths

* apply pr comments

* run black

* feat: collect sam metadata resources to be processed by the enrich method (#4152)

* feat: do metadata resource type property validations (#4153)

* feat: do metadata resource type property validations

* apply pr comments

* run black

* feat: get the cfn resource that match the sam metadata resource name property (#4154)

* feat: get the cfn resource that match the sam metadata resource name property

* apply pr comments

* The sam metadata resource name will be always a postfix to the module address.

* feat: extract the lambda function source code or docker context paths (#4155)

* feat: extract the lambda function source code or docker context paths

* apply pr comments

* feat: enrich zip lambda functions (#4156)

* feat: enrich image lambda functions (#4157)

* chore: refactor enrich_zip_lambda_functions, and enrich_image_lambda_functions outside of enrich_mapped_resources (#4171)

* feat: create stub function for generating custom makefile (#4174)

* fix: make terraform project root arg required, and move script into the terraform hook package dir (#4182)

* Test: add integration test cases to cover Custom build with custom working directory (#4184)

* fix: check for `hook-package-id` only being allowed if a beta feature on `sam build` (#4185)

* fix: check for `hook-package-id` only being allowed if a beta feature on
`sam build`

* fix: add tests for `sam build` with hook package id

* test: add additional unit test

* comments: address comments

* feat: raise error in case of build in container with hooks package and no build image. (#4192)

* feat: raise error in case of build in container with hooks package and no build image.

* fix lint, and black issues

* increase unit testing coverage

* resolve PR comments

* upgrade go version in Appveyor

* feat: implement _enrich_resources_and_generate_makefile (#4187)

* feat: Implement _get_python_command_name (#4210)

* feat: Read lambda layer property from terraform (#4212)

* feat: Read lambda layer property from terraform

* Remove unused file

* Fix test imports

* Add additional unit tests

* Update unit tests with latest changes

* feat: Generate Makefile Rule (#4233)

* Add makefile rule generation

* Update unit test

* Fix Windows test failures

* Fix script call

* Put jpath string in quotes

* Address comments

* Update jpath for child modules, change iacs metadata build dir

* Use os.linesep

* refactor: move hook into (new) prepare dir, and create resource_linking.py file (#4247)

* fix: remove the terraform project root parameter from built artifacts copy script. (#4251)

* fix: remove the terraform project root parameter from built artifacts copy script.

* update the copy script testing

* Fix copy script loc (#4253)

* Fix path of copy script

* remove commented line

* feat: Clean Terraform References List (#4252)

* Function for cleaning layer reference list

* Merge from upstream

* Add test case for for each

* Don't mutate existing reference list

* refactor: delete layer_linking, move to resource_linking (#4255)

* feat: Added configuration address cleaning function (#4248)

* Added configuration address cleaning function

* Fixed spelling mistake

* Added additional test case

* feat: define resource linking data representation structures (#4249)

* Terraform hooks - Adding integration tests for `sam build` (Terraform Hook) with Zip-based Lambda Functions (#4258)

* Add integ test cases for sam build terraform applications zip based lambda function local backend

* Update tearDown to delete terraform state files properly

* Update makefile rule to address an issue that the metadata resource might not be refreshed

* Add integ tests sam build for terraform on s3 backend

* Update to skip s3 backend test in ci runs

* Update SKIP_S3_BACKEND_TESTS

* Update skip flag

* Add logging

* fix the skip condition

* fix the skip condition, plan command needs credentials

* Update skip flag

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* feat: Resolve Module Variables (#4260)

* feat: Resolve module variables

* Address comments

* Update for readability

* Add integ test for sam build terraform image based Lambda Function lo… (#4264)

* Add integ test for sam build terraform image based Lambda Function local backend

* Add test cases for image based LF on s3 backend

* Correct test data path for s3 backend test

* uncomment skip

* feat: Added module output resolving (#4259)

* Added module output resolving

* Address comments

* Change magic constant to use find() instead

* Addressed comments

* Added check for variable type

* Update makefile rule generate to create temp backend override (#4265)

* Update makefile rule generate to create temp backend override

* Revert "Update makefile rule generate to create temp backend override"

This reverts commit d50cc31.

* Add generate backend override file and update make rule

* Update unit test

* Not make it a tf file when creating the override file

* feat: Add resource attribute resolving (#4271)

* feat: Add resource attribute resolving

* Fixing a spelling mistake

* fix spelling mistake

* fix lint issue

* feat: implement function to construct Modules out of the terraform configuration (#4269)

* feat: add _link_lambda_function_to_layer function stub (#4280)

* add _link_lambda_function_to_layer function stub

* fix lint error

* feat: implement linking lambda layers to function (#4284)

* feat: implement linking lambda layers to function

* fix the PR comments

* apply the PR comments

* apply the PR comments

* apply the PR comments

* apply the PR comments

* feat: Add logic for linking functions to layers (#4295)

* Add logic for linking functions to layers

* Add unit tests

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* test: Add integration test cases for building terraform projects with lambda layers support (#4311)

* fix resolving layers for lambda functions that does not have layers attribute

* skip mapping the resource attributes of type dictionary to the TF Resource object. We do not need these resources for now as we do not process any of these resources during the lambda - layer linking

* fix dealing with attributes that customer defined by null in the terraform project

* add build terraform with layers integration testing

* apply pr comments

* test: Add integration test cases for local testing terraform projects with lambda layers support (#4309)

* fix resolving layers for lambda functions that does not have layers attribute

* skip mapping the resource attributes of type dictionary to the TF Resource object. We do not need these resources for now as we do not process any of these resources during the lambda - layer linking

* add sam local invoke testcases for lambda layer support

* add sam local start-lambda testcases for lambda layer support

* run black

* fix lint

* undo committed debugging code

* rerun black

* apply pr comments

* feat: Update client-side telemetry (#4323)

* Update client-side telemetry

* Address comments

* Address comments

* fix: Update logging and removed tree command in test file (#4322)

* Update logging and removed tree command in test file

* Removed redundant code

* Removed pwd command

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* Update make rule to use one python script to support both Windows and… (#4316)

* Update make rule to use one python script to support both Windows and MacOS/Linux

* Address feedback (copy_terraform_built_artifacts.py)

* address feedback (hook.py)

* (temp) add windows test case

* Update integ test to work on both Linux and Windows

* update copy script tests

* fix missing IS_WINDOWS import

* Handle OSError while check python executable

* update testdata for windows

* black reformat

* omit copy_terraform_built_artifacts.py in unit test coverage requirement

* Remove unused code in testdata py files

* fix some errors based on the merge conflicts

* feat: link the lambda resources to the correct s3_object in case if the s3 bucket data are not available in the planned values (#4325)

* feat: link the lambda resources to the correct s3_object in case if the s3 bucket data are not available in the planned values

* apply pr comments

* fix unit testing issues

* fix metadata file generation performance issue (#4334)

* fix: remove the dependency on six module (#4340)

* fix: building terraform projects in container (#4344)

* fix: building terraform projects in container

* fix mypy issue

* apply PR comments

* fix the testing failures

* fix: Updated build success message for hook packages (#4351)

* Updated build success message for hook packages

* Fix linting error

* Removed start API suggested command

* Added hook flag to invoke suggestion

* docs: ignore pycache when listing available hook package ids in help text (#4349)

* docs: ignore pycache when listing available hook package ids in help text

* check if first char is alphabetic

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* test: add integration test cases for serverless tf module (#4354)

* Integration Test Cases for Unsupported TF cases (#4343)

* Add integ test for unsupported cases

* Update error message

* Add test case for unsupport local var layer

* fix black format issue

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* feat: error out if input is required on terraform init (#4370)

* fix: Ignore template option when using hook package (#4371)

* Ignore template option when using hook package

* Added unit tests for _gen_success_msg

* fix the unit testing in windows

* fix unit testing

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* fix: skip processing references to Data sources resources, and constant values. (#4366)

* fix: skip processing references to Data sources resources.

* run black

* apply pr comments

* Fix escape double quotes in generating Make rule (#4368)

* Fix escape double quotes in generating Make rule

* black reformat

* Remove unnecessary commented code

* Update integration test to test for_each

* fix: make the experimental warning message printed before the prepare hook logic (#4365)

* fix: make the experimental warning message printed before the prepare hook logic

* apply PR comments

* apply pr comments

* support reading the beta-features flag form the environment variable and toml config

* apply pr comments

* fix testing failures

* enable the debugging for prepare hook

* fix the experimental warning message testing

* fix the experimental warning message testing

* fix the experimental warning message testing

* fix the experimental warning message testing

* feat: link sam metadata resource to lambda resource if there is no resource_name attribute (#4353)

* feat: link sam metadata resource to lambda resource if there is no resource_name attribute

* apply pr comments

* apply pr comments

* version: bring in latest lambda builders version (#4380)

- run integ tests on this branch once brought in.

* rename hook-package-id option to hook-name (#4382)

* test: increase terraform sample functions timeout (#4384)

* Increase the Lambda functions timeout

* Copy the TF project to the temp directory to allow parallel testing executions

* apply black

* feat: Run subprocess with loading pattern (#4379)

* Run subprocess with loading pattern

* Stream stdout if logging level is set to debug

* Address comments

* Update unit tests

* Use os.linesep

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* feat: add hook metadata to generated template and use for telemetry (#4378)

* feat: add hook metadata to generated template and use for telemetry

* remove unused import

* fix metadata key

* fix unit test

* address review feedback

* remove unused import

* rename hook package id to hook name in new metadata field

* fix testing after merge

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* feat: Added --skip-prepare-infra to skip the prepare hook stage (#4377)

* Added --skip-prepare-iac option

* Updated and added more tests

* Added flag to invoke and start-lambda

* Moved skip prepare to terraform hook

* Added checks for default_map

* Changed hook package to hook name and remove LOG assert

* Added log message if we provided --skip-prepare-infra but have no metadata file

* Removed lingering extra decorator from earlier merge

Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>

* feat: raise error for terraform expression that mix between constant values and references  (#4387)

* raise OneLambdaLayerLinkingLimitationException exception if the function is referring to a lyer using mix of constant and references

* apply black

* chore: add terraform Appveyor testing jobs (#4386)

* add Terraform testing jobs

* raise OneLambdaLayerLinkingLimitationException exception if the function is referring to a lyer using mix of constant and references

* fix: run terraform testing in sequence

* split the terraform build integration test cases in 2 files

* run terraform build integration testing in parallel

* update the building job

* fix merge issues

* test: use Serverless TF module from terraform registry (#4388)

* Ignore ResourceWarning in pytest (#4392)

* Ignore ResourceWarning in pytest

* Keep showing warning but not raise as error

* Updated help string message (#4394)

* chore: Add issue links (#4397)

* chore: Merge develop to terraform (#4398)

* feat: updating app templates repo hash with (ef97aef12ebad99ffd8e14ed7abf9549007cb1aa) (#4381)

Co-authored-by: GitHub Action <action@github.com>

* exclude installer dir in build (#4389)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

* fix: fix integration testing (#4391)

* raise OneLambdaLayerLinkingLimitationException exception if the function is referring to a lyer using mix of constant and references

* add Terraform testing jobs

* test: use Serverless TF module from terraform registry

* apply black

* fix: fix integration testing issues

* fix: fix integration testing issues

* fix: fix linux build script

* fix: run terraform testing in sequence

* fix: fix linux build script

* fix: fix linux build script

* split the terraform build integration test cases in 2 files

* add logging to check the invoke failure

* run terraform build integration testing in parallel

* add missing artifacts

* update the building job

* fix docker login in windows

* fix docker login in windows

* fix docker login in windows

* fix docker login in windows

* fix docker login in windows

* fix docker login in windows

* fix docker login in windows

* fix docker login in windows

* fix windows build script

* remove Serverless TF modules from windows TF example as it is not supported in windows

* fix windows python build scripts

* fix writing makefile in windows

* fix black & unit testing

* fix black & unit testing

* fix black & unit testing

* fix black & unit testing

* fix s3 terraform testing in windows

* fix terraform testing in windows

* fix terraform testing in windows

* fix terraform testing in windows

* fix terraform testing in windows

* fix terraform testing in windows

* fix terraform testing in windows

* test os environment variables

* test os environment variables

* fix terraform integration testing

* fix terraform integration testing

* fix terraform integration testing

* fix terraform integration testing

* chore: add terraform Appveyor testing jobs (#4386)

* add Terraform testing jobs

* raise OneLambdaLayerLinkingLimitationException exception if the function is referring to a lyer using mix of constant and references

* fix: run terraform testing in sequence

* split the terraform build integration test cases in 2 files

* run terraform build integration testing in parallel

* update the building job

* fix merge issues

* Revert "fix terraform testing in windows"

This reverts commit a0d2fc9

* Revert "fix terraform integration testing"

This reverts commit be3a233

* fix build in container windows

* run terraform testing in sequence

* fix unit testing issue

* split the terraform integration testing in 2 modules

* fix windows local testing issue

* fix windows local testing issue

* complete hook package id rename

* revert docker login change, and run build integration testing in parallel

* use Pure Path instead of manually replace \ to / in windows paths

* remove the region config from the tf projects

* revert back the progress bas to be printed into stdout

* add comments

* remove the powershell command to login to public ecr docker

Co-authored-by: moelasmar <eng.mohamed.asmar\2gmail.com>

* chore: merge develop into develop-terraform-hooks (#4399)

* feat: updating app templates repo hash with (ef97aef12ebad99ffd8e14ed7abf9549007cb1aa) (#4381)

Co-authored-by: GitHub Action <action@github.com>

* exclude installer dir in build (#4389)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

* chore: merge from develop (#4400)

* feat: updating app templates repo hash with (ef97aef12ebad99ffd8e14ed7abf9549007cb1aa) (#4381)

Co-authored-by: GitHub Action <action@github.com>

* exclude installer dir in build (#4389)

* chore: bump version to 1.62.0 (#4393)

Co-authored-by: Lau <lauwing@f8ffc25e8e59.ant.amazon.com>

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Lau <lauwing@f8ffc25e8e59.ant.amazon.com>

* chore: run sam build in container integration test cases for terraform projects in sequence (#4401)

* fix the docker login command

Co-authored-by: Ruperto Torres <86501267+torresxb1@users.noreply.github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Co-authored-by: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com>
Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com>
Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Lau <lauwing@f8ffc25e8e59.ant.amazon.com>
Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build sam build command pr/internal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants