web - migrate from azure-sdk-for-go to go-azure-sdk#31996
web - migrate from azure-sdk-for-go to go-azure-sdk#31996sreallymatt wants to merge 17 commits intomainfrom
web - migrate from azure-sdk-for-go to go-azure-sdk#31996Conversation
|
Build failure This pull request contains a build failure which needs addressed here . |
|
Build failure This pull request contains a build failure which needs addressed here . |
028dc00 to
4dda026
Compare
|
Build failure This pull request contains a build failure which needs addressed here . |
… to `go-azure-sdk`
36ce222 to
1a20b07
Compare
jackofallops
left a comment
There was a problem hiding this comment.
Thanks @sreallymatt - I know this is based on another open PR, so hopefully I've not repeated myself! Just a few comments to take a look at.
| } | ||
|
|
||
| appSettingsResp, err := clients.Web.AppServicesClient.ListApplicationSettings(ctx, id.ResourceGroup, id.SiteName) | ||
| appSettingsResp, err := clients.Web.AppServicesClientV1.ListApplicationSettings(ctx, id.ResourceGroup, id.SiteName) |
There was a problem hiding this comment.
as mentioned in #31967 comments, this should be using the AppService client as per the resource.
| existing, err := client.Get(ctx, id) | ||
| if !response.WasNotFound(existing.HttpResponse) { | ||
| if err != nil { |
There was a problem hiding this comment.
whilst this will work, idiom say check err first, which will keep this consistent with the rest of the provider.
| ValidityInYears: pointer.To(int64(d.Get("validity_in_years").(int))), | ||
| }, | ||
| Location: location.Normalize(d.Get("location").(string)), | ||
| Tags: utils.ExpandPtrMapStringString(d.Get("tags").(map[string]interface{})), |
There was a problem hiding this comment.
should probably switch this to tags? (needs import)
| Tags: utils.ExpandPtrMapStringString(d.Get("tags").(map[string]interface{})), | |
| Tags: tags.Expand(d.Get("tags").(map[string]interface{})), |
|
|
||
| if model := resp.Model; model != nil { | ||
| d.Set("location", location.Normalize(model.Location)) | ||
| d.Set("tags", model.Tags) |
There was a problem hiding this comment.
Should probably flatten this for consistency?
| d.Set("tags", model.Tags) | |
| d.Set("tags", tags.Flatten(model.Tags)) |
| } | ||
|
|
||
| if d.HasChange("tags") { | ||
| existing.Model.Tags = utils.ExpandPtrMapStringString(d.Get("tags").(map[string]interface{})) |
There was a problem hiding this comment.
| existing.Model.Tags = utils.ExpandPtrMapStringString(d.Get("tags").(map[string]interface{})) | |
| existing.Model.Tags = tags.Expand(d.Get("tags").(map[string]interface{})) |
| if props.IssueDate != nil { | ||
| issueDate = props.IssueDate.Format(time.RFC3339) | ||
| if model := resp.Model; model != nil { | ||
| d.Set("tags", model.Tags) |
There was a problem hiding this comment.
| d.Set("tags", model.Tags) | |
| d.Set("tags", tags.Flatten(model.Tags)) |
| } | ||
|
|
||
| payload := certificates.CertificatePatchResource{ | ||
| Tags: utils.ExpandPtrMapStringString(d.Get("tags").(map[string]interface{})), |
There was a problem hiding this comment.
| Tags: utils.ExpandPtrMapStringString(d.Get("tags").(map[string]interface{})), | |
| Tags: tags.Expand(d.Get("tags").(map[string]interface{})), |
| if !response.WasNotFound(existing.HttpResponse) { | ||
| if err != nil { |
There was a problem hiding this comment.
again, should check err first.
| if response.WasNotFound(existing.HttpResponse) { | ||
| d.SetId("") |
There was a problem hiding this comment.
Looks like we're historically missing a log message here to inform the user why the resource is disappearing from state?
|
|
||
| d.SetId(id.ID()) | ||
|
|
||
| return pluginsdk.Retry(d.Timeout(pluginsdk.TimeoutCreate), func() *pluginsdk.RetryError { |
There was a problem hiding this comment.
Not one for now, but this needs reworking to remove the Retry()
Community Note
Description
This PR migrates all the non-deprecated
webresources/datasources fromazure-sdk-for-gotogo-azure-sdk. Additionally I've cleaned up some old resource IDs and replaced them with resource IDs fromgo-azure-sdkorgo-azure-helpers.Most changes are split into separate commits per resource to help the reviewers. Not feasible to split into separate PRs due to most changes being intertwined
I did not replace the deprecated resources in the non-deprecated resources tests', I'll follow up with a separate PR to do so
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Tests running...
State Migration for
azurerm_app_service_custom_hostname_binding:manual tests for state migration
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource- support for thething1property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #25942
AI Assistance Disclosure
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.