Fix: Cron Trigger Plan-time Validation; Cron Trigger Updates; Cron Trigger Testing (#100)

## What

* Add Cron Trigger plan-time validation
* Fix Cron Trigger updating (force replacement of resource on any
change, because there is no update verb in the cron trigger API)
* Improve Cron Trigger Testing
* Misc: remove use of "Hermes" service name in Provider logs. While
accurate in the [API
docs](https://g.codefresh.io/api/#tag/Triggers-Events), this may cause
unnecessary confusion to users of the Terraform provider.

## Why

* Updating a Cron Trigger results in duplicates
* No plan-time validation on Cron Triggers

## Notes

* Closes #99 
* Closes #98 
* Adding a `minor` label because now we have plan-validation, which can
potentially break existing cron trigger configurations in Terraform)

Co-authored-by: Yonatan Koren <10080107+korenyoni@users.noreply.github.com>

Author: korenyoni

client/hermes_trigger.go

@@ -45,7 +45,7 @@ func (client *Client) GetHermesTriggerByEventAndPipeline(event string, pipeline
 		}
 	}
 	if hermesTrigger.Event == "" {
-		return nil, fmt.Errorf("No trigger found for event %s and pipeline %s", event, pipeline)
+		return nil, fmt.Errorf("no Trigger found for event: %s, pipeline: %s", event, pipeline)
 	}
 
 	return &hermesTrigger, nil
@@ -73,7 +73,7 @@ func (client *Client) DeleteHermesTriggerByEventAndPipeline(event string, pipeli
 	_, err := client.RequestAPI(&opts)
 
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to delete Trigger: \n%v", err)
 	}
 
 	return nil

client/hermes_trigger_event.go

@@ -22,7 +22,7 @@ func (client *Client) GetHermesTriggerEvent(event string) (*HermesTriggerEvent,
 
 	resp, err := client.RequestAPI(&opts)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to retrieve Trigger Event: \n%v", err)
 	}
 
 	var hermesTriggerEvent HermesTriggerEvent
@@ -49,6 +49,9 @@ func (client *Client) CreateHermesTriggerEvent(event *HermesTriggerEvent) (strin
 	}
 
 	resp, err := client.RequestAPI(&opts)
+	if err != nil {
+		return "", fmt.Errorf("failed to create Trigger Event: \n%v", err)
+	}
 
 	var eventString string
 	err = DecodeResponseInto(resp, &eventString)

client/team.go

@@ -90,7 +90,6 @@ func (client *Client) GetTeamByID(id string) (*Team, error) {
 	return nil, nil
 }
 
-//
 func ConvertToNewTeam(team *Team) *NewTeam {
 	var users []string
 

codefresh/resource_idp.go

@@ -13,7 +13,7 @@ func dataSourceIdps() *schema.Resource {
 	}
 }
 
-//IdpSchema -
+// IdpSchema -
 func IdpSchema() map[string]*schema.Schema {
 	return map[string]*schema.Schema{
 		"_id": {

codefresh/resource_pipeline_cron_trigger.go

@@ -1,12 +1,17 @@
 package codefresh
 
 import (
+	"context"
 	"fmt"
 	"regexp"
 	"strings"
 
 	cfClient "github.com/codefresh-io/terraform-provider-codefresh/client"
+	"github.com/hashicorp/go-cty/cty"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/robfig/cron"
 )
 
 func resourcePipelineCronTrigger() *schema.Resource {
@@ -20,7 +25,7 @@ func resourcePipelineCronTrigger() *schema.Resource {
 				idParts := strings.Split(d.Id(), ",")
 
 				if len(idParts) != 2 || idParts[0] == "" || idParts[1] == "" {
-					return nil, fmt.Errorf("Unexpected format of ID (%q), expected EVENT,PIPELINE_ID", d.Id())
+					return nil, fmt.Errorf("unexpected format of ID (%q), expected EVENT,PIPELINE_ID", d.Id())
 				}
 
 				event := idParts[0]
@@ -39,12 +44,55 @@ func resourcePipelineCronTrigger() *schema.Resource {
 			"expression": {
 				Type:     schema.TypeString,
 				Required: true,
+				ValidateDiagFunc: func(v interface{}, path cty.Path) (diags diag.Diagnostics) {
+					expression := v.(string)
+
+					// Cron expression requirements: 6 fields, with ability to use descriptors (e.g. @yearly)
+					parser := cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)
+					if _, err := parser.Parse(expression); err != nil {
+						diags = append(diags, diag.Diagnostic{
+							Severity: diag.Error,
+							Summary:  "Invalid cron expression.",
+							Detail:   fmt.Sprintf("The cron expression %q is invalid: %s", expression, err),
+						})
+					}
+
+					return
+				},
 			},
 			"message": {
 				Type:     schema.TypeString,
 				Required: true,
+				ValidateDiagFunc: func(v interface{}, path cty.Path) (diags diag.Diagnostics) {
+					message := v.(string)
+
+					// https://github.com/codefresh-io/hermes/blob/6d75b347cb8ff471ce970a766b2285788e5e19fe/pkg/backend/dev_compose_types.json#L226
+					re := regexp.MustCompile(`^[a-zA-Z0-9_+\s-#?.:]{2,128}$`)
+
+					if !re.MatchString(message) {
+						diags = append(diags, diag.Diagnostic{
+							Severity: diag.Error,
+							Summary:  "Invalid message.",
+							Detail:   fmt.Sprintf("The message %q is invalid (must match %q).", message, re.String()),
+						})
+					}
+
+					return
+				},
 			},
 		},
+		// Force new resource if any field changes. This is because the Codefresh API does not support updating cron triggers.
+		CustomizeDiff: customdiff.All(
+			customdiff.ForceNewIfChange("pipeline_id", func(ctx context.Context, old, new, meta interface{}) bool {
+				return true
+			}),
+			customdiff.ForceNewIfChange("expression", func(ctx context.Context, old, new, meta interface{}) bool {
+				return true
+			}),
+			customdiff.ForceNewIfChange("message", func(ctx context.Context, old, new, meta interface{}) bool {
+				return true
+			}),
+		),
 	}
 }
 
@@ -97,7 +145,8 @@ func resourcePipelineCronTriggerRead(d *schema.ResourceData, meta interface{}) e
 }
 
 func resourcePipelineCronTriggerUpdate(d *schema.ResourceData, meta interface{}) error {
-	return resourcePipelineCronTriggerCreate(d, meta)
+	// see notes in resourcePipelineCronTrigger()
+	return fmt.Errorf("cron triggers cannot be updated")
 }
 
 func resourcePipelineCronTriggerDelete(d *schema.ResourceData, meta interface{}) error {
@@ -107,7 +156,7 @@ func resourcePipelineCronTriggerDelete(d *schema.ResourceData, meta interface{})
 
 	err := client.DeleteHermesTriggerByEventAndPipeline(hermesTrigger.Event, hermesTrigger.PipelineID)
 	if err != nil {
-		return fmt.Errorf("Failed to delete cron trigger: %s", err)
+		return fmt.Errorf("failed to delete cron trigger: %v", err)
 	}
 
 	return nil
@@ -122,7 +171,7 @@ func mapPipelineCronTriggerToResource(hermesTrigger *cfClient.HermesTrigger, d *
 		r := regexp.MustCompile("[^:]+:[^:]+:[^:]+:[^:]+")
 		eventStringAttributes := strings.Split(hermesTrigger.Event, ":")
 		if !r.MatchString(hermesTrigger.Event) {
-			return fmt.Errorf("Event string must be in format 'cron:codefresh:[expression]:[message]:[uid]': %s", hermesTrigger.Event)
+			return fmt.Errorf("event string must be in format 'cron:codefresh:[expression]:[message]:[uid]': %s", hermesTrigger.Event)
 		}
 		d.Set("expression", eventStringAttributes[2])
 		d.Set("message", eventStringAttributes[3])