New resource: azurerm_eventgrid_partner_configuration #28676
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wyattfry
reviewed
Feb 13, 2025
| @@ -29,12 +29,15 @@ func (r Registration) WebsiteCategories() []string { | |||
| } | |||
|
|
|||
| func (r Registration) DataSources() []sdk.DataSource { | |||
| return []sdk.DataSource{} | |||
| return []sdk.DataSource{ | |||
| // todo: decide if this needs a data source | |||
There was a problem hiding this comment.
Is this still needed? If not, please remove the comment.
Suggested change
| // todo: decide if this needs a data source |
wyattfry
reviewed
Feb 13, 2025
| } | ||
|
|
||
| type PartnerAuthorization struct { | ||
| PartnerRegistrationImmutableId string `tfschema:"partner_registration_immutable_id"` |
There was a problem hiding this comment.
Considering that IDs are generally immutable, could we rename this property?
Suggested change
| PartnerRegistrationImmutableId string `tfschema:"partner_registration_immutable_id"` | |
| PartnerRegistrationId string `tfschema:"partner_registration_id"` |
| MinItems: 1, | ||
| Elem: &pluginsdk.Resource{ | ||
| Schema: map[string]*pluginsdk.Schema{ | ||
| "partner_registration_immutable_id": &schema.Schema{ |
| } | ||
|
|
||
| param := partnerconfigurations.PartnerConfiguration{ | ||
| Location: pointer.To("global"), |
There was a problem hiding this comment.
Is location not configurable? If it is, then could we add an input property above?
There was a problem hiding this comment.
Location is not configurable, only "global" is a valid value. Please refer to the create screen in portal below. Note that there no location input.
| return commonids.ValidateResourceGroupID | ||
| } | ||
|
|
||
| func expandAuthorizedPartnersList(partnerAuthorization *[]PartnerAuthorization) *[]partnerconfigurations.Partner { |
There was a problem hiding this comment.
Suggested change
| func expandAuthorizedPartnersList(partnerAuthorization *[]PartnerAuthorization) *[]partnerconfigurations.Partner { | |
| func expandAuthorizedPartnersList(partnerAuthorization []PartnerAuthorization) *[]partnerconfigurations.Partner { |
In the model, it's defined as a value, not a pointer.
Comment on lines
237
to
241
| partners := []partnerconfigurations.Partner{} | ||
|
|
||
| if partnerAuthorization == nil { | ||
| return &partners | ||
| } |
There was a problem hiding this comment.
Suggested change
| partners := []partnerconfigurations.Partner{} | |
| if partnerAuthorization == nil { | |
| return &partners | |
| } | |
| if len(partnerAuthorization) == 0 { | |
| return nil | |
| } | |
| partners := []partnerconfigurations.Partner{} |
Sending an empty slice is not equivalent to sending a nil, which is preferred for an unspecified property.
Comment on lines
96
to
103
| template := r.basic(data) | ||
| return fmt.Sprintf(` | ||
| %s | ||
|
|
||
| resource "azurerm_eventgrid_partner_configuration" "import" { | ||
| resource_group_name = azurerm_resource_group.test.name | ||
| } | ||
| `, template) |
There was a problem hiding this comment.
Suggested change
| template := r.basic(data) | |
| return fmt.Sprintf(` | |
| %s | |
| resource "azurerm_eventgrid_partner_configuration" "import" { | |
| resource_group_name = azurerm_resource_group.test.name | |
| } | |
| `, template) | |
| return fmt.Sprintf(` | |
| %s | |
| resource "azurerm_eventgrid_partner_configuration" "import" { | |
| resource_group_name = azurerm_resource_group.test.name | |
| } | |
| `, r.basic(data)) |
| }) | ||
| } | ||
|
|
||
| func TestAccEventGridPartnerConfiguration_complete(t *testing.T) { |
There was a problem hiding this comment.
Could you please add more steps so the update path is also tested? e.g. complete and then basic. This will ensure that the other properties in complete also work in create.
| authorization_expiration_time_in_utc = "%s" | ||
| } | ||
|
|
||
| tags = { "foo" = "bar" } |
There was a problem hiding this comment.
Suggested change
| tags = { "foo" = "bar" } | |
| tags = { | |
| foo = "bar" | |
| } |
Just to make it more consistent with other test configurations.
|
|
||
| * `partner_registration_immutable_id` - (Required) The immutable id of the corresponding partner registration. | ||
|
|
||
| * `authorization_expiration_time_in_utc` - (Optional) Expiration time of the partner authorization. If this timer expires, any request from this partner to create, update or delete resources in subscriber's context will fail. Value should be in RFC 3339 format in UTC time zone, for example: "2025-02-04T00:00:00Z". If not specified, the authorization will expire after `default_maximum_expiration_time_in_days`. |
There was a problem hiding this comment.
Suggested change
| * `authorization_expiration_time_in_utc` - (Optional) Expiration time of the partner authorization. If this timer expires, any request from this partner to create, update or delete resources in subscriber's context will fail. Value should be in RFC 3339 format in UTC time zone, for example: "2025-02-04T00:00:00Z". If not specified, the authorization will expire after `default_maximum_expiration_time_in_days`. | |
| * `authorization_expiration_time_in_utc` - (Optional) Expiration time of the partner authorization. Value should be in RFC 3339 format in UTC time zone, for example: "2025-02-04T00:00:00Z". | |
| -> **Note:** If the time from `authorization_expiration_time_in_utc` expires, any request from this partner to create, update or delete resources in the subscriber's context will fail. If not specified, the authorization will expire after `default_maximum_expiration_time_in_days`. |
| The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/language/resources/syntax#operation-timeouts) for certain actions: | ||
|
|
||
| * `create` - (Defaults to 30 minutes) Used when creating the Event Grid Partner Configuration. | ||
| * `read` - (Defaults to 30 minutes) Used when retrieving the Event Grid Partner Configuration. |
There was a problem hiding this comment.
Suggested change
| * `read` - (Defaults to 30 minutes) Used when retrieving the Event Grid Partner Configuration. | |
| * `read` - (Defaults to 5 minutes) Used when retrieving the Event Grid Partner Configuration. |
…horization as a value instead of a pointer
|
Thx @wyattfry , I have addressed all the feedbacks (marked with 👍emoji) and all acctest passes (log attached below). Please give this another review.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Community Note
Description
New resource for Event Grid Partner Configuration
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Testing
Acceptance test ran with command:
make acctests SERVICE='eventgrid' TESTARGS=' run="TestAccEventGridPartnerConfiguration_"' TESTTIMEOUT='120m'Test output: link (access restricted, please DM me on Slack)
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_eventgrid_partner_configuration- new resource [Support for Event Grid Partner resources #13611]This is a (please select all that apply):
Related Issue(s)
#13611 Note: please do not close the issue as there are other resources still yet to be implemented.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.