`azurerm_app_service_connection`: add support for `scope`, `configuration_info` and `public_network_solution` #26387

jiaweitao001 · 2024-06-19T07:48:24Z

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

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.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

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

azurerm_app_service_connection: add support for scope, configuration_info and public_network_solution

This is a (please select all that apply):

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

Related Issue(s)

Fixes #0000

Note

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

katbyte · 2024-07-23T16:32:09Z

website/docs/r/app_service_connection.html.markdown

+
+---
+
+A `configuration_info` block supports the following:


info is redundent can we remove it?

Suggested change

A `configuration_info` block supports the following:

A `configuration` block supports the following:

jackofallops

Thanks @jiaweitao001 - This appears to be fine, however, the _complete test now always fails due to timeout? Can you take a look? Also, there's no update coverage at all for this resource, can you take a look at covering that also?

Thanks!

jiaweitao001 · 2024-07-31T09:36:21Z

Thanks @jiaweitao001 - This appears to be fine, however, the _complete test now always fails due to timeout? Can you take a look? Also, there's no update coverage at all for this resource, can you take a look at covering that also?

Thanks!

Thanks @jackofallops , I've fixed the failing test, the config was not correctly configured. Actually there's update acc test coverage for this resource.

jackofallops

Thanks for the changes @jiaweitao001 - just a couple of items to take another look at if you could?

Thanks!

internal/services/serviceconnector/service_connector_app_service_resource.go

jackofallops · 2024-08-05T08:51:45Z

internal/services/serviceconnector/service_connector_app_service_resource_test.go

@@ -551,6 +560,16 @@ resource "azurerm_app_service_connection" "test" {
  authentication {
    type = "systemAssignedIdentity"
  }
+  scope = "default"


This suggests that the scope value might possibly need a default setting? And will need some validation in the schema?

This is an optional field, no default value is required. As service team said, no specific validation is required.

is "" a valid? ie should this at least not permit empty strings?

@jiaweitao001 - could you address this comment?

Thanks for reminding. Addressed.

website/docs/r/app_service_connection.html.markdown

jackofallops

Thanks @jiaweitao001 - Just one more comment to check, and as soon as we have the response and values from the service team for the docs / validation we can look to get this merged.

jackofallops · 2024-08-07T10:17:48Z

internal/services/serviceconnector/service_connector_app_service_resource.go

+					"action": {
+						Type:     pluginsdk.TypeString,
+						Optional: true,
+						ValidateFunc: validation.StringInSlice([]string{
+							string(servicelinker.ActionTypeOptOut),
+							string(servicelinker.ActionTypeEnable),
+						}, false),


given this is the only property, this should be Required or there's no point specifying the block?

This is not a Required field, under some circumstances, users can either omit this or configuration_store.

Hi @jiaweitao001 - This is the only property in this block? Is there any point specifying the public_network_solution block if action is not specified? If not, then action should be required, or have a default? I'm referring here just to the terraform config, not whether the API marks it as required.

I got the point. Will change this.

jackofallops · 2024-08-07T12:06:07Z

@jiaweitao001 - also, looks like the delete for this now consistently times out on TestAccServiceConnectorAppService_complete can you investigate, and see if there's a way to fix without needing to just increase the timeout? Thanks!

jiaweitao001 · 2024-08-08T02:06:28Z

@jiaweitao001 - also, looks like the delete for this now consistently times out on TestAccServiceConnectorAppService_complete can you investigate, and see if there's a way to fix without needing to just increase the timeout? Thanks!

I don't see timeout in Team city now, could you please take a look again?

katbyte

Hi, we have a test failure:

------- Stdout: -------
=== RUN   TestAccServiceConnectorAppServiceCosmosdb_update
=== PAUSE TestAccServiceConnectorAppServiceCosmosdb_update
=== CONT  TestAccServiceConnectorAppServiceCosmosdb_update
    testcase.go:173: Step 2/4 error: Pre-apply plan check(s) failed:
        'azurerm_app_service_connection.test' - expected action to not be Replace
--- FAIL: TestAccServiceConnectorAppServiceCosmosdb_update (949.36s)
FAIL

…info and public_network_solution

github-actions bot added documentation service/service-connector size/L waiting-response and removed waiting-response labels Jun 19, 2024

katbyte requested changes Jul 23, 2024

View reviewed changes

github-actions bot added waiting-response and removed waiting-response labels Jul 23, 2024

jiaweitao001 requested a review from katbyte July 30, 2024 02:31

jackofallops requested changes Jul 30, 2024

View reviewed changes

jackofallops self-assigned this Jul 30, 2024

github-actions bot added waiting-response and removed waiting-response labels Jul 30, 2024

jackofallops requested changes Aug 5, 2024

View reviewed changes

github-actions bot added the waiting-response label Aug 5, 2024

jiaweitao001 force-pushed the upgrade_serviceconnector_version branch from d6b62c9 to 3abe61f Compare August 7, 2024 08:56

github-actions bot removed the waiting-response label Aug 7, 2024

jackofallops requested changes Aug 7, 2024

View reviewed changes

github-actions bot added the waiting-response label Aug 7, 2024

github-actions bot removed the waiting-response label Aug 8, 2024

jackofallops added the waiting-response label Aug 15, 2024

github-actions bot removed the waiting-response label Aug 15, 2024

jiaweitao001 requested a review from jackofallops August 27, 2024 02:59

katbyte requested changes Sep 10, 2024

View reviewed changes

github-actions bot added the waiting-response label Sep 10, 2024

jiaweitao001 force-pushed the upgrade_serviceconnector_version branch from cb8cfb7 to c05d200 Compare September 11, 2024 08:20

github-actions bot removed the waiting-response label Sep 11, 2024

github-actions bot added the waiting-response label Dec 5, 2024

jiaweitao001 added 15 commits December 5, 2024 10:51

add new features

eab397e

azurerm_app_service_connection: Add support for scope, configuration_…

49f6522

…info and public_network_solution

Rename attribute

ddbe4c6

fix failing acc test

49ca608

remove unnecessary nil check

75f78a5

Change public network solution action to required field

c10e070

add description for attributes in doc

5e76f4a

fix conflicts

9d1755f

add upgrade test

6a23545

use servicelinker to replace links

345b436

remove hard coded location in tests

8cc441f

fix target service unaligned issue in update

b1153d3

add validation func for scope

137a296

address comments on docs

f8f003c

fix acc test

9e0f6b1

jiaweitao001 force-pushed the upgrade_serviceconnector_version branch from c217e2c to 9e0f6b1 Compare December 5, 2024 02:53

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

remove network solution in acc test

115b863

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

jiaweitao001 added 2 commits December 5, 2024 16:21

fix subnet upgrade issue

a84ac65

Fix complete acc test

77bca43

github-actions bot added waiting-response and removed waiting-response labels Dec 10, 2024

Fix indent

9cfbc5c

github-actions bot removed the waiting-response label Dec 16, 2024

jiaweitao001 requested review from jackofallops and katbyte January 16, 2025 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_app_service_connection`: add support for `scope`, `configuration_info` and `public_network_solution` #26387

`azurerm_app_service_connection`: add support for `scope`, `configuration_info` and `public_network_solution` #26387

jiaweitao001 commented Jun 19, 2024

katbyte Jul 23, 2024

jiaweitao001 Jul 30, 2024

jackofallops left a comment

jiaweitao001 commented Jul 31, 2024

jackofallops left a comment

jackofallops Aug 5, 2024

jiaweitao001 Aug 7, 2024

katbyte Oct 29, 2024

katbyte Nov 19, 2024

jiaweitao001 Nov 21, 2024

jackofallops left a comment

jackofallops Aug 7, 2024

jiaweitao001 Aug 8, 2024

jackofallops Aug 15, 2024

jiaweitao001 Aug 15, 2024

jackofallops commented Aug 7, 2024

jiaweitao001 commented Aug 8, 2024

katbyte left a comment

	A `configuration_info` block supports the following:
	A `configuration` block supports the following:

azurerm_app_service_connection: add support for scope, configuration_info and public_network_solution #26387

Are you sure you want to change the base?

azurerm_app_service_connection: add support for scope, configuration_info and public_network_solution #26387

Conversation

jiaweitao001 commented Jun 19, 2024

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackofallops left a comment

Choose a reason for hiding this comment

jiaweitao001 commented Jul 31, 2024

jackofallops left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackofallops left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackofallops commented Aug 7, 2024

jiaweitao001 commented Aug 8, 2024

katbyte left a comment

Choose a reason for hiding this comment

`azurerm_app_service_connection`: add support for `scope`, `configuration_info` and `public_network_solution` #26387

`azurerm_app_service_connection`: add support for `scope`, `configuration_info` and `public_network_solution` #26387