Skip to content
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

Update costmanagement API to 2023-08-01 #27680

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

s4heid
Copy link

@s4heid s4heid commented Oct 17, 2024

Community Note

  • Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
  • Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

This change updates the costmanagement API to version 2023-08-01, ensuring compatibility with both the public AzureCloud and AzureChinaCloud. It's important to note that AzureChinaCloud has specific API version requirements, and version 2023-08-01 meets these.

In addition, the azurerm_cost_anomaly_alert resource now includes a new optional parameter, email_address_sender, which is sent to the API as properties.notificationEmail. Should this parameter be omitted, the TF provider will default to using the first email address listed in email_addresses, aligning with the default behavior observed in the Azure Portal.

The ScheduledActionsClient_v2022_10_01, which previously existed alongside the ScheduledActionsClient, has been discontinued. This is because the API version 2023-08-01 seems to be compatible with all uses.

PR Checklist

  • I have followed the guidelines in our Contributing Documentation.
  • I have checked to ensure there aren't other open Pull Requests for the same update/change.
  • I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
  • -I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.-
  • I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
    For example: “resource_name_here - description of change e.g. adding property new_property_name_here

Changes to existing Resource / Data Source

  • I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
  • I have written new tests for my resource or datasource changes & updated any relevent documentation.
  • I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
  • (For changes that include a state migration only). -I have manually tested the migration path between relevant versions of the provider.-

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

  • azurerm_resource_group_cost_management_export - upgrade the API version to 2023-08-01 which also supports AzureChinaCloud

This is a (please select all that apply):

  • Bug Fix
  • New Feature (ie adding a service, resource, or data source)
  • Enhancement
  • Breaking Change

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

Additonal Context

When creating the resource with an old API version (let's say 2022-06-01-preview), the properties.notificationEmail is null in the response. With API version 2023-11-01, if you try to get the previously created alert, the response contains the first entry of the list properties.notification.to.

This change updates the costmanagement API to version 2023-08-01, ensuring
compatibility with both the public Azure Cloud and AzureChinaCloud. It's
important to note that AzureChinaCloud has specific API version requirements,
and version 2023-08-01 meets these.

In addition, the `azurerm_cost_anomaly_alert` resource now includes a new
optional parameter, `email_address_sender`, introduced in this API version.
Should this parameter be omitted, the the TF provider will default to using the
first email address listed in `email_addresses`, aligning with the default
behavior observed in the Azure Portal.

The ScheduledActionsClient_v2022_10_01, which previously existed alongside the
ScheduledActionsClient, has been discontinued. This is because the API version
2023-08-01 seems to be compatible with all uses.
@katbyte katbyte requested review from katbyte and a team as code owners November 5, 2024 19:17
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @s4heid - we will need to update the docs with this new property.

@@ -48,6 +48,13 @@ func (AnomalyAlertResource) Arguments() map[string]*pluginsdk.Schema {
ValidateFunc: commonids.ValidateSubscriptionID,
},

"email_address_sender": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you explain what this property does as this is different from the API/SDK name here.
notification email and sender email seem rather different

my guess is it is the email the notification is sent from? if so perhapes it would be better as

Suggested change
"email_address_sender": {
"notification_from_email_address": {

Copy link
Author

@s4heid s4heid Nov 6, 2024

Choose a reason for hiding this comment

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

@katbyte Based on my understanding, this email is not the sender of the notifications. In the Azure REST API specifications, the property is called notificationEmail and it is used to specify the email address of the person who should receive notifications related to the scheduled actions (e.g. if someone likes to unsubscribe from the notifications).

Email address of the point of contact that should get the unsubscribe requests and notification emails.

The reason why I used email_address_sender, is because scheduled actions already sets the field NotificationEmail for ScheduledActionProperties and called the parameter email_address_sender.

However, I agree with you that it would be odd to have a different naming than API / SDK.

I'll update the parameter to notification_email if that works for you.

@s4heid
Copy link
Author

s4heid commented Nov 6, 2024

@katbyte I added a commit to reflect the new parameter in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants