Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_data_factory_integration_runtime_azure_ssis support for the credential_name property #24458

Merged
merged 6 commits into from
Jan 24, 2024
Merged

Conversation

bruceharrison1984
Copy link
Contributor

@bruceharrison1984 bruceharrison1984 commented Jan 10, 2024

This adds a new user_assigned_identity_credential_name property to azurerm_data_factory_integration_runtime_azure_ssis so it can make use of Data Factory credential resources.

This completes the end run of Data Factory SSIS being able to access SQL Servers that are secured via Entra authentication.

This PR is a follow on to #24307.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @bruceharrison1984 - aside from one minor comment on the property name i think this looks good

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bruceharrison1984 but we also have a panic in the tests:

Test ended in panic.
------- Stdout: -------
=== RUN   TestAccDataFactoryIntegrationRuntimeManagedSsis_basic
=== PAUSE TestAccDataFactoryIntegrationRuntimeManagedSsis_basic
=== CONT  TestAccDataFactoryIntegrationRuntimeManagedSsis_basic
------- Stderr: -------
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x61bbf10]
goroutine 398 [running]:
github.com/hashicorp/terraform-provider-azurerm/internal/services/datafactory.resourceDataFactoryIntegrationRuntimeAzureSsisRead(0xc003354200, {0x8464860?, 0xc003cf7200?})
  /opt/teamcity-agent/work/3337027aeff310bf/internal/services/datafactory/data_factory_integration_runtime_azure_ssis_resource.go:573 +0x910
github.com/hashicorp/terraform-provider-azurerm/internal/services/datafactory.resourceDataFactoryIntegrationRuntimeAzureSsisCreateUpdate(0x0?, {0x8464860?, 0xc003cf7200?})
 ...

I left a fix in-line.

@@ -564,6 +570,10 @@ func resourceDataFactoryIntegrationRuntimeAzureSsisRead(d *pluginsdk.ResourceDat
return fmt.Errorf("setting `catalog_info`: %+v", err)
}

if err := d.Set("credential_user_assigned_identity_name", ssisProps.Credential.ReferenceName); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing tests to panic, we probably want to nil check this

Suggested change
if err := d.Set("credential_user_assigned_identity_name", ssisProps.Credential.ReferenceName); err != nil {
credentialName := ""
if ssisProps.Credential != nil && ssisProps.Credential.ReferenceName != nil {
credentialName = *ssisProps.Credential.ReferenceName
}
if err := d.Set("credential_user_assigned_identity_name", credentialName); err != nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I never ran into any issues locally. I'll get this fixed up and resubmitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephybun

I added a nil check similar to other nested properties and all tests were passing locally against a test subscription.

@bruceharrison1984
Copy link
Contributor Author

bruceharrison1984 commented Jan 24, 2024

@katbyte

After further review, I renamed the credential_user_assigned_identity_name property to the shorter credential_name.

The name of any type of Data Factory Credential(UAMI, Principal, some new kind) could be used here, so the extremely specific naming was not necessary and likely would've caused confusion(and rework) later.

DataFactory IR REST

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good @bruceharrison1984 - reunning the tests i'm seeing this error thou

Test ended in panic.

------- Stdout: -------
=== RUN   TestAccDataFactoryIntegrationRuntimeManagedSsis_basic
=== PAUSE TestAccDataFactoryIntegrationRuntimeManagedSsis_basic
=== CONT  TestAccDataFactoryIntegrationRuntimeManagedSsis_basic

------- Stderr: -------
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x61bbf10]

goroutine 439 [running]:
github.com/hashicorp/terraform-provider-azurerm/internal/services/datafactory.resourceDataFactoryIntegrationRuntimeAzureSsisRead(0xc00211a380, {0x8464860?, 0xc00231e900?})
	/opt/teamcity-agent/work/3337027aeff310bf/internal/services/datafactory/data_factory_integration_runtime_azure_ssis_resource.go:573 +0x910
github.com/hashicorp/terraform-provider-azurerm/internal/services/datafactory.resourceDataFactoryIntegrationRuntimeAzureSsisCreateUpdate(0x0?, {0x8464860?, 0xc00231e900?})
	/opt/teamcity-agent/work/3337027aeff310bf/internal/services/datafactory/data_factory_integration_runtime_azure_ssis_resource.go:506 +0xae5
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).create(0x9d222c0?, {0x9d222c0?, 0xc003a9f230?}, 0xd?, {0x8464860?, 0xc00231e900?})
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:766 +0x163
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0xc000938c40, {0x9d222c0, 0xc003a9f230}, 0xc00250ef70, 0xc00211a200, {0x8464860, 0xc00231e900})
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/resource.go:909 +0xa7e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ApplyResourceChange(0xc002201920, {0x9d222c0?, 0xc003a9f1a0?}, 0xc003d32d70)
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema/grpc_provider.go:1060 +0xdbc
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ApplyResourceChange(0xc001e06a00, {0x9d222c0?, 0xc003a9e6c0?}, 0xc0008eca10)
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server/server.go:859 +0x56a
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ApplyResourceChange_Handler({0x8ff7600?, 0xc001e06a00}, {0x9d222c0, 0xc003a9e6c0}, 0xc0008ec9a0, 0x0)
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:467 +0x169
google.golang.org/grpc.(*Server).processUnaryRPC(0xc002a505a0, {0x9d478e0, 0xc00316e000}, 0xc0044f26c0, 0xc0014e8960, 0xfb4e718, 0x0)
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:1374 +0xde7
google.golang.org/grpc.(*Server).handleStream(0xc002a505a0, {0x9d478e0, 0xc00316e000}, 0xc0044f26c0, 0x0)
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:1751 +0x9e7
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:986 +0xbb
created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 340
	/opt/teamcity-agent/work/3337027aeff310bf/vendor/google.golang.org/grpc/server.go:997 +0x145

@bruceharrison1984
Copy link
Contributor Author

@katbyte Hm, I was seeing that error prior to fixing the nil check, at the same location. I'll take another look and see if I can sort it out.

@katbyte
Copy link
Collaborator

katbyte commented Jan 24, 2024

It could be the test ran before you fixed it, i'll fire it off again

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests now pass LGTM 🧶

@katbyte katbyte changed the title Allow azurerm_data_factory_integration_runtime_azure_ssis to use azurerm_data_factory_credential_user_managed_identity azurerm_data_factory_integration_runtime_azure_ssis support for the credential_name property Jan 24, 2024
@katbyte katbyte merged commit 36ef0d1 into hashicorp:main Jan 24, 2024
35 checks passed
katbyte added a commit that referenced this pull request Jan 24, 2024
@github-actions github-actions bot added this to the v3.89.0 milestone Jan 24, 2024
Copy link

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

dduportal added a commit to jenkins-infra/azure that referenced this pull request Jan 25, 2024
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>changes detected:&#xA;&#x9;&#34;hashicorp/azurerm&#34; updated from
&#34;3.88.0&#34; to &#34;3.89.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.89.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.89.0&#xA;FEATURES:&#xA;&#xA;*
New Data Source: `azurerm_data_factory_trigger_schedule`
([#24572](https://github.com/hashicorp/terraform-provider-azurerm/issues/24572))&#xA;*
New Data Source: `azurerm_data_factory_trigger_schedules`
([#24572](https://github.com/hashicorp/terraform-provider-azurerm/issues/24572))&#xA;*
New Data Source: `azurerm_ip_groups`
([#24540](https://github.com/hashicorp/terraform-provider-azurerm/issues/24540))&#xA;*
New Data Source: `azurerm_nginx_certificate`
([#24577](https://github.com/hashicorp/terraform-provider-azurerm/issues/24577))&#xA;*
New Resource: `azurerm_chaos_studio_target`
([#24580](https://github.com/hashicorp/terraform-provider-azurerm/issues/24580))&#xA;*
New Resource: `azurerm_elastic_san_volume_group`
([#24166](https://github.com/hashicorp/terraform-provider-azurerm/issues/24166))&#xA;*
New Resource: `azurerm_netapp_account_encryption`
([#23733](https://github.com/hashicorp/terraform-provider-azurerm/issues/23733))&#xA;*
New Resource: `azurerm_redhat_openshift_cluster`
([#24375](https://github.com/hashicorp/terraform-provider-azurerm/issues/24375))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.66.1` of
`github.com/hashicorp/go-azure-helpers`
([#24561](https://github.com/hashicorp/terraform-provider-azurerm/issues/24561))&#xA;*
dependencies: updating to `v0.20240124.1115501` of
`github.com/hashicorp/go-azure-sdk`
([#24619](https://github.com/hashicorp/terraform-provider-azurerm/issues/24619))&#xA;*
`bot`: updating to API Version `2021-05-01-preview`
([#24555](https://github.com/hashicorp/terraform-provider-azurerm/issues/24555))&#xA;*
`containerservice`: the SDK Clients now support logging
([#24564](https://github.com/hashicorp/terraform-provider-azurerm/issues/24564))&#xA;*
`cosmosdb`: updating to API Version `2023-04-15`
([#24541](https://github.com/hashicorp/terraform-provider-azurerm/issues/24541))&#xA;*
`loadtestservice`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest` (and support
logging)
([#24578](https://github.com/hashicorp/terraform-provider-azurerm/issues/24578))&#xA;*
`managedidentity`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest` (and support
logging)
([#24578](https://github.com/hashicorp/terraform-provider-azurerm/issues/24578))&#xA;*
`azurerm_api_management_api` - change the `id` format so specific
`revision`s can be managed by Terraform
([#23031](https://github.com/hashicorp/terraform-provider-azurerm/issues/23031))&#xA;*
`azurerm_data_protection_backup_vault` - the `redundancy` propety can
now be set to `ZoneRedundant`
([#24556](https://github.com/hashicorp/terraform-provider-azurerm/issues/24556))&#xA;*
`azurerm_data_factory_integration_runtime_azure_ssis` - support for the
`credential_name` property
([#24458](https://github.com/hashicorp/terraform-provider-azurerm/issues/24458))&#xA;*
`azurerm_orchestrated_virtual_machine_scale_set` - support
`2022-datacenter-azure-edition-hotpatch` and
`2022-datacenter-azure-edition-hotpatch-smalldisk` hotpatching images
([#23500](https://github.com/hashicorp/terraform-provider-azurerm/issues/23500))&#xA;*
`azurerm_stream_analytics_job` - support for the `sku_name` property
([#24554](https://github.com/hashicorp/terraform-provider-azurerm/issues/24554))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* Data Source: `azurerm_app_service` - parsing the API
Response for `app_service_plan_id` case-insensitively
([#24626](https://github.com/hashicorp/terraform-provider-azurerm/issues/24626))&#xA;*
Data Source: `azurerm_function_app` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](https://github.com/hashicorp/terraform-provider-azurerm/issues/24626))&#xA;*
`azurerm_app_configuration_key` - the value for the `value` property can
now be removed/emptied
([#24582](https://github.com/hashicorp/terraform-provider-azurerm/issues/24582))&#xA;&#xA;*
`azurerm_app_service` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](https://github.com/hashicorp/terraform-provider-azurerm/issues/24626))&#xA;*
`azurerm_app_service_plan` - fix casing in `serverFarms` due to ID
update
([#24562](https://github.com/hashicorp/terraform-provider-azurerm/issues/24562))&#xA;*
`azurerm_app_service_slot` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](https://github.com/hashicorp/terraform-provider-azurerm/issues/24626))&#xA;*
`azurerm_automation_schedule` - only one `monthly_occurence` block can
now be specified
([#24614](https://github.com/hashicorp/terraform-provider-azurerm/issues/24614))&#xA;*
`azurerm_cognitive_deployment` - the `model.version` property is no
longer required
([#24264](https://github.com/hashicorp/terraform-provider-azurerm/issues/24264))&#xA;*
`azurerm_container_app` - multiple `custom_scale_rule` can not be
updated
([#24509](https://github.com/hashicorp/terraform-provider-azurerm/issues/24509))&#xA;*
`azurerm_container_registry_task_schedule_run_now` - prevent issue where
the incorrect scheduled run in tracked if there have been multiple
([#24592](https://github.com/hashicorp/terraform-provider-azurerm/issues/24592))&#xA;*
`azurerm_function_app` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](https://github.com/hashicorp/terraform-provider-azurerm/issues/24626))&#xA;*
`azurerm_function_app_slot` - parsing the API Response for
`app_service_plan_id` case-insensitively
([#24626](https://github.com/hashicorp/terraform-provider-azurerm/issues/24626))&#xA;*
`azurerm_logic_app_standard` - now will parse the app service ID
insensitively
([#24562](https://github.com/hashicorp/terraform-provider-azurerm/issues/24562))&#xA;*
`azurerm_logic_app_workflow` - the `workflow_parameters` will now
correctly handle information specified by `$connections`
([#24141](https://github.com/hashicorp/terraform-provider-azurerm/issues/24141))&#xA;*
`azurerm_mssql_managed_instance_security_alert_policy` - can not update
empty storage attributes
([#24553](https://github.com/hashicorp/terraform-provider-azurerm/issues/24553))&#xA;*
`azurerm_network_interface` - the `ip_configuration` properties are no
longer added to a Load Balancer Backend if one of those
`ip_configurations` is associated with a backend
([#24470](https://github.com/hashicorp/terraform-provider-azurerm/issues/24470))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1052/">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>
Co-authored-by: Damien Duportal <damien.duportal@gmail.com>
Copy link

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 github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants