-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
New Resource: resource_arm_servicebus_subscription_rule
#1124
Changes from 1 commit
39fd30f
4795beb
4511fe1
e76dbc7
65107b9
6138226
d79e461
f38c500
40fd30f
1f834d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,354 @@ | ||
package azurerm | ||
|
||
import ( | ||
"fmt" | ||
"log" | ||
|
||
"github.com/Azure/azure-sdk-for-go/services/servicebus/mgmt/2017-04-01/servicebus" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
"github.com/hashicorp/terraform/helper/validation" | ||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" | ||
) | ||
|
||
func resourceArmServiceBusRule() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceArmServiceBusRuleCreate, | ||
Read: resourceArmServiceBusRuleRead, | ||
Update: resourceArmServiceBusRuleCreate, | ||
Delete: resourceArmServiceBusRuleDelete, | ||
Importer: &schema.ResourceImporter{ | ||
State: schema.ImportStatePassthrough, | ||
}, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"namespace_name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"topic_name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"subscription_name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"filter_type": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ValidateFunc: validation.StringInSlice([]string{ | ||
string(servicebus.FilterTypeSQLFilter), | ||
string(servicebus.FilterTypeCorrelationFilter), | ||
}, true), | ||
DiffSuppressFunc: ignoreCaseDiffSuppressFunc, | ||
}, | ||
|
||
"location": deprecatedLocationSchema(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove this, since it's not being used? |
||
|
||
"resource_group_name": resourceGroupNameSchema(), | ||
|
||
"sql_filter": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
|
||
"correlation_filter": { | ||
Type: schema.TypeList, | ||
Optional: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from what I can see from the schema below, there can only be one of these, can we add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can also add |
||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"correlation_id": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"message_id": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"to": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"reply_to": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"label": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"session_id": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"reply_to_session_id": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
"content_type": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
}, | ||
}, | ||
}, | ||
|
||
"action": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func resourceArmServiceBusRuleCreate(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).serviceBusRulesClient | ||
ctx := meta.(*ArmClient).StopContext | ||
log.Printf("[INFO] preparing arguments for Azure ARM ServiceBus Rule creation.") | ||
|
||
name := d.Get("name").(string) | ||
topicName := d.Get("topic_name").(string) | ||
subscriptionName := d.Get("subscription_name").(string) | ||
namespaceName := d.Get("namespace_name").(string) | ||
resourceGroup := d.Get("resource_group_name").(string) | ||
filterType := d.Get("filter_type").(string) | ||
|
||
ruleProperties := servicebus.Ruleproperties{ | ||
FilterType: servicebus.FilterType(filterType), | ||
Action: getAzureRmServiceBusAction(d), | ||
SQLFilter: getAzureRmServiceBusSQLFilter(d), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given these two methods only extract a single field - I think it'd be simpler to just in-line these below? e.g.
that said - looking at the model in the SDK, I think it'd make sense for this to be in a sub-object like
|
||
CorrelationFilter: getAzureRmServiceBusCorrelationFilter(d), | ||
} | ||
|
||
rule := servicebus.Rule{ | ||
Ruleproperties: &ruleProperties, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: we can actually just inline these two objects e.g.
|
||
} | ||
|
||
err := validateArmServiceBusRule(name, rule) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
_, err = client.CreateOrUpdate(ctx, resourceGroup, namespaceName, topicName, subscriptionName, name, rule) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
read, err := client.Get(ctx, resourceGroup, namespaceName, topicName, subscriptionName, name) | ||
if err != nil { | ||
return err | ||
} | ||
if read.ID == nil { | ||
return fmt.Errorf("Cannot read ServiceBus Rule %s (resource group %s) ID", name, resourceGroup) | ||
} | ||
|
||
d.SetId(*read.ID) | ||
|
||
return resourceArmServiceBusRuleRead(d, meta) | ||
} | ||
|
||
func resourceArmServiceBusRuleRead(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).serviceBusRulesClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
resourceGroup := id.ResourceGroup | ||
namespaceName := id.Path["namespaces"] | ||
topicName := id.Path["topics"] | ||
subscriptionName := id.Path["subscriptions"] | ||
name := id.Path["rules"] | ||
|
||
resp, err := client.Get(ctx, resourceGroup, namespaceName, topicName, subscriptionName, name) | ||
if err != nil { | ||
if utils.ResponseWasNotFound(resp.Response) { | ||
d.SetId("") | ||
return nil | ||
} | ||
return fmt.Errorf("Error making Read request on Azure ServiceBus Rule %q (Resource Group %q): %+v", name, resourceGroup, err) | ||
} | ||
|
||
d.Set("name", resp.Name) | ||
d.Set("resource_group_name", resourceGroup) | ||
d.Set("namespace_name", namespaceName) | ||
d.Set("topic_name", topicName) | ||
d.Set("subscription_name", subscriptionName) | ||
|
||
if properties := resp.Ruleproperties; properties != nil { | ||
d.Set("filter_type", properties.FilterType) | ||
|
||
if properties.Action != nil { | ||
d.Set("action", properties.Action.SQLExpression) | ||
} | ||
|
||
if properties.SQLFilter != nil { | ||
d.Set("sql_filter", properties.SQLFilter.SQLExpression) | ||
} | ||
|
||
if properties.CorrelationFilter != nil { | ||
if err := d.Set("correlation_filter", flattenAzureRmServiceBusCorrelationFilter(properties.CorrelationFilter)); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we update this to always set the
|
||
return fmt.Errorf("Error setting `correlation_filter` on Azure ServiceVus Rule (%q): %+v", name, err) | ||
} | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func resourceArmServiceBusRuleDelete(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).serviceBusRulesClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
resourceGroup := id.ResourceGroup | ||
namespaceName := id.Path["namespaces"] | ||
topicName := id.Path["topics"] | ||
subscriptionName := id.Path["subscriptions"] | ||
name := id.Path["rules"] | ||
|
||
_, err = client.Delete(ctx, resourceGroup, namespaceName, topicName, subscriptionName, name) | ||
|
||
return err | ||
} | ||
|
||
func getAzureRmServiceBusAction(d *schema.ResourceData) *servicebus.Action { | ||
if action := d.Get("action").(string); action != "" { | ||
serviceBusAction := servicebus.Action{ | ||
SQLExpression: &action, | ||
} | ||
return &serviceBusAction | ||
} | ||
return nil | ||
} | ||
|
||
func getAzureRmServiceBusSQLFilter(d *schema.ResourceData) *servicebus.SQLFilter { | ||
if sqlFilter := d.Get("sql_filter").(string); sqlFilter != "" { | ||
serviceBusSQLFilter := servicebus.SQLFilter{ | ||
SQLExpression: &sqlFilter, | ||
} | ||
return &serviceBusSQLFilter | ||
} | ||
return nil | ||
} | ||
|
||
func getAzureRmServiceBusCorrelationFilter(d *schema.ResourceData) *servicebus.CorrelationFilter { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we rename this |
||
if config := d.Get("correlation_filter").([]interface{}); len(config) > 0 { | ||
filter := config[0].(map[string]interface{}) | ||
correlationFilter := servicebus.CorrelationFilter{} | ||
|
||
if correlationID := filter["correlation_id"].(string); correlationID != "" { | ||
correlationFilter.CorrelationID = &correlationID | ||
} | ||
|
||
if messageID := filter["message_id"].(string); messageID != "" { | ||
correlationFilter.MessageID = &messageID | ||
} | ||
|
||
if to := filter["to"].(string); to != "" { | ||
correlationFilter.To = &to | ||
} | ||
|
||
if replyTo := filter["reply_to"].(string); replyTo != "" { | ||
correlationFilter.ReplyTo = &replyTo | ||
} | ||
|
||
if label := filter["label"].(string); label != "" { | ||
correlationFilter.Label = &label | ||
} | ||
|
||
if sessionID := filter["session_id"].(string); sessionID != "" { | ||
correlationFilter.SessionID = &sessionID | ||
} | ||
|
||
if replyToSessionID := filter["reply_to_session_id"].(string); replyToSessionID != "" { | ||
correlationFilter.ReplyToSessionID = &replyToSessionID | ||
} | ||
|
||
if contentType := filter["content_type"].(string); contentType != "" { | ||
correlationFilter.ContentType = &contentType | ||
} | ||
|
||
return &correlationFilter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. based on this comment: https://github.com/terraform-providers/terraform-provider-azurerm/pull/1124/files#diff-5ae250cddcb7acd8b53f87d1465954bfR168 - is it worth raising an error if no properties have been set? |
||
} | ||
return nil | ||
} | ||
|
||
func flattenAzureRmServiceBusCorrelationFilter(f *servicebus.CorrelationFilter) []interface{} { | ||
filters := make([]interface{}, 0, 1) | ||
filter := make(map[string]interface{}) | ||
|
||
if f.CorrelationID != nil { | ||
filter["correlation_id"] = *f.CorrelationID | ||
} | ||
|
||
if f.MessageID != nil { | ||
filter["message_id"] = *f.MessageID | ||
} | ||
|
||
if f.To != nil { | ||
filter["to"] = *f.To | ||
} | ||
|
||
if f.ReplyTo != nil { | ||
filter["reply_to"] = *f.ReplyTo | ||
} | ||
|
||
if f.Label != nil { | ||
filter["label"] = *f.Label | ||
} | ||
|
||
if f.SessionID != nil { | ||
filter["session_id"] = *f.SessionID | ||
} | ||
|
||
if f.ReplyToSessionID != nil { | ||
filter["reply_to_session_id"] = *f.ReplyToSessionID | ||
} | ||
|
||
if f.ContentType != nil { | ||
filter["content_type"] = *f.ContentType | ||
} | ||
|
||
filters = append(filters, filter) | ||
return filters | ||
} | ||
|
||
func validateArmServiceBusRule(name string, rule servicebus.Rule) error { | ||
if rule.Ruleproperties.FilterType == servicebus.FilterTypeSQLFilter { | ||
if rule.Ruleproperties.SQLFilter == nil { | ||
return fmt.Errorf("Cannot create ServiceBus Rule (%s). 'sql_filter' must be specified when 'filter_type' is set to 'SqlFilter'", name) | ||
} | ||
if rule.Ruleproperties.CorrelationFilter != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we use the |
||
return fmt.Errorf("ServiceBus Rule (%s) does not support `correlation_filter` when 'filter_type' is set to 'SqlFilter'", name) | ||
} | ||
} | ||
|
||
if rule.Ruleproperties.FilterType == servicebus.FilterTypeCorrelationFilter { | ||
if rule.Ruleproperties.CorrelationFilter == nil { | ||
return fmt.Errorf("Cannot create ServiceBus Rule (%s). 'correlation_filter' must be specified when 'filter_type' is set to 'CorrelationFilter'", name) | ||
} | ||
if rule.Ruleproperties.SQLFilter != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we use the |
||
return fmt.Errorf("ServiceBus Rule (%s) does not support `sql_filter` when 'filter_type' is set to 'CorrelationFilter'", name) | ||
} | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from what I can tell, these are known as ServiceBus Subscription Rules - as such would it make sense for this to be
azurerm_servicebus_subscription_rule
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Tom
I did consider this but in both the Service Bus documentation and in the go SDK they are known simply as 'rules' and i wanted to keep consistency with this terraform module.
https://docs.microsoft.com/en-us/rest/api/servicebus/rules/createorupdate
https://github.com/Azure/azure-sdk-for-go/blob/56332fec5b308fbb6615fa1af6117394cdba186d/services/servicebus/mgmt/2017-04-01/servicebus/rules.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the Documentation and Go SDK are actually both generated from the same Swagger file - so the Azure Portal tends to be a more reliable source of truth. Given we've already got support for ServiceBus Topic Authorization Rules, I think this would be best as
Subscription Rule
(given it's scoped to that level) in-case a Namespace-wide rule is added in the future - WDYT?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sounds good to me. Will get this sorted and updated soon