Skip to content

Prevent AttributeError during get_certificate_version #12747

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
chlowell merged 7 commits into from
Jul 29, 2020
Merged

Prevent AttributeError during get_certificate_version #12747

chlowell merged 7 commits into from
Jul 29, 2020

Conversation

@chlowell
Copy link
Member

@chlowell chlowell commented Jul 27, 2020

When a client GETs a specific version of a certificate, Key Vault doesn't return that certificate's management policy. So get_certificate_version calls KeyVaultCertificate._from_certificate_bundle with a bundle whose policy attribute is None, provoking an AttributeError because that code path assumes policy is a model instance. This PR prevents the error and adds test coverage around get_certificate_version and list_properties_of_certificate_versions. I also noticed that some lists in test_example_certificate_list_operations are empty, meaning these tests aren't as useful as they appear, and tweaked them to ensure this is no longer the case.

@chlowell chlowell added KeyVault Client This issue points to a problem in the data-plane of the library. blocking-release Blocks release labels Jul 27, 2020
@chlowell chlowell requested a review from iscai-msft July 27, 2020 21:17
@chlowell chlowell requested a review from schaabs as a code owner July 27, 2020 21:17
iscai-msft
iscai-msft previously approved these changes Jul 28, 2020
View reviewed changes
sdk/keyvault/azure-keyvault-certificates/tests/test_certificates_client.py Outdated
@ResourceGroupPreparer(random_name_enabled=True)
@KeyVaultPreparer()
@KeyVaultClientPreparer()
def test_versions(self, client, **kwargs):
Copy link
Contributor

@iscai-msft iscai-msft Jul 28, 2020

Choose a reason for hiding this comment

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

nit: i wonder if it might be better to call it test_certificate_versions since I thought this had to do with API versions first

@chlowell chlowell requested a review from iscai-msft July 29, 2020 16:24
@chlowell chlowell merged commit 9b8d5c9 into Azure:master Jul 29, 2020
@chlowell chlowell deleted the list-cert-versions branch July 29, 2020 17:40
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 29, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
2549e53 
…into regenerate_certs

* 'master' of https://github.com/Azure/azure-sdk-for-python: (45 commits)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  Move InteractiveCredential to a new module (Azure#12706)
  Add foundations for eng/common restructure (Azure#12725)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12730)
  fix flaky test (Azure#12721)
  python link verification (Azure#12543)
  [ServiceBus] Remove legacy control client (Azure#12644)
  mirror update from MicrosoftDocs/azure-docs-sdk-python#709 (Azure#12691)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 29, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
40ae41c 
…into add_back_pii

* 'master' of https://github.com/Azure/azure-sdk-for-python: (40 commits)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  Move InteractiveCredential to a new module (Azure#12706)
  Add foundations for eng/common restructure (Azure#12725)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12730)
  fix flaky test (Azure#12721)
  python link verification (Azure#12543)
  [ServiceBus] Remove legacy control client (Azure#12644)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 30, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
7dc065c 
…into opinion_mining

* 'master' of https://github.com/Azure/azure-sdk-for-python: (69 commits)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  Move InteractiveCredential to a new module (Azure#12706)
  Add foundations for eng/common restructure (Azure#12725)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12730)
  fix flaky test (Azure#12721)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 30, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
2bd6154 
…into regenerate_secrets

* 'master' of https://github.com/Azure/azure-sdk-for-python: (52 commits)
  [key vault] Regenerate keys (Azure#12101)
  [Tables] Azure Data Tables SDK, sync and async code (Azure#12766)
  fix doc, sample, docstring (Azure#12784)
  clean up links for PII samples (Azure#12826)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 31, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
6cd0f4f 
…into ta_opinion_mining_sample

* 'master' of https://github.com/Azure/azure-sdk-for-python: (47 commits)
  [text analytics] opinion mining support (Azure#12542)
  [key vault] Regenerate keys (Azure#12101)
  [Tables] Azure Data Tables SDK, sync and async code (Azure#12766)
  fix doc, sample, docstring (Azure#12784)
  clean up links for PII samples (Azure#12826)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iscai-msft iscai-msft iscai-msft approved these changes

@schaabs schaabs Awaiting requested review from schaabs

Assignees

No one assigned

Labels

blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. KeyVault

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chlowell @iscai-msft