`azurerm_data_factory_linked_service_azure_sql_database` - adding `credential` block by chilledornaments · Pull Request #27629 · hashicorp/terraform-provider-azurerm

chilledornaments · 2024-10-11T17:24:26Z

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

Adds a credential block attribute to azurerm_data_factory_linked_service_azure_sql_database in order to support User-assigned managed identity authentication. This resource currently supports the other available authentication types.

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.

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)

New test:

/opt/homebrew/opt/go/libexec/bin/go tool test2json -t /Users/mitch/Library/Caches/JetBrains/GoLand2024.2/tmp/GoLand/___1data_factory_linked_service_azure_sql_database_resource_test_go.test -test.v=test2json -test.paniconexit0 -test.run ^\QTestAccDataFactoryLinkedServiceAzureSQLDatabase_basic\E|\QTestAccDataFactoryLinkedServiceAzureSQLDatabase_update\E|\QTestAccDataFactoryLinkedServiceAzureSQLDatabase_managed_id\E|\QTestAccDataFactoryLinkedServiceAzureSQLDatabase_PasswordKeyVaultReference\E|\QTestAccDataFactoryLinkedServiceAzureSQLDatabase_ConnectionStringKeyVaultReference\E|\QTestAccDataFactoryLinkedServiceAzureSQLDatabase_Credential\E$
=== RUN   TestAccDataFactoryLinkedServiceAzureSQLDatabase_basic
=== PAUSE TestAccDataFactoryLinkedServiceAzureSQLDatabase_basic
=== CONT  TestAccDataFactoryLinkedServiceAzureSQLDatabase_basic
--- PASS: TestAccDataFactoryLinkedServiceAzureSQLDatabase_basic (122.86s)
=== RUN   TestAccDataFactoryLinkedServiceAzureSQLDatabase_update
=== PAUSE TestAccDataFactoryLinkedServiceAzureSQLDatabase_update
=== CONT  TestAccDataFactoryLinkedServiceAzureSQLDatabase_update
--- PASS: TestAccDataFactoryLinkedServiceAzureSQLDatabase_update (142.63s)
=== RUN   TestAccDataFactoryLinkedServiceAzureSQLDatabase_managed_id
=== PAUSE TestAccDataFactoryLinkedServiceAzureSQLDatabase_managed_id
=== CONT  TestAccDataFactoryLinkedServiceAzureSQLDatabase_managed_id
--- PASS: TestAccDataFactoryLinkedServiceAzureSQLDatabase_managed_id (77.66s)
=== RUN   TestAccDataFactoryLinkedServiceAzureSQLDatabase_PasswordKeyVaultReference
=== PAUSE TestAccDataFactoryLinkedServiceAzureSQLDatabase_PasswordKeyVaultReference
=== CONT  TestAccDataFactoryLinkedServiceAzureSQLDatabase_PasswordKeyVaultReference
--- PASS: TestAccDataFactoryLinkedServiceAzureSQLDatabase_PasswordKeyVaultReference (885.46s)
=== RUN   TestAccDataFactoryLinkedServiceAzureSQLDatabase_ConnectionStringKeyVaultReference
=== PAUSE TestAccDataFactoryLinkedServiceAzureSQLDatabase_ConnectionStringKeyVaultReference
=== CONT  TestAccDataFactoryLinkedServiceAzureSQLDatabase_ConnectionStringKeyVaultReference
--- PASS: TestAccDataFactoryLinkedServiceAzureSQLDatabase_ConnectionStringKeyVaultReference (896.26s)
=== RUN   TestAccDataFactoryLinkedServiceAzureSQLDatabase_Credential
=== PAUSE TestAccDataFactoryLinkedServiceAzureSQLDatabase_Credential
=== CONT  TestAccDataFactoryLinkedServiceAzureSQLDatabase_Credential
--- PASS: TestAccDataFactoryLinkedServiceAzureSQLDatabase_Credential (70.00s)
PASS

Process finished with the exit code 0

Change Log

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

azurerm_data_factory_linked_service_azure_sql_database - support for the credential property [Support for User-assigned managed identity in azurerm_data_factory_linked_service_azure_sql_database #27628]

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 #27628

Note

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

stephybun

Thanks @chilledornaments, could you take a look at the comment I left in-line?

stephybun · 2024-10-24T08:50:04Z

internal/services/datafactory/data_factory_linked_service_azure_sql_database_resource.go

+			"credential": {
+				Type:     pluginsdk.TypeList,
+				Optional: true,
+				MaxItems: 1,
+				Elem: &pluginsdk.Resource{
+					Schema: map[string]*pluginsdk.Schema{
+						"reference_name": {
+							Type:         pluginsdk.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringIsNotEmpty,
+						},
+
+						"type": {
+							Type:         pluginsdk.TypeString,
+							Required:     true,
+							ValidateFunc: validation.StringIsNotEmpty,
+						},
+					},
+				},
+			},


In azurerm_data_factory_integration_runtime_managed we just expose this as credential_name since type only has one valid value. To be consistent with other data factory resources can we please update this to

Suggested change

"credential": {

Type: pluginsdk.TypeList,

Optional: true,

MaxItems: 1,

Elem: &pluginsdk.Resource{

Schema: map[string]*pluginsdk.Schema{

"reference_name": {

Type: pluginsdk.TypeString,

Required: true,

ValidateFunc: validation.StringIsNotEmpty,

},

"type": {

Type: pluginsdk.TypeString,

Required: true,

ValidateFunc: validation.StringIsNotEmpty,

},

},

},

},

"credential_name": {

Type: pluginsdk.TypeString,

Optional: true,

ValidateFunc: validation.StringIsNotEmpty,

},

@stephybun thanks for the feedback 🙂 I've made the changes you requested.

stephybun · 2024-10-24T15:27:10Z

internal/services/datafactory/data_factory_linked_service_azure_sql_database_resource.go

 	}

+	if credential := sql.Credential; credential != nil {
+		d.Set("credential_name", pointer.From(sql.Credential.ReferenceName))


Suggested change

d.Set("credential_name", pointer.From(sql.Credential.ReferenceName))

d.Set("credential_name", pointer.From(credential.ReferenceName))

Good catch, thank you. Fix pushed.

stephybun

Tests look happy, thanks @chilledornaments LGTM 🍕

…edential` block (hashicorp#27629) * add credential to azurerm_data_factory_linked_service_azure_sql_database * align with msft docs * change credential block to credential_name string * fix reference

github-actions · 2024-11-25T02:16:44Z

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.

chilledornaments added 2 commits October 11, 2024 11:01

add credential to azurerm_data_factory_linked_service_azure_sql_database

9d9cd7a

align with msft docs

e842054

github-actions bot added documentation service/data-factory size/M labels Oct 11, 2024

rcskosir added the enhancement label Oct 14, 2024

stephybun suggested changes Oct 24, 2024

View reviewed changes

github-actions bot added the waiting-response label Oct 24, 2024

change credential block to credential_name string

12becb8

chilledornaments requested review from a team and katbyte as code owners October 24, 2024 15:02

github-actions bot removed the waiting-response label Oct 24, 2024

stephybun reviewed Oct 24, 2024

View reviewed changes

github-actions bot added the waiting-response label Oct 24, 2024

fix reference

a7fb183

github-actions bot removed the waiting-response label Oct 24, 2024

stephybun approved these changes Oct 25, 2024

View reviewed changes

stephybun merged commit 7bd7388 into hashicorp:main Oct 25, 2024

github-actions bot added this to the v4.8.0 milestone Oct 25, 2024

stephybun added a commit that referenced this pull request Oct 25, 2024

Update for #27629

a9f1b17

djryanj pushed a commit to djryanj/terraform-provider-azurerm that referenced this pull request Oct 26, 2024

Update for hashicorp#27629

2636b18

github-actions bot locked as resolved and limited conversation to collaborators Nov 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_data_factory_linked_service_azure_sql_database` - adding `credential` block#27629

`azurerm_data_factory_linked_service_azure_sql_database` - adding `credential` block#27629
stephybun merged 4 commits intohashicorp:mainfrom
chilledornaments:feat/adf-sql-db-credential

chilledornaments commented Oct 11, 2024 •

edited

Loading

Uh oh!

stephybun left a comment

Uh oh!

stephybun Oct 24, 2024

Uh oh!

chilledornaments Oct 24, 2024

Uh oh!

stephybun Oct 24, 2024

Uh oh!

chilledornaments Oct 24, 2024

Uh oh!

stephybun left a comment

Uh oh!

github-actions bot commented Nov 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	d.Set("credential_name", pointer.From(sql.Credential.ReferenceName))
	d.Set("credential_name", pointer.From(credential.ReferenceName))

Conversation

chilledornaments commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

Uh oh!

stephybun left a comment

Choose a reason for hiding this comment

Uh oh!

stephybun Oct 24, 2024

Choose a reason for hiding this comment

Uh oh!

chilledornaments Oct 24, 2024

Choose a reason for hiding this comment

Uh oh!

stephybun Oct 24, 2024

Choose a reason for hiding this comment

Uh oh!

chilledornaments Oct 24, 2024

Choose a reason for hiding this comment

Uh oh!

stephybun left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chilledornaments commented Oct 11, 2024 •

edited

Loading