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_api_management_api - deprecate sku in favour of the sku_name property #3154

Merged
merged 15 commits into from
Sep 24, 2019
Merged
34 changes: 8 additions & 26 deletions azurerm/data_source_api_management.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,9 @@ func dataSourceApiManagementService() *schema.Resource {
Computed: true,
},

"sku": {
Type: schema.TypeList,
"sku_name": {
katbyte marked this conversation as resolved.
Show resolved Hide resolved
Type: schema.TypeString,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Computed: true,
},
"capacity": {
Type: schema.TypeInt,
Computed: true,
},
},
},
},

"notification_sender_email": {
Expand Down Expand Up @@ -198,8 +186,8 @@ func dataSourceApiManagementRead(d *schema.ResourceData, meta interface{}) error
}
}

if err := d.Set("sku", flattenDataSourceApiManagementServiceSku(resp.Sku)); err != nil {
return fmt.Errorf("Error setting `sku`: %+v", err)
if err := d.Set("sku_name", flattenDataSourceApiManagementServiceSku(resp.Sku)); err != nil {
return fmt.Errorf("Error setting `sku_name`: %+v", err)
}

flattenAndSetTags(d, resp.Tags)
Expand Down Expand Up @@ -289,20 +277,14 @@ func flattenDataSourceApiManagementAdditionalLocations(input *[]apimanagement.Ad
return results
}

func flattenDataSourceApiManagementServiceSku(profile *apimanagement.ServiceSkuProperties) []interface{} {
func flattenDataSourceApiManagementServiceSku(profile *apimanagement.ServiceSkuProperties) string {
if profile == nil {
return []interface{}{}
return ""
}

sku := make(map[string]interface{})

sku["name"] = string(profile.Name)

if profile.Capacity != nil {
sku["capacity"] = *profile.Capacity
}
skuName := fmt.Sprintf("%s_%d", string(profile.Name), *profile.Capacity)

return []interface{}{sku}
return skuName
}

func apiManagementDataSourceHostnameSchema() map[string]*schema.Schema {
Expand Down
58 changes: 58 additions & 0 deletions azurerm/helpers/azure/sku.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package azure

import (
"fmt"
"strconv"
"strings"

"github.com/hashicorp/terraform/helper/schema"
)

func SplitSku(sku string) (string, int32, error) {
skuParts := strings.Split(sku, "_")

if len(skuParts) != 2 {
return "", -1, fmt.Errorf("sku_name(%s) is not formatted properly.", sku)
}

capacity, err := strconv.Atoi(skuParts[1])

if err != nil {
return "", -1, fmt.Errorf("%s in sku_name is not a valid value.", skuParts[1])
}

return skuParts[0], int32(capacity), nil
}

// SkuNameStringInSlice returns a SchemaValidateFunc which tests if the provided value
// is of type string and matches the value of an element in the valid slice
// will test with in lower case if ignoreCase is true will also validate if the
// capacity if above passed minCapacity value
func SkuNameStringInSlice(valid []string, minCapacity int32, ignoreCase bool) schema.SchemaValidateFunc {
return func(i interface{}, k string) (s []string, es []error) {
v, ok := i.(string)
if !ok {
es = append(es, fmt.Errorf("expected type of %s to be string", k))
return
}

name, capacity, err := SplitSku(v)

if err != nil {
es = append(es, err)
return
}

for _, str := range valid {
if name == str || (ignoreCase && strings.ToLower(name) == strings.ToLower(str)) {
if capacity < minCapacity {
es = append(es, fmt.Errorf("expected %s capacity value to be greater that %d, got %d", k, minCapacity, capacity))
}
return
}
}

es = append(es, fmt.Errorf("expected %s to be one of %v, got %s", k, valid, name))
return
}
}
92 changes: 78 additions & 14 deletions azurerm/resource_arm_api_management.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@ func resourceArmApiManagementService() *schema.Resource {
ValidateFunc: validate.ApiManagementServicePublisherEmail,
},

// Remove in 2.0
"sku": {
Type: schema.TypeList,
Required: true,
MaxItems: 1,
Type: schema.TypeList,
Optional: true,
Deprecated: "This property has been deprecated in favour of the 'sku_name' property and will be removed in version 2.0 of the provider",
ConflictsWith: []string{"sku_name"},
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Expand All @@ -77,15 +80,28 @@ func resourceArmApiManagementService() *schema.Resource {
string(apimanagement.SkuTypePremium),
}, false),
},

"capacity": {
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IntAtLeast(0),
ValidateFunc: validation.IntAtLeast(1),
Copy link
Collaborator

Choose a reason for hiding this comment

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

How come this value changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Per the SDK implementation:

// Capacity - Capacity of the SKU (number of deployed units of the SKU). The default value is 1.

Copy link
Collaborator

Choose a reason for hiding this comment

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

that says default not minimum? thou it does make sense

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh didn't mean it needed to have a default added, I mean it's all moot as this is removed in a couple version so 🤷‍♀

Copy link
Collaborator

Choose a reason for hiding this comment

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

was more this would break anyone using 0 currently

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reverted code and made it like it was, to avoid potential impact to existing users...

},
},
},
},

"sku_name": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"sku"},
ValidateFunc: azure.SkuNameStringInSlice([]string{
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function seems rather specific to this service?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are lots of SKU's that have the format of Name_Capacity so I will be able to reuse this function in other resources flatten SKU PR's. I have updated the name so it is more descriptive of what it is actually doing.

string(apimanagement.SkuTypeDeveloper),
string(apimanagement.SkuTypeBasic),
string(apimanagement.SkuTypeStandard),
string(apimanagement.SkuTypePremium),
}, 1, false),
},

"identity": {
Type: schema.TypeList,
Optional: true,
Expand Down Expand Up @@ -366,6 +382,14 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in
client := meta.(*ArmClient).apiManagementServiceClient
ctx := meta.(*ArmClient).StopContext

log.Printf("[INFO] validating SKU for API Management Service.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems like a mistake?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the line of code...


sku := expandAzureRmApiManagementSku(d)

if sku == nil {
return fmt.Errorf("either 'sku_name' or 'sku' must be defined in the configuration file")
}

log.Printf("[INFO] preparing arguments for API Management Service creation.")

name := d.Get("name").(string)
Expand All @@ -387,8 +411,6 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in
location := azureRMNormalizeLocation(d.Get("location").(string))
tags := d.Get("tags").(map[string]interface{})

sku := expandAzureRmApiManagementSku(d)

publisherName := d.Get("publisher_name").(string)
publisherEmail := d.Get("publisher_email").(string)
notificationSenderEmail := d.Get("notification_sender_email").(string)
Expand Down Expand Up @@ -493,6 +515,7 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{

resourceGroup := id.ResourceGroup
name := id.Path["service"]
skuName := d.Get("sku_name").(string)

resp, err := client.Get(ctx, resourceGroup, name)
if err != nil {
Expand Down Expand Up @@ -562,8 +585,21 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{
}
}

if err := d.Set("sku", flattenApiManagementServiceSku(resp.Sku)); err != nil {
return fmt.Errorf("Error setting `sku`: %+v", err)
if skuName == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As mentioned in other PR, we can just set both and simplify this logic

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I now set both and removed the logic...

// Remove in 2.0
if sku := resp.Sku; sku != nil {
if err := d.Set("sku", flattenApiManagementServiceSku(resp.Sku)); err != nil {
return fmt.Errorf("Error setting `sku`: %+v", err)
}
}
d.Set("sku_name", "")
} else {
if sku := resp.Sku; sku != nil {
if err := d.Set("sku_name", flattenApiManagementServiceSkuName(resp.Sku)); err != nil {
return fmt.Errorf("Error setting `sku_name`: %+v", err)
}
}
d.Set("sku", "")
}

if err := d.Set("sign_in", flattenApiManagementSignInSettings(signInSettings)); err != nil {
Expand Down Expand Up @@ -850,21 +886,49 @@ func flattenAzureRmApiManagementMachineIdentity(identity *apimanagement.ServiceI
}

func expandAzureRmApiManagementSku(d *schema.ResourceData) *apimanagement.ServiceSkuProperties {
katbyte marked this conversation as resolved.
Show resolved Hide resolved
vs := d.Get("sku").([]interface{})
// guaranteed by MinItems in the schema
v := vs[0].(map[string]interface{})
var name string
var capacity int32
var err error

name := apimanagement.SkuType(v["name"].(string))
capacity := int32(v["capacity"].(int))
if s := d.Get("sku_name").(string); s != "" {
name, capacity, err = azure.SplitSku(s)

if err != nil {
return nil
}
} else {
// Remove in 2.0 timeframe
vs := d.Get("sku").([]interface{})

if len(vs) == 0 {
return nil
}

// guaranteed by MinItems in the schema
v := vs[0].(map[string]interface{})

name = v["name"].(string)
capacity = int32(v["capacity"].(int))
}

sku := &apimanagement.ServiceSkuProperties{
Name: name,
Name: apimanagement.SkuType(name),
Capacity: utils.Int32(capacity),
}

return sku
}

func flattenApiManagementServiceSkuName(input *apimanagement.ServiceSkuProperties) string {
if input == nil {
return ""
}

sku := fmt.Sprintf("%s_%d", string(input.Name), *input.Capacity)

return sku
}

func flattenApiManagementServiceSku(input *apimanagement.ServiceSkuProperties) []interface{} {
if input == nil {
return []interface{}{}
Expand Down
Loading