Skip to content

fix(bug): Pass boto_session to SAM Translator library #2759

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.

Sign up for GitHub

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

Merged
jfuss merged 4 commits into from
Apr 19, 2021
Merged

fix(bug): Pass boto_session to SAM Translator library #2759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
444bc52
fix(bug): Pass boto_session to SAM Translator library
jfuss Mar 18, 2021
58a774b
Merge branch 'develop' into issues/442
mgrandis Apr 13, 2021
a092f10
Merge branch 'develop' into issues/442
jfuss Apr 16, 2021
902288e
Merge branch 'develop' into issues/442
jfuss Apr 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions samcli/commands/validate/lib/sam_template_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from samtranslator.public.exceptions import InvalidDocumentException
from samtranslator.parser import parser
from samtranslator.translator.translator import Translator
from boto3.session import Session

from samcli.lib.utils.packagetype import ZIP
from samcli.yamlhelper import yaml_dump
Expand All @@ -16,7 +17,7 @@


class SamTemplateValidator:
def __init__(self, sam_template, managed_policy_loader):
def __init__(self, sam_template, managed_policy_loader, profile=None, region=None):
"""
Construct a SamTemplateValidator

Expand All @@ -40,6 +41,7 @@ def __init__(self, sam_template, managed_policy_loader):
self.sam_template = sam_template
self.managed_policy_loader = managed_policy_loader
self.sam_parser = parser.Parser()
self.boto3_session = Session(profile_name=profile, region_name=region)

def is_valid(self):
"""
Expand All @@ -53,7 +55,12 @@ def is_valid(self):
"""
managed_policy_map = self.managed_policy_loader.load()

sam_translator = Translator(managed_policy_map=managed_policy_map, sam_parser=self.sam_parser, plugins=[])
sam_translator = Translator(
managed_policy_map=managed_policy_map,
sam_parser=self.sam_parser,
plugins=[],
boto_session=self.boto3_session,
)

self._replace_local_codeuri()

Expand Down
11 changes: 10 additions & 1 deletion samcli/commands/validate/validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from botocore.exceptions import NoCredentialsError
import click

from samtranslator.translator.arn_generator import NoRegionFound

from samcli.cli.main import pass_context, common_options as cli_framework_options, aws_creds_options, print_cmdline_args
from samcli.commands._utils.options import template_option_without_build
from samcli.lib.telemetry.metric import track_command
Expand Down Expand Up @@ -49,13 +51,20 @@ def do_cli(ctx, template):
sam_template = _read_sam_file(template)

iam_client = boto3.client("iam")
validator = SamTemplateValidator(sam_template, ManagedPolicyLoader(iam_client))
validator = SamTemplateValidator(
sam_template, ManagedPolicyLoader(iam_client), profile=ctx.profile, region=ctx.region
)

try:
validator.is_valid()
except InvalidSamDocumentException as e:
click.secho("Template provided at '{}' was invalid SAM Template.".format(template), bg="red")
raise InvalidSamTemplateException(str(e)) from e
except NoRegionFound as no_region_found_e:
raise UserException(
"AWS Region was not found. Please configure your region through a profile or --region option",
Copy link
Contributor

@sriram-mv sriram-mv Apr 13, 2021

Choose a reason for hiding this comment

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

should we add AWS_REGION env var too?

Copy link
Contributor Author

@jfuss jfuss Apr 14, 2021

Choose a reason for hiding this comment

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

I am not sold on adding AWS_DEFAULT_REGION. Yes customers can set this as it is resolved by boto3, but I don't see the need to add every single option here. Most customers are going to use --region or define them through a profile locally.

Is there a specific reason why you think we should add it?

Copy link
Contributor

@sriram-mv sriram-mv Apr 15, 2021

Choose a reason for hiding this comment

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

does sam validate automatically pick up the env vars? just trying to see if we can reduce friction where we can.

Copy link
Contributor Author

@jfuss jfuss Apr 16, 2021

Choose a reason for hiding this comment

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

Yes. We create a boto_session like we do in other places throughout. When None is passed, boto will resolve for us.

wrapped_from=no_region_found_e.__class__.__name__,
) from no_region_found_e
except NoCredentialsError as e:
raise UserException(
"AWS Credentials are required. Please configure your credentials.", wrapped_from=e.__class__.__name__
Expand Down
16 changes: 8 additions & 8 deletions tests/functional/commands/validate/lib/test_sam_template_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def test_valid_template(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
Expand All @@ -56,7 +56,7 @@ def test_invalid_template(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

with self.assertRaises(InvalidSamDocumentException):
validator.is_valid()
Expand All @@ -76,7 +76,7 @@ def test_valid_template_with_local_code_for_function(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
Expand All @@ -93,7 +93,7 @@ def test_valid_template_with_local_code_for_layer_version(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
Expand All @@ -113,7 +113,7 @@ def test_valid_template_with_local_code_for_api(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
Expand All @@ -133,7 +133,7 @@ def test_valid_template_with_DefinitionBody_for_api(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
Expand Down Expand Up @@ -165,7 +165,7 @@ def test_valid_template_with_s3_object_passed(self):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
Expand All @@ -187,7 +187,7 @@ def test_valid_api_request_model_template(self, template_path):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"PolicyName": "FakePolicy"}

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, region="us-east-1")

# Should not throw an exception
validator.is_valid()
21 changes: 16 additions & 5 deletions tests/unit/commands/validate/lib/test_sam_template_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,41 +9,50 @@


class TestSamTemplateValidator(TestCase):
@patch("samcli.commands.validate.lib.sam_template_validator.Session")
@patch("samcli.commands.validate.lib.sam_template_validator.Translator")
@patch("samcli.commands.validate.lib.sam_template_validator.parser")
def test_is_valid_returns_true(self, sam_parser, sam_translator):
def test_is_valid_returns_true(self, sam_parser, sam_translator, boto_session_patch):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"policy": "SomePolicy"}
template = {"a": "b"}

parser = Mock()
sam_parser.Parser.return_value = parser

boto_session_mock = Mock()
boto_session_patch.return_value = boto_session_mock

translate_mock = Mock()
translate_mock.translate.return_value = {"c": "d"}
sam_translator.return_value = translate_mock

validator = SamTemplateValidator(template, managed_policy_mock)
validator = SamTemplateValidator(template, managed_policy_mock, profile="profile", region="region")

# Should not throw an Exception
validator.is_valid()

boto_session_patch.assert_called_once_with(profile_name="profile", region_name="region")
sam_translator.assert_called_once_with(
managed_policy_map={"policy": "SomePolicy"}, sam_parser=parser, plugins=[]
managed_policy_map={"policy": "SomePolicy"}, sam_parser=parser, plugins=[], boto_session=boto_session_mock
)
translate_mock.translate.assert_called_once_with(sam_template=template, parameter_values={})
sam_parser.Parser.assert_called_once()

@patch("samcli.commands.validate.lib.sam_template_validator.Session")
@patch("samcli.commands.validate.lib.sam_template_validator.Translator")
@patch("samcli.commands.validate.lib.sam_template_validator.parser")
def test_is_valid_raises_exception(self, sam_parser, sam_translator):
def test_is_valid_raises_exception(self, sam_parser, sam_translator, boto_session_patch):
managed_policy_mock = Mock()
managed_policy_mock.load.return_value = {"policy": "SomePolicy"}
template = {"a": "b"}

parser = Mock()
sam_parser.Parser.return_value = parser

boto_session_mock = Mock()
boto_session_patch.return_value = boto_session_mock

translate_mock = Mock()
translate_mock.translate.side_effect = InvalidDocumentException([Exception("message")])
sam_translator.return_value = translate_mock
Expand All @@ -54,8 +63,10 @@ def test_is_valid_raises_exception(self, sam_parser, sam_translator):
validator.is_valid()

sam_translator.assert_called_once_with(
managed_policy_map={"policy": "SomePolicy"}, sam_parser=parser, plugins=[]
managed_policy_map={"policy": "SomePolicy"}, sam_parser=parser, plugins=[], boto_session=boto_session_mock
)

boto_session_patch.assert_called_once_with(profile_name=None, region_name=None)
translate_mock.translate.assert_called_once_with(sam_template=template, parameter_values={})
sam_parser.Parser.assert_called_once()

Expand Down
9 changes: 6 additions & 3 deletions tests/unit/commands/validate/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from unittest import TestCase
from unittest.mock import Mock, patch
from collections import namedtuple

from botocore.exceptions import NoCredentialsError

Expand All @@ -8,6 +9,8 @@
from samcli.commands.validate.lib.exceptions import InvalidSamDocumentException
from samcli.commands.validate.validate import do_cli, _read_sam_file

ctx_mock = namedtuple("ctx", ["profile", "region"])


class TestValidateCli(TestCase):
@patch("samcli.commands.validate.validate.click")
Expand Down Expand Up @@ -46,7 +49,7 @@ def test_template_fails_validation(self, read_sam_file_patch, click_patch, templ
template_valiadator.return_value = is_valid_mock

with self.assertRaises(InvalidSamTemplateException):
do_cli(ctx=None, template=template_path)
do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path)

@patch("samcli.commands.validate.lib.sam_template_validator.SamTemplateValidator")
@patch("samcli.commands.validate.validate.click")
Expand All @@ -60,7 +63,7 @@ def test_no_credentials_provided(self, read_sam_file_patch, click_patch, templat
template_valiadator.return_value = is_valid_mock

with self.assertRaises(UserException):
do_cli(ctx=None, template=template_path)
do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path)

@patch("samcli.commands.validate.lib.sam_template_validator.SamTemplateValidator")
@patch("samcli.commands.validate.validate.click")
Expand All @@ -73,4 +76,4 @@ def test_template_passes_validation(self, read_sam_file_patch, click_patch, temp
is_valid_mock.is_valid.return_value = True
template_valiadator.return_value = is_valid_mock

do_cli(ctx=None, template=template_path)
do_cli(ctx=ctx_mock(profile="profile", region="region"), template=template_path)