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

azuread_application: work around very buggy API when instantiating from template #1406

Merged

Conversation

manicminer
Copy link
Member

@manicminer manicminer commented Jun 11, 2024

Important

Depends on manicminer/hamilton#285, requires rebase prior to merging

The /instantiate operation can return a 404 whlst also processing the request completely normally. This leads to orphaned objects in the directory, and a resource that cannot successfully Create.

Work around this by polling for the application and service principal that you'd expect to see created out-of-band, whenever a 404 is received. Also set a temporary displayName for the application, as this is the only means we have to identify the resulting object is the one we are looking for.

Unfortunately this means that this workaround cannot be implemented for the azuread_application_from_template resource, since that resource intentionally avoids changing the implicitly created user_impersonation scope - this will get created with nonsensical display names / descriptions in the consent flow. Since the whole point of the standalone resource is to inherit scopes (and other fields) from the template, this makes it infeasible to add this workaround there.

Tested locally by intercepting the response to POST /v1.0/applicationTemplates/4601ed45-8ff3-4599-8377-b6649007e876/instantiate and sending a 404 back to the provider.

Screenshot 2024-06-11 at 18 23 59

Test results:

Screenshot 2024-06-11 at 18 19 26

…om template

The `/instantiate` operation can return a 404 whlst also processing the
request completely normally. This leads to orphaned objects in the
directory, and a resource that cannot successfully Create.

Work around this by polling for the application and service principal
that you'd expect to see created out-of-band, whenever a 404 is
received. Also set a temporary `displayName` for the application, as
this is the only means we have to identify the resulting object is the
one we are looking for.

Unfortunately this means that this workaround cannot be implemented for
the `azuread_application_from_template` resource, since that resource
intentionally avoids changing the implicitly created
`user_impersonation` scope - this will get created with nonsensical
display names / descriptions in the consent flow. Since the whole point
of the standalone resource is to inherit scopes from the template, this
makes it infeasible to add this workaround there.
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

One comment but otherwise 👍

@@ -63,3 +63,5 @@ require (
)

go 1.21.3

replace github.com/manicminer/hamilton => github.com/MarkDordoy/hamilton v0.17.1-0.20240611151114-899c6ce169f6
Copy link
Contributor

Choose a reason for hiding this comment

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

revert?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry forgot to note that this depends on manicminer/hamilton#285, will rebase prior to merge

@manicminer manicminer merged commit ed1121c into main Jun 13, 2024
26 checks passed
@manicminer manicminer deleted the bugfix/application-instantiate-404-api-bug-workaround branch June 13, 2024 21:15
@github-actions github-actions bot added this to the v2.52.0 milestone Jun 13, 2024
manicminer added a commit that referenced this pull request Jun 13, 2024
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Jun 18, 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.51.0&#34; to &#34;2.52.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>2.52.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azuread/releases/tag/v2.52.0&#xA;BUG
FIXES:&#xA;&#xA;* `azuread_application` - fix a bug that could prevent
the `ignore_changes` lifecycle argument from working for the `app_role`,
`oauth2_permission_scope`, `identifier_uris`, `optional_claims`, and
`required_resource_access` properties
([#1403](https://github.com/hashicorp/terraform-provider-azuread/issues/1403))&#xA;*
`azuread_application` - add a workaround for an API bug when
instantiating an application from template using the `template_id`
property
([#1406](https://github.com/hashicorp/terraform-provider-azuread/issues/1406))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/245/">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 pull request Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants