refactor: status condition validations

internal/controllers/v1beta2/build_helpers.go

@@ -992,12 +992,7 @@ Build cancelled
 		// if the build was cancelled at the queued phase of a build, then it is likely it was cancelled by a new build
 		// update the buildstep to be cancelled by new build for clearer visibility in the resource status
 		if value, ok := lagoonBuild.ObjectMeta.Labels["lagoon.sh/cancelledByNewBuild"]; ok && value == "true" {
-			condition := metav1.Condition{
-				Type:               "BuildStep",
-				Reason:             "CancelledByNewBuild",
-				Status:             metav1.ConditionTrue,
-				LastTransitionTime: metav1.NewTime(time.Now().UTC()),
-			}
+			condition, _ := helpers.BuildStepToStatusConditions("CancelledByNewBuild", "", time.Now().UTC())
 			_ = meta.SetStatusCondition(&lagoonBuild.Status.Conditions, condition)
 		}
 		// finaly patch the build with the cancelled status phase

internal/controllers/v1beta2/podmonitor_buildhandlers.go

@@ -16,8 +16,6 @@ import (
 	lagooncrd "github.com/uselagoon/remote-controller/api/lagoon/v1beta2"
 	"github.com/uselagoon/remote-controller/internal/helpers"
 	"github.com/uselagoon/remote-controller/internal/metrics"
-	"golang.org/x/text/cases"
-	"golang.org/x/text/language"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	meta "k8s.io/apimachinery/pkg/api/meta"
@@ -555,23 +553,9 @@ Build %s
 				},
 			},
 		}
-		condition := metav1.Condition{
-			Type: "BuildStep",
-			// Reason needs to be CamelCase not camelCase. Would need to update the `build-deploy-tool` to use CamelCase
-			// to eventually remove the need for `cases`
-			Reason:             cases.Title(language.English, cases.NoLower).String(buildStep),
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.NewTime(time.Now().UTC()),
-		}
-		_ = meta.SetStatusCondition(&lagoonBuild.Status.Conditions, condition)
-		// add every build step as its own status condition too
-		condition = metav1.Condition{
-			Type:               cases.Title(language.English, cases.NoLower).String(buildStep),
-			Reason:             buildCondition.String(),
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.NewTime(time.Now().UTC()),
-		}
-		_ = meta.SetStatusCondition(&lagoonBuild.Status.Conditions, condition)
+		condition1, condition2 := helpers.BuildStepToStatusConditions(buildStep, buildCondition.String(), time.Now().UTC())
+		_ = meta.SetStatusCondition(&lagoonBuild.Status.Conditions, condition1)
+		_ = meta.SetStatusCondition(&lagoonBuild.Status.Conditions, condition2)
 		mergeMap["status"] = map[string]interface{}{
 			"conditions": lagoonBuild.Status.Conditions,
 			"phase":      buildCondition.String(),

internal/controllers/v1beta2/podmonitor_taskhandlers.go

@@ -337,12 +337,7 @@ Task %s
 			},
 		}
 
-		condition := metav1.Condition{
-			Reason:             taskCondition.String(),
-			Type:               "TaskCondition",
-			Status:             metav1.ConditionTrue,
-			LastTransitionTime: metav1.NewTime(time.Now().UTC()),
-		}
+		condition := helpers.TaskStepToStatusCondition(taskCondition.String(), time.Now().UTC())
 		_ = meta.SetStatusCondition(&lagoonTask.Status.Conditions, condition)
 		mergeMap["status"] = map[string]interface{}{
 			"conditions": lagoonTask.Status.Conditions,

internal/helpers/helpers.go

@@ -12,7 +12,12 @@ import (
 	"strings"
 	"time"
 
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 const (
@@ -272,3 +277,44 @@ func GetEnvBool(key string, fallback bool) bool {
 	}
 	return fallback
 }
+
+// this converts the buildstep to a status condition reason where possible
+// otherwise it will just return an unknown condition
+func BuildStepToStatusConditions(s, c string, t time.Time) (metav1.Condition, metav1.Condition) {
+	reason := cases.Title(language.English, cases.NoLower).String(s)
+	reason = strings.ReplaceAll(reason, "-", "_")
+	condition1 := metav1.Condition{
+		Type:               "BuildStep",
+		Reason:             reason,
+		Status:             metav1.ConditionTrue,
+		LastTransitionTime: metav1.NewTime(t),
+	}
+	errs := metav1validation.ValidateCondition(condition1, field.NewPath("reason"))
+	if len(errs) > 0 {
+		// if the build step can't be converted, just return unknown
+		condition1.Reason = "Unknown"
+	}
+	condition2 := metav1.Condition{
+		Type:               reason,
+		Reason:             c,
+		Status:             metav1.ConditionTrue,
+		LastTransitionTime: metav1.NewTime(t),
+	}
+	errs = metav1validation.ValidateCondition(condition2, field.NewPath("type"))
+	if len(errs) > 0 {
+		// if the build step can't be converted, just return unknown
+		condition2.Type = "Unknown"
+	}
+	return condition1, condition2
+}
+
+// this converts the taskstep to a status condition reason where possible
+// otherwise it will just return an unknown condition
+func TaskStepToStatusCondition(c string, t time.Time) metav1.Condition {
+	return metav1.Condition{
+		Type:               "TaskCondition",
+		Reason:             c,
+		Status:             metav1.ConditionTrue,
+		LastTransitionTime: metav1.NewTime(t),
+	}
+}

internal/helpers/helpers_test.go

@@ -4,6 +4,9 @@ import (
 	"os"
 	"reflect"
 	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestShortName(t *testing.T) {
@@ -256,3 +259,143 @@ func TestGetLagoonFeatureFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestBuildStepToStatusConditions(t *testing.T) {
+	type args struct {
+		step      string
+		condition string
+		time      int64
+	}
+	tests := []struct {
+		name  string
+		args  args
+		want  metav1.Condition
+		want2 metav1.Condition
+	}{
+		{
+			name: "test1-valid",
+			args: args{
+				step:      "buildingImages",
+				condition: "Running",
+				time:      1735516457,
+			},
+			want: metav1.Condition{
+				Type:               "BuildStep",
+				Reason:             "BuildingImages",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+			want2: metav1.Condition{
+				Type:               "BuildingImages",
+				Reason:             "Running",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+		},
+		{
+			name: "test2-valid",
+			args: args{
+				step:      "buildingImages-nginx",
+				condition: "Running",
+				time:      1735516457,
+			},
+			want: metav1.Condition{
+				Type:               "BuildStep",
+				Reason:             "BuildingImages_Nginx",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+			want2: metav1.Condition{
+				Type:               "BuildingImages_Nginx",
+				Reason:             "Running",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+		},
+		{
+			name: "test3-valid",
+			args: args{
+				step:      "buildingImages-nginx-3",
+				condition: "Running",
+				time:      1735516457,
+			},
+			want: metav1.Condition{
+				Type:               "BuildStep",
+				Reason:             "BuildingImages_Nginx_3",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+			want2: metav1.Condition{
+				Type:               "BuildingImages_Nginx_3",
+				Reason:             "Running",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+		},
+		{
+			name: "test4-invalid",
+			args: args{
+				step:      "buildingImages-nginx-3#",
+				condition: "Running",
+				time:      1735516457,
+			},
+			want: metav1.Condition{
+				Type:               "BuildStep",
+				Reason:             "Unknown",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+			want2: metav1.Condition{
+				Type:               "Unknown",
+				Reason:             "Running",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got1, got2 := BuildStepToStatusConditions(tt.args.step, tt.args.condition, time.Unix(tt.args.time, 0))
+			if got1 != tt.want {
+				t.Errorf("BuildStepToStatusConditions() = %v, want %v", got1, tt.want)
+			}
+			if got2 != tt.want2 {
+				t.Errorf("BuildStepToStatusConditions() = %v, want %v", got2, tt.want2)
+			}
+		})
+	}
+}
+
+func TestTaskStepToStatusCondition(t *testing.T) {
+	type args struct {
+		condition string
+		time      int64
+	}
+	tests := []struct {
+		name string
+		args args
+		want metav1.Condition
+	}{
+		{
+			name: "test1",
+			args: args{
+				condition: "Running",
+				time:      1735516457,
+			},
+			want: metav1.Condition{
+				Type:               "TaskCondition",
+				Reason:             "Running",
+				Status:             metav1.ConditionTrue,
+				LastTransitionTime: metav1.NewTime(time.Unix(1735516457, 0)),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := TaskStepToStatusCondition(tt.args.condition, time.Unix(tt.args.time, 0))
+			if got != tt.want {
+				t.Errorf("TaskStepToStatusCondition() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}