azurerm_nginx_deployment - support for the configuration block #24276

wuxu92 · 2023-12-19T06:51:35Z

deprecate independent azurerm_nginx_configuration resource and skip exists AccTests for it. This PR supersedes #24041.

--- PASS: TestAccNginxDeployment_withConfiguration (511.17s)
--- PASS: TestAccNginxDeployment_updateWithConfiguration (484.44s)
PASS

katbyte · 2024-02-20T22:31:32Z

internal/services/nginx/nginx_configuration_resource_test.go

@@ -32,6 +32,7 @@ func (a ConfigurationResource) Exists(ctx context.Context, client *clients.Clien
 }

 func TestAccConfiguration_basic(t *testing.T) {
+	t.Skipf("Skipping test due to deprecated nginx_configuration resource")


since these are broken and never will be fixed should we just remove them,?

katbyte · 2024-02-20T22:32:37Z

website/docs/r/nginx_configuration.html.markdown

@@ -10,6 +10,8 @@ description: |-

 Manages the configuration for a Nginx Deployment.

+~> **NOTE** Do not use the independent `azurerm_nginx_configuration` resource anymore due to a [known API issue](https://docs.nginx.com/nginxaas/azure/known-issues/#i-classfa-solid-fa-bug-stylecolore4002bi-terraform-shows-an-error-while-trying-to-manage-configuration-of-a-fresh-deployment-id-891). Use the `configuration` block of [azurerm_nginx_deployment](./nginx_deployment#configuration) instead.


should we just remove this resource documentation entirely as it no longer works on creation & deprecate the resouce?

wuxu92 · 2024-03-05T02:43:03Z

AccTest pass

katbyte · 2024-03-05T04:28:00Z

internal/services/nginx/nginx_deployment_resource_test.go

+
+
+
+


Suggested change

katbyte · 2024-03-05T04:28:26Z

internal/services/nginx/nginx_configuration_resource.go

@@ -17,6 +17,10 @@ import (
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
 )

+// This resource has been deprecated and will be removed in the next major release.


we should mark this resource as deprecated with instructions to use the new resource?

katbyte

LGTM 🎫

katbyte · 2024-03-21T23:15:22Z

good to mege once merge conflicts are resolved

wuxu92 · 2024-03-22T02:20:51Z

@katbyte rebased main.

github-actions · 2024-03-22T09:03:52Z

This functionality has been released in v3.97.1 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!

puneetsarna · 2024-03-22T15:18:00Z

Hi @katbyte @wuxu92 https://docs.nginx.com/nginxaas/azure/changelog/#march-13-2024 mentions that the issue with the default configuration was fixed. Can we please restore the azurerm_nginx_configuration resource?

<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.96.0" to "3.97.1" in file ".terraform.lock.hcl"</p> <details> <summary>3.97.1</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.97.1
ENHANCEMENTS:

* `azurerm_nginx_deployment` - support for the `configuration` block ([#24276](https://github.com/hashicorp/terraform-provider-azurerm/issues/24276))

BUG FIXES:

* `azurerm_data_factory_integration_runtime_self_hosted` - ensure that autorizationh keys are exported ([#25246](hashicorp/terraform-provider-azurerm#25246 `azurerm_storage_account` - defaulting the value for `dns_endpoint_type` to `Standard` when it's not returned from the Azure API ([#25367](https://github.com/hashicorp/terraform-provider-azurerm/issues/25367))



</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/69/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>

…ashicorp#24276)" This reverts commit 7a1f9f7.

wuxu92 · 2024-03-26T06:08:21Z

@puneetsarna The azurerm_nginx_configuration can be used until the next major release. Since only one default configuration can be created, it makes more sense to embed it into the nginx deployment resource rather than using a separate resource.

puneetsarna · 2024-03-26T14:11:21Z

@wuxu92 I see what you mean here around the config resource being a singleton. I have called out the explanation around why having the separate resource is very useful for us (see PR description here #25402). Let's continue to discuss it over the other PR.

I understand that the resource was only deprecated and not removed but the users upgrading to the latest terraform won't see any documentation either thinking that the resource was retired (or will be soon). I believe this will also cause user panic as they would have to migrate to the new embedded resource if they want to use the latest provider. Lastly, users might see issues while transitioning to this new embedded resource as the transition would involve downtime:

User removes TF resource and tries to use embedded resource.
Apply would trigger a conf destruction (🔴 ) and re-application.
Config destruction would cause downtime for the user.

Let's continue these discussions over here: #25402

…ashicorp#24276)" This reverts commit 7a1f9f7.

github-actions · 2024-04-26T02:06:58Z

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

wuxu92 marked this pull request as ready for review December 19, 2023 07:47

catriona-m added the service/nginx label Dec 20, 2023

tombuildsstuff mentioned this pull request Jan 22, 2024

tools/importer-rest-api-specs: adding a workaround covering Azure/azure-rest-api-specs#26883 hashicorp/pandora#3417

Closed

katbyte requested changes Feb 20, 2024

View reviewed changes

katbyte added the waiting-response label Feb 21, 2024

wuxu92 force-pushed the nginx/config branch from 00a0523 to 8afade1 Compare March 5, 2024 01:48

github-actions bot added documentation size/XL waiting-response and removed waiting-response labels Mar 5, 2024

wuxu92 force-pushed the nginx/config branch from 8afade1 to 711e909 Compare March 5, 2024 01:51

github-actions bot added waiting-response and removed waiting-response labels Mar 5, 2024

katbyte requested changes Mar 5, 2024

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Mar 5, 2024

katbyte approved these changes Mar 21, 2024

View reviewed changes

embed nginx configuration into deployment resource

734c975

wuxu92 force-pushed the nginx/config branch from d81b470 to 734c975 Compare March 22, 2024 01:23

wuxu92 added 3 commits March 22, 2024 09:27

return deprecate in favour of resource

42b69c7

ignore not found error for configuration get operation

f2d6de0

format

e3e9055

katbyte merged commit 7a1f9f7 into hashicorp:main Mar 22, 2024
32 checks passed

github-actions bot added this to the v3.98.0 milestone Mar 22, 2024

katbyte changed the title ~~Nginx: embed nginx configuration into deployment resource~~ azurerm_nginx_deployment - support for the configuration block Mar 22, 2024

katbyte added a commit that referenced this pull request Mar 22, 2024

Update CHANGELOG.md #24276

6e413bc

wuxu92 deleted the nginx/config branch March 22, 2024 07:03

tombuildsstuff modified the milestones: v3.98.0, v3.97.1 Mar 22, 2024

puneetsarna added a commit to puneetsarna/terraform-provider-azurerm that referenced this pull request Mar 25, 2024

Revert "Nginx: embed nginx configuration into deployment resource (h…

6996302

…ashicorp#24276)" This reverts commit 7a1f9f7.

puneetsarna mentioned this pull request Mar 25, 2024

Restore azurerm_nginx_configuration resource and data source #25402

Closed

valyria257 added a commit to valyria257/terraform-provider-azurerm that referenced this pull request Apr 24, 2024

Revert "Nginx: embed nginx configuration into deployment resource (h…

6daeb9a

…ashicorp#24276)" This reverts commit 7a1f9f7.

valyria257 mentioned this pull request Apr 24, 2024

Restore azurerm_nginx_configuration resource and data source #25740

Closed

github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azurerm_nginx_deployment - support for the configuration block #24276

azurerm_nginx_deployment - support for the configuration block #24276

wuxu92 commented Dec 19, 2023 •

edited

Loading

katbyte Feb 20, 2024

katbyte Feb 20, 2024

wuxu92 commented Mar 5, 2024

katbyte Mar 5, 2024

wuxu92 Mar 5, 2024

katbyte Mar 5, 2024

wuxu92 Mar 5, 2024

katbyte left a comment

katbyte commented Mar 21, 2024

wuxu92 commented Mar 22, 2024

github-actions bot commented Mar 22, 2024

puneetsarna commented Mar 22, 2024

wuxu92 commented Mar 26, 2024

puneetsarna commented Mar 26, 2024

github-actions bot commented Apr 26, 2024

		@@ -10,6 +10,8 @@ description: \|-

		Manages the configuration for a Nginx Deployment.

		~> NOTE Do not use the independent `azurerm_nginx_configuration` resource anymore due to a [known API issue](https://docs.nginx.com/nginxaas/azure/known-issues/#i-classfa-solid-fa-bug-stylecolore4002bi-terraform-shows-an-error-while-trying-to-manage-configuration-of-a-fresh-deployment-id-891). Use the `configuration` block of [azurerm_nginx_deployment](./nginx_deployment#configuration) instead.

azurerm_nginx_deployment - support for the configuration block #24276

azurerm_nginx_deployment - support for the configuration block #24276

Conversation

wuxu92 commented Dec 19, 2023 • edited Loading

katbyte Feb 20, 2024

Choose a reason for hiding this comment

katbyte Feb 20, 2024

Choose a reason for hiding this comment

wuxu92 commented Mar 5, 2024

katbyte Mar 5, 2024

Choose a reason for hiding this comment

wuxu92 Mar 5, 2024

Choose a reason for hiding this comment

katbyte Mar 5, 2024

Choose a reason for hiding this comment

wuxu92 Mar 5, 2024

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

katbyte commented Mar 21, 2024

wuxu92 commented Mar 22, 2024

github-actions bot commented Mar 22, 2024

puneetsarna commented Mar 22, 2024

wuxu92 commented Mar 26, 2024

puneetsarna commented Mar 26, 2024

github-actions bot commented Apr 26, 2024

wuxu92 commented Dec 19, 2023 •

edited

Loading