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_bot_channel_email - support for magic_code #23129

Merged
merged 7 commits into from
Jan 30, 2024

Conversation

neil-yechenwei
Copy link
Contributor

@neil-yechenwei neil-yechenwei commented Aug 30, 2023

fixes #20982

--- PASS: TestAccBotChannelsRegistration/channel/emailMagicCode (70.20s)


if v, ok := d.GetOk("magic_code"); ok {
channel, _ := channel.Properties.AsEmailChannel()
channel.Properties.AuthMethod = utils.Float(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

this indicates a Swagger/API issue since these should be exposed as Constant values and not an integer - can we raise a Swagger issue for that

Copy link
Contributor Author

@neil-yechenwei neil-yechenwei Sep 4, 2023

Choose a reason for hiding this comment

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

@tombuildsstuff , I assume it's by design since they added the meaning/description of the value. "0" means "Password" by default. "1" means "OAuth". See more details from rest api spec. Does it make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Service team confirmed that it's by design to them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated PR for this concern.

@neil-yechenwei
Copy link
Contributor Author

@tombuildsstuff , thanks for the comment and I replied. Please take another look. Thanks.

@stephybun stephybun added the upstream/pandora This issue/PR has a dependency on an issue in `github.com/hashicorp/pandora` label Sep 14, 2023
@stephybun stephybun added this to the Blocked milestone Sep 14, 2023
@neil-yechenwei
Copy link
Contributor Author

@stephybun , may I ask why this PR is added "upstream-pandora"?

@stephybun
Copy link
Member

@neil-yechenwei the values for Auth method aren't being surfaced correctly as constants in Pandora.

@tombuildsstuff tombuildsstuff self-assigned this Sep 15, 2023
@neil-yechenwei
Copy link
Contributor Author

@stephybun & @tombuildsstuff , is it possible to merge this PR first to unblock users since AuthMethod isn't exposed to users?

@katbyte
Copy link
Collaborator

katbyte commented Nov 7, 2023

"0" means "Password" by default. "1" means "OAuth".

this should still be 2 constants defined in swagger? so can you please open a rest API specs issue and add a comment with it inline explaining what 1 and 0 means and why it should be fixed so we can merge this with the link to the bug? thanks

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Nov 7, 2023

Hi @katbyte , the "AuthMethod" property has been defined as enum constants in azure rest api spec. And service team has confirmed that defining this property as number type is by API design so that they won't change it. As @stephybun mentioned, it's a pandora issue since the values for this property aren't being surfaced correctly as constants in Pandora. So I filed an issue on hashicorp/pandora#3254. As we don't expose AuthMethod as parameter to user in this resource, so I assume we can merge this PR first. Once this pandora issue is fixed, we can submit another PR to update this resource with pandora change. Does it make sense?

Note: there is a GH issue that is requesting this feature so that I linked that issue to here.

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Nov 24, 2023

@katbyte , is it good to be merged?

@tombuildsstuff
Copy link
Contributor

I've got a pending review for this one in-flight but this is blocked on hashicorp/go-azure-helpers#183 which'll enable switching this over to hashicorp/go-azure-sdk which'll allow us to fix this in the SDK.

@neil-yechenwei
Copy link
Contributor Author

@tombuildsstuff , @katbyte , @stephybun , I updated PR. Please take another look. Thanks.

@tombuildsstuff tombuildsstuff removed this from the Blocked milestone Jan 24, 2024
@tombuildsstuff tombuildsstuff removed their assignment Jan 24, 2024
@tombuildsstuff tombuildsstuff removed the upstream/pandora This issue/PR has a dependency on an issue in `github.com/hashicorp/pandora` label Jan 24, 2024
@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Jan 25, 2024

@tombuildsstuff , @katbyte , @stephybun , Below is the latest test result.

--- PASS: TestAccBotChannelsRegistration/channel/emailBasic (198.77s)
--- PASS: TestAccBotChannelsRegistration/channel/emailUpdate (244.39s)
--- PASS: TestAccBotChannelsRegistration/channel/emailMagicCode (65.30s)

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.

Aside from 1 minor comment LGTM ☔

website/docs/r/bot_channel_email.html.markdown Outdated Show resolved Hide resolved
@katbyte katbyte merged commit de887a9 into hashicorp:main Jan 30, 2024
3 checks passed
katbyte added a commit that referenced this pull request Jan 30, 2024
@github-actions github-actions bot added this to the v3.90.0 milestone Jan 30, 2024
rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Jan 30, 2024
* azurerm_bot_channel_email - support for magic_code

* update code

* update code

* update code

* update code

* Update website/docs/r/bot_channel_email.html.markdown

---------

Co-authored-by: kt <[email protected]>
rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Jan 30, 2024
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Feb 5, 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.89.0&#34; to &#34;3.90.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.90.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.90.0&#xA;UPGRADE
NOTES:&#xA;&#xA;* provider - The provider will now automatically
register the `AppConfiguration`, `DataFactory`, and `SignalRService`
Resource Providers. When running Terraform with limited permissions,
note that you [must disable automatic Resource Provider Registration and
ensure that any Resource Providers Terraform requires are
registered]([XXX](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#skip_provider_registration)).
([#24645](hashicorp/terraform-provider-azurerm#24645;
&#xA;FEATURES:&#xA;&#xA;* **New Data Source**:
`azurerm_nginx_configuration`
([#24642](hashicorp/terraform-provider-azurerm#24642
**New Data Source**: `azurerm_virtual_desktop_workspace`
([#24732](hashicorp/terraform-provider-azurerm#24732
**New Resource**: `azurerm_kubernetes_fleet_update_strategy`
([#24328](hashicorp/terraform-provider-azurerm#24328
**New Resource**: `azurerm_site_recovery_vmware_replicated_vm`
([#22477](hashicorp/terraform-provider-azurerm#22477
**New Resource**:
`azurerm_spring_cloud_new_relic_application_performance_monitoring`
([#24699](https://github.com/hashicorp/terraform-provider-azurerm/issues/24699))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
provider: registering the Resource Provider `Microsoft.AppConfiguration`
([#24645](hashicorp/terraform-provider-azurerm#24645
provider: registering the Resource Provider `Microsoft.DataFactory`
([#24645](hashicorp/terraform-provider-azurerm#24645
provider: registering the Resource Provider `Microsoft.SignalRService`
([#24645](hashicorp/terraform-provider-azurerm#24645
provider: the Provider is now built using Go 1.21.6
([#24653](hashicorp/terraform-provider-azurerm#24653
dependencies: the dependency `github.com/hashicorp/go-azure-sdk` has
been split into multiple Go Modules - and as such will be referred to by
those paths going forwards
([#24636](hashicorp/terraform-provider-azurerm#24636
dependencies: updating to ``v0.20240201.1064937` of
`github.com/hashicorp/go-azure-sdk/resource-manager`
([#24738](hashicorp/terraform-provider-azurerm#24738
dependencies: updating to `v0.20240201.1064937` of
`github.com/hashicorp/go-azure-sdk/sdk`
([#24738](hashicorp/terraform-provider-azurerm#24738
`appservice`: update to `go-azure-sdk` and API version `2023-01-01`
([#24688](hashicorp/terraform-provider-azurerm#24688
`datafactory`: updating to use `tombuildsstuff/kermit`
([#24675](hashicorp/terraform-provider-azurerm#24675
`hdinsight`: refactoring to use
`github.com/hashicorp/go-azure-sdk/resource-manager`
([#24011](hashicorp/terraform-provider-azurerm#24011
`hdinsight`: updating to API Version `2021-06-01`
([#24011](hashicorp/terraform-provider-azurerm#24011
`loadbalancer`: updating to use `hashicorp/go-azure-sdk`
([#24291](hashicorp/terraform-provider-azurerm#24291
`nginx`: updating to API Version `2023-09-01`
([#24640](hashicorp/terraform-provider-azurerm#24640
`servicefabricmanagedcluster`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
([#24654](hashicorp/terraform-provider-azurerm#24654
`springcloud`: updating to use API Version `2023-11-01-preview`
([#24690](hashicorp/terraform-provider-azurerm#24690
`subscriptions`: refactoring to use `hashicorp/go-azure-sdk`
([#24663](hashicorp/terraform-provider-azurerm#24663
Data Source: `azurerm_stream_analytics_job` - support for User Assigned
Identities
([#24738](hashicorp/terraform-provider-azurerm#24738
`azurerm_cosmosdb_account` - support for the `gremlin_database` and
`tables_to_restore` properties
([#24627](hashicorp/terraform-provider-azurerm#24627
`azurerm_bot_channel_email` - support for the `magic_code` property
([#23129](hashicorp/terraform-provider-azurerm#23129
`azurerm_cosmosdb_account` - support for the `partition_merge_enabled`
property
([#24615](hashicorp/terraform-provider-azurerm#24615
`azurerm_mssql_managed_database` - support for the
`immutable_backups_enabled` property
([#24745](hashicorp/terraform-provider-azurerm#24745
`azurerm_mssql_database` - support for the `immutable_backups_enabled`
property
([#24745](hashicorp/terraform-provider-azurerm#24745
`azurerm_palo_alto_next_generation_firewall_virtual_hub_panorama` -
support for the `trusted_address_ranges` property
([#24459](hashicorp/terraform-provider-azurerm#24459
`azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack`
- support for the `trusted_address_ranges` property
([#24459](hashicorp/terraform-provider-azurerm#24459
`azurerm_palo_alto_next_generation_firewall_virtual_network_panorama` -
support for the `trusted_address_ranges` property
([#24459](hashicorp/terraform-provider-azurerm#24459
`azurerm_servicebus_namespace` - updating to use API Version
`2022-10-01-preview`
([#24650](hashicorp/terraform-provider-azurerm#24650
`azurerm_spring_cloud_api_portal` - support for the
`api_try_out_enabled` property
([#24696](hashicorp/terraform-provider-azurerm#24696
`azurerm_spring_cloud_gateway` - support for the
`local_response_cache_per_route` and `local_response_cache_per_instance`
properties
([#24697](hashicorp/terraform-provider-azurerm#24697
`azurerm_stream_analytics_job` - support for User Assigned Identities
([#24738](hashicorp/terraform-provider-azurerm#24738
`azurerm_subscription` - refactoring to use `hashicorp/go-azure-sdk` to
set tags on the subscription
([#24734](hashicorp/terraform-provider-azurerm#24734
`azurerm_virtual_desktop_workspace` - correctly validate the `name`
property
([#24668](https://github.com/hashicorp/terraform-provider-azurerm/issues/24668))&#xA;&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* provider: skip registration for resource providers
that are unavailable
([#24571](hashicorp/terraform-provider-azurerm#24571
`azurerm_app_configuration` - no longer require
`lifecycle_ignore_changes` for the `value` property when using a key
vault reference
([#24702](hashicorp/terraform-provider-azurerm#24702
`azurerm_app_service_managed_certificate` - fix casing issue in
`app_service_plan_id` by parsing insensitively
([#24664](hashicorp/terraform-provider-azurerm#24664
`azurerm_cognitive_deployment` - updates now include the `version`
property
([#24700](hashicorp/terraform-provider-azurerm#24700
`azurerm_dns_cname_record` - prevent casing issue in
`target_resource_id` by parsing the ID insensitively
([#24181](hashicorp/terraform-provider-azurerm#24181
`azurerm_mssql_managed_instance_failover_group` - prevent an issue when
trying to create a failover group with a managed instance from a
different subscription
([#24646](hashicorp/terraform-provider-azurerm#24646
`azurerm_storage_account` - conditionally update properties only when
needed
([#24669](hashicorp/terraform-provider-azurerm#24669
`azurerm_storage_account` - change update order for `access_tier`to
prevent errors when uploading blobs to the archive tier
([#22250](https://github.com/hashicorp/terraform-provider-azurerm/issues/22250))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1075/">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) <[email protected]>
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 26, 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.

Argument missing | azurerm_bot_channel_email_v3.33.0 module
4 participants