Skip to content

azurerm_mssql_job_step – support managed-identity authentication by making job_credential_id optional#30031

Merged
sreallymatt merged 13 commits intohashicorp:mainfrom
rorsgo:feature/azurerm_mssql_job_step-optional-job-credential-id
Jul 14, 2025
Merged

azurerm_mssql_job_step – support managed-identity authentication by making job_credential_id optional#30031
sreallymatt merged 13 commits intohashicorp:mainfrom
rorsgo:feature/azurerm_mssql_job_step-optional-job-credential-id

Conversation

@rorsgo
Copy link
Contributor

@rorsgo rorsgo commented Jun 27, 2025

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

Resolves #29076 – Support managed identity instead of job_credential_id

Elastic Job Steps can now run under the Job Agent’s User/System-Assigned Managed Identity by simply omitting job_credential_id from azurerm_mssql_job_step.
This change makes hybrid and key-vault scenarios easier because secrets no longer need to be stored in SQL credentials.

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 relevant 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.

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

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the provider.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Note

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

@rorsgo rorsgo requested a review from a team as a code owner June 27, 2025 10:20
@rorsgo
Copy link
Contributor Author

rorsgo commented Jul 1, 2025

Hi @WodansSon, would you be able to take a look when you get a chance? We'd appreciate your help in moving this PR forward.

@katbyte katbyte requested a review from Copilot July 2, 2025 18:44

This comment was marked as outdated.

@rorsgo
Copy link
Contributor Author

rorsgo commented Jul 3, 2025

Hi @katbyte, I've fixed the errors. Could you please re-run the tests?

Copy link
Collaborator

@sreallymatt sreallymatt left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR @rorsgo, I've left a few comments inline, additionally could you also run make terrafmt to format the test configurations? (This should resolve the failing GHA)

@rorsgo
Copy link
Contributor Author

rorsgo commented Jul 4, 2025

Hi @sreallymatt — thanks for the quick turnaround on the feedback! Let me know if you spot anything else or would like additional tweaks. Appreciate the review! 🥇

@rorsgo rorsgo force-pushed the feature/azurerm_mssql_job_step-optional-job-credential-id branch from 7d06d9b to 1558992 Compare July 4, 2025 14:31
@rorsgo
Copy link
Contributor Author

rorsgo commented Jul 4, 2025

Still having issues with the docs. I’ll correct the typo and push an update shortly.

@sreallymatt
Copy link
Collaborator

Hi @rorsgo, to provide an update to this PR. We've unfortunately got 2 test failures (TestAccMsSqlJobStep_update and TestAccMsSqlJobStep_updateFromCredentialToManagedIdentity), it looks like omitting the credential property from the PUT request doesn't remove an existing credential.

This likely means we'll need to send an explicit null value, which isn't supported by go-azure-sdk at the moment, if I have some free time this week I'll have a peek at the API behaviour to see if I can find a workaround. (or, if you have some time and find a workaround, please do let me know!)

@rorsgo
Copy link
Contributor Author

rorsgo commented Jul 8, 2025

Hi @sreallymatt, thanks for the update and for looking into this! Please let me know what you find, and if there's anything I can do to help :)

@ivanl-out
Copy link

Hi @sreallymatt and @rorsgo, I have been following this PR closely because I too need this functionality.

Until this PR is merged, I did work around the job_credential_id being required, by actually provisioning a fake azurerm_mssql_job_credential, setting the job_credential_id to the provisioned credentials ID, then clearing the credential reference on the job step manually in the jobs database, and for good measure adding an ignore_changes on the job step so that the credential reference does not get set again:

resource "azurerm_mssql_job_credential" "my_unused_job_credential" {
  ...
  password     = random_password.temp.result
}

resource "azurerm_mssql_job_step" "my_job_step" {
  lifecycle {
    ignore_changes = [job_credential_id]
  }
  ...
  job_credential_id   = azurerm_mssql_job_credential.my_unused_job_credential.id
}

I did see that once you set a job step's credential via the Azure Portal, you cannot unset it. Upon further investigation, I also found that Microsoft's [jobs].[sp_update_jobstep] stored procedure in the jobs database does not allow unsetting of a job step's credential either, in that even if you do pass in a NULL for the @credential_name parameter, it coalesces the NULL back to the job step's existing credential_name. So there does not appear to be any way to unset a job credential via the UI or stored procedure. I have not tested out the behaviour via the API but I suspect this would be the same.

How did I clear the job step's credential in my workaround above? I edited the [jobs_internal].[jobstep_data] table directly and set the value of credential_name to NULL.

Given this, I would suggest that the markdown description for the job_credential_id in this PR is updated to indicate that once a value for this has been set, it cannot be unset due to lack of support on Microsoft's end for unsetting that value. I am not sure if that can be enforced in the code as well.

@sreallymatt
Copy link
Collaborator

sreallymatt commented Jul 14, 2025

Looking into this, it seems that once a job credential is set, it's not meant to be removed. However, in case that behaviour should be supported I have opened an issue on the Azure REST API spec repository. (Azure/azure-rest-api-specs#35881)

To make this clear to users, and to avoid a constant diff, we'll have to use a CustomizeDiff to conditionally ForceNew if the job_credential_id field is removed from the config.

@rorsgo, did you want to take a look at adding a CustomizeDiff for this behaviour, or if you prefer I can push changes to your branch?

@rorsgo
Copy link
Contributor Author

rorsgo commented Jul 14, 2025

@sreallymatt thanks! That would be great — I'm currently away from my computer, so feel free to push the changes to my branch. Appreciate the help!

@sreallymatt
Copy link
Collaborator

Test results:

image

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

LGTM!

@sreallymatt sreallymatt merged commit 06bfc69 into hashicorp:main Jul 14, 2025
34 checks passed
@github-actions github-actions bot added this to the v4.37.0 milestone Jul 14, 2025
sreallymatt added a commit that referenced this pull request Jul 14, 2025
mbfrahry pushed a commit that referenced this pull request Jul 17, 2025
@github-actions
Copy link
Contributor

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 Aug 14, 2025
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.

Support for user assigned managed identities in azurerm_mssql_job_step

5 participants