`azurerm_managed_application` - extend supported types for `parameters`/`parameter_values`/`output` and deprecate `parameters` #21541

myc2h6o · 2023-04-26T07:22:28Z

Fix #21267
Fix #13427
Fix #20016

Code
- Split flatten for parameters and outputs, as outputs does not support secureString but parameters does. And moved common logic into functions.
- while parsing secureString, use the value from local config, as it's not returned from server due to security concern.
- fixed a bug in expandManagedApplicationParameters, previously it always read the value from parameters after parameter_values, which will cause an issue when updating parameter_values, because on the second apply, even parameter is not set in config, it's still present in the state from the first apply.
- add ForceNew to plan, plan cannot be changed according to error returned by service Code="CannotUpdatePlan".
Document
- added note in parameters, it only supports string and secureString due to the limitation of tf schema, for other types, parmeter_values could be used as it is a pure JSON string
Test cases
- updated azurerm_managed_application_definition to use inline template as dependency instead of online template to ensure stability
- remove the old updatetest, cause it's updating forceNew properties, which is a re-creation, updates of certain properties are put into their own tests
- added tests to cover all supported types within parameters and parameter_values
Examples
- added examples to use parameters and parameter_values, to show how to set parameters of different types.

stephybun

Thanks for this @myc2h6o - could you add a test to validate this?

stephybun · 2023-06-20T12:26:38Z

Any updates @myc2h6o

myc2h6o · 2023-06-20T14:03:11Z

@stephybun I found some more issue while updating this pr, and I'm working on the update in a separate branch main...myc2h6o:terraform-provider-azurerm:managed_app_type_backup currently, I'll update it here once I resolved all the issues

myc2h6o · 2023-06-30T07:27:35Z

Hi @stephybun sorry for the delayed update, I finally get this to work. I've updated the code and acc tests, and put the updated PR details into the pr descriptions, please take a look.

myc2h6o · 2023-07-03T05:59:28Z

Updated test result

katbyte · 2023-07-06T21:29:40Z

#21571 should probably be merged first as it will be easier to fix conflicts in this PR then that one

myc2h6o · 2023-07-07T01:26:37Z

@katbyte sure, I'll wait for the sdk to be upgraded first

myc2h6o · 2023-07-08T05:10:28Z

I have resolved conflicts after the sdk upgrade, below is the updated test result:

manicminer

Thanks for updating and adding the tests @myc2h6o. I think this mostly looks good, just a couple of potential crashes I spotted, and given the messiness of this double implementation I wonder if we should consider deprecating parameters with a view to removing it in v4.0? Although it's an attempt at a more native implementation, it currently has to coexist with parameter_values because it's only a subset - we can avoid a lot of unnecessary gymnastics here by removing it. WDYT?

manicminer · 2023-07-25T19:41:58Z

internal/services/managedapplications/managed_application_resource.go

+					// Secure values are not returned, thus settings it with local value
+					v = ""
+					if oldValueStruct, oldValueStructOK := localParameters[k]; oldValueStructOK {
+						if oldValue, oldValueOK := oldValueStruct.(map[string]interface{})["value"]; oldValueOK {


I think we'll need some type-checking here because oldValueStruct is an interface{} and no guarantee that it will be a map? If it isn't, this will panic

yes make sense, have added the type check

manicminer · 2023-07-25T19:43:06Z

internal/services/managedapplications/managed_application_resource.go

+				if _, ok := value["value"]; !ok {
+					value["value"] = ""
+					if localParam, localParamOK := localParameters[k]; localParamOK {
+						if localParamValue, localParamValueOK := localParam.(map[string]interface{})["value"]; localParamValueOK {


As above, we should add more type-checking here

yes make sense, have added the type check

myc2h6o · 2023-08-07T07:27:17Z

@manicminer thanks for reviewing the change! I've updated the code to deprecate parameters in 4.0 and added the type check. Please take a look.

myc2h6o · 2023-08-07T08:34:59Z

updated test result

…_values` and `output`

myc2h6o · 2023-08-07T11:13:57Z

have rebased to main to resolve ci failure of the example issue fixed by #22839

manicminer

Thanks for updating @myc2h6o, this LGTM 👍

internal/services/managedapplications/managed_application_resource_test.go

website/docs/r/managed_application.html.markdown

github-actions · 2024-05-18T02:09:06Z

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.

github-actions bot added service/managed-apps size/XS labels Apr 26, 2023

stephybun requested changes Apr 26, 2023

View reviewed changes

stephybun added the bug label Apr 26, 2023

github-actions bot added the waiting-response label Apr 26, 2023

github-actions bot removed the waiting-response label Jun 20, 2023

stephybun added the waiting-response label Jun 26, 2023

myc2h6o force-pushed the managed_app_type branch from 087cea1 to cd10cd6 Compare June 30, 2023 06:43

github-actions bot added size/XL and removed waiting-response size/XS labels Jun 30, 2023

myc2h6o changed the title ~~azurerm_managed_application - fix parameter types~~ azurerm_managed_application - extend supported types for parameter_values and output Jun 30, 2023

myc2h6o changed the title ~~azurerm_managed_application - extend supported types for parameter_values and output~~ azurerm_managed_application - extend supported types for parameters/parameter_values and output Jun 30, 2023

myc2h6o force-pushed the managed_app_type branch from cd10cd6 to 830e765 Compare June 30, 2023 07:13

github-actions bot added the documentation label Jun 30, 2023

myc2h6o force-pushed the managed_app_type branch from 830e765 to 2efcf60 Compare July 3, 2023 05:03

myc2h6o force-pushed the managed_app_type branch 3 times, most recently from 6e03df4 to b09176c Compare July 8, 2023 04:50

catriona-m assigned manicminer Jul 24, 2023

manicminer suggested changes Jul 25, 2023

View reviewed changes

github-actions bot added the waiting-response label Jul 25, 2023

myc2h6o changed the title ~~azurerm_managed_application - extend supported types for parameters/parameter_values and output~~ azurerm_managed_application - extend supported types for parameters/parameter_values and output and deprecate parameters Aug 7, 2023

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

github-actions bot added waiting-response and removed waiting-response labels Aug 7, 2023

myc2h6o added 3 commits August 7, 2023 19:11

azurerm_managed_application - extend supported types for `parameter…

afcd1ad

…_values` and `output`

deprecate parameters

8bc41c0

add type check

63238af

myc2h6o force-pushed the managed_app_type branch from 557e0ad to 63238af Compare August 7, 2023 11:12

myc2h6o changed the title ~~azurerm_managed_application - extend supported types for parameters/parameter_values and output and deprecate parameters~~ azurerm_managed_application - extend supported types for parameters/parameter_values/output and deprecate parameters Aug 8, 2023

manicminer approved these changes Aug 9, 2023

View reviewed changes

internal/services/managedapplications/managed_application_resource_test.go Outdated Show resolved Hide resolved

website/docs/r/managed_application.html.markdown Outdated Show resolved Hide resolved

Apply suggestions from code review

9e5290d

manicminer added this to the v3.69.0 milestone Aug 9, 2023

manicminer merged commit 5a65857 into hashicorp:main Aug 9, 2023

manicminer added a commit that referenced this pull request Aug 9, 2023

Changelog for #21541

94718dc

myc2h6o deleted the managed_app_type branch August 10, 2023 02:33

chkp-stuartgreen mentioned this pull request Oct 12, 2023

Error when creating / destroying Managed Application - "Error: extracting parameters: unexpected type []interface {}" #23527

Closed

1 task

github-actions bot locked as resolved and limited conversation to collaborators May 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_managed_application` - extend supported types for `parameters`/`parameter_values`/`output` and deprecate `parameters` #21541

`azurerm_managed_application` - extend supported types for `parameters`/`parameter_values`/`output` and deprecate `parameters` #21541

myc2h6o commented Apr 26, 2023 •

edited

Loading

stephybun left a comment

stephybun commented Jun 20, 2023

myc2h6o commented Jun 20, 2023

myc2h6o commented Jun 30, 2023

myc2h6o commented Jul 3, 2023

katbyte commented Jul 6, 2023

myc2h6o commented Jul 7, 2023

myc2h6o commented Jul 8, 2023

manicminer left a comment

manicminer Jul 25, 2023

myc2h6o Aug 7, 2023

manicminer Jul 25, 2023

myc2h6o Aug 7, 2023

myc2h6o commented Aug 7, 2023

myc2h6o commented Aug 7, 2023

myc2h6o commented Aug 7, 2023 •

edited

Loading

manicminer left a comment

github-actions bot commented May 18, 2024

azurerm_managed_application - extend supported types for parameters/parameter_values/output and deprecate parameters #21541

azurerm_managed_application - extend supported types for parameters/parameter_values/output and deprecate parameters #21541

Conversation

myc2h6o commented Apr 26, 2023 • edited Loading

stephybun left a comment

Choose a reason for hiding this comment

stephybun commented Jun 20, 2023

myc2h6o commented Jun 20, 2023

myc2h6o commented Jun 30, 2023

myc2h6o commented Jul 3, 2023

katbyte commented Jul 6, 2023

myc2h6o commented Jul 7, 2023

myc2h6o commented Jul 8, 2023

manicminer left a comment

Choose a reason for hiding this comment

manicminer Jul 25, 2023

Choose a reason for hiding this comment

myc2h6o Aug 7, 2023

Choose a reason for hiding this comment

manicminer Jul 25, 2023

Choose a reason for hiding this comment

myc2h6o Aug 7, 2023

Choose a reason for hiding this comment

myc2h6o commented Aug 7, 2023

myc2h6o commented Aug 7, 2023

myc2h6o commented Aug 7, 2023 • edited Loading

manicminer left a comment

Choose a reason for hiding this comment

github-actions bot commented May 18, 2024

`azurerm_managed_application` - extend supported types for `parameters`/`parameter_values`/`output` and deprecate `parameters` #21541

`azurerm_managed_application` - extend supported types for `parameters`/`parameter_values`/`output` and deprecate `parameters` #21541

myc2h6o commented Apr 26, 2023 •

edited

Loading

myc2h6o commented Aug 7, 2023 •

edited

Loading