🐛 Fix panic when executionMode is set to agent but agentPool is not set

CHANGELOG.md

@@ -3,6 +3,7 @@
 BUG FIXES:
 * `AgentPool`: fix an issue when `plan_queued` and `apply_queued` statuses do not trigger agent scaling. [[GH-215](https://github.com/hashicorp/terraform-cloud-operator/pull/215)]
 * `Helm Chart`: fix an issue with the Deployment template in the Helm chart where `name` in path `spec.template.spec.containers[0]` was duplicated. [[GH-216](https://github.com/hashicorp/terraform-cloud-operator/pull/216)]
+* `Workspace`: fix an issue when the Operator panics when `spec.executionMode` is configured as `agent` but `spec.agentPool` is not set which is mandatory in this case. [[GH-242](https://github.com/hashicorp/terraform-cloud-operator/pull/242)]
 
 ENHANCEMENT:
 * `Operator`: Add the ability to skip TLS certificate validation for communication between the Operator and the TFC/E endpoint. A new environment variable `TFC_TLS_SKIP_VERIFY` should be set to `true` to skip the validation. Default: `false`. [[GH-222](https://github.com/hashicorp/terraform-cloud-operator/pull/222)]

api/v1alpha2/workspace_validation.go

@@ -16,6 +16,7 @@ func (w *Workspace) ValidateSpec() error {
 	var allErrs field.ErrorList
 
 	allErrs = append(allErrs, w.validateSpecAgentPool()...)
+	allErrs = append(allErrs, w.validateSpecExecutionMode()...)
 	allErrs = append(allErrs, w.validateSpecNotifications()...)
 	allErrs = append(allErrs, w.validateSpecRemoteStateSharing()...)
 	allErrs = append(allErrs, w.validateSpecRunTasks()...)
@@ -62,6 +63,22 @@ func (w *Workspace) validateSpecAgentPool() field.ErrorList {
 	return allErrs
 }
 
+func (w *Workspace) validateSpecExecutionMode() field.ErrorList {
+	allErrs := field.ErrorList{}
+	spec := w.Spec.ExecutionMode
+
+	f := field.NewPath("spec").Child("executionMode")
+
+	if spec == "agent" && w.Spec.AgentPool == nil {
+		allErrs = append(allErrs, field.Required(
+			f,
+			"'spec.agentPool' must be set when 'spec.executionMode' is set to 'agent'"),
+		)
+	}
+
+	return allErrs
+}
+
 func (w *Workspace) validateSpecNotifications() field.ErrorList {
 	allErrs := field.ErrorList{}
 	spec := w.Spec.Notifications
@@ -417,6 +434,5 @@ func (w *Workspace) validateSpecSSHKey() field.ErrorList {
 // + EnvironmentVariables names duplicate: spec.environmentVariables[].name
 // + TerraformVariables names duplicate: spec.terraformVariables[].name
 // + Tags duplicate: spec.tags[]
-// + AgentPool must be set when ExecutionMode = 'agent': spec.agentPool <- spec.executionMode['agent']
 //
 // + Invalid CR cannot be deleted until it is fixed -- need to discuss if we want to do something about it

api/v1alpha2/workspace_validation_test.go

@@ -60,6 +60,51 @@ func TestValidateWorkspaceSpecAgentPool(t *testing.T) {
 	}
 }
 
+func TestValidateWorkspaceSpecExecutionMode(t *testing.T) {
+	successCases := map[string]Workspace{
+		"AgentWithAgentPoolWithID": {
+			Spec: WorkspaceSpec{
+				ExecutionMode: "agent",
+				AgentPool: &WorkspaceAgentPool{
+					ID: "this",
+				},
+			},
+		},
+		"AgentWithAgentPoolWithName": {
+			Spec: WorkspaceSpec{
+				ExecutionMode: "agent",
+				AgentPool: &WorkspaceAgentPool{
+					Name: "this",
+				},
+			},
+		},
+	}
+
+	for n, c := range successCases {
+		t.Run(n, func(t *testing.T) {
+			if errs := c.validateSpecExecutionMode(); len(errs) != 0 {
+				t.Errorf("Unexpected validation errors: %v", errs)
+			}
+		})
+	}
+
+	errorCases := map[string]Workspace{
+		"AgentWithoutAgentPool": {
+			Spec: WorkspaceSpec{
+				ExecutionMode: "agent",
+			},
+		},
+	}
+
+	for n, c := range errorCases {
+		t.Run(n, func(t *testing.T) {
+			if errs := c.validateSpecExecutionMode(); len(errs) == 0 {
+				t.Error("Unexpected failure, at least one error is expected")
+			}
+		})
+	}
+}
+
 func TestValidateWorkspaceSpecNotifications(t *testing.T) {
 	token := "token"
 	url := "https://example.com"

controllers/workspace_controller_agents.go

@@ -32,6 +32,10 @@ func (r *WorkspaceReconciler) getAgentPoolIDByName(ctx context.Context, w *works
 func (r *WorkspaceReconciler) getAgentPoolID(ctx context.Context, w *workspaceInstance) (string, error) {
 	specAgentPool := w.instance.Spec.AgentPool
 
+	if specAgentPool == nil {
+		return "", fmt.Errorf("'spec.agentPool' is not set")
+	}
+
 	if specAgentPool.Name != "" {
 		w.log.Info("Reconcile Agent Pool", "msg", "getting agent pool ID by name")
 		return r.getAgentPoolIDByName(ctx, w)