-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Adding Operational Insight Workspace (a.k.a Log Analytics) #331
Changes from 2 commits
78cca50
db9621c
357bc06
ae309e0
d5ebef8
75dc27c
1fe74ea
e86a8a2
c3408b2
2f74b8e
4d4678f
011e38f
c821a98
577502d
3e8c7b5
a578ba2
0cd140a
ea0692e
87b6e9a
9b64611
7c6ffaa
a260e4f
204c2fb
68bec07
01fa92d
5ced761
8e33ee6
57b177f
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,56 @@ | ||
package azurerm | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform/helper/acctest" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
) | ||
|
||
func TestAccAzureRMOperationalInsightWorkspace_importrequiredOnly(t *testing.T) { | ||
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'd tend to name this 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. Done! |
||
resourceName := "azurerm_operational_insight_workspace.test" | ||
|
||
ri := acctest.RandInt() | ||
config := testAccAzureRMOperationalInsightWorkspace_requiredOnly(ri, testLocation()) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testCheckAzureRMOperationalInsightWorkspaceDestroy, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: config, | ||
}, | ||
|
||
{ | ||
ResourceName: resourceName, | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func TestAccAzureRMOperationalInsightWorkspace_importoptional(t *testing.T) { | ||
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'd tend to name this 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. Done |
||
resourceName := "azurerm_operational_insight_workspace.test" | ||
|
||
ri := acctest.RandInt() | ||
config := testAccAzureRMOperationalInsightWorkspace_optional(ri, testLocation()) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testCheckAzureRMOperationalInsightWorkspaceDestroy, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: config, | ||
}, | ||
|
||
{ | ||
ResourceName: resourceName, | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
}, | ||
}, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,11 +104,12 @@ func Provider() terraform.ResourceProvider { | |
"azurerm_lb_rule": resourceArmLoadBalancerRule(), | ||
"azurerm_local_network_gateway": resourceArmLocalNetworkGateway(), | ||
|
||
"azurerm_managed_disk": resourceArmManagedDisk(), | ||
"azurerm_network_interface": resourceArmNetworkInterface(), | ||
"azurerm_network_security_group": resourceArmNetworkSecurityGroup(), | ||
"azurerm_network_security_rule": resourceArmNetworkSecurityRule(), | ||
"azurerm_public_ip": resourceArmPublicIp(), | ||
"azurerm_managed_disk": resourceArmManagedDisk(), | ||
"azurerm_network_interface": resourceArmNetworkInterface(), | ||
"azurerm_network_security_group": resourceArmNetworkSecurityGroup(), | ||
"azurerm_network_security_rule": resourceArmNetworkSecurityRule(), | ||
"azurerm_operational_insight_workspace": resourceArmOperationalInsightWorkspaceService(), | ||
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'm a little confused around the naming of this resource, is the latest product name for this "Operational Insights" or "Log Analytics"? 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 also confused about that. I discuss with the production team. In conclusion, they'd love to go "Log Analytics" for terraform.tf. However, SDK and REST-API is "Operational Insights". I edit my code to fit to it. |
||
"azurerm_public_ip": resourceArmPublicIp(), | ||
|
||
"azurerm_redis_cache": resourceArmRedisCache(), | ||
"azurerm_route": resourceArmRoute(), | ||
|
@@ -254,21 +255,22 @@ func registerAzureResourceProvidersWithSubscription(providerList []resources.Pro | |
var err error | ||
providerRegistrationOnce.Do(func() { | ||
providers := map[string]struct{}{ | ||
"Microsoft.Cache": struct{}{}, | ||
"Microsoft.Cdn": struct{}{}, | ||
"Microsoft.Compute": struct{}{}, | ||
"Microsoft.ContainerRegistry": struct{}{}, | ||
"Microsoft.ContainerService": struct{}{}, | ||
"Microsoft.DocumentDB": struct{}{}, | ||
"Microsoft.EventHub": struct{}{}, | ||
"Microsoft.KeyVault": struct{}{}, | ||
"Microsoft.Insights": struct{}{}, | ||
"Microsoft.Network": struct{}{}, | ||
"Microsoft.Resources": struct{}{}, | ||
"Microsoft.Search": struct{}{}, | ||
"Microsoft.ServiceBus": struct{}{}, | ||
"Microsoft.Sql": struct{}{}, | ||
"Microsoft.Storage": struct{}{}, | ||
"Microsoft.Cache": struct{}{}, | ||
"Microsoft.Cdn": struct{}{}, | ||
"Microsoft.Compute": struct{}{}, | ||
"Microsoft.ContainerRegistry": struct{}{}, | ||
"Microsoft.ContainerService": struct{}{}, | ||
"Microsoft.DocumentDB": struct{}{}, | ||
"Microsoft.EventHub": struct{}{}, | ||
"Microsoft.KeyVault": struct{}{}, | ||
"Microsoft.Insights": struct{}{}, | ||
"Microsoft.Network": struct{}{}, | ||
"Microsoft.OperationalInsights": struct{}{}, | ||
"Microsoft.Resources": struct{}{}, | ||
"Microsoft.Search": struct{}{}, | ||
"Microsoft.ServiceBus": struct{}{}, | ||
"Microsoft.Sql": struct{}{}, | ||
"Microsoft.Storage": struct{}{}, | ||
} | ||
|
||
// filter out any providers already registered | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,224 @@ | ||
package azurerm | ||
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. (as above) - could we rename this to 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. Done! 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. resource_arm_log_analytics_workspace.go instead. :) Done! |
||
|
||
import ( | ||
"fmt" | ||
"log" | ||
"net/http" | ||
"regexp" | ||
|
||
"github.com/Azure/azure-sdk-for-go/arm/operationalinsights" | ||
"github.com/hashicorp/terraform/helper/schema" | ||
) | ||
|
||
func resourceArmOperationalInsightWorkspaceService() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceArmOperationalInsightWorkspaceCreateUpdate, | ||
Read: resourceArmOperationalInsightWorkspaceRead, | ||
Update: resourceArmOperationalInsightWorkspaceCreateUpdate, | ||
Delete: resourceArmOperationalInsightWorkspaceDelete, | ||
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. as above - could we rename this to 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. Done |
||
Importer: &schema.ResourceImporter{ | ||
State: schema.ImportStatePassthrough, | ||
}, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
ValidateFunc: validateAzureRmOperationalInsightWorkspaceName, | ||
}, | ||
"location": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
StateFunc: azureRMNormalizeLocation, | ||
}, | ||
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 swap location over to using the locationSchema? i.e. 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. Done |
||
"resource_group_name": { | ||
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. as of earlier in the week there's now a 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. this'll also fix the following test failure:
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. Great! I did it. |
||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
"workspace_id": { // a.k.a. customer_id | ||
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'd tend to place this comment within the documentation e.g.
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'll just remove this from code and add it on the documentation. |
||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"portal_url": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"sku": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: 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. can we add some validation to this for the possible SKU types, i.e.
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. The
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. Oh! This is good one! I've done! |
||
}, | ||
"retention_in_days": { | ||
Type: schema.TypeInt, | ||
Optional: true, | ||
Computed: true, | ||
}, | ||
"primary_shared_key": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"secondary_shared_key": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
}, | ||
"tags": tagsSchema(), | ||
}, | ||
} | ||
} | ||
|
||
func resourceArmOperationalInsightWorkspaceCreateUpdate(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).workspacesClient | ||
log.Printf("[INFO] preparing arguments for AzureRM Operational Insight workspace creation.") | ||
|
||
name := d.Get("name").(string) | ||
location := d.Get("location").(string) | ||
resGroup := d.Get("resource_group_name").(string) | ||
|
||
skuName := d.Get("sku") | ||
sku, err := getSku(skuName) | ||
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. since we can just cast directly to the Enum, we can replace this with:
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. Done. Also I removed the getSku and related function. |
||
if err != nil { | ||
return err | ||
} | ||
|
||
retentionInDays := int32(d.Get("retention_in_days").(int)) | ||
|
||
tags := d.Get("tags").(map[string]interface{}) | ||
|
||
parameters := operationalinsights.Workspace{ | ||
Name: &name, | ||
Location: &location, | ||
Tags: expandTags(tags), | ||
WorkspaceProperties: &operationalinsights.WorkspaceProperties{ | ||
Sku: sku, | ||
RetentionInDays: &retentionInDays, | ||
}, | ||
} | ||
|
||
cancel := make(chan struct{}) | ||
workspaceChannel, error := client.CreateOrUpdate(resGroup, name, parameters, cancel) | ||
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. two things:
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've done. I'm new to go-lang. However, I don't like copy-and-paste programming without understanding the reason. Since I just couldn't understand why you don't use the return value of CreateOrUpdate. Could you tell me the reason for my learning? :) |
||
workspace := <-workspaceChannel | ||
err = <-error | ||
if err != nil { | ||
return err | ||
} | ||
// The cosmos DB read rest api again for getting id. Try remove it. | ||
// read, err := client.Get(resGroup, name) | ||
// if err != nil { | ||
// return err | ||
//} | ||
|
||
//if read.ID == nil { | ||
// return fmt.Errorf("Cannot read Operational Inight Workspace '%s' (resource group %s) ID", name, resGroup) | ||
//} | ||
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. (as above) - is there any reason why we're not re-requesting the resource here? 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. Also, I just forget to remove it. However, I follow your coding style, I mend it. |
||
d.SetId(*workspace.ID) | ||
|
||
return resourceArmOperationalInsightWorkspaceRead(d, meta) | ||
|
||
} | ||
|
||
func resourceArmOperationalInsightWorkspaceRead(d *schema.ResourceData, meta interface{}) error { | ||
// I don't understand why we can get the meta data. How the framework set it. | ||
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. this is set by the calls from Terraform Core - which injects the Provider as the 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. Thank you for the explanation! It help me to understand the behavior! Thanks. I removed the comment. |
||
client := meta.(*ArmClient).workspacesClient | ||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
resGroup := id.ResourceGroup | ||
name := id.Path["workspaces"] | ||
|
||
resp, err := client.Get(resGroup, name) | ||
if err != nil { | ||
if responseWasNotFound(resp.Response) { | ||
d.SetId("") | ||
return nil | ||
} | ||
return fmt.Errorf("Error making Read request on AzureRM Operational Insight workspaces '%s': %s", name, err) | ||
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 make this second formatting argument 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. Done! |
||
} | ||
|
||
d.Set("name", resp.Name) | ||
d.Set("location", resp.Location) | ||
d.Set("resource_group_name", resGroup) | ||
d.Set("workspace_id", resp.CustomerID) | ||
d.Set("portal_url", resp.PortalURL) | ||
d.Set("sku", resp.Sku.Name) | ||
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. could we add a check to ensure that the returned SKU object isn't nil here? (this can happen when the Swagger doesn't match the API response, when the SDK is upgraded)
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. Thanks, I didn't know that! Good info! 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. Done |
||
d.Set("retention_in_days", resp.RetentionInDays) | ||
|
||
sharedKeys, err := client.GetSharedKeys(resGroup, name) | ||
if err != nil { | ||
log.Printf("[ERROR] Unable to List Shared keys for Operatinal Insight workspaces %s: %s", name, err) | ||
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 the secondary formatting argument to 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. Done! |
||
} else { | ||
d.Set("primary_shared_key", sharedKeys.PrimarySharedKey) | ||
d.Set("secondary_shared_key", sharedKeys.SecondarySharedKey) | ||
} | ||
|
||
flattenAndSetTags(d, resp.Tags) | ||
return nil | ||
} | ||
|
||
func resourceArmOperationalInsightWorkspaceDelete(d *schema.ResourceData, meta interface{}) error { | ||
client := meta.(*ArmClient).workspacesClient | ||
|
||
id, err := parseAzureResourceID(d.Id()) | ||
if err != nil { | ||
return err | ||
} | ||
resGroup := id.ResourceGroup | ||
name := id.Path["workspaces"] | ||
|
||
resp, err := client.Delete(resGroup, name) | ||
|
||
if err != nil { | ||
if resp.StatusCode == http.StatusNotFound { | ||
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've got a helper method which also handles connection drops available in 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. Done! |
||
return nil | ||
} | ||
|
||
return fmt.Errorf("Error issuing AzureRM delete request for Operational Insight Workspaces '%s': %+v", name, err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func getSku(skuName interface{}) (*operationalinsights.Sku, error) { | ||
if skuName == nil { | ||
return nil, nil | ||
} | ||
skuEnum, err := getSkuNameEnum(skuName.(string)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return &operationalinsights.Sku{ | ||
Name: skuEnum, | ||
}, nil | ||
} | ||
|
||
func getSkuNameEnum(skuName string) (operationalinsights.SkuNameEnum, error) { | ||
switch skuName { | ||
case "Free": | ||
return operationalinsights.Free, nil | ||
case "PerNode": | ||
return operationalinsights.PerNode, nil | ||
case "Premium": | ||
return operationalinsights.Premium, nil | ||
case "Standalone": | ||
return operationalinsights.Standalone, nil | ||
case "Standard": | ||
return operationalinsights.Standard, nil | ||
case "Unlimited": | ||
return operationalinsights.Unlimited, nil | ||
default: | ||
return operationalinsights.Free, fmt.Errorf("Sku name not found") | ||
} | ||
} | ||
|
||
func validateAzureRmOperationalInsightWorkspaceName(v interface{}, k string) (ws []string, errors []error) { | ||
value := v.(string) | ||
|
||
r, _ := regexp.Compile("^[A-Za-z0-9][A-Za-z0-9-]+[A-Za-z0-9]$") | ||
if !r.MatchString(value) { | ||
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 can't see a max length listed in the API Documentation - is there one in the Portal we should add checks for? 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. Name requirements are shown in the azure portal as a tooltip.
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. Thank you @piotrgo ! I added the validation with test. |
||
errors = append(errors, fmt.Errorf("Workspace Name can only contain alphabet, number, and '-' charactor. You can not use '-' as the start and end of the name.")) | ||
} | ||
return | ||
} |
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.
given the resource has been renamed - can we rename this file to
import_arm_log_analytics_workspace_test.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.
Done!