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

State is updated with new password, even when password change failed. #1308

Closed
jamesw4 opened this issue Feb 9, 2024 · 1 comment · Fixed by #1379
Closed

State is updated with new password, even when password change failed. #1308

jamesw4 opened this issue Feb 9, 2024 · 1 comment · Fixed by #1379

Comments

@jamesw4
Copy link

jamesw4 commented Feb 9, 2024

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritise this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritise the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureAD Provider) Version

Terraform v1.7.3
on windows_amd64

  • provider registry.terraform.io/hashicorp/azuread v2.47.0
  • provider registry.terraform.io/hashicorp/random v3.6.0

Affected Resource(s)

  • azuread_user

Terraform Configuration Files

resource "random_password" "test" {
  length = 16
}

resource "azuread_user" "example" {
  user_principal_name = "test@test.com"
  display_name = "test"
  password            = random_password.test.result
}

terraform {
  required_providers {
    azuread = {
       version = "=2.47.0"
    }
  }
}

Debug Output

https://gist.github.com/jamesw4/ab2f287eb5519d329e0245ddae398763

Panic Output

Expected Behavior

On password update failure, state should not be updated with the attempted password.

Actual Behavior

Even though password update failed, and the API threw an error, state is updated with the attempted password.

Steps to Reproduce

  1. terraform apply the above example to create test user.
  2. Stage a failure by setting length of random_password resource to 1.
  3. terraform apply again and observe error thrown.
  4. Examine state file and observe state has the password as the 1 char password that failed to be applied.

Important Factoids

References

  • #0000
@manicminer
Copy link
Contributor

Thanks for reporting @jamesw4, sorry for the delay. I'm able to reproduce this and with a little digging I found a workaround that avoids this.

@manicminer manicminer added this to the v2.50.0 milestone May 16, 2024
manicminer added a commit that referenced this issue May 16, 2024
dduportal referenced this issue in jenkins-infra/azure May 20, 2024
<Actions>
<action
id="6d17e7acdb2f3311576150379e22805f2f9b4aa72ff00ec136aceee45cae4b98">
        <h3>Bump Terraform `azuread` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>changes detected:&#xA;&#x9;&#34;hashicorp/azuread&#34; updated from
&#34;2.49.1&#34; to &#34;2.50.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>2.50.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azuread/releases/tag/v2.50.0&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.68.0` of `github.com/manicminer/hamilton`
([#1382](hashicorp/terraform-provider-azuread#1382
`data.azuread_application` - support looking up applications with the
`identifier_uri` property [GH 1303]&#xA;*
`azuread_conditional_access_policy` - improve handling of the
`session_controls` block
([#1382](https://github.com/hashicorp/terraform-provider-azuread/issues/1382))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* `data.azuread_service_principal` - treat the
`display_name` property case-insensitively
([#1381](hashicorp/terraform-provider-azuread#1381
`azuread_conditional_access_policy` - fix a bug that could cause a
persistent diff when setting certain properties in the
`session_controls` block
([#1382](hashicorp/terraform-provider-azuread#1382
`azuread_user` - don&#39;t overwrite the existing password in state,
when a password change fails
([#1308](https://github.com/hashicorp/terraform-provider-azuread/issues/1308))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/188/">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>
BrendanThompson pushed a commit to BrendanThompson/terraform-provider-azuread that referenced this issue Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants