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_web_chat - site_names is removed in favour of site #23161

Merged
merged 18 commits into from
Sep 19, 2023
195 changes: 180 additions & 15 deletions internal/services/bot/bot_channel_web_chat_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/bot/parse"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/bot/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
Expand All @@ -23,7 +24,7 @@ import (
)

func resourceBotChannelWebChat() *pluginsdk.Resource {
return &pluginsdk.Resource{
resource := &pluginsdk.Resource{
Create: resourceBotChannelWebChatCreate,
Read: resourceBotChannelWebChatRead,
Delete: resourceBotChannelWebChatDelete,
Expand Down Expand Up @@ -53,16 +54,72 @@ func resourceBotChannelWebChat() *pluginsdk.Resource {
ValidateFunc: validate.BotName,
},

"site_names": {
"site": {
Type: pluginsdk.TypeSet,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
Optional: true,
Computed: !features.FourPointOhBeta(),
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
},

"application_id": {
Type: pluginsdk.TypeString,
Optional: true,
ValidateFunc: validation.StringIsNotEmpty,
neil-yechenwei marked this conversation as resolved.
Show resolved Hide resolved
},

"block_user_upload_enabled": {
Copy link
Member

Choose a reason for hiding this comment

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

This name is confusing - this has to be renamed to

Suggested change
"block_user_upload_enabled": {
"user_upload_enabled": {

Copy link
Contributor Author

@neil-yechenwei neil-yechenwei Sep 8, 2023

Choose a reason for hiding this comment

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

I assume we may not rename it to "user_upload_enabled" because user_upload_enabled has different meaning than block_user_upload_enabled.

When block_user_upload_enabled is true, it means the feature of blocking user upload is enabled and then user cannot upload.
When user_upload_enabled is true, it means the feature of blocking user upload is disabled and then user can upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed it to "user_upload_enabled".

Type: pluginsdk.TypeBool,
Optional: true,
Default: false,
},

"endpoint_parameters_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
},

"no_storage_enabled": {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, this should be renamed to

Suggested change
"no_storage_enabled": {
"storage_enabled": {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume we may not rename it to "storage_enabled" because storage_enabled has different meaning than no_storage_enabled.

When no_storage_enabled is true, it means the site without storage is added.
When storage_enabled is true, it means the site with storage is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed it to "storage_enabled".

Type: pluginsdk.TypeBool,
Optional: true,
},

"tenant_id": {
Type: pluginsdk.TypeString,
Optional: true,
ValidateFunc: validation.IsUUID,
},
},
},
ExactlyOneOf: func() []string {
if !features.FourPointOhBeta() {
return []string{"site_names", "site"}
}
return []string{}
}(),
},
},
}

if !features.FourPointOhBeta() {
resource.Schema["site_names"] = &pluginsdk.Schema{
Type: pluginsdk.TypeSet,
Optional: true,
Computed: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
Deprecated: "`site_names` will be removed in favour of the property `site` in version 4.0 of the AzureRM Provider.",
ExactlyOneOf: []string{"site_names", "site"},
}
}

return resource
}

func resourceBotChannelWebChatCreate(d *pluginsdk.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -99,20 +156,36 @@ func resourceBotChannelWebChatCreate(d *pluginsdk.ResourceData, meta interface{}

channel := botservice.BotChannel{
Properties: botservice.WebChatChannel{
Properties: &botservice.WebChatChannelProperties{
Sites: expandSiteNames(d.Get("site_names").(*pluginsdk.Set).List()),
},
Properties: &botservice.WebChatChannelProperties{},
ChannelName: botservice.ChannelNameBasicChannelChannelNameWebChatChannel,
},
Location: utils.String(azure.NormalizeLocation(d.Get("location").(string))),
Kind: botservice.KindBot,
}

if !features.FourPointOhBeta() {
if v, ok := d.GetOk("site_names"); ok {
channel, _ := channel.Properties.AsWebChatChannel()
channel.Properties.Sites = expandSiteNames(v.(*pluginsdk.Set).List())
}
}

if v, ok := d.GetOk("site"); ok {
channel, _ := channel.Properties.AsWebChatChannel()
channel.Properties.Sites = expandSites(v.(*pluginsdk.Set).List())
}

if _, err := client.Create(ctx, id.ResourceGroup, id.BotServiceName, botservice.ChannelNameWebChatChannel, channel); err != nil {
return fmt.Errorf("creating %s: %+v", id, err)
}

// Unable to add a new site with block_user_upload_enabled, endpoint_parameters_enabled, no_storage_enabled in the same operation, so we need to make two calls
if _, err := client.Update(ctx, id.ResourceGroup, id.BotServiceName, botservice.ChannelNameWebChatChannel, channel); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}

d.SetId(id.ID())

return resourceBotChannelWebChatRead(d, meta)
}

Expand Down Expand Up @@ -144,8 +217,14 @@ func resourceBotChannelWebChatRead(d *pluginsdk.ResourceData, meta interface{})
if props := resp.Properties; props != nil {
if channel, ok := props.AsWebChatChannel(); ok {
if channelProps := channel.Properties; channelProps != nil {
if err := d.Set("site_names", flattenSiteNames(channelProps.Sites)); err != nil {
return fmt.Errorf("setting `site_names`: %+v", err)
if !features.FourPointOhBeta() {
if err := d.Set("site_names", flattenSiteNames(channelProps.Sites)); err != nil {
return fmt.Errorf("setting `site_names`: %+v", err)
}
}

if err := d.Set("site", flattenSites(channelProps.Sites)); err != nil {
return fmt.Errorf("setting `site`: %+v", err)
}
}
}
Expand All @@ -166,15 +245,30 @@ func resourceBotChannelWebChatUpdate(d *pluginsdk.ResourceData, meta interface{}

channel := botservice.BotChannel{
Properties: botservice.WebChatChannel{
Properties: &botservice.WebChatChannelProperties{
Sites: expandSiteNames(d.Get("site_names").(*pluginsdk.Set).List()),
},
Properties: &botservice.WebChatChannelProperties{},
ChannelName: botservice.ChannelNameBasicChannelChannelNameWebChatChannel,
},
Location: utils.String(azure.NormalizeLocation(d.Get("location").(string))),
Kind: botservice.KindBot,
}

if !features.FourPointOhBeta() {
if d.HasChange("site_names") {
channel, _ := channel.Properties.AsWebChatChannel()
channel.Properties.Sites = expandSiteNames(d.Get("site_names").(*pluginsdk.Set).List())
}
}

if d.HasChange("site") {
channel, _ := channel.Properties.AsWebChatChannel()
channel.Properties.Sites = expandSites(d.Get("site").(*pluginsdk.Set).List())
}

if _, err := client.Update(ctx, id.ResourceGroup, id.BotServiceName, botservice.ChannelNameWebChatChannel, channel); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}

// Unable to add a new site with block_user_upload_enabled, endpoint_parameters_enabled, no_storage_enabled in the same operation, so we need to make two calls
Copy link
Member

Choose a reason for hiding this comment

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

Copy paste error? I don't see a second call here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first call is at line 267. The second call is at line 272. Seems it's just a display problem.

if _, err := client.Update(ctx, id.ResourceGroup, id.BotServiceName, botservice.ChannelNameWebChatChannel, channel); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}
Expand All @@ -192,6 +286,11 @@ func resourceBotChannelWebChatDelete(d *pluginsdk.ResourceData, meta interface{}
return err
}

existing, err := client.Get(ctx, id.ResourceGroup, id.BotServiceName, string(botservice.ChannelNameWebChatChannel))
if err != nil {
return err
}

channel := botservice.BotChannel{
Properties: botservice.WebChatChannel{
Properties: &botservice.WebChatChannelProperties{
Expand All @@ -204,7 +303,7 @@ func resourceBotChannelWebChatDelete(d *pluginsdk.ResourceData, meta interface{}
},
ChannelName: botservice.ChannelNameBasicChannelChannelNameWebChatChannel,
},
Location: utils.String(azure.NormalizeLocation(d.Get("location").(string))),
Location: utils.String(azure.NormalizeLocation(*existing.Location)),
Kind: botservice.KindBot,
}

Expand All @@ -230,6 +329,36 @@ func expandSiteNames(input []interface{}) *[]botservice.WebChatSite {
return &results
}

func expandSites(input []interface{}) *[]botservice.WebChatSite {
results := make([]botservice.WebChatSite, 0)

for _, item := range input {
site := item.(map[string]interface{})
result := botservice.WebChatSite{
IsEnabled: utils.Bool(true),
IsBlockUserUploadEnabled: utils.Bool(site["block_user_upload_enabled"].(bool)),
IsEndpointParametersEnabled: utils.Bool(site["endpoint_parameters_enabled"].(bool)),
IsNoStorageEnabled: utils.Bool(site["no_storage_enabled"].(bool)),
}

if siteName := site["name"].(string); siteName != "" {
result.SiteName = utils.String(siteName)
}

if v := site["application_id"].(string); v != "" {
result.AppID = utils.String(v)
}

if v := site["tenant_id"].(string); v != "" {
result.TenantID = utils.String(v)
}

results = append(results, result)
}

return &results
}

func flattenSiteNames(input *[]botservice.WebChatSite) []interface{} {
results := make([]interface{}, 0)
if input == nil {
Expand All @@ -248,6 +377,42 @@ func flattenSiteNames(input *[]botservice.WebChatSite) []interface{} {
return results
}

func flattenSites(input *[]botservice.WebChatSite) []interface{} {
results := make([]interface{}, len(*input))
Copy link
Member

Choose a reason for hiding this comment

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

We should explicitly initialise it with 0 length

Suggested change
results := make([]interface{}, len(*input))
results := make([]interface{}, 0)

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


for k, item := range *input {
result := make(map[string]interface{})

if v := item.SiteName; v != nil {
result["name"] = *v
}

if v := item.AppID; v != nil {
result["application_id"] = *v
}

if v := item.IsBlockUserUploadEnabled; v != nil {
result["block_user_upload_enabled"] = *v
}

if v := item.IsEndpointParametersEnabled; v != nil {
result["endpoint_parameters_enabled"] = *v
}

if v := item.IsNoStorageEnabled; v != nil {
result["no_storage_enabled"] = *v
}

if v := item.TenantID; v != nil {
result["tenant_id"] = *v
}

results[k] = result
}

return results
}

func includeDefaultWebChatSite(sites *[]botservice.WebChatSite) bool {
includeDefaultSite := false
for _, site := range *sites {
Expand Down
29 changes: 26 additions & 3 deletions internal/services/bot/bot_channel_web_chat_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ resource "azurerm_bot_channel_web_chat" "test" {
bot_name = azurerm_bot_channels_registration.test.name
location = azurerm_bot_channels_registration.test.location
resource_group_name = azurerm_resource_group.test.name
site_names = ["TestSite"]

site {
Copy link
Member

Choose a reason for hiding this comment

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

We can't completely remove the deprecated property site_named from all tests, otherwise we're not checking that it's still usable, which it is. Please add it back to one of the tests so we can verify that it still works.

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

name = "TestSite"
}
}
`, BotChannelsRegistrationResource{}.basicConfig(data))
}
Expand All @@ -128,7 +131,10 @@ resource "azurerm_bot_channel_web_chat" "import" {
bot_name = azurerm_bot_channel_web_chat.test.bot_name
location = azurerm_bot_channel_web_chat.test.location
resource_group_name = azurerm_bot_channel_web_chat.test.resource_group_name
site_names = ["TestSite"]

site {
name = "TestSite"
}
}
`, r.basic(data))
}
Expand All @@ -141,7 +147,24 @@ resource "azurerm_bot_channel_web_chat" "test" {
bot_name = azurerm_bot_channels_registration.test.name
location = azurerm_bot_channels_registration.test.location
resource_group_name = azurerm_resource_group.test.name
site_names = ["TestSite2", "TestSite3"]

site {
name = "TestSite1"
application_id = "testAppId1"
tenant_id = data.azurerm_client_config.current.tenant_id
block_user_upload_enabled = true
endpoint_parameters_enabled = true
no_storage_enabled = true
}

site {
name = "TestSite2"
application_id = "testAppId2"
tenant_id = data.azurerm_client_config.current.tenant_id
block_user_upload_enabled = false
endpoint_parameters_enabled = false
no_storage_enabled = false
}
}
`, BotChannelsRegistrationResource{}.basicConfig(data))
}
28 changes: 24 additions & 4 deletions website/docs/r/bot_channel_web_chat.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ resource "azurerm_bot_channel_web_chat" "example" {
location = azurerm_bot_channels_registration.example.location
resource_group_name = azurerm_resource_group.example.name

site_names = [
"TestSite",
]
site {
name = "TestSite"
}
}
```

Expand All @@ -49,7 +49,27 @@ The following arguments are supported:

* `bot_name` - (Required) The name of the Bot Resource this channel will be associated with. Changing this forces a new resource to be created.

* `site_names` - (Required) A list of Web Chat Site names.
* `site_names` - (Optional) A list of Web Chat Site names.

~> **NOTE:** `site_names` is deprecated and will be removed in favour of the property `site` in version 4.0 of the AzureRM Provider.

* `site` - (Optional) A site represents a client application that you want to connect to your bot. Multiple `site` blocks may be defined as below

---

A `site` block has the following properties:

* `name` - (Required) The name of the site.

* `application_id` - (Optional) The application ID for this site.

* `block_user_upload_enabled` - (Optional) Is the block user upload enabled for this site? Defaults to `false`.

* `endpoint_parameters_enabled` - (Optional) Is the endpoint parameters enabled for this site?

* `no_storage_enabled` - (Optional) Is no storage enabled for this site?

* `tenant_id` - (Optional) The Tenant ID for this site.

## Attributes Reference

Expand Down