{Role} Bump ResourceType.MGMT_AUTHORIZATION to 2020-04-01-preview by jiasli · Pull Request #14317 · Azure/azure-cli

jiasli · 2020-07-10T09:00:08Z

Description

Bump ResourceType.MGMT_AUTHORIZATION to 2020-04-01-preview.
Fix live tests for Role module.

jiasli · 2020-07-10T09:09:17Z

test_create_for_rbac_with_existing_kv_cert fails, because of:

`azure-devtools` bug: incorrect `bytes` -> `str` conversion

Azure CLI pinned azure-devtools to 1.0.0, which has a bug with SingleValueReplacer (fixed in Azure/azure-python-devtools#56, but not released even until 1.1.1) at this line

https://github.com/Azure/azure-python-devtools/blob/1.0.0/src/azure_devtools/scenario_tests/preparers.py#L108

body = str(request.body)

This is the wrong way to convert bytes to str:

https://docs.python.org/3/library/stdtypes.html#str

Passing a bytes object to str() without the encoding or errors arguments falls under the first case of returning the informal string representation (see also the -b command-line option to Python)> . For example:
>>> str(b'Zoot!')
"b'Zoot!'"

This causes all the weird request.body in recorded YAMLs:

azure-cli/src/azure-cli/azure/cli/command_modules/resource/tests/latest/recordings/test_generic_subscription_locks.yaml

Lines 303 to 304 in 55d53ba

    
               body: 'b''b\''{"properties": {"level": "ReadOnly", "notes": "notes000002"}, "name": 
        
                 "cli-test-lock000001"}\'''''

`vcrpy` bug: replacers are called twice

Due to another bug in vcrpy (kevin1024/vcrpy#543), replacers are called twice. When <Request (POST) https://graph.windows.net/00000000-0000-0000-0000-000000000000/applications?api-version=1.6> is recorded, the replacers are executed in the following order:

azure.cli.testsdk.utilities.GraphClientPasswordReplacer
azure.cli.testsdk.preparers.ResourceGroupPreparer        << This corrupts the JSON

azure.cli.testsdk.utilities.GraphClientPasswordReplacer  << This fails with JSONDecodeError
azure.cli.testsdk.preparers.ResourceGroupPreparer

As ResourceGroupPreparer inherits from SingleValueReplacer, it corrupts the JSON as

b'{"availableToOtherTenants": false, ...}'

Then GraphClientPasswordReplacer fails to load the JSON with JSONDecodeError:

azure-cli/src/azure-cli-testsdk/azure/cli/testsdk/utilities.py

Line 132 in a305967

body = json.loads(body)

Solution

We need to release the latest azure-devtools and use it to correctly converts bytes to str:

https://github.com/Azure/azure-python-devtools/blob/master/src/azure_devtools/scenario_tests/preparers.py#L108

body = str(request.body, 'utf-8') if isinstance(request.body, bytes) else str(request.body)

jiasli · 2020-07-10T09:28:23Z

...command_modules/role/tests/latest/recordings/test_create_for_rbac_with_existing_kv_cert.yaml

The weird b''b\'' prefix has been removed by using the latest azure-devtools.

- body: 'b''b\''{"availableToOtherTenants": false, "homepage": "https://cli_test_sp_with_kv_existing_cert000001", + body: '{"availableToOtherTenants": false, "homepage": "https://cli_test_sp_with_kv_existing_cert000001",

yonzhan · 2020-07-10T11:29:58Z

add to S173

qwordy · 2020-09-09T08:18:52Z

src/azure-cli-core/azure/cli/core/profiles/_shared.py

-        ResourceType.MGMT_AUTHORIZATION: SDKProfile('2018-09-01-preview', {
+        ResourceType.MGMT_AUTHORIZATION: SDKProfile('2020-04-01-preview', {
            'classic_administrators': '2015-06-01',
            'role_definitions': '2018-01-01-preview',


Do we need update this line?

2018-01-01-preview is the latest version which contains RoleDefinition.

# Conflicts: # src/azure-cli/azure/cli/command_modules/role/custom.py # src/azure-cli/azure/cli/command_modules/vm/tests/latest/recordings/test_disk_encryption_set.yaml # src/azure-cli/azure/cli/command_modules/vm/tests/latest/recordings/test_disk_encryption_set_disk_update.yaml # src/azure-cli/azure/cli/command_modules/vm/tests/latest/recordings/test_disk_encryption_set_snapshot.yaml # src/azure-cli/azure/cli/command_modules/vm/tests/latest/recordings/test_disk_encryption_set_update.yaml # src/azure-cli/azure/cli/command_modules/vm/tests/latest/recordings/test_gallery_e2e.yaml

jiasli · 2020-09-10T03:34:21Z

src/azure-cli/azure/cli/command_modules/role/tests/hybrid_2018_03_01/test_role.py

 class RbacSPKeyVaultScenarioTest2(ScenarioTest):
    @ResourceGroupPreparer(name_prefix='cli_test_sp_with_kv_new_cert')
-    @KeyVaultPreparer(name_prefix='test_create_for_rbac_with_new_kv_cert')
+    @KeyVaultPreparer(name_prefix='test-rbac-new-kv')


The name is too long for name_prefix. Shortening it.

jiasli · 2020-09-10T03:34:51Z

src/azure-cli/azure/cli/command_modules/role/tests/latest/test_graph.py

+        # TODO: https://github.com/Azure/azure-cli/pull/13769 fails to work
+        # Cert created with
+        # openssl req -newkey rsa:2048 -nodes -keyout key.pem -x509 -days 10000 -out certificate.pem
+        self.kwargs['cert_file'] = """


Regenerate an un-expired cert.

arrownj · 2020-09-10T04:59:23Z

src/azure-cli/azure/cli/command_modules/role/tests/util.py

+    # Due to unstable role definition ARIs: https://github.com/Azure/azure-cli/issues/3187
+    import time
+    result = None
+    while not result:


Better to add a max_retry

I don't exactly know what number is suitable. The result is pretty unpredictable. It won't affect the CI as the tests will definitely succeed. So I want to put it on hold for now.

Added as requested. Let's use ROLE_COMMAND_MAX_RETRY = 20 for now.

jiasli requested review from arrownj, bim-msft, haroldrandom, mmyyrroonn and qianwens as code owners July 10, 2020 09:00

jiasli commented Jul 10, 2020

View reviewed changes

jiasli marked this pull request as draft July 10, 2020 09:34

yonzhan assigned jiasli Jul 10, 2020

yonzhan added this to the S173 milestone Jul 10, 2020

jiasli mentioned this pull request Jul 22, 2020

{Role} Fix: az role assignment list-changelogs fails with KeyError #14461

Merged

jiasli modified the milestones: S173, S174 Jul 29, 2020

jiasli force-pushed the role-live branch from 257b6c2 to a43397e Compare August 21, 2020 05:56

yonzhan modified the milestones: S174, S175 - For Ignite Aug 22, 2020

yonzhan requested a review from jsntcy August 22, 2020 17:01

jiasli closed this Aug 28, 2020

jiasli mentioned this pull request Aug 28, 2020

[Role] az role assignment create/update: Support --description, --condition and --condition-version #14958

Merged

rerun-tests

3a6ead7

jiasli reopened this Sep 9, 2020

jiasli force-pushed the role-live branch from a43397e to 3a6ead7 Compare September 9, 2020 02:56

jiasli changed the title ~~{Role} Fix live tests~~ {Role} Bump ResourceType.MGMT_AUTHORIZATION to 2020-04-01-preview Sep 9, 2020

Revert src/azure-cli-testsdk/azure/cli/testsdk/utilities.py

6c1e633

jiasli marked this pull request as ready for review September 9, 2020 03:09

jiasli requested review from Juliehzl, fengzhou-msft, qwordy and zhoxing-ms as code owners September 9, 2020 03:09

jiasli removed request for bim-msft and qianwens September 9, 2020 03:09

qwordy reviewed Sep 9, 2020

View reviewed changes

qwordy approved these changes Sep 9, 2020

View reviewed changes

jiasli added 2 commits September 10, 2020 10:20

rerun tests

fd9a419

jiasli commented Sep 10, 2020

View reviewed changes

arrownj reviewed Sep 10, 2020

View reviewed changes

zhoxing-ms approved these changes Sep 10, 2020

View reviewed changes

arrownj approved these changes Sep 10, 2020

View reviewed changes

mmyyrroonn approved these changes Sep 10, 2020

View reviewed changes

Add max_retry

0437a86

jiasli requested a review from arrownj September 10, 2020 06:11

arrownj approved these changes Sep 10, 2020

View reviewed changes

jsntcy approved these changes Sep 11, 2020

View reviewed changes

jiasli merged commit 5b30f29 into Azure:dev Sep 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

{Role} Bump ResourceType.MGMT_AUTHORIZATION to 2020-04-01-preview#14317

{Role} Bump ResourceType.MGMT_AUTHORIZATION to 2020-04-01-preview#14317
jiasli merged 5 commits intoAzure:devfrom
jiasli:role-live

jiasli commented Jul 10, 2020 •

edited

Loading

Uh oh!

jiasli commented Jul 10, 2020 •

edited

Loading

Uh oh!

jiasli Jul 10, 2020 •

edited

Loading

Uh oh!

yonzhan commented Jul 10, 2020

Uh oh!

qwordy Sep 9, 2020

Uh oh!

jiasli Sep 9, 2020

Uh oh!

jiasli Sep 10, 2020

Uh oh!

jiasli Sep 10, 2020

Uh oh!

arrownj Sep 10, 2020

Uh oh!

jiasli Sep 10, 2020 •

edited

Loading

Uh oh!

jiasli Sep 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

jiasli commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jiasli commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

azure-devtools bug: incorrect bytes -> str conversion

vcrpy bug: replacers are called twice

Solution

Uh oh!

jiasli Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yonzhan commented Jul 10, 2020

Uh oh!

qwordy Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

jiasli Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

jiasli Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

jiasli Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

arrownj Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

jiasli Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiasli Sep 10, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jiasli commented Jul 10, 2020 •

edited

Loading

jiasli commented Jul 10, 2020 •

edited

Loading

`azure-devtools` bug: incorrect `bytes` -> `str` conversion

`vcrpy` bug: replacers are called twice

jiasli Jul 10, 2020 •

edited

Loading

jiasli Sep 10, 2020 •

edited

Loading