Typed SDK and pluginsdk wrapper #1188

manicminer · 2023-09-13T01:21:17Z

Bump all dependencies including github.com/hashicorp/terraform-plugin-sdk/v2
Add Typed SDK
Add pluginsdk wrapper package
Update acceptance framework
Use terraform-plugin-testing module
Convert data.azuread_client_config and data.azuread_domains to typed data source
Convert azuread_directory_role to typed resource

To aid review, here's a rough breakdown of the changes by package:

Package/File	Ported from AzureRM	Notes
internal/acceptance	Partially	Merged with AzureRM package as there are local additions to help with AAD constructs
internal/acceptance/plugin_sdk_aliases.go	Yes
internal/helpers	-	Use pluginsdk wrappers
internal/helpers/tf	Yes	From helpers/tf
internal/provider	-	Support typed resources
internal/sdk	Yes	-
internal/services/...	-	Use pluginsdk and acceptance wrappers, also validation refactoring
internal/services/domains	-	Port data.azuread_domains to typed SDK
internal/services/serviceprincipals	-	Port data.azuread_client_config to typed SDK
internal/tf/import.go	-	Removed, boilerplate
internal/tf/pluginsdk	Yes	Addition of wrappers for `diag` package
internal/tf/validation/pluginsdk.go	Yes	-
internal/validate	-	Moved to internal/tf/validation

…tance framework, convert data.azuread_domains to typed data source

…source

tombuildsstuff

Left some comments inline, but if we can fix those up then this should otherwise be good to go 👍

internal/acceptance/testcase.go

tombuildsstuff · 2023-10-13T14:33:42Z

internal/acceptance/testcase.go

+			azurerm := provider.AzureADProvider()
+			return azurerm, nil
+		},
+		"azurerm-alt": func() (*schema.Provider, error) { //nolint:unparam


although does AAD need both provider instances?

Suggested change

"azurerm-alt": func() (*schema.Provider, error) { //nolint:unparam

"azuread-alt": func() (*schema.Provider, error) { //nolint:unparam

Gonna remove the alt instance for now, we'll need 1-2 alt providers in time, but those can be added later

internal/acceptance/testcase.go

internal/sdk/README.md

tombuildsstuff · 2023-10-13T14:57:26Z

internal/services/administrativeunits/administrative_unit_data_source.go

 				Optional:         true,
 				Computed:         true,
-				ValidateDiagFunc: validate.NoEmptyStrings,
+				ValidateDiagFunc: validation.ValidateDiag(validation.StringIsNotEmpty),


rather than wrapping this, why not make the StringIsNotEmpty functions return diags?

as discussed, wrapping these will help when migrating back to error-based validation funcs

internal/services/domains/domains_data_source.go

manicminer · 2023-10-16T22:10:02Z

Test results

<Actions> <action id="c2aadc6326b4b0bc58df11ee286b0f67ccdb5888bd77f391e6473570113337ec"> <h3>Bump Terraform `azuread` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azuread" updated from "2.43.0" to "2.44.0" in file ".terraform.lock.hcl"</p> <details> <summary>2.44.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azuread/releases/tag/v2.44.0
* Developer Note: the Typed Resource SDK, as also used in the AzureRM provider, is now the preferred way of introducing new resources ([#1188](https://github.com/hashicorp/terraform-provider-azuread/issues/1188))

FEATURES:

* **New Resource:** `azuread_application_api_access` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_app_role` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_fallback_public_client` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_from_template` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_identifier_uri` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_known_clients` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_owner` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_permission_scope ([#1214](https://github.com/hashicorp/terraform-provider-azuread/issues/1214))`
* **New Resource:** `azuread_application_redirect_uris` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_application_registration` ([#1214](hashicorp/terraform-provider-azuread#1214 **New Resource:** `azuread_authentication_strength_policy` ([#1171](https://github.com/hashicorp/terraform-provider-azuread/issues/1171))

ENHANCEMENTS:

* `data.azuread_application` - export the `client_id` attribute, deprecate the `application_id` attribute ([#1214](hashicorp/terraform-provider-azuread#1214 `data.azuread_service_principal` - support for the `client_id` property, deprecate the `application_id` property ([#1214](hashicorp/terraform-provider-azuread#1214 `data.azuread_service_principals` - support for the `client_ids` property, deprecate the `application_ids` property ([#1214](hashicorp/terraform-provider-azuread#1214 `data.azuread_service_principals` - export the `client_id` attribute in the `service_principals` block, deprecate the `application_id` attribute ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_application` - export the `client_id` attribute, deprecate the `application_id` attribute ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_application_federated_identity_credential` - support for the `application_id` property, deprecate the `application_object_id` property ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_application_certificate` - support for the `application_id` property, deprecate the `application_object_id` property ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_application_password` - support for the `application_id` property, deprecate the `application_object_id` property ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_application_pre_authorized` - support for the `application_id` property, deprecate the `application_object_id` property ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_service_principal` - support for the `client_id` property, deprecate the `application_id` property ([#1214](hashicorp/terraform-provider-azuread#1214 `azuread_conditional_access_policy` - support for the `authentication_strength_policy_id` property in the `grant_controls` block [GH_1171]

BUG FIXES:

* `azuread_group_member` - resolve a bug when refreshing state if the group is missing ([#1198](https://github.com/hashicorp/terraform-provider-azuread/issues/1198))


</pre> </details> <details> <summary>2.44.1</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azuread/releases/tag/v2.44.1
BUG FIXES:

* `azuread_application_certificate` - work around an unexpected diff with the `application_object_id` property ([#1221](hashicorp/terraform-provider-azuread#1221 `azuread_application_federated_identity_credential` - work around an unexpected diff with the `application_object_id` property ([#1221](hashicorp/terraform-provider-azuread#1221 `azuread_application_password` - work around an unexpected diff with the `application_object_id` property ([#1221](hashicorp/terraform-provider-azuread#1221 `azuread_application_pre_authorized` - work around an unexpected diff with the `application_object_id` property ([#1221](https://github.com/hashicorp/terraform-provider-azuread/issues/1221))


</pre> </details> </details> </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>

Update go modules, add Typed SDK, add pluginsdk wrapper, update accep…

2f4a871

…tance framework, convert data.azuread_domains to typed data source

manicminer added the refactor label Sep 13, 2023

manicminer requested a review from a team September 13, 2023 01:21

github-actions bot added dependencies size/XL labels Sep 13, 2023

manicminer force-pushed the refactor/typed-sdk branch 2 times, most recently from 2355ee6 to a02dcaf Compare September 13, 2023 13:19

Use pluginsdk shims, convert data.azuread_client_config to typed data…

2eac106

…source

manicminer force-pushed the refactor/typed-sdk branch from a02dcaf to 2eac106 Compare September 13, 2023 23:32

linting

57ad291

manicminer added feature/domains provider labels Sep 14, 2023

manicminer added 2 commits September 20, 2023 18:25

Merge branch 'main' into refactor/typed-sdk

96313a0

Fix up bad merge

be6fb53

manicminer force-pushed the refactor/typed-sdk branch from 5f06812 to be6fb53 Compare September 20, 2023 17:34

Merge branch 'main' into refactor/typed-sdk

93a61f3

manicminer force-pushed the refactor/typed-sdk branch from af67e10 to 76e4deb Compare September 27, 2023 14:37

pluginsdk wrapper for diag package

91d9736

manicminer force-pushed the refactor/typed-sdk branch from 76e4deb to 91d9736 Compare September 27, 2023 14:41

tombuildsstuff suggested changes Oct 13, 2023

View reviewed changes

manicminer added 3 commits October 16, 2023 22:47

address review, fix typos, copypastas and bugs

19cd10d

Merge branch 'main' into refactor/typed-sdk

23da82f

acceptance: remove check for unneeded env vars

ffbbaee

manicminer added this to the v2.44.0 milestone Oct 16, 2023

manicminer merged commit 520c3eb into main Oct 16, 2023
15 checks passed

manicminer deleted the refactor/typed-sdk branch October 16, 2023 22:12

manicminer added a commit that referenced this pull request Oct 16, 2023

Changelog for #1188

298d7dc

github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typed SDK and pluginsdk wrapper #1188

Typed SDK and pluginsdk wrapper #1188

manicminer commented Sep 13, 2023 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Oct 13, 2023

manicminer Oct 16, 2023 •

edited

Loading

tombuildsstuff Oct 13, 2023

manicminer Oct 16, 2023

manicminer commented Oct 16, 2023

	"azurerm-alt": func() (*schema.Provider, error) { //nolint:unparam
	"azuread-alt": func() (*schema.Provider, error) { //nolint:unparam

Typed SDK and pluginsdk wrapper #1188

Typed SDK and pluginsdk wrapper #1188

Conversation

manicminer commented Sep 13, 2023 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Oct 13, 2023

Choose a reason for hiding this comment

manicminer Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

tombuildsstuff Oct 13, 2023

Choose a reason for hiding this comment

manicminer Oct 16, 2023

Choose a reason for hiding this comment

manicminer commented Oct 16, 2023

manicminer commented Sep 13, 2023 •

edited

Loading

manicminer Oct 16, 2023 •

edited

Loading