Storageaccount tlsversion #207

paultaiton · 2020-07-27T05:35:23Z

SUMMARY

This change adds support to azure_rm_storageaccounts.py for the "MinimumTlsVersion" parameter for new and updated storage accounts.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

azure_rm_storageaccount

ADDITIONAL INFORMATION

paultaiton · 2020-07-27T05:37:31Z

This is the first code of any significance I've written with Azure python SDK and for Ansible modules. I made a lot of assumptions to get this working, so I really appreciate any help with reviewing and getting this to an acceptable quality.

Fred-sun · 2020-07-27T07:24:02Z

@paultaiton Thank you for your contribution, we will review and promote PR merger as soon as possible. Can you add test cases to this feature? Thank you!

tests/integration/targets/azure_rm_storageaccount/tasks/main.yml

paultaiton · 2020-07-27T15:39:04Z

@Fred-sun
I have not done ansible tests before, but I'll see what I can throw together and will let you know when I think I have it.
Thanks Fred.

paultaiton · 2020-07-30T00:39:52Z

@Fred-sun
I added some tests, but in the process realized that I did not add sufficient support for the _info module. That has also been added in the latest commit.

During my own testing of adding the TLS feature, I realized that there were errors with the code as it existed, and had to make modifications outside the scope of MinimumTlsVersion to make execution complete without errors.

As mentioned I added some code for the integration tests, but I have no idea how to run it. I THINK it is correct syntactically and logically, but lacking the actual test I cannot confirm. Could I please ask that you either run the test for me, or point me in the right direction for documentation on how to accomplish that myself?

Thanks again for your help.

Fred-sun · 2020-07-30T01:52:59Z

@Fred-sun
I added some tests, but in the process realized that I did not add sufficient support for the _info module. That has also been added in the latest commit.

During my own testing of adding the TLS feature, I realized that there were errors with the code as it existed, and had to make modifications outside the scope of MinimumTlsVersion to make execution complete without errors.

As mentioned I added some code for the integration tests, but I have no idea how to run it. I THINK it is correct syntactically and logically, but lacking the actual test I cannot confirm. Could I please ask that you either run the test for me, or point me in the right direction for documentation on how to accomplish that myself?

Thanks again for your help.

I will check your change! Thank you very much!

Fred-sun

@paultaiton Please help change as suggested below. Thank you!

plugins/modules/azure_rm_storageaccount.py

plugins/modules/azure_rm_storageaccount_info.py

Co-authored-by: Fred-sun <[email protected]>

Fred-sun

@paultaiton Thanks!

plugins/modules/azure_rm_storageaccount.py

Co-authored-by: Fred-sun <[email protected]>

paultaiton · 2020-07-30T03:54:04Z

Suggestions look good.

Fred-sun · 2020-07-30T05:49:48Z

ready_for_review

paultaiton · 2020-08-03T21:09:44Z

Hello @Fred-sun .

Was there any further action required on my part? I'm not sure what the next steps are.

Fred-sun · 2020-08-04T02:42:49Z

Hello @Fred-sun .

Was there any further action required on my part? I'm not sure what the next steps are.

Because the maintainer was busy with other things during this time, there are no time to review these PRs. I will push maintainer to handle these PRs today or tomorrow. If it causes you inconvenience, please forgive me. Thank you!

paultaiton · 2020-08-04T05:22:35Z

No problem, just wanted to make sure it was not waiting on me. Appreciate the response.

paultaiton · 2020-08-04T16:21:23Z

Thanks for your help @haiyuazhang , and especially for your help @Fred-sun . It is much appreciated.

… of a Storage Account. (ansible-collections#108) * Add support for managing the 'Firewall and virtual networks' settings of a Storage Account. Co-authored-by: Fred-sun <[email protected]> Update azure_rm_publicipaddress add test sample codebase cleanup: update format (ansible-collections#131) * Update format refine azure devops pipeline (ansible-collections#138) * refine azure devops pipeline enable single module test (ansible-collections#144) * enable single module test bug fixing in azure_rm_aks module(ansible-collections#170) * bug fixing in azure_rm_aks module For Test( 2.9 to 2.10) (ansible-collections#140) * adding more testing dimensions Add FileEndpoint to azure_rm_storageaccount_info (ansible-collections#102) * Add FileEndpoint to azure_rm_storageaccount_info Use hasattr instead of dir (ansible-collections#75) Add ephemeral OS disk for azure_rm_virtualmachine (ansible-collections#124) * Add ephemeral OS disk support for azure_rm_virtualmachine Add ephemeral OS disk for azure_rm_virtualmachinescaleset (ansible-collections#128) * Add ephemeral OS disk support for azure_rm_virtualmachinescaleset Change network_client api_version to match latest default api_version (ansible-collections#157) * #Fixes 156 Change network_client api_version to match latest default api_version * #Fixes 156 Update azure-mgmt-network to 10.2.0 Update 'latest' version listing and default version for Network Client to 2018-08-01 Add ability to remove all Subnet Service Endpoints when supplying an empty list. (ansible-collections#148) New azure_rm_privatednszone module with tests (ansible-collections#122) * New azure_rm_privatednszone module with tests Co-authored-by: Fred-sun <[email protected]> Co-authored-by: Fred-sun <[email protected]> Add new azure active directory related modules (ansible-collections#179) * add ad related modules * release v0.2.0 preparation and minor ad module bug fixing fixing sanity testing errors in ad related modules (ansible-collections#182) * disable ad relate module test * fixing sanity testing errors in ad related modules Fixing issues in azure_rm_deployment_info.py (ansible-collections#180) - Fixing getting the template_link when it does not exist. - Feature: Return the correlation_id Co-authored-by: Steve Kieu <[email protected]> azure_rm_virtualmachine: Protect against no diskSizeGB (ansible-collections#185) Disks `diskSizeGB` is not reported if the Virtual Machine is deallocated. Trying to fetch it is causing a `KeyError`. This guards against that KeyError and then nulls out the key so it isn't set incorrectly in the update call. Fix wrong module deprecate statement format (ansible-collections#176) * fix wrong module deprecate statement format update doc add batch upload feature in azure_rm_storageblob (ansible-collections#203) Co-authored-by: haiyuazhang <[email protected]> fixing status code issue in azure_rm_deployment (ansible-collections#204) Co-authored-by: haiyuazhang <[email protected]> fixing update check bug in azure_rm_adserviceprincipal (ansible-collections#205) Bump version to v0.3.0 update doc change service_url to service_uri (ansible-collections#212) Storageaccount tlsversion (ansible-collections#207) * add support for minimum_tls_version fix(vm): fix boot diagnistics option "no" caused an error (ansible-collections#200) Fix the issue ansible-collections#158 (ansible-collections#214) * fix issue ansible-collections#158 and add tests azure_rm_appgateway.py: Support Version 2 SKUs (ansible-collections#198) fix sanity test issues (ansible-collections#223) Bump version to v0.4.0 lift v0.4.0 to v1.0.0 update new pr-pipeline (ansible-collections#229) Add execution environment metadata (ansible-collections#220) Storage account allow blob public access parameter (ansible-collections#219) * Storage account allow blob public access parameter * Change name of test to match correct attribute. Update azure_rm_virtualmachine boot_diagnostics resource group config… (ansible-collections#208) * Update azure_rm_virtualmachine boot_diagnostics resource group configuration Update azure_rm_storageaccount.py file (ansible-collections#233) * Uneatable parameters do not need to set default values fix the problem of disk lun self-increment (ansible-collections#237) Fixing rule_type reference (ansible-collections#99) Reference to ev instead of item is preventing the expected rule_type values from functioning Co-authored-by: Fred-sun <[email protected]> add load_balancer_sku option for aks (ansible-collections#199) * added load_balancer_sku option for aks Co-authored-by: Fred-sun <[email protected]> Improve OS detection when VM has no osProfile (ansible-collections#197) We have VM without OS profile (checked on https://resources.azure.com/) and they appear as "unknown" os :-/ This appear to be a known issue: https://support.microsoft.com/en-ph/help/4018140/computer-names-of-specialized-virtual-machines-are-missing-or-blank-in Co-authored-by: Fred-sun <[email protected]> Co-authored-by: haiyuan_zhang <[email protected]> Add IPv6 address for azure_rm_subnet (ansible-collections#240) * Add IPv6 address for azure_rm_subnet Get address_prefixes info from virtualnetwork (ansible-collections#239) * Add new parameter for get subnet IPv6 info Support azure cli credentials with multiple `subscription_id`s (ansible-collections#195) * feat: Support azure cli credentials with multiple `subscription_id`s If the `subscription_id` is specified as module parameter or in the environment then try to find that subscription in either the MSI (existing) or CLI credentials (new). This patch brings those two scenarios in line. * docs: Improve documentation on auth_source * refactor: Move defaults up to class Just trying to make the `__init__` fn a bit slimmer and easier to reason about. * refactor: Use python kwargs instead of passing dict This is an isomorphic change, just using python syntax to accomplish the exact same thing. * refactor: Use ansible builtin `env_fallback` for `auth_source` Ansible modules have a pattern for looking up a module parameter in the environment with precedence of explicit param -> env -> default. Use this pattern to simplify our code here. This shouldn't change any behavior of `auth_source`, just using standard ansible patterns to accomplish it. * style: Split long line No semantic change, just wrapping a long line to be a bit more readable. * refactor: helper fun _get_env Somewhat frequently there is a lookup in the environment for the key that matches a module parameter. This simple helper just encapsulates that to make it a bit easier elsewhere-- lookup the same key in params, credentials, env * fix: typo in log message Co-authored-by: Justin Ossevoort <[email protected]> Add 10.3 version for azure_rm_mariadbserver module (ansible-collections#244) * add new version for azure_rm_mariadbserver Update azure_rm_storageaccount yaml (ansible-collections#226) * update azure_rm_storageaccount test * update pipeline file

paultaiton added 4 commits July 26, 2020 19:32

update azure storage mgmt package to minimum required for tls_version

2335aec

minimum sdk version and api version to support minimum_tls_version

46c42c2

no .value properties, sku.tier and sku.name are strings.

c88040c

added minimum_tls_version parameter for storageaccount

9b01f05

paultaiton closed this Jul 27, 2020

paultaiton reopened this Jul 27, 2020

add support for minimum_tls_version

eca024e

Fred-sun reviewed Jul 30, 2020

View reviewed changes

plugins/modules/azure_rm_storageaccount.py Outdated Show resolved Hide resolved

plugins/modules/azure_rm_storageaccount_info.py Outdated Show resolved Hide resolved

paultaiton and others added 2 commits July 29, 2020 20:50

Update plugins/modules/azure_rm_storageaccount.py

2dff5d2

Co-authored-by: Fred-sun <[email protected]>

Update plugins/modules/azure_rm_storageaccount_info.py

b690dbe

Co-authored-by: Fred-sun <[email protected]>

Fred-sun reviewed Jul 30, 2020

View reviewed changes

plugins/modules/azure_rm_storageaccount.py Outdated Show resolved Hide resolved

Update plugins/modules/azure_rm_storageaccount.py

a08dbf0

Co-authored-by: Fred-sun <[email protected]>

haiyuazhang merged commit 0328969 into ansible-collections:dev Aug 4, 2020

paultaiton deleted the storageaccount-tlsversion branch August 4, 2020 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storageaccount tlsversion #207

Storageaccount tlsversion #207

paultaiton commented Jul 27, 2020

paultaiton commented Jul 27, 2020

Fred-sun commented Jul 27, 2020 •

edited

Loading

paultaiton commented Jul 27, 2020

paultaiton commented Jul 30, 2020

Fred-sun commented Jul 30, 2020

Fred-sun left a comment

Fred-sun left a comment

paultaiton commented Jul 30, 2020

Fred-sun commented Jul 30, 2020

paultaiton commented Aug 3, 2020

Fred-sun commented Aug 4, 2020

paultaiton commented Aug 4, 2020

paultaiton commented Aug 4, 2020

Storageaccount tlsversion #207

Storageaccount tlsversion #207

Conversation

paultaiton commented Jul 27, 2020

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

paultaiton commented Jul 27, 2020

Fred-sun commented Jul 27, 2020 • edited Loading

paultaiton commented Jul 27, 2020

paultaiton commented Jul 30, 2020

Fred-sun commented Jul 30, 2020

Fred-sun left a comment

Choose a reason for hiding this comment

Fred-sun left a comment

Choose a reason for hiding this comment

paultaiton commented Jul 30, 2020

Fred-sun commented Jul 30, 2020

paultaiton commented Aug 3, 2020

Fred-sun commented Aug 4, 2020

paultaiton commented Aug 4, 2020

paultaiton commented Aug 4, 2020

Fred-sun commented Jul 27, 2020 •

edited

Loading