Restrict/warn overridding of pre/post definitions in tests and requires a manual flag

Author: deepsm007

pkg/api/types.go

@@ -1224,6 +1224,10 @@ type MultiStageTestConfiguration struct {
 	// NodeArchitectureOverrides is a map that allows overriding the node architecture for specific steps
 	// that exist in the Pre, Test and Post steps. The key is the name of the step and the value is the architecture.
 	NodeArchitectureOverrides NodeArchitectureOverrides `json:"node_architecture_overrides,omitempty"`
+	// AllowPrePostStepOverrides must be set to true when a test configuration overrides the pre or post steps
+	// from a workflow. This is a safety mechanism to prevent accidental overriding of critical setup and
+	// teardown steps that could cause resource leaks.
+	AllowPrePostStepOverrides *bool `json:"allow_pre_post_step_overrides,omitempty"`
 }
 
 type NodeArchitectureOverrides map[string]NodeArchitecture

pkg/api/zz_generated.deepcopy.go

@@ -2,6 +2,7 @@ package registry
 
 import (
 	"fmt"
+	"strings"
 
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -68,6 +69,12 @@ func NewResolver(stepsByName ReferenceByName, chainsByName ChainByName, workflow
 
 func (r *registry) Resolve(name string, config api.MultiStageTestConfiguration) (api.MultiStageTestConfigurationLiteral, error) {
 	var overridden [][]api.TestStep
+
+	// Validate workflow overrides before merging
+	if err := r.validateWorkflowOverrides(config); err != nil {
+		return api.MultiStageTestConfigurationLiteral{}, err
+	}
+
 	if config.Workflow != nil {
 		var errs []error
 		overridden, errs = r.mergeWorkflow(&config)
@@ -78,6 +85,35 @@ func (r *registry) Resolve(name string, config api.MultiStageTestConfiguration)
 	return r.resolveTest(config, stackForTest(name, config.Environment, config.Dependencies, config.DNSConfig, config.NodeArchitecture), overridden)
 }
 
+// validateWorkflowOverrides validates that tests properly acknowledge when they override workflow pre/post steps
+func (r *registry) validateWorkflowOverrides(config api.MultiStageTestConfiguration) error {
+	if config.Workflow == nil {
+		return nil
+	}
+
+	workflow, ok := r.workflowsByName[*config.Workflow]
+	if !ok {
+		// This will be caught by mergeWorkflow, no need to duplicate the error
+		return nil
+	}
+
+	hasPreOverride := config.Pre != nil && len(workflow.Pre) > 0
+	hasPostOverride := config.Post != nil && len(workflow.Post) > 0
+
+	if (hasPreOverride || hasPostOverride) && (config.AllowPrePostStepOverrides == nil || !*config.AllowPrePostStepOverrides) {
+		var overriddenSections []string
+		if hasPreOverride {
+			overriddenSections = append(overriddenSections, "pre")
+		}
+		if hasPostOverride {
+			overriddenSections = append(overriddenSections, "post")
+		}
+		return fmt.Errorf("test configuration overrides %s steps from workflow %q but does not have 'allow_pre_post_step_overrides: true' set. This flag is required to prevent accidental overriding of critical setup and teardown steps that could cause resource leaks", strings.Join(overriddenSections, " and "), *config.Workflow)
+	}
+
+	return nil
+}
+
 func (r *registry) mergeWorkflow(config *api.MultiStageTestConfiguration) ([][]api.TestStep, []error) {
 	var overridden [][]api.TestStep
 	workflow, ok := r.workflowsByName[*config.Workflow]
@@ -103,6 +139,7 @@ func (r *registry) mergeWorkflow(config *api.MultiStageTestConfiguration) ([][]a
 	} else {
 		overridden = append(overridden, workflow.Post)
 	}
+
 	config.Environment = mergeEnvironments(workflow.Environment, config.Environment)
 	config.Dependencies = mergeDependencies(workflow.Dependencies, config.Dependencies)
 	config.DependencyOverrides = mergeDependencyOverrides(workflow.DependencyOverrides, config.DependencyOverrides)