Skip to content

Define new replacer to replace keys in recording #18294

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 6, 2021
Merged

Define new replacer to replace keys in recording #18294

merged 2 commits into from
May 6, 2021

Conversation

00Kai0
Copy link
Contributor

@00Kai0 00Kai0 commented Apr 26, 2021

No description provided.

@00Kai0 00Kai0 requested a review from changlong-liu as a code owner April 26, 2021 06:42
msyyc
msyyc reviewed Apr 26, 2021
View reviewed changes
sdk/automation/azure-mgmt-automation/tests/test_cli_mgmt_automation.py
@@ -30,10 +30,12 @@ class MgmtAutomationClientTest(AzureMgmtTestCase):

def setUp(self):
super(MgmtAutomationClientTest, self).setUp()
self.re_replacer.register_pattern_pair('"value": ".{64}"', '"value": "FakeValue"')
self.re_replacer.register_pattern_pair('"Value":".{88}"', '"Value":"FakeValue"')
Copy link
Member

Choose a reason for hiding this comment

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

Better to give an example as annotation to explain how regular expression works. It is hard to understand it.

seankane-msft
seankane-msft suggested changes Apr 26, 2021
View reviewed changes
tools/azure-sdk-tools/devtools_testutils/mgmt_testcase.py
@@ -76,6 +80,7 @@ def __init__(
replay_patches=replay_patches,
**kwargs
)
self.recording_processors.append(self.re_replacer)
Copy link
Member

Choose a reason for hiding this comment

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

Do you need access to the RENameReplacer specifically? I think you can replace this with: self.recording_processors.append(RENameReplacer()) and remove the self.re_replacer line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I need to add regex to this replacer.

...omation/azure-mgmt-automation/tests/recordings/test_cli_mgmt_automation.test_automation.yaml Outdated
@@ -20,22 +20,22 @@ interactions:
response:
body:
string: '{"name":"myAutomationAccount9","id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test_cli_mgmt_automation_test_automation53db109a/providers/Microsoft.Automation/automationAccounts/myAutomationAccount9","type":"Microsoft.Automation/AutomationAccounts","location":"East
US 2","tags":{},"etag":null,"properties":{"sku":{"name":"Basic","family":null,"capacity":null},"state":"Ok","RegistrationUrl":"https://4ce1a53f-b815-4200-af5f-96601848a9d9.agentsvc.eus2.azure-automation.net/accounts/4ce1a53f-b815-4200-af5f-96601848a9d9","creationTime":"2021-03-16T09:08:29.5+00:00","lastModifiedBy":null,"lastModifiedTime":"2021-03-16T09:08:29.5+00:00"}}'
US 2","tags":{},"etag":null,"properties":{"sku":{"name":"Basic","family":null,"capacity":null},"state":"Ok","RegistrationUrl":"https://b2be1457-9afa-4398-8c77-0e9aa7479152.agentsvc.eus2.azure-automation.net/accounts/b2be1457-9afa-4398-8c77-0e9aa7479152","creationTime":"2021-04-26T06:16:28.93+00:00","lastModifiedBy":null,"lastModifiedTime":"2021-04-26T06:16:28.93+00:00"}}'
Copy link
Member

Choose a reason for hiding this comment

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

These both look like secrets, can you confirm they are not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a regex to hide it.

self.re_replacer.register_pattern_pair(
    '"RegistrationUrl":"https://.*/accounts/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"',
    '"RegistrationUrl":"FakeUrl"'
)

seankane-msft
seankane-msft reviewed Apr 27, 2021
View reviewed changes
...omation/azure-mgmt-automation/tests/recordings/test_cli_mgmt_automation.test_automation.yaml
expires:
- '-1'
location:
- https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test_cli_mgmt_automation_test_automation53db109a/providers/Microsoft.Automation/automationAccounts/myAutomationAccount9/runbooks/runbook_draft?api-version=2019-06-01
ocp-location:
- https://management.azure.com/subscriptions/92f95d8f-3c67-4124-91c7-8cf07cdbf241/resourceGroups/test_cli_mgmt_automation_test_automation53db109a/providers/Microsoft.Automation/automationAccounts/myAutomationAccount9/runbooks/runbook_draft?api-version=2019-06-01
- https://management.azure.com/subscriptions/92f95d8f-3c67-4124-91c7-8cf07cdbf241/resourceGroups/rgpy-test_cli_mgmt_automation_test_automation53db109a/providers/Microsoft.Automation/automationAccounts/myAutomationAccount9/runbooks/runbook_draft?api-version=2019-06-01
Copy link
Member

Choose a reason for hiding this comment

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

Is this the subscription ID?

...omation/azure-mgmt-automation/tests/recordings/test_cli_mgmt_automation.test_automation.yaml
@@ -165,7 +164,7 @@ interactions:
x-content-type-options:
- nosniff
x-ms-location:
- https://management.azure.com/subscriptions/92f95d8f-3c67-4124-91c7-8cf07cdbf241/resourceGroups/test_cli_mgmt_automation_test_automation53db109a/providers/Microsoft.Automation/automationAccounts/myAutomationAccount9/modules/module1?api-version=2019-06-01
- https://management.azure.com/subscriptions/92f95d8f-3c67-4124-91c7-8cf07cdbf241/resourceGroups/rgpy-test_cli_mgmt_automation_test_automation53db109a/providers/Microsoft.Automation/automationAccounts/myAutomationAccount9/modules/module1?api-version=2019-06-01
Copy link
Member

Choose a reason for hiding this comment

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

Is this a subscription id in the x-ms-location header too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is. But this one is in header and it's been around here for a long time . This PR is used to solve secrets in requests and responses.
We need another PR to fix replace_header to solve it.

@seankane-msft seankane-msft self-requested a review April 28, 2021 13:23
@msyyc msyyc merged commit 7472807 into Azure:master May 6, 2021
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request May 6, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
893fa04 
…into add_purview_nspkg_ci

* 'master' of https://github.com/Azure/azure-sdk-for-python:
  del useless files (Azure#18528)
  delete existing useless files for trafficmanager (Azure#18525)
  Define new replacer to replace keys in recording (Azure#18294)
  [purview] add purview nspkg to ci (Azure#18523)
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request May 6, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
3dc1198 
…into azure_purview_scanning

* 'master' of https://github.com/Azure/azure-sdk-for-python: (550 commits)
  del useless files (Azure#18528)
  delete existing useless files for trafficmanager (Azure#18525)
  Define new replacer to replace keys in recording (Azure#18294)
  [purview] add purview nspkg to ci (Azure#18523)
  ignore analysis for swagger readmes (Azure#18520)
  [purview] add azure-purview-nspkg (Azure#18518)
  [AppConfiguration] Appconfig consistency (Azure#18493)
  [Container Registry] Improved samples (Azure#18263)
  [Container Registry] renamings (Azure#18492)
  [ServiceBus] internal code rename and sample readme update (Azure#18516)
  [EventHub] update link in sample readme (Azure#18517)
  Post Process Event Names Script (Azure#18419)
  [Tables] use etag from entity if match condition is given (Azure#18271)
  adding operation-location to headers that are scrubbed (Azure#18514)
  [Tables] Adds support for AzureNamedKeyCredential (Azure#18456)
  [Tables] delete_entity takes an entity instead of row and partition key (Azure#18269)
  [Tables] Removed TableEntity attribute wrapper (Azure#18489)
  [EventHub&ServiceBus] Bump uAMQP dependency (Azure#17942)
  [ServiceBus] add keyword override support to update_ methods in mgmt module (Azure#18210)
  Add compatibility switch to disable CAE (Azure#18148)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seankane-msft seankane-msft seankane-msft approved these changes

@msyyc msyyc msyyc approved these changes

@jsntcy jsntcy Awaiting requested review from jsntcy

@changlong-liu changlong-liu Awaiting requested review from changlong-liu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@00Kai0 @seankane-msft @msyyc