Skip to content

Commit

Permalink
Merge pull request #501 from aws/lb-fix2
Browse files Browse the repository at this point in the history
Fix for lb setting when using .ebextensions or save config files
  • Loading branch information
NihalM99 authored Oct 28, 2023
2 parents 53460c0 + 9f05da7 commit 87c8352
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 8 deletions.
51 changes: 46 additions & 5 deletions ebcli/controllers/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import argparse
import os
import time

import yaml
from ebcli.core import io, fileoperations, hooks
from ebcli.core.abstractcontroller import AbstractBaseController
from ebcli.lib import elasticbeanstalk, utils, iam
Expand Down Expand Up @@ -239,7 +239,7 @@ def do_command(self):
cname = cname or get_environment_cname(env_name, provided_env_name, tier)
key_name = key_name or commonops.get_default_keyname()
vpc = self.form_vpc_object(tier, single)
elb_type = elb_type or get_elb_type_from_customer(interactive, single, tier)
elb_type = elb_type or get_elb_type_from_customer(interactive, single, tier,cfg_flag_used=cfg)
shared_lb = get_shared_load_balancer(interactive, elb_type, platform, shared_lb, vpc)
shared_lb_port = shared_lb_port or shared_lb_ops.get_shared_lb_port_from_customer(interactive, shared_lb)
enable_spot = enable_spot or spotops.get_spot_request_from_customer(interactive)
Expand Down Expand Up @@ -533,8 +533,48 @@ def get_cname_from_customer(env_name):
break
return cname

def check_elb_type_from_configs(use_saved_config=False):
"""
Checks if the ELB type is present from either the .ebextensions or saved_configs directories.
:param use_saved_config: Boolean indicating if --cfg flag was used.
:return: True, else False.
"""

# If --cfg flag is used, prioritize checking saved_configs first
if use_saved_config:
saved_configs_dir = './.elasticbeanstalk/saved_configs'
if os.path.exists(saved_configs_dir):
for config_file in os.listdir(saved_configs_dir):
with open(os.path.join(saved_configs_dir, config_file), 'r') as f:
try:
config = yaml.safe_load(f)
option_settings = config.get('OptionSettings', {})
for namespace, setting in option_settings.items():
if namespace == 'aws:elasticbeanstalk:environment' and setting.get('LoadBalancerType'):
return True
except yaml.YAMLError:
raise ValueError(f"Malformed YAML in file {config_file} in saved_configs.")

def get_elb_type_from_customer(interactive, single, tier):
else:
# Then check in .ebextensions
ebextensions_dir = './.ebextensions'
if os.path.exists(ebextensions_dir):
for config_file in os.listdir(ebextensions_dir):
with open(os.path.join(ebextensions_dir, config_file), 'r') as f:
try:
config = yaml.safe_load(f)
option_settings = config.get('option_settings', [])
for setting in option_settings:
if setting.get('namespace') == 'aws:elasticbeanstalk:environment' and setting.get('option_name') == 'LoadBalancerType':
if setting.get('value'):
return True
except yaml.YAMLError:
raise ValueError(f"Malformed YAML in file {config_file} in .ebextensions.")


return False

def get_elb_type_from_customer(interactive, single, tier, cfg_flag_used=False):
"""
Prompt customer to specify the ELB type if operating in the interactive mode and
on a load-balanced environment.
Expand All @@ -546,8 +586,9 @@ def get_elb_type_from_customer(interactive, single, tier):
:param tier: the tier type of the environment
:return: selected ELB type which is one among ['application', 'classic', 'network']
"""
if single or (tier and not tier.is_webserver()):
return
elb_type_is_configured = check_elb_type_from_configs(use_saved_config=cfg_flag_used)
if single or (tier and not tier.is_webserver()) or elb_type_is_configured:
return
elif not interactive:
return elb_names.APPLICATION_VERSION

Expand Down
108 changes: 105 additions & 3 deletions tests/unit/controllers/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@
import os
import shutil

import mock
#import mock
import unittest
from unittest import mock
from unittest.mock import patch, mock_open

from ebcli.controllers import create
from ebcli.controllers.create import check_elb_type_from_configs

from ebcli.core import fileoperations
from ebcli.core.ebcore import EB
from ebcli.objects.exceptions import (
Expand Down Expand Up @@ -2373,6 +2377,77 @@ def test_get_template_name__config_file_name_passed_in(

self.assertEqual('some_cfg', create.get_template_name('some_app', 'some_cfg'))

@mock.patch('yaml.safe_load')
@mock.patch('os.path.exists')
@mock.patch('os.listdir')
@mock.patch('builtins.open', new_callable=mock.mock_open, read_data="valid_yaml_content")
def test_check_elb_type_from_configs_saved_configs_prioritized(self, mock_file, mock_listdir, mock_exists, mock_yaml):
mock_exists.return_value = True
mock_listdir.return_value = ['config1.yaml']
mock_yaml.return_value = {
'OptionSettings': {
'aws:elasticbeanstalk:environment': {
'LoadBalancerType': True
}
}
}

self.assertTrue(check_elb_type_from_configs(use_saved_config=True))

@mock.patch('yaml.safe_load')
@mock.patch('os.path.exists')
@mock.patch('os.listdir')
@mock.patch('builtins.open', new_callable=mock.mock_open, read_data="valid_yaml_content")
def test_check_elb_type_from_configs_ebextensions_checked(self, mock_file, mock_listdir, mock_exists, mock_yaml):
mock_exists.return_value = True
mock_listdir.return_value = ['config1.yaml']
mock_yaml.return_value = {
'option_settings': [
{
'namespace': 'aws:elasticbeanstalk:environment',
'option_name': 'LoadBalancerType',
'value': True
}
]
}

self.assertTrue(check_elb_type_from_configs(use_saved_config=False))

@mock.patch('os.path.exists')
@mock.patch('os.listdir')
@mock.patch('builtins.open', new_callable=mock.mock_open, read_data="invalid: yaml: data")
def test_check_elb_type_from_configs_malformed_yaml_raises_value_error(self, mock_file, mock_listdir, mock_exists):
mock_exists.return_value = True
mock_listdir.return_value = ['config1.yaml']

with self.assertRaises(ValueError):
check_elb_type_from_configs(use_saved_config=False)

@mock.patch('os.path.exists')
@mock.patch('os.listdir')
@mock.patch('builtins.open', new_callable=mock.mock_open, read_data="option_settings: []")
def test_check_elb_type_from_configs_returns_none_when_elb_not_found(self, mock_file, mock_listdir, mock_exists):
mock_exists.return_value = True
mock_listdir.return_value = ['config1.yaml']

self.assertFalse(check_elb_type_from_configs(use_saved_config=False))

@mock.patch('yaml.safe_load')
@mock.patch('os.path.exists')
@mock.patch('os.listdir')
@mock.patch('builtins.open', new_callable=mock.mock_open, read_data="OptionSettings: {random_namespace: {random_option: random_value}}")
def test_check_elb_type_from_configs_setting_not_present_in_saved_configs(self, mock_file, mock_listdir, mock_exists, mock_yaml):
mock_exists.return_value = True
mock_listdir.return_value = ['config1.yaml']
mock_yaml.return_value = {
'random_namespace': {
'random_option': 'random_value'
}}

self.assertFalse(check_elb_type_from_configs(use_saved_config=True))



def test_get_elb_type_from_customer__single_instance_environment(self):
self.assertIsNone(
create.get_elb_type_from_customer(
Expand All @@ -2381,13 +2456,14 @@ def test_get_elb_type_from_customer__single_instance_environment(self):
tier=Tier.from_raw_string('webserver')
)
)

def test_get_elb_type_from_customer__non_interactive_mode(self):
self.assertEqual(
create.get_elb_type_from_customer(
interactive=False,
single=False,
tier=None
tier=None,

),
'application'
)
Expand Down Expand Up @@ -2448,6 +2524,32 @@ def test_get_elb_type_from_customer__interactive_mode__not_applicable_for_worker
)
)

@mock.patch('ebcli.controllers.create.check_elb_type_from_configs')
def test_get_elb_type_from_customer_elb_type_configured_interactive(self, mock_check_elb):
mock_check_elb.return_value = True # ELB type is configured

result = create.get_elb_type_from_customer(
interactive=True,
single=False,
tier=None,
cfg_flag_used=True
)

self.assertIsNone(result)

@mock.patch('ebcli.controllers.create.check_elb_type_from_configs')
def test_get_elb_type_from_customer_elb_type_configured_non_interactive(self, mock_check_elb):
mock_check_elb.return_value = True # ELB type is configured

result = create.get_elb_type_from_customer(
interactive=False,
single=False,
tier=None,
cfg_flag_used=True
)

self.assertIsNone(result)

@mock.patch('ebcli.controllers.create.shared_lb_ops.validate_shared_lb_for_non_interactive')
@mock.patch('ebcli.controllers.create.shared_lb_ops.get_shared_lb_from_customer')
def test_get_shared_load_balancer__for_interactive(
Expand Down

0 comments on commit 87c8352

Please sign in to comment.