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

Deprecation notices for application_oauth2_permission{,_scope} and azuread_application_app_role resources #465

Merged
merged 2 commits into from
Jun 21, 2021

Conversation

manicminer
Copy link
Contributor

@manicminer manicminer commented Jun 18, 2021

Both the azuread_application_app_role and azuread_application_oauth2_permission
resources were added to the provider as a workaround for several bugs in our
implementations of the corresponding properties in the azuread_application resource.

  • Both these properties (blocks) are TypeSet, and currently Optional + Computed,
    which stems from API inconsistencies and (in the case of oauth2 permissions)
    default values applied by the AAD Graph API.
  • The id field in each of these blocks is Computed (read-only),
    however there are use cases where it's necessary to be able to specify
    this field. We couldn't make this field Optional + Computed inside a TypeSet
    that is also Optional + Computed, due to an upstream limitation / bug.

We therefore introduced these virtual resources to enable users to specify an
id for a role/scope.

In version 2.0 both these blocks in the azuread_application resource
will lose their Computed field, to eliminate these problems for users:

  • Although we use ConfigMode: schema.SchemaConfigModeAttr, it's not
    always possible to clear the values of these properties because the
    actual value in configuration is not exposed to the provider (via
    either schema.ResourceData or schema.ResourceDiff).
  • Changes to these properties are not always detected by Terraform when
    there are no other changes to that same resource, and then the
    UpdateFunc is never actually invoked.
  • When updating these fields, the API requires them to be orchestrated
    so that each role/scope is first disabled before updating or removing
    it. With id being Computed, we have no way to identify which
    role/scope was updated by the user and so we have no choice but to
    disable all of them, before making our changes and quickly re-enabling
    them. This causes momentary outages for users and has sometimes left
    roles/scopes disabled after the fact due to an API bug ignoring our
    re-enable request (breaking customers' applications).

All of these issues will be fixed in v2.0, by way of removing the Computed
field from these properties. This means we can no longer support these
virtual resources, but also that they are no longer needed, so in v2.0
they will be removed.

…uread_application_app_role resources

Both the azuread_application_app_role and azuread_application_oauth2_permission
resources were added to the provider as a workaround for several bugs in our
implementations of the corresponding properties in the azuread_application resource.

- Both these properties (blocks) are TypeSet, and currently Optional + Computed,
  which stems from API inconsistencies and (in the case of oauth2 permissions)
  default values applied by the AAD Graph API.
- The `id` field in each of these blocks is Computed (read-only),
  however there are use cases where it's necessary to be able to specify
  this field. We couldn't make this field Optional + Computed inside a TypeSet
  that is also Optional + Computed, due to an upstream limitation / bug.

We therefore introduced these virtual resources to enable users to specify an
`id` for a role/scope.

In version 2.0 both these blocks in the azuread_application resource
will lose their Computed field, to eliminate these problems for users:

- Although we use `ConfigMode: schema.SchemaConfigModeAttr`, it's not
  always possible to clear the values of these properties because the
  actual value in configuration is not exposed to the provider (via
  either schema.ResourceData or schema.ResourceDiff).
- Changes to these properties are not always detected by Terraform when
  there are no other changes to that same resource, and then the
  UpdateFunc is never actually invoked.
- When updating these fields, the API requires them to be orchestrated
  so that each role/scope is first disabled before updating or removing
  it. With `id` being Computed, we have no way to identify which
  role/scope was updated by the user and so we have no choice but to
  disable all of them, before making our changes and quickly re-enabling
  them. This causes momentary outages for users and has sometimes left
  roles/scopes disabled after the fact due to an API bug ignoring our
  re-enable request (breaking customers' applications).

All of these issues are fixed in v2.0, by way of removing the Computed
field from these properties. This means we can no longer support these
virtual resources, but also that they are no longer needed, so in v2.0
they will be removed.
@manicminer manicminer added this to the v1.6.0 milestone Jun 18, 2021
@manicminer manicminer requested a review from a team June 18, 2021 16:03
@manicminer manicminer mentioned this pull request Jun 21, 2021
10 tasks
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.

LGTM 👍

@manicminer manicminer merged commit 82a68d3 into main Jun 21, 2021
@manicminer manicminer deleted the deprecate-app-virtual-resources branch June 21, 2021 19:15
manicminer added a commit that referenced this pull request Jun 21, 2021
@github-actions
Copy link

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

@github-actions
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 Jul 25, 2021
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.

2 participants