Improve configuration comparison and event handling (#162)

* utils/cmp: Remove event related content from cmp package and improve tests

* cluster/event: Cleanup event processing

* cluster: Update cluster plan to match new event changes

* cluster/executors/kubespray: Update tests

* cluster/provisioner/terraform: Update to match new event changes

pkg/cluster/action_apply.go

@@ -29,14 +29,14 @@ func (a ApplyAction) String() string {
 }
 
 // events returns events of the corresponding action.
-func (a ApplyAction) events() event.Events {
+func (a ApplyAction) rules() []event.Rule {
 	switch a {
 	case CREATE:
-		return event.ModifyEvents
+		return event.ModifyRules
 	case SCALE:
-		return event.ScaleEvents
+		return event.ScaleRules
 	case UPGRADE:
-		return event.UpgradeEvents
+		return event.UpgradeRules
 	default:
 		return nil
 	}
@@ -65,7 +65,7 @@ func (c *Cluster) Apply(a string) error {
 	}
 
 	if c.AppliedConfig == nil && (action == SCALE || action == UPGRADE) {
-		ui.Printf(ui.INFO, "Cannot %s cluster '%s'. It has not been created yet.\n\n", action, c.Name)
+		ui.Printf(ui.INFO, "Cannot %s cluster %q. It has not been created yet.\n\n", action, c.Name)
 
 		err := ui.Ask("Would you like to create it instead?")
 		if err != nil {
@@ -75,18 +75,14 @@ func (c *Cluster) Apply(a string) error {
 		action = CREATE
 	}
 
-	var events event.Events
-
-	if c.AppliedConfig != nil {
-		events, err = c.plan(action)
-		if err != nil {
-			return err
-		}
+	events, err := c.plan(action)
+	if err != nil {
+		return err
+	}
 
-		if len(events) == 0 {
-			ui.Println(ui.INFO, "No changes detected.")
-			return nil
-		}
+	if c.AppliedConfig != nil && len(events) == 0 {
+		ui.Println(ui.INFO, "No changes detected.")
+		return nil
 	}
 
 	if err := c.prepare(); err != nil {
@@ -109,46 +105,83 @@ func (c *Cluster) Apply(a string) error {
 	return c.ApplyNewConfig()
 }
 
-// plan compares new and applied configuration files, and detects
-// events based on the apply action.
-//
-// If applied configuration file does not exist, no events and no
-// error is returned.
-// If blocking changes are detected, an error is returned.
-// If warnings are detected, user is asked for permission to continue.
+// plan compares an already applied configuration file with the new one, and
+// detects events based on the apply action. If cluster has not been
+// initialized yet, nil is returned both for an error and events.
 func (c *Cluster) plan(action ApplyAction) (event.Events, error) {
 	if c.AppliedConfig == nil {
 		return nil, nil
 	}
 
-	comp := cmp.NewComparator()
-	comp.Tag = "opt"
-	comp.ExtraNameTags = []string{"yaml"}
-	comp.IgnoreEmptyChanges = true
-	comp.PopulateStructNodes = true
-
-	diff, err := comp.Compare(c.AppliedConfig, c.NewConfig)
+	cmpOptions := cmp.Options{
+		Tag:                "opt",
+		ExtraNameTags:      []string{"yaml"},
+		RespectSliceOrder:  false,
+		IgnoreEmptyChanges: true,
+		PopulateAllNodes:   true,
+	}
 
-	if err != nil || len(diff.Changes()) == 0 {
+	// Compare configuration files.
+	res, err := cmp.Compare(c.AppliedConfig, c.NewConfig, cmpOptions)
+	if err != nil {
 		return nil, err
 	}
 
+	// Return if there is no changes.
+	if !res.HasChanges() {
+		return nil, nil
+	}
+
+	fmtOptions := cmp.FormatOptions{
+		ShowColor:            ui.HasColor(),
+		ShowDiffOnly:         true,
+		ShowChangeTypePrefix: true,
+	}
+
 	fmt.Printf("Following changes have been detected:\n\n")
-	fmt.Println(diff.ToYamlDiff())
+	fmt.Println(res.ToYaml(fmtOptions))
+
+	// Generate events from detected configuration changes and provided rules.
+	events, err := event.GenerateEvents(res.Tree(), action.rules())
+	if err != nil {
+		return nil, err
+	}
 
-	events := event.TriggerEvents(diff, action.events())
-	blocking := events.Blocking()
+	hasError := false
+	for _, e := range events {
+		if !e.Rule.IsOfType(event.Error) {
+			continue
+		}
+
+		hasError = true
+		if e.Change.Type == cmp.Create || e.Change.Type == cmp.Delete {
+			// For create and delete events, only change's
+			// path is shown.
+			err := NewConfigChangeError(e.Rule.Message, e.Change.Path)
+			ui.PrintBlockE(err)
+		} else {
+			err := NewConfigChangeError(e.Rule.Message, e.MatchedChangePaths...)
+			ui.PrintBlockE(err)
+		}
+	}
 
-	if len(blocking) > 0 {
-		ui.PrintBlockE(blocking.Errors()...)
-		return nil, fmt.Errorf("Aborted. Configuration file contains errors.")
+	if hasError {
+		return nil, fmt.Errorf("Configuration file contains errors.")
 	}
 
-	warnings := events.Warns()
+	hasWarnings := false
+	for _, e := range events {
+		if !e.Rule.IsOfType(event.Warn) {
+			continue
+		}
+
+		hasWarnings = true
+		err := NewConfigChangeWarning(e.Rule.Message, e.Change.Path)
+		ui.PrintBlockE(err)
+	}
 
-	if len(warnings) > 0 {
-		ui.PrintBlockE(warnings.Errors()...)
-		fmt.Println("Above warnings indicate potentially destructive actions.")
+	if hasWarnings {
+		ui.Println(ui.INFO, "Above warnings indicate potentially dangerous actions.")
 	}
 
 	return events, ui.Ask()
@@ -210,7 +243,7 @@ func (c *Cluster) upgrade() error {
 }
 
 // scale scales an existing cluster.
-func (c *Cluster) scale(events event.Events) error {
+func (c *Cluster) scale(events []event.Event) error {
 	if err := c.Executor().Init(); err != nil {
 		return err
 	}

pkg/cluster/action_apply_test.go

@@ -1,7 +1,6 @@
 package cluster
 
 import (
-	"fmt"
 	"os"
 	"testing"
 	"time"
@@ -78,12 +77,11 @@ func TestPlan(t *testing.T) {
 	assert.NoError(t, c.ApplyNewConfig())
 	assert.NoError(t, c.Sync())
 
-	// Make "blocking" change
-	ver := fmt.Sprintf("%s.%s", env.ProjectK8sVersions[0], "99")
-	c.NewConfig.Kubernetes.Version = config.KubernetesVersion(ver)
+	// Make a change that will result in a configuration error.
+	c.NewConfig.Kubernetes.Version = config.KubernetesVersion("v1.26.5")
 
 	_, err := c.plan(SCALE)
-	assert.EqualError(t, err, "Aborted. Configuration file contains errors.")
+	assert.EqualError(t, err, "Configuration file contains errors.")
 }
 
 func TestApply_Create(t *testing.T) {
@@ -106,7 +104,7 @@ func TestApply_Upgrade_AskToCreate(t *testing.T) {
 	defer func() { env.ProjectRequiredFiles = tmp }()
 
 	assert.NoError(t, c.Apply(UPGRADE.String()))
-	assert.Contains(t, c.Ui().ReadStdout(t), "Cannot upgrade cluster 'cluster-mock'. It has not been created yet.")
+	assert.Contains(t, c.Ui().ReadStdout(t), "Cannot upgrade cluster")
 }
 
 func TestApply_Upgrade_NoChanges(t *testing.T) {
@@ -130,11 +128,10 @@ func TestApply_Upgrade(t *testing.T) {
 	assert.NoError(t, c.ApplyNewConfig())
 	assert.NoError(t, c.Sync())
 
-	// Make some changes to the new config
-	ver := fmt.Sprintf("%s.%s", env.ProjectK8sVersions[0], "99")
-	c.NewConfig.Kubernetes.Version = config.KubernetesVersion(ver)
+	// Make a valid configuration change.
+	c.NewConfig.Kubernetes.Version = config.KubernetesVersion("v1.26.5")
 
-	// Skip required files check
+	// Skip required files check.
 	tmp := env.ProjectRequiredFiles
 	env.ProjectRequiredFiles = []string{}
 	defer func() { env.ProjectRequiredFiles = tmp }()

pkg/cluster/action_destroy_test.go

@@ -7,16 +7,18 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDestroy(t *testing.T) {
 	c := MockCluster(t)
 
 	// Create terraform state file
 	err := os.MkdirAll(path.Dir(c.TfStatePath()), os.ModePerm)
-	assert.NoError(t, err)
+	require.NoError(t, err)
+
 	err = ioutil.WriteFile(c.TfStatePath(), []byte(""), os.ModePerm)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.NoError(t, c.Destroy())
 }

pkg/cluster/errors.go

@@ -20,3 +20,23 @@ func NewValidationError(msg string, path string) error {
 		},
 	)
 }
+
+func NewConfigChangeError(msg string, paths ...string) error {
+	return ui.NewErrorBlock(ui.ERROR,
+		[]ui.Content{
+			ui.NewErrorLine("Error type:", "Invalid Configuration Change"),
+			ui.NewErrorSection("Config path:", paths...),
+			ui.NewErrorSection("Error:", msg),
+		},
+	)
+}
+
+func NewConfigChangeWarning(msg string, paths ...string) error {
+	return ui.NewErrorBlock(ui.WARN,
+		[]ui.Content{
+			ui.NewErrorLine("Warning type:", "Dangerous Configuration Change"),
+			ui.NewErrorSection("Config path:", paths...),
+			ui.NewErrorSection("Warning:", msg),
+		},
+	)
+}

pkg/cluster/event/README.md

@@ -0,0 +1,107 @@
+# Event Generation Behavior
+
+The `event` package is designed to detect changes and generate events based on the provided rules.
+
+## Event
+
+An `Event` represents a detected change and its associated rule.
+It contains:
+
+- The rule that was matched.
+- The detected change.
+- Paths of changes that matched the rule (triggered an event).
+
+## Rule
+
+A `Rule` defines the conditions that trigger events based on detected changes.
+It specifies:
+
+- The rule's type.
+- The rule's path.
+- The type of change it observes.
+
+## Event Generation
+
+The core of the event generation process is the `GenerateEvents` function.
+It evaluates the changes from the comparison tree against the provided rules and returns a list of corresponding events.
+A single change can match at most one rule and thus produce at most one event.
+
+The process involves:
+
+- Validating the provided rules.
+- Recursively traversing through each node in the comparison tree.
+- Matching each leaf change against the provided rules and determining the best match.
+- Creating and adding events based on best matched rule.
+
+## Matching Rules
+
+The matchRule function evaluates each change to identify the best matching rule.
+A change matches a rule if both the path and change type align.
+
+For each change, the `matchRule` function determines the most appropriate rule (best match).
+A change matches a rule if both the path and change type align.
+If no rules match with a given change, the change is ignored.
+If multiple rules match, they are prioritized based on:
+
+1. Path Length: Longer paths are prioritized.
+2. Wildcard Count: Paths with fewer wildcards are considered more specific and are prioritized.
+3. Rule Priority: Higher priority rules are prioritized.
+
+# RulePath
+
+To improve rule matching flexibility, the rule's path uses specific operators.
+While these resemble regex, they are much simpler and less powerful.
+
+## Operators
+
+- `.`: Separates path into multiple segments.
+- `{}`: Option block defines multiple options divided with comma for matching path segments.
+- `*`: Acts as a wildcard, matching any path segment.
+- `@`: Denotes an anchor for a path segment.
+- `!`: Indicates path termination, ensuring an exact match.
+
+## Simplifications
+
+- `{a}` can be simplified to `a` when the option block contains only one option.
+- `@{a}` can be simplified to `@a` when using an anchor with a single segment option.
+- `@*` can be simplified to `@` when using an anchor as a wildcard.
+- Spaces within rules are ignored and can be omitted.
+
+## Reserved Characters
+
+The characters `*`, `@`, `{`, `}`, `.`, `!` and space (` `) are considered reserved.
+Rule paths containing these characters outside their intended context will be considered invalid.
+Therefore, change paths containing these characters will never be matched.
+
+## Matching Scenarios
+
+Consider changes on the following paths:
+- `a`
+- `a.b`
+- `a.b.c`
+- `a.B.c`
+- `a.C.c`
+- `A.B.c`
+- `a.b.c.d`
+
+1. **Normal Path**:
+   - **Description**: Normally, rule path matches change paths that begin with the same segments.
+   - **Example**: Rule path `a.b.c` matches both `a.b.c` and `a.b.c.d`.
+
+2. **Exact Match**:
+   - **Description**: For a precise match without considering extended paths, end the rule path with `!`.
+   - **Example**: Rule path `a.b.c!` strictly matches `a.b.c`.
+
+3. **Path Options**:
+   - **Description**: Option blocks in rule paths allow matching multiple potential segments.
+   - **Example**: Rule path `a.{b, B}.c` can match either `a.b.c` or `a.B.c`.
+
+4. **Wildcard Path**:
+   - **Description**: Wildcards allow matching any segment in the change path.
+   - **Example**: Rule path `a.*.c` can match any of `a.b.c`, `a.B.c`, `a.C.c`, or `a.b.c.d`.
+
+5. **Anchor Path**:
+   - **Description**: Anchors ensure that the change in an event is extracted from the anchored path segment, rather than the segment where the change was actually detected.
+   - **Examples**:
+     - Rule path `a.@*.c` (or its shorthand `[email protected]`) matches paths like `a.b.c`, `a.B.c`, `a.C.c`, and `a.b.c.d`.
+     - Rule path `a.@{b}.c` (or its shorthand `[email protected]`) specifically matches `a.b.c` and its extension `a.b.c.d`.