SPIRE-365: create-only mode false status is not set on the main CR once the create-only mode is disabled

pkg/controller/utils/utils.go

@@ -6,10 +6,12 @@ import (
 	"os"
 	"sort"
 	"strings"
+	"sync"
 
 	routev1 "github.com/openshift/api/route/v1"
 	securityv1 "github.com/openshift/api/security/v1"
 	spiffev1alpha1 "github.com/spiffe/spire-controller-manager/api/v1alpha1"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -23,6 +25,9 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 )
 
+// logInvalidCreateOnlyModeOnce ensures we only log the warning once
+var logInvalidCreateOnlyModeOnce sync.Once
+
 var (
 	scheme = runtime.NewScheme()
 	codecs serializer.CodecFactory
@@ -225,12 +230,28 @@ func GetLogFormatFromString(logFormat string) string {
 	return logFormat
 }
 
-// IsInCreateOnlyMode checks if create-only mode is enabled
-// If the environment variable is set to "true", it returns true
-// Otherwise, it returns false
+// IsInCreateOnlyMode checks if create-only mode is enabled.
+// It accepts case-insensitive values:
+//   - "true", "TRUE", "True" -> returns true (enabled)
+//   - "false", "FALSE", "False", empty, or invalid -> returns false (disabled)
 func IsInCreateOnlyMode() bool {
-	createOnlyEnvValue := os.Getenv(createOnlyEnvName)
-	return createOnlyEnvValue == "true"
+	value := strings.TrimSpace(os.Getenv(createOnlyEnvName))
+	normalized := strings.ToUpper(value)
+
+	switch normalized {
+	case "TRUE":
+		return true
+	case "FALSE", "":
+		return false
+	default:
+		// Log warning once for invalid value
+		logInvalidCreateOnlyModeOnce.Do(func() {
+			ctrl.Log.WithName("create-only-mode").Info("Invalid CREATE_ONLY_MODE value, using default (disabled)",
+				"value", value,
+				"validValues", "true, false (case-insensitive)")
+		})
+		return false
+	}
 }
 
 // ZTWIMSpecChangedPredicate triggers reconciliation when ZTWIM spec is created

pkg/controller/utils/utils_test.go

@@ -742,22 +742,64 @@ func TestIsInCreateOnlyModeEnvCheck(t *testing.T) {
 		description string
 	}{
 		{
-			name:        "env var set to true",
+			name:        "env var set to true (lowercase)",
 			envValue:    "true",
 			expected:    true,
 			description: "should return true when CREATE_ONLY_MODE is 'true'",
 		},
 		{
-			name:        "env var set to false",
+			name:        "env var set to TRUE (uppercase)",
+			envValue:    "TRUE",
+			expected:    true,
+			description: "should return true when CREATE_ONLY_MODE is 'TRUE'",
+		},
+		{
+			name:        "env var set to True (mixed case)",
+			envValue:    "True",
+			expected:    true,
+			description: "should return true when CREATE_ONLY_MODE is 'True'",
+		},
+		{
+			name:        "env var set to false (lowercase)",
 			envValue:    "false",
 			expected:    false,
 			description: "should return false when CREATE_ONLY_MODE is 'false'",
 		},
+		{
+			name:        "env var set to FALSE (uppercase)",
+			envValue:    "FALSE",
+			expected:    false,
+			description: "should return false when CREATE_ONLY_MODE is 'FALSE'",
+		},
+		{
+			name:        "env var set to False (mixed case)",
+			envValue:    "False",
+			expected:    false,
+			description: "should return false when CREATE_ONLY_MODE is 'False'",
+		},
 		{
 			name:        "env var not set",
 			envValue:    "",
 			expected:    false,
-			description: "should return false when CREATE_ONLY_MODE is not set",
+			description: "should return false (default) when CREATE_ONLY_MODE is not set",
+		},
+		{
+			name:        "env var with whitespace around true",
+			envValue:    "  true  ",
+			expected:    true,
+			description: "should trim whitespace and return true",
+		},
+		{
+			name:        "env var with whitespace around false",
+			envValue:    "  FALSE  ",
+			expected:    false,
+			description: "should trim whitespace and return false",
+		},
+		{
+			name:        "env var with only whitespace",
+			envValue:    "   ",
+			expected:    false,
+			description: "should treat whitespace-only as empty and return false (default)",
 		},
 		{
 			name:        "env var set to invalid value",
@@ -769,11 +811,7 @@ func TestIsInCreateOnlyModeEnvCheck(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if tt.envValue != "" {
-				t.Setenv(createOnlyEnvName, tt.envValue)
-			} else {
-				t.Setenv(createOnlyEnvName, "")
-			}
+			t.Setenv(createOnlyEnvName, tt.envValue)
 			result := IsInCreateOnlyMode()
 			if result != tt.expected {
 				t.Errorf("IsInCreateOnlyMode() = %v, expected %v - %s", result, tt.expected, tt.description)

pkg/controller/zero-trust-workload-identity-manager/controller.go

@@ -196,14 +196,18 @@ func New(mgr ctrl.Manager) (*ZeroTrustWorkloadIdentityManagerReconciler, error)
 	}, nil
 }
 
-// setCreateOnlyModeCondition sets the CreateOnlyMode condition on the main CR if any operand has it
-func setCreateOnlyModeCondition(statusMgr *status.Manager, anyOperandHasCreateOnlyCondition, anyCreateOnlyModeEnabled bool) {
-	if anyOperandHasCreateOnlyCondition {
-		if anyCreateOnlyModeEnabled {
-			statusMgr.AddCondition(CreateOnlyMode, utils.CreateOnlyModeEnabled,
-				"One or more operands have create-only mode enabled",
-				metav1.ConditionTrue)
-		} else {
+// setCreateOnlyModeCondition sets the CreateOnlyMode condition on the main CR based on the environment variable
+func setCreateOnlyModeCondition(statusMgr *status.Manager, existingConditions []metav1.Condition) {
+	createOnlyMode := utils.IsInCreateOnlyMode()
+
+	if createOnlyMode {
+		statusMgr.AddCondition(CreateOnlyMode, utils.CreateOnlyModeEnabled,
+			"Create-only mode is enabled: Updates are not reconciled to existing resources",
+			metav1.ConditionTrue)
+	} else {
+		// Only set to False if we previously had it set to True (to show the transition)
+		existingCondition := apimeta.FindStatusCondition(existingConditions, CreateOnlyMode)
+		if existingCondition != nil && existingCondition.Status == metav1.ConditionTrue {
 			statusMgr.AddCondition(CreateOnlyMode, utils.CreateOnlyModeDisabled,
 				"Create-only mode is disabled",
 				metav1.ConditionFalse)
@@ -305,14 +309,16 @@ func (r *ZeroTrustWorkloadIdentityManagerReconciler) Reconcile(ctx context.Conte
 			metav1.ConditionFalse)
 	}
 
-	// Set CreateOnlyMode condition if any operand has it
-	setCreateOnlyModeCondition(statusMgr, result.anyOperandHasCreateOnlyCondition, result.anyCreateOnlyModeEnabled)
+	// Set CreateOnlyMode condition based on environment variable (simpler than aggregating from operands)
+	setCreateOnlyModeCondition(statusMgr, config.Status.ConditionalStatus.Conditions)
 
-	r.log.Info("Aggregated operand status", "allReady", result.allReady, "notCreated", result.notCreatedCount, "failed", result.failedCount, "anyCreateOnlyModeEnabled", result.anyCreateOnlyModeEnabled, "anyOperandHasCreateOnlyCondition", result.anyOperandHasCreateOnlyCondition, "anyOperandExists", result.anyOperandExists)
+	// Check create-only mode from environment variable for logging and OLM update
+	createOnlyModeEnabled := utils.IsInCreateOnlyMode()
+	r.log.Info("Aggregated operand status", "allReady", result.allReady, "notCreated", result.notCreatedCount, "failed", result.failedCount, "createOnlyModeEnabled", createOnlyModeEnabled, "anyOperandExists", result.anyOperandExists)
 
 	// Update OperatorCondition for OLM integration (best effort - don't fail reconciliation if it fails)
 	// Upgradeable condition is only set on OperatorCondition, not on ZTWIM CR
-	if err := r.updateOperatorCondition(ctx, result.anyCreateOnlyModeEnabled, result.operandStatuses); err != nil {
+	if err := r.updateOperatorCondition(ctx, createOnlyModeEnabled, result.operandStatuses); err != nil {
 		r.log.Error(err, "failed to update OperatorCondition, continuing (operator may be running outside OLM)")
 	}
 
@@ -321,39 +327,26 @@ func (r *ZeroTrustWorkloadIdentityManagerReconciler) Reconcile(ctx context.Conte
 
 // operandAggregateState holds the aggregate state tracked across all operands
 type operandAggregateState struct {
-	allReady                         bool
-	notCreatedCount                  int
-	failedCount                      int
-	anyCreateOnlyModeEnabled         bool
-	anyOperandHasCreateOnlyCondition bool
-	anyOperandExists                 bool
+	allReady         bool
+	notCreatedCount  int
+	failedCount      int
+	anyOperandExists bool
 }
 
 // operandAggregateResult holds the result of aggregating operand statuses
 type operandAggregateResult struct {
-	operandStatuses                  []v1alpha1.OperandStatus
-	allReady                         bool
-	notCreatedCount                  int
-	failedCount                      int
-	anyCreateOnlyModeEnabled         bool
-	anyOperandHasCreateOnlyCondition bool
-	anyOperandExists                 bool
+	operandStatuses  []v1alpha1.OperandStatus
+	allReady         bool
+	notCreatedCount  int
+	failedCount      int
+	anyOperandExists bool
 }
 
 // processOperandStatus processes a single operand's status and updates aggregate state
 func processOperandStatus(operand v1alpha1.OperandStatus, state *operandAggregateState) {
 	// Check if operand exists
 	if operand.Message != OperandMessageCRNotFound {
 		state.anyOperandExists = true
-
-		// Check if this operand has CreateOnlyMode condition
-		createOnlyCondition := apimeta.FindStatusCondition(operand.Conditions, utils.CreateOnlyModeStatusType)
-		if createOnlyCondition != nil {
-			state.anyOperandHasCreateOnlyCondition = true
-			if createOnlyCondition.Status == metav1.ConditionTrue {
-				state.anyCreateOnlyModeEnabled = true
-			}
-		}
 	}
 
 	// Check if operand is ready
@@ -391,13 +384,11 @@ func (r *ZeroTrustWorkloadIdentityManagerReconciler) aggregateOperandStatus(ctx
 	}
 
 	return operandAggregateResult{
-		operandStatuses:                  operandStatuses,
-		allReady:                         state.allReady,
-		notCreatedCount:                  state.notCreatedCount,
-		failedCount:                      state.failedCount,
-		anyCreateOnlyModeEnabled:         state.anyCreateOnlyModeEnabled,
-		anyOperandHasCreateOnlyCondition: state.anyOperandHasCreateOnlyCondition,
-		anyOperandExists:                 state.anyOperandExists,
+		operandStatuses:  operandStatuses,
+		allReady:         state.allReady,
+		notCreatedCount:  state.notCreatedCount,
+		failedCount:      state.failedCount,
+		anyOperandExists: state.anyOperandExists,
 	}
 }
 
@@ -482,19 +473,19 @@ func (r *ZeroTrustWorkloadIdentityManagerReconciler) getSpireOIDCDiscoveryProvid
 }
 
 // extractKeyConditions extracts key conditions from operand status
-// Only includes CreateOnlyMode condition when it's enabled (True) - needed for ZTWIM aggregation
+// Includes CreateOnlyMode condition when enabled (for visibility on operand status)
 // When operand is not ready, also includes Ready condition and other failed conditions
 func extractKeyConditions(conditions []metav1.Condition, isReady bool) []metav1.Condition {
 	keyConditions := []metav1.Condition{}
 
-	// Only include CreateOnlyMode condition if it's enabled (True)
-	// When disabled (False), it's just clutter and not needed
+	// Include CreateOnlyMode condition only when enabled (for visibility)
+	// ZTWIM now checks the environment variable directly instead of aggregating from operands
 	createOnlyCondition := apimeta.FindStatusCondition(conditions, utils.CreateOnlyModeStatusType)
 	if createOnlyCondition != nil && createOnlyCondition.Status == metav1.ConditionTrue {
 		keyConditions = append(keyConditions, *createOnlyCondition)
 	}
 
-	// If operand is ready and no CreateOnlyMode, return empty (reduces clutter)
+	// If operand is ready, return only the CreateOnlyMode condition if present (reduces clutter)
 	if isReady {
 		return keyConditions
 	}