diff --git a/pkg/controller/utils/utils.go b/pkg/controller/utils/utils.go index f5667098..285bbb09 100644 --- a/pkg/controller/utils/utils.go +++ b/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 diff --git a/pkg/controller/utils/utils_test.go b/pkg/controller/utils/utils_test.go index f6de7b66..d8651845 100644 --- a/pkg/controller/utils/utils_test.go +++ b/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) diff --git a/pkg/controller/zero-trust-workload-identity-manager/controller.go b/pkg/controller/zero-trust-workload-identity-manager/controller.go index 3d7552b1..75891716 100644 --- a/pkg/controller/zero-trust-workload-identity-manager/controller.go +++ b/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,23 +327,19 @@ 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 @@ -345,15 +347,6 @@ func processOperandStatus(operand v1alpha1.OperandStatus, state *operandAggregat // 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 }