SAM calls that use AWS resources now correctly use an AWS profile #554

bryceitoc9 · 2019-05-10T21:35:37Z

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Infrastructure (change which improves the lifecycle management (CI/CD, build, package, deploy, lint, etc) of the application, but does not change functionality.)
Technical debt (change which improves the maintainability of the codebase, but does not change functionality)
Testing (change which modifies or adds test coverage, but does not change functionality)
Documentation (change which modifies or adds documentation, but does not change functionality)

Description

First crack at a fix for #553 . Adds the currently-immediately active AWS profile's credentials to SAM calls that interact with AWS resources.

Consierations:

By 'currently-immediately active', I mean that if you swap the profile while the workflow is already running, whatever the next step is will use the currently-set profile. If we do not like this behavior, we can make the changes to the SAM Deploy workflow instead Implemented
~~Should we be using the AWS Context instead of directly pulling from the settingsConfiguration? What's the point in having a specific getter for this value if we're not going to use it?~~ Yes we should. Implemented.

Motivation and Context

Functions would only deploy to the default AWS CLI profile. Additionally, functions would be able to deploy without setting credentials in VS Code at all if a default CLI profile existed.

Related Issue(s)

#553

Testing

Tested the same deployment against a profile that does have valid credentials and one that doesn't. Confirmed that the deployments worked as expected.

Screenshots (if appropriate)

Checklist

I have read the README document
I have read the CONTRIBUTING document
My code follows the code style of this project
I have added tests to cover my changes
All new and existing tests passed
A short description of the change has been added to the CHANGELOG

License

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

bryceitoc9 · 2019-05-10T22:23:15Z

src/lambda/commands/deploySamApplication.ts

+                }
+            )
+
+            restParams.outputChannel.appendLine(localize(


Should we consider moving these calls to a channelLogger?

Seems reasonable, but not necessary as part of this PR

src/test/shared/sam/cli/samCliDeploy.test.ts

package.nls.json

src/lambda/commands/deploySamApplication.ts

awschristou

I like that the messages now include profile name. Good catch!

src/lambda/commands/deploySamApplication.ts

awschristou · 2019-05-12T18:39:35Z

src/lambda/commands/deploySamApplication.ts

+                }
+            )
+
+            restParams.outputChannel.appendLine(localize(


Seems reasonable, but not necessary as part of this PR

src/lambda/commands/deploySamApplication.ts

awschristou · 2019-05-12T18:42:35Z

src/lambda/commands/deploySamApplication.ts

+                'AWS.samcli.deploy.workflow.error',
+                'Failed to deploy SAM application. Error while {0}: {1}',
+                stage, String(err)
+            )
            // tslint:disable-next-line:max-line-length
            // detect error message from https://github.com/aws/aws-cli/blob/4ff0cbacbac69a21d4dd701921fe0759cf7852ed/awscli/customizations/cloudformation/exceptions.py#L42
            // and append region to assist in troubleshooting the error
            // (command uses CLI configured value--users that don't know this and omit region won't see error)
            if (msg.includes(`aws cloudformation describe-stack-events --stack-name ${args.stackName}`)) {


Consider moving all msg decoration out to a separate method.

awschristou · 2019-05-12T18:44:22Z

src/shared/sam/cli/samCliPackage.ts

        const { exitCode, error, stderr, stdout }: ChildProcessResult = await this.invoker.invoke(
            'package',
            '--template-file', this.templateFile,
            '--s3-bucket', this.s3Bucket,
            '--output-template-file', this.outputTemplateFile,
-            '--region', this.region
+            '--region', this.region,
+            '--profile', this.profile


samCliLocalInvoke.test.ts contains unit tests that verify we pass expected values to the invoker.invoke call.
Add a similar one for sam cli package to test profile. Tests can be backfilled for existing untested code later, but incremental changes that can be tested should be tested.

src/test/shared/sam/cli/samCliDeploy.test.ts

src/lambda/commands/deploySamApplication.ts

codecov-io · 2019-05-13T16:59:22Z

Codecov Report

Merging #554 into develop will decrease coverage by 0.05%.
The diff coverage is 23.52%.

@@             Coverage Diff             @@
##           develop     #554      +/-   ##
===========================================
- Coverage    53.09%   53.04%   -0.06%     
===========================================
  Files          107      107              
  Lines         4211     4221      +10     
  Branches       643      645       +2     
===========================================
+ Hits          2236     2239       +3     
- Misses        1975     1982       +7

Impacted Files	Coverage Δ
src/shared/sam/cli/samCliPackage.ts	`12.5% <0%> (-0.55%)`	⬇️
src/lambda/lambdaTreeDataProvider.ts	`47.36% <0%> (ø)`	⬆️
src/shared/sam/cli/samCliDeploy.ts	`70% <100%> (+1.03%)`	⬆️
src/lambda/commands/deploySamApplication.ts	`25% <18.18%> (-1.09%)`	⬇️
src/shared/awsContext.ts	`75% <50%> (-25%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e6954d...719ef0a. Read the comment docs.

src/lambda/commands/deploySamApplication.ts

…le (#554)' from develop.

…y. (#599) * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

Due to a series of misteps, we are squashing this entire feature branch into develop. 1. We allowed `feature/dotnet` to drift too far from `develop`, which required significant manual conflict resolution to resolve. 2. We should have squashed the conflict-resolution history *before* merging develop, but we did not. Therefore in order to squash the pre-merge commits, we would need to recreate the merge and re-resolve conflicts. This would reduce our confidence in the quality of the branch. In the future: 1. We will regularly merge into feature branches to prevent drift. 2. If we do encounter a situation that involves manually porting, we will squash the ports *before* merging develop. * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

SAM calls that use AWS resources now correctly use an AWS profile

8d28b50

bryceitoc9 requested review from mpiroc and awschristou May 10, 2019 21:35

Profile is verified on SAM deploy invocation, additional housekeeping

ebbe28b

bryceitoc9 commented May 10, 2019

View reviewed changes

src/test/shared/sam/cli/samCliDeploy.test.ts Show resolved Hide resolved

mpiroc suggested changes May 10, 2019

View reviewed changes

package.nls.json Outdated Show resolved Hide resolved

src/lambda/commands/deploySamApplication.ts Outdated Show resolved Hide resolved

awschristou requested changes May 12, 2019

View reviewed changes

awschristou reviewed May 12, 2019

View reviewed changes

src/lambda/commands/deploySamApplication.ts Outdated Show resolved Hide resolved

Using context instead of going straight to the SettingsConfiguration

9d07f2c

awschristou requested changes May 13, 2019

View reviewed changes

src/lambda/commands/deploySamApplication.ts Outdated Show resolved Hide resolved

Responding to feedback

fc510cc

mpiroc approved these changes May 15, 2019

View reviewed changes

bryceitoc9 mentioned this pull request May 16, 2019

Update Toolkit with changes to credentials file #168

Closed

Added test to samCliPackage and minor refactoring

719ef0a

awschristou approved these changes May 16, 2019

View reviewed changes

mpiroc approved these changes May 16, 2019

View reviewed changes

bryceitoc9 merged commit 28ce37b into develop May 16, 2019

bryceitoc9 deleted the bryceito/sam-with-credentials branch May 16, 2019 20:21

mpiroc added a commit that referenced this pull request May 29, 2019

Port 'SAM calls that use AWS resources now correctly use an AWS profi…

e116cd0

…le (#554)' from develop.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAM calls that use AWS resources now correctly use an AWS profile #554

SAM calls that use AWS resources now correctly use an AWS profile #554

bryceitoc9 commented May 10, 2019 •

edited

Loading

bryceitoc9 May 10, 2019

awschristou May 12, 2019

awschristou left a comment

awschristou May 12, 2019

awschristou May 12, 2019

awschristou May 12, 2019

bryceitoc9 May 13, 2019

codecov-io commented May 13, 2019 •

edited

Loading

SAM calls that use AWS resources now correctly use an AWS profile #554

SAM calls that use AWS resources now correctly use an AWS profile #554

Conversation

bryceitoc9 commented May 10, 2019 • edited Loading

Types of changes

Description

Consierations:

Motivation and Context

Related Issue(s)

Testing

Screenshots (if appropriate)

Checklist

License

bryceitoc9 May 10, 2019

Choose a reason for hiding this comment

awschristou May 12, 2019

Choose a reason for hiding this comment

awschristou left a comment

Choose a reason for hiding this comment

awschristou May 12, 2019

Choose a reason for hiding this comment

awschristou May 12, 2019

Choose a reason for hiding this comment

awschristou May 12, 2019

Choose a reason for hiding this comment

bryceitoc9 May 13, 2019

Choose a reason for hiding this comment

codecov-io commented May 13, 2019 • edited Loading

Codecov Report

bryceitoc9 commented May 10, 2019 •

edited

Loading

codecov-io commented May 13, 2019 •

edited

Loading