Merge pull request #9 from gessnerfl/feature/8_user_friendly_severity

Close #8: integer value replaced by strings (critical and warning) in terraform provider

README.md

@@ -79,7 +79,7 @@ The ID of the resource which is also used as unique identifier in Instana is aut
 resource "instana_rule_binding" "example" {
   enabled = true
   triggering = true
-  severity = 5
+  severity = warning
   text = "text"
   description = "description"
   expiration_time = 60000

instana/commons_test.go

@@ -83,14 +83,14 @@ func validateRequiredSchemaOfTypeListOfString(schemaField string, schemaMap map[
 	if s.Elem == nil {
 		t.Fatal("Expected list element to be defined")
 	}
-	if s.Elem.(schema.Schema).Type != schema.TypeString {
+	if s.Elem.(*schema.Schema).Type != schema.TypeString {
 		t.Fatal("Expected list element to be of type string")
 	}
 	if len(s.Description) == 0 {
 		t.Fatal("Expected description for schema")
 	}
-	if s.Required {
-		t.Fatalf("Expected %s to be optional", schemaField)
+	if !s.Required {
+		t.Fatalf("Expected %s to be required", schemaField)
 	}
 }
 

instana/provider.go

@@ -20,6 +20,24 @@ const ResourceInstanaRuleBinding = "instana_rule_binding"
 //ResourceInstanaUserRole the name of the terraform-provider-instana resource to manage user roles
 const ResourceInstanaUserRole = "instana_user_role"
 
+//Severity representation of the severity in both worlds Instana API and Terraform Provider
+type Severity struct {
+	apiRepresentation       int
+	terraformRepresentation string
+}
+
+//GetAPIRepresentation returns the integer representation of the Instana API
+func (s Severity) GetAPIRepresentation() int { return s.apiRepresentation }
+
+//GetTerraformRepresentation returns the string representation of the Terraform Provider
+func (s Severity) GetTerraformRepresentation() string { return s.terraformRepresentation }
+
+//SeverityCritical representation of the critical severity
+var SeverityCritical = Severity{apiRepresentation: 10, terraformRepresentation: "critical"}
+
+//SeverityWarning representation of the warning severity
+var SeverityWarning = Severity{apiRepresentation: 5, terraformRepresentation: "warning"}
+
 //Provider interface implementation of hashicorp terraform provider
 func Provider() *schema.Provider {
 	return &schema.Provider{

instana/resource-rule-binding.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/gessnerfl/terraform-provider-instana/instana/restapi"
 	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/hashicorp/terraform/helper/validation"
 )
 
 //RuleBindingFieldEnabled constant value for the schema field enabled
@@ -41,42 +42,51 @@ func CreateResourceRuleBinding() *schema.Resource {
 
 		Schema: map[string]*schema.Schema{
 			RuleBindingFieldEnabled: &schema.Schema{
-				Type:     schema.TypeBool,
-				Default:  true,
-				Optional: true,
+				Type:        schema.TypeBool,
+				Default:     true,
+				Optional:    true,
+				Description: "Configures if the rule binding is enabled or not",
 			},
 			RuleBindingFieldTriggering: &schema.Schema{
-				Type:     schema.TypeBool,
-				Default:  false,
-				Optional: true,
+				Type:        schema.TypeBool,
+				Default:     false,
+				Optional:    true,
+				Description: "Configures the issue should trigger an incident",
 			},
 			RuleBindingFieldSeverity: &schema.Schema{
-				Type:     schema.TypeInt,
-				Required: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ValidateFunc: validation.StringInSlice([]string{SeverityWarning.terraformRepresentation, SeverityCritical.terraformRepresentation}, false),
+				Description:  "Configures the severity of the issue",
 			},
 			RuleBindingFieldText: &schema.Schema{
-				Type:     schema.TypeString,
-				Required: true,
+				Type:        schema.TypeString,
+				Required:    true,
+				Description: "Configures the title of the rule binding",
 			},
 			RuleBindingFieldDescription: &schema.Schema{
-				Type:     schema.TypeString,
-				Required: true,
+				Type:        schema.TypeString,
+				Required:    true,
+				Description: "Configures the description text of the rule binding",
 			},
 			RuleBindingFieldExpirationTime: &schema.Schema{
-				Type:     schema.TypeInt,
-				Required: true,
+				Type:        schema.TypeInt,
+				Required:    true,
+				Description: "Configures the expiration time (grace period) to wait before the issue is closed",
 			},
 			RuleBindingFieldQuery: &schema.Schema{
-				Type:     schema.TypeString,
-				Required: false,
-				Optional: true,
+				Type:        schema.TypeString,
+				Required:    false,
+				Optional:    true,
+				Description: "Configures the dynamic focus query for the rule binding",
 			},
 			RuleBindingFieldRuleIds: &schema.Schema{
 				Type:     schema.TypeList,
 				Required: true,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
 				},
+				Description: "Configures the list of rule ids which should be considered by the rule binding",
 			},
 		},
 	}
@@ -97,8 +107,7 @@ func ReadRuleBinding(d *schema.ResourceData, meta interface{}) error {
 		}
 		return err
 	}
-	updateRuleBindingState(d, ruleBinding)
-	return nil
+	return updateRuleBindingState(d, ruleBinding)
 }
 
 //CreateRuleBinding creates the configured rule binding through the Instana API and updates the resource state.
@@ -110,50 +119,66 @@ func CreateRuleBinding(d *schema.ResourceData, meta interface{}) error {
 //UpdateRuleBinding updates the configured rule binding through the Instana API and updates the resource state.
 func UpdateRuleBinding(d *schema.ResourceData, meta interface{}) error {
 	instanaAPI := meta.(restapi.InstanaAPI)
-	ruleBinding := createRuleBindingFromResourceData(d)
+	ruleBinding, err := createRuleBindingFromResourceData(d)
+	if err != nil {
+		return err
+	}
 	updatedRuleBinding, err := instanaAPI.RuleBindings().Upsert(ruleBinding)
 	if err != nil {
 		return err
 	}
-	updateRuleBindingState(d, updatedRuleBinding)
-	return nil
+	return updateRuleBindingState(d, updatedRuleBinding)
 }
 
 //DeleteRuleBinding deletes the configured rule binding through the Instana API and deletes the resource state.
 func DeleteRuleBinding(d *schema.ResourceData, meta interface{}) error {
 	instanaAPI := meta.(restapi.InstanaAPI)
-	ruleBinding := createRuleBindingFromResourceData(d)
-	err := instanaAPI.RuleBindings().DeleteByID(ruleBinding.ID)
+	ruleBinding, err := createRuleBindingFromResourceData(d)
+	if err != nil {
+		return err
+	}
+	err = instanaAPI.RuleBindings().DeleteByID(ruleBinding.ID)
 	if err != nil {
 		return err
 	}
 	d.SetId("")
 	return nil
 }
 
-func createRuleBindingFromResourceData(d *schema.ResourceData) restapi.RuleBinding {
+func createRuleBindingFromResourceData(d *schema.ResourceData) (restapi.RuleBinding, error) {
+	severity, err := ConvertSeverityFromTerraformToInstanaAPIRepresentation(d.Get(RuleBindingFieldSeverity).(string))
+	if err != nil {
+		return restapi.RuleBinding{}, err
+	}
+
 	return restapi.RuleBinding{
 		ID:             d.Id(),
 		Enabled:        d.Get(RuleBindingFieldEnabled).(bool),
 		Triggering:     d.Get(RuleBindingFieldTriggering).(bool),
-		Severity:       d.Get(RuleBindingFieldSeverity).(int),
+		Severity:       severity,
 		Text:           d.Get(RuleBindingFieldText).(string),
 		Description:    d.Get(RuleBindingFieldDescription).(string),
 		ExpirationTime: d.Get(RuleBindingFieldExpirationTime).(int),
 		Query:          d.Get(RuleBindingFieldQuery).(string),
 		RuleIds:        ReadStringArrayParameterFromResource(d, RuleBindingFieldRuleIds),
-	}
+	}, nil
 }
 
-func updateRuleBindingState(d *schema.ResourceData, ruleBinding restapi.RuleBinding) {
+func updateRuleBindingState(d *schema.ResourceData, ruleBinding restapi.RuleBinding) error {
+	severity, err := ConvertSeverityFromInstanaAPIToTerraformRepresentation(ruleBinding.Severity)
+	if err != nil {
+		return err
+	}
+
 	d.Set(RuleBindingFieldEnabled, ruleBinding.Enabled)
 	d.Set(RuleBindingFieldTriggering, ruleBinding.Triggering)
-	d.Set(RuleBindingFieldSeverity, ruleBinding.Severity)
+	d.Set(RuleBindingFieldSeverity, severity)
 	d.Set(RuleBindingFieldText, ruleBinding.Text)
 	d.Set(RuleBindingFieldDescription, ruleBinding.Description)
 	d.Set(RuleBindingFieldExpirationTime, ruleBinding.ExpirationTime)
 	d.Set(RuleBindingFieldQuery, ruleBinding.Query)
 	d.Set(RuleBindingFieldRuleIds, ruleBinding.RuleIds)
 
 	d.SetId(ruleBinding.ID)
+	return nil
 }

instana/resource-rule-binding_test.go

@@ -27,13 +27,13 @@ var testRuleBindingProviders = map[string]terraform.ResourceProvider{
 const resourceRuleBindingDefinitionTemplate = `
 provider "instana" {
   api_token = "test-token"
-  endpoint = "localhost:{{PORT}}"
+  endpoint = "localhost:{{port}}"
 }
 
 resource "instana_rule_binding" "example" {
   enabled = true
   triggering = true
-  severity = 5
+  severity = "{{severity}}"
   text = "text"
   description = "description"
   expiration_time = 60000
@@ -45,34 +45,42 @@ resource "instana_rule_binding" "example" {
 const ruleBindingApiPath = restapi.RuleBindingsResourcePath + "/{id}"
 const testRuleBindingDefinition = "instana_rule_binding.example"
 
-func TestCRUDOfRuleBindingResourceWithMockServer(t *testing.T) {
+func TestCRUDOfRuleBindingResourceOfSeverityCriticalWithMockServer(t *testing.T) {
+	testCRUDOfRuleBindingResourceWithMockServer(SeverityCritical, t)
+}
+
+func TestCRUDOfRuleBindingResourceOfSeverityWarningWithMockServer(t *testing.T) {
+	testCRUDOfRuleBindingResourceWithMockServer(SeverityWarning, t)
+}
+
+func testCRUDOfRuleBindingResourceWithMockServer(severity Severity, t *testing.T) {
 	testutils.DeactivateTLSServerCertificateVerification()
 	httpServer := testutils.NewTestHTTPServer()
 	httpServer.AddRoute(http.MethodPut, ruleBindingApiPath, testutils.EchoHandlerFunc)
 	httpServer.AddRoute(http.MethodDelete, ruleBindingApiPath, testutils.EchoHandlerFunc)
 	httpServer.AddRoute(http.MethodGet, ruleBindingApiPath, func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
-		json := strings.ReplaceAll(`
+		json := strings.ReplaceAll(strings.ReplaceAll(`
 		{
 			"id" : "{{id}}",
 			"enabled" : true,
 			"triggering" : true,
-			"severity" : 5,
+			"severity" : {{severity}},
 			"text" : "text",
 			"description" : "description",
 			"expirationTime" : 60000,
 			"query" : "query",
 			"ruleIds" : [ "rule-id-1", "rule-id-2" ]
 		}
-		`, "{{id}}", vars["id"])
+		`, "{{id}}", vars["id"]), "{{severity}}", strconv.Itoa(severity.GetAPIRepresentation()))
 		w.Header().Set("Content-Type", r.Header.Get("Content-Type"))
 		w.WriteHeader(http.StatusOK)
 		w.Write([]byte(json))
 	})
 	httpServer.Start()
 	defer httpServer.Close()
 
-	resourceRuleBindingDefinition := strings.ReplaceAll(resourceRuleBindingDefinitionTemplate, "{{PORT}}", strconv.Itoa(httpServer.GetPort()))
+	resourceRuleBindingDefinition := strings.ReplaceAll(strings.ReplaceAll(resourceRuleBindingDefinitionTemplate, "{{port}}", strconv.Itoa(httpServer.GetPort())), "{{severity}}", severity.GetTerraformRepresentation())
 
 	resource.UnitTest(t, resource.TestCase{
 		Providers: testRuleBindingProviders,
@@ -83,7 +91,7 @@ func TestCRUDOfRuleBindingResourceWithMockServer(t *testing.T) {
 					resource.TestCheckResourceAttrSet(testRuleBindingDefinition, "id"),
 					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldEnabled, "true"),
 					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldTriggering, "true"),
-					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldSeverity, "5"),
+					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldSeverity, severity.GetTerraformRepresentation()),
 					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldText, "text"),
 					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldDescription, "description"),
 					resource.TestCheckResourceAttr(testRuleBindingDefinition, RuleBindingFieldExpirationTime, "60000"),
@@ -97,9 +105,9 @@ func TestCRUDOfRuleBindingResourceWithMockServer(t *testing.T) {
 }
 
 func TestResourceRuleBindingDefinition(t *testing.T) {
-	resource := CreateResourceRule()
+	resource := CreateResourceRuleBinding()
 
-	validateRuleResourceSchema(resource.Schema, t)
+	validateRuleBindingResourceSchema(resource.Schema, t)
 
 	if resource.Create == nil {
 		t.Fatal("Create function expected")
@@ -118,7 +126,7 @@ func TestResourceRuleBindingDefinition(t *testing.T) {
 func validateRuleBindingResourceSchema(schemaMap map[string]*schema.Schema, t *testing.T) {
 	validateSchemaOfTypeBoolWithDefault(RuleBindingFieldEnabled, true, schemaMap, t)
 	validateSchemaOfTypeBoolWithDefault(RuleBindingFieldTriggering, false, schemaMap, t)
-	validateOptionalSchemaOfTypeInt(RuleBindingFieldSeverity, schemaMap, t)
+	validateRequiredSchemaOfTypeString(RuleBindingFieldSeverity, schemaMap, t)
 	validateRequiredSchemaOfTypeString(RuleBindingFieldText, schemaMap, t)
 	validateRequiredSchemaOfTypeString(RuleBindingFieldDescription, schemaMap, t)
 	validateRequiredSchemaOfTypeInt(RuleBindingFieldExpirationTime, schemaMap, t)
@@ -176,6 +184,28 @@ func TestShouldFailToReadRuleBindingFromInstanaAPIWhenIDIsMissing(t *testing.T)
 	}
 }
 
+func TestShouldFailToReadRuleBindingFromInstanaAPIWhenSeverityCannotBeMapped(t *testing.T) {
+	modelData := createFullTestRuleBindingModel()
+	modelData.Severity = 1
+	resourceData := createEmptyRuleBindingResourceData(t)
+	ruleBindingID := "rule-binding-id"
+	resourceData.SetId(ruleBindingID)
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockRuleBindingApi := mocks.NewMockRuleBindingResource(ctrl)
+	mockInstanaAPI := mocks.NewMockInstanaAPI(ctrl)
+
+	mockInstanaAPI.EXPECT().RuleBindings().Return(mockRuleBindingApi).Times(1)
+	mockRuleBindingApi.EXPECT().GetOne(gomock.Eq(ruleBindingID)).Return(modelData, nil).Times(1)
+
+	err := ReadRuleBinding(resourceData, mockInstanaAPI)
+
+	if err == nil || !strings.HasPrefix(err.Error(), "1 is not a valid severity") {
+		t.Fatal("Expected error to occur because of invalid severity")
+	}
+}
+
 func TestShouldFailToReadRuleBindingFromInstanaAPIAndDeleteResourceWhenBindingDoesNotExist(t *testing.T) {
 	resourceData := createEmptyRuleBindingResourceData(t)
 	ruleBindingID := "rule-binding-id"
@@ -264,6 +294,22 @@ func TestShouldReturnErrorWhenCreateRuleBindingFailsThroughInstanaAPI(t *testing
 	}
 }
 
+func TestShouldFailToCreateRuleBindingThroughInstanaAPIWhenSeverityCannotBeMappedToRepresentationOfInstanaAPI(t *testing.T) {
+	data := createFullTestRuleBindingData()
+	data[RuleBindingFieldSeverity] = "INVALID_SEVERITY"
+	resourceData := createRuleBindingResourceData(t, data)
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockInstanaAPI := mocks.NewMockInstanaAPI(ctrl)
+
+	err := CreateRuleBinding(resourceData, mockInstanaAPI)
+
+	if err == nil || !strings.HasPrefix(err.Error(), "INVALID_SEVERITY is not a valid severity") {
+		t.Fatal("Expected error to occur because of invalid severity")
+	}
+}
+
 func TestShouldDeleteRuleBindingThroughInstanaAPI(t *testing.T) {
 	id := "test-id"
 	data := createFullTestRuleBindingData()
@@ -323,7 +369,11 @@ func verifyRuleBindingModelAppliedToResource(model restapi.RuleBinding, resource
 	if model.Triggering != resourceData.Get(RuleBindingFieldTriggering).(bool) {
 		t.Fatal("Expected Triggering to be identical")
 	}
-	if model.Severity != resourceData.Get(RuleBindingFieldSeverity).(int) {
+	severity, err := ConvertSeverityFromInstanaAPIToTerraformRepresentation(model.Severity)
+	if err != nil {
+		t.Fatalf("Expected convertion of severity to be successful but got '%s'", err)
+	}
+	if severity != resourceData.Get(RuleBindingFieldSeverity).(string) {
 		t.Fatal("Expected Severity to be identical")
 	}
 	if model.Text != resourceData.Get(RuleBindingFieldText).(string) {
@@ -375,6 +425,7 @@ func createFullTestRuleBindingData() map[string]interface{} {
 	data[RuleBindingFieldText] = "text"
 	data[RuleBindingFieldDescription] = "description"
 	data[RuleBindingFieldExpirationTime] = 1234
+	data[RuleBindingFieldSeverity] = SeverityWarning.GetTerraformRepresentation()
 	data[RuleBindingFieldQuery] = "query"
 	data[RuleBindingFieldRuleIds] = []string{"test-rule-id-1", "test-rule-id-2"}
 	return data