-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
azurerm_notification_hub
: Support new property browser_credential
#27058
azurerm_notification_hub
: Support new property browser_credential
#27058
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sinbai, please take a look at the comments left in-line.
Required: true, | ||
ValidateFunc: validation.StringIsNotEmpty, | ||
}, | ||
"vapid_private_key": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be marked as Sensitive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -74,6 +76,16 @@ A `apns_credential` block contains: | |||
|
|||
--- | |||
|
|||
A `browser_credential` block contains: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A `browser_credential` block contains: | |
A `browser_credential` supports the following: |
Can you please update this for apns_credential
and gcm_credential
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Hi @stephybun thanks for your time and feedback. I have updated the code, could you please take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sinbai LGTM 👍
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
Support new property
browser_credential
.Swagger:
https://github.com/Azure/azure-rest-api-specs/blob/abad0096677005817d2c19df2364663e5583c8fc/specification/notificationhubs/resource-manager/Microsoft.NotificationHubs/stable/2023-09-01/notificationhubs.json#L2069
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
https://hashicorp.teamcity.com/buildConfiguration/TF_AzureRM_AZURERM_SERVICE_PUBLIC_NOTIFICATIONHUB/219255?buildTab=tests
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_notification_hub
- support for thebrowser_credential
property [Support for Azure Notification Hub browser (web push) credentials #25588]This is a (please select all that apply):
Related Issue(s)
Fixes #25588