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 parameters passed as string that should be integer #36818

Closed
wants to merge 1 commit into from

Conversation

zikalino
Copy link
Contributor

SUMMARY

Fixes bug #35536
Converts parameters passed as string to int based on template definition.
Note: template has to be specified via template parameter, for example using lookup instead of template_url

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_deployment

ANSIBLE VERSION

2.4

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2018

@ansibot ansibot added azure 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 Feb 28, 2018
@yuwzho
Copy link
Contributor

yuwzho commented Feb 28, 2018

shipit

@zikalino
Copy link
Contributor Author

bot_status

@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2018

Components

lib/ansible/modules/cloud/azure/azure_rm_deployment.py
support: certified
maintainers: devigned lmazuel obsoleted

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 2
ansible_shipits (core team members): 0
shipit_actors (maintainers or core team members): yuwzho zikalino
shipit_actors_other: []

click here for bot help

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed committer_review In order to be merged, this PR must follow the certified review workflow. labels Feb 28, 2018
@zikalino
Copy link
Contributor Author

ready_for_review

@Akasurde Akasurde removed the needs_triage Needs a first human triage before being processed. label Feb 28, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
# this is to fix issue #35536 -- it's impossible to pass int parameters
if self.parameters and self.template:
for k in self.parameters:
if self.template['parameters'][k]['type'] == "int":
Copy link
Contributor

Choose a reason for hiding this comment

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

So to understand this correctly, the user must specify the type as an int in the parameter definition like so?

- azure_rm_deployment:
    template:
      parameters:
        intFieldParameter:
          type: int
          value: 123

Copy link
Contributor

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

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

Are you able to add a test case for this?

@jborean93 jborean93 self-requested a review March 22, 2018 00:10
@yungezz
Copy link
Contributor

yungezz commented Apr 27, 2018

agree with @jborean93 . it's necessary to add test to cover the change since azure_rm_deployment module is quite general

@@ -490,6 +490,12 @@ def deploy_template(self):
uri=self.template_link
)

# this is to fix issue #35536 -- it's impossible to pass int parameters
if self.parameters and self.template:
Copy link
Contributor

Choose a reason for hiding this comment

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

pls also help to fix in paremeters_link

@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 19, 2018
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. support:certified This issue/PR relates to certified code. and removed support:certified This issue/PR relates to certified code. support:core This issue/PR relates to code supported by the Ansible Engineering Team. shipit This PR is ready to be merged by Core labels Jun 5, 2018
@ansibot ansibot added support:certified This issue/PR relates to certified code. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. shipit This PR is ready to be merged by Core and removed shipit This PR is ready to be merged by Core needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jun 6, 2018
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed shipit This PR is ready to be merged by Core labels Jun 13, 2018
@ansibot ansibot added small_patch shipit This PR is ready to be merged by Core needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. shipit This PR is ready to be merged by Core labels Jun 21, 2018
@ansibot ansibot added shipit This PR is ready to be merged by Core and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 6, 2018
@Fred-sun
Copy link
Contributor

@zikalino This PR exist conflicting files, Could you help to resolve it? Thanks!

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed shipit This PR is ready to be merged by Core labels Aug 10, 2018
@zikalino
Copy link
Contributor Author

closing this, as this is not a good fix, a hack really. it needs to be fixed in ansible core. so closing...……...

@zikalino zikalino closed this Aug 13, 2018
@zikalino zikalino deleted the deployment-int-params branch August 13, 2018 03:12
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 azure bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. small_patch stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:certified This issue/PR relates to certified code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants