PaloAltoNetworks / terraform-provider-panos

Terraform Panos provider

https://www.terraform.io/docs/providers/panos/

MIT License

89 stars 71 forks source link

add resource panos_ssl_decrypt_exclude_certificate_entry #345

Closed TiantongHu closed 2 years ago

TiantongHu commented 2 years ago

Description

add resource panos_ssl_decrypt_exclude_certificate_entry which manages SSL Decrypt Exclusion setting without toching other SSL decrypt settings

Motivation and Context

342

How Has This Been Tested?

Tested using the resource to modify ssl decryption exclusion with exsiting forward_trust_certificate_rsa and other ssl drcrypt settings, nothing gets changed except ssl decryption exclusion setting. Tested create, update, delete. All working correctly (only changing ssl decrption exclusion) Tested with resource ssl_decrypt, working as expected.

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

[x] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes if appropriate.
[x] All new and existing tests passed.

welcome-to-palo-alto-networks[bot] commented 2 years ago

:tada: Thanks for opening this pull request! We really appreciate contributors like you! :raised_hands:

TiantongHu commented 2 years ago

Hi @shinmog , the PR is ready for review. Please let me know if anything needs to be changed :) thanks

TiantongHu commented 2 years ago

Hi @shinmog , is there anything that needs to be added?

TiantongHu commented 2 years ago

I appreciate that you did documentation updates in this PR..!

But unfortunately this code has a race condition if used in real life. If you look at the other code for modules that do _entry operations you'll see that they tend to use special functions that operate on individual members, which prevents race conditions if there are multiple _entry resources running at the same time.

Implementing a panos_ssl_decrypt_exclude_certificate_entry resource will require a pango SDK update first and foremost.

Hi, I'm working on pango changes. I see the SslDecryptTrustedRootCaEntry sets ForceNew to true. For SslDecryptExcludeCertificateEntry, does it matter to have an Update function? Is it OK to also set ForceNew to true? Thanks

TiantongHu commented 2 years ago

I've updated the PR with another account in https://github.com/PaloAltoNetworks/terraform-provider-panos/pull/348 Please review that one instead. Tanks