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 a crash when the volume_size for an EBS volume is not an int #32291

Merged
merged 1 commit into from Oct 30, 2017
Merged

Fix a crash when the volume_size for an EBS volume is not an int #32291

merged 1 commit into from Oct 30, 2017

Conversation

josephtate
Copy link
Contributor

@josephtate josephtate commented Oct 28, 2017

(because of templating for example)

SUMMARY

boto3 added type validation to many methods. Similar to #18999, the ec2_lc module didn't cast the volume_size parameter to int before passing it to boto. This was fine if the yaml used an integer value, but if a string was used (because of templating, or variable substitution mostly), a type error was raised.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_lc

ANSIBLE VERSION
ansible 2.5.0 (boto3-non-int-volume-size-crash e4f6b8a5cb) last updated 2017/10/28 07:05:08 (GMT -400)
  config file = None
  configured module search path = [u'/home/jtate/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/jtate/Work/crunch.io/misc/ansible/lib/ansible
  executable location = /home/jtate/Work/crunch.io/misc/ansible/bin/ansible
  python version = 2.7.13 (default, Sep  5 2017, 08:53:59) [GCC 7.1.1 20170622 (Red Hat 7.1.1-3)]

Broken in 6d402de all the way to current devel HEAD.

ADDITIONAL INFORMATION

Imagine a task as follows:

- name: set up the launch configuration
  ec2_lc:
    name: blargh
    volumes:
      - device_name: /dev/sda1
        volume_size: "{{ebs_vol_size|int}}"
        device_type: gp2
        delete_on_termination: true
# ... etc

Because of the forced use of quotes, volume_size is passed to the module as a string.

The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_Ip2uRQ/ansible_module_ec2_lc.py", line 426, in <module>
    main()
  File "/tmp/ansible_Ip2uRQ/ansible_module_ec2_lc.py", line 420, in main
    create_launch_config(connection, module)
  File "/tmp/ansible_Ip2uRQ/ansible_module_ec2_lc.py", line 317, in create_launch_config
    connection.create_launch_configuration(**launch_config)
  File "/home/jtate/.virtualenvs/ansible2/lib/python2.7/site-packages/botocore/client.py", line 312, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/jtate/.virtualenvs/ansible2/lib/python2.7/site-packages/botocore/client.py", line 579, in _make_api_call
    api_params, operation_model, context=request_context)
  File "/home/jtate/.virtualenvs/ansible2/lib/python2.7/site-packages/botocore/client.py", line 634, in _convert_to_request_dict
    api_params, operation_model)
  File "/home/jtate/.virtualenvs/ansible2/lib/python2.7/site-packages/botocore/validate.py", line 291, in serialize_to_request
    raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter BlockDeviceMappings[0].Ebs.VolumeSize, value: 100, type: <type 'str'>, valid types: <type 'int'>, <type 'long'>

After applying this change, the command succeeds.

@ansibot
Copy link
Contributor

ansibot commented Oct 28, 2017

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 aws bugfix_pull_request cloud committer_review In order to be merged, this PR must follow the certified review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:certified This issue/PR relates to certified code. labels Oct 28, 2017
@wilvk
Copy link
Contributor

wilvk commented Oct 30, 2017

@willthames - may need to add this to ec2_ami as well

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Oct 30, 2017
@s-hertel s-hertel merged commit 77061f5 into ansible:devel Oct 30, 2017
@josephtate
Copy link
Contributor Author

Could we get this pulled into the 2.3 and 2.4 branches? It still affects them.

@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@mentos1386
Copy link
Contributor

mentos1386 commented May 25, 2018

@willthames as @wilvk mentioned. Can this be implemented for ec2_ami as well. We can provide volume_size inside device_mapping

https://docs.ansible.com/ansible/latest/modules/ec2_ami_module.html?highlight=ec2_ami

EDIT: Created a PR #40938

@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 aws bug This issue/PR relates to a bug. cloud committer_review In order to be merged, this PR must follow the certified review workflow. module This issue/PR relates to a module. support:certified This issue/PR relates to certified code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants