-
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
Upgrade sentinel API to 2021-09-01-preview #14983
Upgrade sentinel API to 2021-09-01-preview #14983
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.
hi @magodo
Thanks for this PR.
Taking a look through as you've mentioned there's some breaking changes in this PR - what's the migration path for existing users?
Thanks!
string(securityinsight.EntitiesMatchingMethodAll), | ||
string(securityinsight.EntitiesMatchingMethodCustom), | ||
string(securityinsight.EntitiesMatchingMethodNone), | ||
string(securityinsight.MatchingMethodAnyAlert), | ||
string(securityinsight.MatchingMethodSelected), | ||
string(securityinsight.MatchingMethodAllEntities), |
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.
what's the migration path for users/resources here?
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.
Just confirmed with the service team and verified in my local: the original enums All
, Custom
, None
corresponds to the AllEntities
, Selected
and AnyAlert
. If the user creats the alert rule using the old API version, and do a GET
using the new one, the enums are returned in their new form.
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.
Reckon for any enums that dont do this could translate them and provide a warning to the user for a few versions - although if all old enums behave like this users will know to fix their configuration when their is idempotency
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.
Good idea, I'll make the change.
@magodo - we have some licensing issues:
Do you know how we can resolve this in our tenant? |
@katbyte The license issue has been last for quite a while, I've ping you in the slack channel for the context of that issue. Personally, I don't have the knowledge about the license setup, since my account don't have that permission in the tenant. |
…i_to_2021_09_01_preview
@katbyte Is there any chance that we can get this PR merged? There are a couple of sentinel related new features pending on this PR. |
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 @magodo - LGTM 🏗️
This functionality has been released in v2.99.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
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. |
Upgrade sentinel API to 2021-09-01-preview.
Note that there is one breaking change in the
azurerm_sentinel_alert_rule_scheduled
resource, where the allowed enums of theentity_matching_method
changed from[All, Custom, None]
to[AnyAlert, Selected, AllEntities]
.Related issues: #14244, #14973