-
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
Update NGINXaaS API: 2024-09-01-preview #27776
Conversation
Depends on #27775. Will rebase and pull latest main once the latest SDK is in. |
Need to fix up property deprecation. |
7c0417f
to
865c786
Compare
This is a rough/incomplete draft for now. CC @stephybun (Based on our discussion, this needs to be chatted about within the Terraform team before we move forward with this work). |
865c786
to
3870172
Compare
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.
Hey @puneetsarna
Before I dive into a detailed review here, I noticed that the acceptance tests for the configuration resource begun failing from the 15th of October. There is a related PR open that attempts to fix this but it seems there are some issues around testing here.
This test passing is a pre-requisite to reviewing and merging this change since we rely on those to validate that the deprecation and changes in this PR have been done correctly and won't cause any additional issues or unexpected side effects for users.
Would you be able to work with the author of the PR linked above to get that test fixed?
Thanks @stephybun I took a look at that PR and have left some comments. Can you help review it as well please? then we can start on this PR also? |
@puneetsarna could you rebase this on top of main to pull in those nginx configuration test fixes? |
3870172
to
bb5ce8a
Compare
@stephybun Done 👍 |
bb5ce8a
to
c8b71af
Compare
I was missing the tag I ran the tests locally at 5.0 and they are passing now 👍 |
6a593c3
to
002563e
Compare
002563e
to
0207626
Compare
This commit updates the NGINXaaS API version to be the latest: 2024-09-01-preview.
0207626
to
3c4a4db
Compare
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 @puneetsarna! There are still a few test failures in 5.0 mode, once those are fixed this should be good to go!
Test ended in panic.
------- Stdout: -------
=== RUN TestAccNginxConfigurationDataSource_basic
=== PAUSE TestAccNginxConfigurationDataSource_basic
=== CONT TestAccNginxConfigurationDataSource_basic
------- Stderr: -------
panic: Invalid address to set: []string{"protected_file", "0", "content"}
goroutine 2231 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc0029fbc80, {0x6ca2375, 0xe}, {0x6f80740, 0xc004f68b70})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:233 +0x2b4
github.com/hashicorp/terraform-provider-azurerm/internal/sdk.ResourceMetaData.Encode({0xc004e52488, {0x8c6ca20, 0xc001ed4678}, 0xc0029fbc80, 0x0, {0x8c6ca58, 0xe52f5c0}}, {0x6e03c40, 0xc00421d6e0})
/opt/teamcity-agent/work/3337027aeff310bf/internal/sdk/resource_encode.go:31 +0x247
github.com/hashicorp/terraform-provider-azurerm/internal/services/nginx.(*ConfigurationDataSource).Read.ConfigurationDataSource.Read.func1({0x8c6c898, 0xc005137180}, {0xc004e52488, {0x8c6ca20, 0xc001ed4678}, 0xc0029fbc80, 0x0, {0x8c6ca58, 0xe52f5c0}})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/nginx/nginx_configuration_data_source.go:163 +0x5b3
Test ended in panic.
------- Stdout: -------
=== RUN TestAccNginxDeploymentDataSource_basic
=== PAUSE TestAccNginxDeploymentDataSource_basic
=== CONT TestAccNginxDeploymentDataSource_basic
------- Stderr: -------
panic: Invalid address to set: []string{"managed_resource_group"}
goroutine 2163 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc003ed7100, {0x6f7f3d1, 0x16}, {0x7102740, 0xe531ba0})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:233 +0x2b4
github.com/hashicorp/terraform-provider-azurerm/internal/sdk.ResourceMetaData.Encode({0xc00252ed88, {0x8c6ca20, 0xc005a2f2f0}, 0xc003ed7100, 0x0, {0x8c6ca58, 0xe52f5c0}}, {0x6e03d00, 0xc002a89040})
/opt/teamcity-agent/work/3337027aeff310bf/internal/sdk/resource_encode.go:31 +0x247
github.com/hashicorp/terraform-provider-azurerm/internal/services/nginx.(*DeploymentDataSource).Read.DeploymentDataSource.Read.func1({0x8c6c898, 0xc003e9f960}, {0xc00252ed88, {0x8c6ca20, 0xc005a2f2f0}, 0xc003ed7100, 0x0, {0x8c6ca58, 0xe52f5c0}})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/nginx/nginx_deployment_data_source.go:324 +0x898
Test ended in panic.
------- Stdout: -------
=== RUN TestAccNginxDeploymentDataSource_autoscaling
=== PAUSE TestAccNginxDeploymentDataSource_autoscaling
=== CONT TestAccNginxDeploymentDataSource_autoscaling
------- Stderr: -------
panic: Invalid address to set: []string{"managed_resource_group"}
goroutine 2122 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*ResourceData).Set(0xc002bbac80, {0x6f7f3d1, 0x16}, {0x7102740, 0xe531ba0})
/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource_data.go:233 +0x2b4
github.com/hashicorp/terraform-provider-azurerm/internal/sdk.ResourceMetaData.Encode({0xc004914908, {0x8c6ca20, 0xc0053f3680}, 0xc002bbac80, 0x0, {0x8c6ca58, 0xe52f5c0}}, {0x6e03d00, 0xc002a6d900})
/opt/teamcity-agent/work/3337027aeff310bf/internal/sdk/resource_encode.go:31 +0x247
github.com/hashicorp/terraform-provider-azurerm/internal/services/nginx.(*DeploymentDataSource).Read.DeploymentDataSource.Read.func1({0x8c6c898, 0xc002835110}, {0xc004914908, {0x8c6ca20, 0xc0053f3680}, 0xc002bbac80, 0x0, {0x8c6ca58, 0xe52f5c0}})
/opt/teamcity-agent/work/3337027aeff310bf/internal/services/nginx/nginx_deployment_data_source.go:324 +0x898
3c4a4db
to
19dc3bb
Compare
19dc3bb
to
f384f1a
Compare
…ntent of protected_files The service provider has never returned the contents of a protected file in the API response. The latest swagger splits up the request,response objects on protected files, and this commit incorporates the swagger change while deprecating the `content` field on the protect file wherever we do a READ on it.
This field was deprecated by the service provider a long time back. This commit just deprectes the same field in terrform in the next provider version.
The provider upgrade guide needs to be updated to call out deprecated fields that will be removed in 5.0.
- Updates to docs to remove deprecated properties.
ebad301
to
5f4f2ad
Compare
Thanks @stephybun. I had missed out on more TF_ACC=1 go test -v ./internal/services/nginx -run=TestAccNginxConfigurationDataSource_basic -timeout 180m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN TestAccNginxConfigurationDataSource_basic
=== PAUSE TestAccNginxConfigurationDataSource_basic
=== CONT TestAccNginxConfigurationDataSource_basic
--- PASS: TestAccNginxConfigurationDataSource_basic (439.64s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/nginx 442.521s
-------------------------------------------------------------------------------------------------
=== CONT TestAccNginxDeploymentDataSource_basic
--- PASS: TestAccNginxDeploymentDataSource_basic (405.34s)
--- PASS: TestAccNginxDeploymentDataSource_autoscaling (393.13s)
PASS |
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 @puneetsarna LGTM 👍
* Update CHANGELOG.md for #26304 * Update CHANGELOG.md for #28211 * Update for #28016 * Update for #28139 * Update for #27776 * Update for #28227 * Update for #28080 * Update for #28228 * Update for #27915 * reword nginx api upgrade * Update for #28160 * Update for #28043 * run changelog-update-for-release.sh --------- Co-authored-by: stephybun <steph@hashicorp.com> Co-authored-by: kt <kt@katbyte.me>
Community Note
Description
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource
- support for thething1
property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #0000
Note
If this PR changes meaningfully during the course of review please update the title and description as required.