From adf081b86f878b101e1ab44d25f9e451fafef9c0 Mon Sep 17 00:00:00 2001 From: Tomasz Janiszewski Date: Wed, 13 May 2026 17:10:56 +0200 Subject: [PATCH 1/2] fix(operator): apply feature flags when scheduled sessions trigger Scheduled sessions were not respecting workspace feature flags like `jira-write` because the session-trigger command creates AgenticSession CRs directly from a pre-stored template without re-evaluating feature flags at trigger time. This fix adds applyFeatureFlagOverrides() to read the workspace's feature-flag-overrides ConfigMap and apply settings (e.g., setting JIRA_READ_ONLY_MODE=false when jira-write is enabled) before creating the session CR. The implementation follows the same pattern as CreateAgenticSession in the backend: read ConfigMap, check flag value, inject environment variable. Errors are logged but non-fatal to ensure degraded operation. Fixes #1506 Co-Authored-By: Claude Sonnet 4.5 --- .../operator/internal/trigger/trigger.go | 67 +++++++++- .../operator/internal/trigger/trigger_test.go | 122 ++++++++++++++++++ 2 files changed, 186 insertions(+), 3 deletions(-) diff --git a/components/operator/internal/trigger/trigger.go b/components/operator/internal/trigger/trigger.go index 25e578634..2a99b0459 100644 --- a/components/operator/internal/trigger/trigger.go +++ b/components/operator/internal/trigger/trigger.go @@ -13,9 +13,11 @@ import ( "strings" "time" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/util/retry" @@ -244,8 +246,67 @@ func resumeSessionWithPrompt(dynamicClient dynamic.Interface, namespace, session }) } +// applyFeatureFlagOverrides reads workspace feature flag overrides from the +// feature-flag-overrides ConfigMap and applies them to the session template. +// Currently supports: jira-write flag → JIRA_READ_ONLY_MODE environment variable. +func applyFeatureFlagOverrides(ctx context.Context, k8sClient kubernetes.Interface, namespace string, template map[string]interface{}) error { + // Read feature-flag-overrides ConfigMap + cm, err := k8sClient.CoreV1().ConfigMaps(namespace).Get(ctx, "feature-flag-overrides", metav1.GetOptions{}) + if errors.IsNotFound(err) { + // No overrides ConfigMap - this is normal, nothing to do + return nil + } + if err != nil { + // Log warning but don't fail - same pattern as backend + log.Printf("WARNING: failed to read feature flag overrides for namespace %s: %v", namespace, err) + return nil + } + + // Check jira-write flag + if val, exists := cm.Data["jira-write"]; exists && val == "true" { + // Get or create environmentVariables map + spec, ok := template["spec"].(map[string]interface{}) + if !ok { + spec = map[string]interface{}{} + template["spec"] = spec + } + + envVars, ok := spec["environmentVariables"].(map[string]interface{}) + if !ok { + envVars = map[string]interface{}{} + } + + // Set JIRA_READ_ONLY_MODE to false to enable write operations + envVars["JIRA_READ_ONLY_MODE"] = "false" + spec["environmentVariables"] = envVars + + log.Printf("Applied jira-write feature flag: JIRA_READ_ONLY_MODE=false") + } + + return nil +} + // createNewSession creates a new AgenticSession CR (original behavior). func createNewSession(dynamicClient dynamic.Interface, namespace, scheduledSessionName string, template map[string]interface{}) { + // Apply feature flag overrides to template before creating the session + cfg, err := rest.InClusterConfig() + if err != nil { + log.Printf("WARNING: failed to get cluster config for feature flag check: %v", err) + // Continue without feature flags - degraded but not fatal + } else { + k8sClient, err := kubernetes.NewForConfig(cfg) + if err != nil { + log.Printf("WARNING: failed to create k8s client for feature flag check: %v", err) + } else { + // Apply feature flag overrides to template + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if err := applyFeatureFlagOverrides(ctx, k8sClient, namespace, template); err != nil { + log.Printf("WARNING: failed to apply feature flag overrides: %v", err) + } + } + } + // Build session name and display name. // The most restrictive derived K8s resource name is the Service: // "session-" (8 chars) + sessionName ≤ 63 → sessionName ≤ 55 @@ -279,9 +340,9 @@ func createNewSession(dynamicClient dynamic.Interface, namespace, scheduledSessi // Create via dynamic client gvr := types.GetAgenticSessionResource() - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - _, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(ctx, session, metav1.CreateOptions{}) + ctx2, cancel2 := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel2() + _, err = dynamicClient.Resource(gvr).Namespace(namespace).Create(ctx2, session, metav1.CreateOptions{}) if err != nil { log.Fatalf("Failed to create AgenticSession %s in namespace %s: %v", sessionName, namespace, err) } diff --git a/components/operator/internal/trigger/trigger_test.go b/components/operator/internal/trigger/trigger_test.go index f119d2406..133fadd9d 100644 --- a/components/operator/internal/trigger/trigger_test.go +++ b/components/operator/internal/trigger/trigger_test.go @@ -1,7 +1,12 @@ package trigger import ( + "context" "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" ) func TestSanitizeName(t *testing.T) { @@ -104,3 +109,120 @@ func TestSanitizeName_TruncationPreservesValidSuffix(t *testing.T) { t.Errorf("sanitizeName(%q) ends with hyphen: %q", input, result) } } + +func TestApplyFeatureFlagOverrides(t *testing.T) { + tests := []struct { + name string + configMapData map[string]string + existingEnvVars map[string]interface{} + expectedEnvVars map[string]interface{} + }{ + { + name: "jira-write enabled sets JIRA_READ_ONLY_MODE to false", + configMapData: map[string]string{"jira-write": "true"}, + existingEnvVars: nil, + expectedEnvVars: map[string]interface{}{"JIRA_READ_ONLY_MODE": "false"}, + }, + { + name: "jira-write disabled does not set env var", + configMapData: map[string]string{"jira-write": "false"}, + existingEnvVars: nil, + expectedEnvVars: nil, + }, + { + name: "no overrides ConfigMap does not set env var", + configMapData: nil, + existingEnvVars: nil, + expectedEnvVars: nil, + }, + { + name: "preserves existing env vars", + configMapData: map[string]string{"jira-write": "true"}, + existingEnvVars: map[string]interface{}{"CUSTOM_VAR": "value"}, + expectedEnvVars: map[string]interface{}{ + "CUSTOM_VAR": "value", + "JIRA_READ_ONLY_MODE": "false", + }, + }, + { + name: "other flags do not affect env vars", + configMapData: map[string]string{"other-flag": "true"}, + existingEnvVars: nil, + expectedEnvVars: nil, + }, + { + name: "jira-write with non-true value does not set env var", + configMapData: map[string]string{"jira-write": "yes"}, + existingEnvVars: nil, + expectedEnvVars: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create fake K8s client + var k8sClient *fake.Clientset + if tt.configMapData != nil { + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "feature-flag-overrides", + Namespace: "test-namespace", + }, + Data: tt.configMapData, + } + k8sClient = fake.NewSimpleClientset(configMap) + } else { + k8sClient = fake.NewSimpleClientset() + } + + // Create template with optional existing env vars + template := map[string]interface{}{ + "spec": map[string]interface{}{}, + } + if tt.existingEnvVars != nil { + spec := template["spec"].(map[string]interface{}) + spec["environmentVariables"] = tt.existingEnvVars + } + + // Apply feature flag overrides + ctx := context.Background() + err := applyFeatureFlagOverrides(ctx, k8sClient, "test-namespace", template) + if err != nil { + t.Fatalf("applyFeatureFlagOverrides() unexpected error: %v", err) + } + + // Verify environment variables + spec, ok := template["spec"].(map[string]interface{}) + if !ok { + t.Fatal("template[spec] is not a map") + } + + envVars, ok := spec["environmentVariables"].(map[string]interface{}) + if tt.expectedEnvVars == nil { + if envVars != nil && len(envVars) > 0 { + t.Errorf("Expected no environmentVariables, got %v", envVars) + } + return + } + + if !ok { + t.Fatal("spec[environmentVariables] is not a map") + } + + if len(envVars) != len(tt.expectedEnvVars) { + t.Errorf("environmentVariables count = %d, want %d", len(envVars), len(tt.expectedEnvVars)) + } + + for key, expectedVal := range tt.expectedEnvVars { + actualVal, exists := envVars[key] + if !exists { + t.Errorf("environmentVariables[%q] missing, want %q", key, expectedVal) + continue + } + if actualVal != expectedVal { + t.Errorf("environmentVariables[%q] = %q, want %q", key, actualVal, expectedVal) + } + } + }) + } +} From 0f878ef8ba29d98311e05320763652e8c89dd6f3 Mon Sep 17 00:00:00 2001 From: Tomasz Janiszewski Date: Wed, 13 May 2026 17:33:33 +0200 Subject: [PATCH 2/2] refactor(operator): improve code quality in session trigger - Add constants for feature flag strings (FeatureFlagOverridesConfigMap, JiraWriteFlagKey, JiraReadOnlyModeEnvVar) to replace hardcoded values - Reuse config.K8sClient and config.DynamicClient initialized in RunSessionTrigger instead of creating clients on every session - Flatten nested conditionals in createNewSession using early returns - Remove WHAT comments that duplicate code, keeping only WHY comments This addresses code review findings: stringly-typed code, redundant client creation, and unnecessary comments. Co-Authored-By: Claude Sonnet 4.5 --- .../operator/internal/trigger/trigger.go | 100 ++++++------------ .../operator/internal/trigger/trigger_test.go | 4 +- .../operator/internal/types/resources.go | 9 ++ 3 files changed, 45 insertions(+), 68 deletions(-) diff --git a/components/operator/internal/trigger/trigger.go b/components/operator/internal/trigger/trigger.go index 2a99b0459..95808ce72 100644 --- a/components/operator/internal/trigger/trigger.go +++ b/components/operator/internal/trigger/trigger.go @@ -18,9 +18,9 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" "k8s.io/client-go/util/retry" + "ambient-code-operator/internal/config" "ambient-code-operator/internal/types" ) @@ -35,39 +35,28 @@ func RunSessionTrigger() { log.Fatalf("Required environment variables SESSION_TEMPLATE, PROJECT_NAMESPACE, and SCHEDULED_SESSION_NAME must be set") } - // Init K8s client - cfg, err := rest.InClusterConfig() - if err != nil { - log.Fatalf("Failed to get in-cluster config: %v", err) + if err := config.InitK8sClients(); err != nil { + log.Fatalf("Failed to initialize Kubernetes clients: %v", err) } - dynamicClient, err := dynamic.NewForConfig(cfg) - if err != nil { - log.Fatalf("Failed to create dynamic client: %v", err) - } - - // Parse session template var template map[string]interface{} if err := json.Unmarshal([]byte(sessionTemplate), &template); err != nil { log.Fatalf("Failed to parse SESSION_TEMPLATE JSON: %v", err) } - // Check if reuse mode is enabled reuseLastSession := strings.EqualFold(strings.TrimSpace(os.Getenv("REUSE_LAST_SESSION")), "true") if reuseLastSession { - reused, err := tryReuseLastSession(dynamicClient, projectNamespace, scheduledSessionName, template) + reused, err := tryReuseLastSession(config.DynamicClient, projectNamespace, scheduledSessionName, template) if err != nil { - // Don't fall through to create — the reuse may have partially succeeded log.Fatalf("Failed to reuse last session for %s: %v", scheduledSessionName, err) } if reused { return } - // No reusable session found — fall through to create a new one } - createNewSession(dynamicClient, projectNamespace, scheduledSessionName, template) + createNewSession(config.DynamicClient, projectNamespace, scheduledSessionName, template) } // tryReuseLastSession finds the most recent session for this scheduled session and either @@ -246,78 +235,56 @@ func resumeSessionWithPrompt(dynamicClient dynamic.Interface, namespace, session }) } -// applyFeatureFlagOverrides reads workspace feature flag overrides from the -// feature-flag-overrides ConfigMap and applies them to the session template. -// Currently supports: jira-write flag → JIRA_READ_ONLY_MODE environment variable. +// applyFeatureFlagOverrides reads workspace feature flag overrides from ConfigMap +// and applies them to the session template. Non-fatal: logs warnings but continues +// if ConfigMap is missing or read fails (degraded operation acceptable). func applyFeatureFlagOverrides(ctx context.Context, k8sClient kubernetes.Interface, namespace string, template map[string]interface{}) error { - // Read feature-flag-overrides ConfigMap - cm, err := k8sClient.CoreV1().ConfigMaps(namespace).Get(ctx, "feature-flag-overrides", metav1.GetOptions{}) + cm, err := k8sClient.CoreV1().ConfigMaps(namespace).Get(ctx, types.FeatureFlagOverridesConfigMap, metav1.GetOptions{}) if errors.IsNotFound(err) { - // No overrides ConfigMap - this is normal, nothing to do return nil } if err != nil { - // Log warning but don't fail - same pattern as backend log.Printf("WARNING: failed to read feature flag overrides for namespace %s: %v", namespace, err) return nil } - // Check jira-write flag - if val, exists := cm.Data["jira-write"]; exists && val == "true" { - // Get or create environmentVariables map - spec, ok := template["spec"].(map[string]interface{}) - if !ok { - spec = map[string]interface{}{} - template["spec"] = spec - } - - envVars, ok := spec["environmentVariables"].(map[string]interface{}) - if !ok { - envVars = map[string]interface{}{} - } + val, exists := cm.Data[types.JiraWriteFlagKey] + if !exists || val != "true" { + return nil + } - // Set JIRA_READ_ONLY_MODE to false to enable write operations - envVars["JIRA_READ_ONLY_MODE"] = "false" - spec["environmentVariables"] = envVars + spec, ok := template["spec"].(map[string]interface{}) + if !ok { + spec = map[string]interface{}{} + template["spec"] = spec + } - log.Printf("Applied jira-write feature flag: JIRA_READ_ONLY_MODE=false") + envVars, ok := spec["environmentVariables"].(map[string]interface{}) + if !ok { + envVars = map[string]interface{}{} } + envVars[types.JiraReadOnlyModeEnvVar] = "false" + spec["environmentVariables"] = envVars + + log.Printf("Applied jira-write feature flag: %s=false", types.JiraReadOnlyModeEnvVar) return nil } -// createNewSession creates a new AgenticSession CR (original behavior). func createNewSession(dynamicClient dynamic.Interface, namespace, scheduledSessionName string, template map[string]interface{}) { - // Apply feature flag overrides to template before creating the session - cfg, err := rest.InClusterConfig() - if err != nil { - log.Printf("WARNING: failed to get cluster config for feature flag check: %v", err) - // Continue without feature flags - degraded but not fatal - } else { - k8sClient, err := kubernetes.NewForConfig(cfg) - if err != nil { - log.Printf("WARNING: failed to create k8s client for feature flag check: %v", err) - } else { - // Apply feature flag overrides to template - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - if err := applyFeatureFlagOverrides(ctx, k8sClient, namespace, template); err != nil { - log.Printf("WARNING: failed to apply feature flag overrides: %v", err) - } + if config.K8sClient != nil { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if err := applyFeatureFlagOverrides(ctx, config.K8sClient, namespace, template); err != nil { + log.Printf("WARNING: failed to apply feature flag overrides: %v", err) } } - // Build session name and display name. - // The most restrictive derived K8s resource name is the Service: - // "session-" (8 chars) + sessionName ≤ 63 → sessionName ≤ 55 - // sanitizeName caps at 40 chars, so namePrefix + "-" + timestamp (10) - // yields at most 51 chars — well within the 55-char budget. now := time.Now() ts := strconv.FormatInt(now.Unix(), 10) namePrefix := sanitizeName(scheduledSessionName) if dn, ok := template["displayName"].(string); ok && dn != "" { namePrefix = sanitizeName(dn) - // Set display name with human-readable timestamp, e.g. "Daily Jira Summary (Jan 1, 2026 - 00:00:00)" template["displayName"] = fmt.Sprintf("%s (%s)", dn, now.UTC().Format("Jan 2, 2006 - 15:04:05")) } sessionName := fmt.Sprintf("%s-%s", namePrefix, ts) @@ -338,11 +305,10 @@ func createNewSession(dynamicClient dynamic.Interface, namespace, scheduledSessi }, } - // Create via dynamic client gvr := types.GetAgenticSessionResource() - ctx2, cancel2 := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel2() - _, err = dynamicClient.Resource(gvr).Namespace(namespace).Create(ctx2, session, metav1.CreateOptions{}) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, err := dynamicClient.Resource(gvr).Namespace(namespace).Create(ctx, session, metav1.CreateOptions{}) if err != nil { log.Fatalf("Failed to create AgenticSession %s in namespace %s: %v", sessionName, namespace, err) } diff --git a/components/operator/internal/trigger/trigger_test.go b/components/operator/internal/trigger/trigger_test.go index 133fadd9d..2637b4a82 100644 --- a/components/operator/internal/trigger/trigger_test.go +++ b/components/operator/internal/trigger/trigger_test.go @@ -7,6 +7,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" + + "ambient-code-operator/internal/types" ) func TestSanitizeName(t *testing.T) { @@ -165,7 +167,7 @@ func TestApplyFeatureFlagOverrides(t *testing.T) { if tt.configMapData != nil { configMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: "feature-flag-overrides", + Name: types.FeatureFlagOverridesConfigMap, Namespace: "test-namespace", }, Data: tt.configMapData, diff --git a/components/operator/internal/types/resources.go b/components/operator/internal/types/resources.go index af6af1343..d253da4dc 100644 --- a/components/operator/internal/types/resources.go +++ b/components/operator/internal/types/resources.go @@ -11,6 +11,15 @@ const ( // manually on private-CA clusters. See issue #1247. TrustedCABundleConfigMapName = "trusted-ca-bundle" + // FeatureFlagOverridesConfigMap is the ConfigMap containing workspace feature flag overrides + FeatureFlagOverridesConfigMap = "feature-flag-overrides" + + // JiraWriteFlagKey enables write operations for Jira MCP tools + JiraWriteFlagKey = "jira-write" + + // JiraReadOnlyModeEnvVar controls read-only mode for Jira integration + JiraReadOnlyModeEnvVar = "JIRA_READ_ONLY_MODE" + // CopiedFromAnnotation is the annotation key used to track secrets copied by the operator CopiedFromAnnotation = "vteam.ambient-code/copied-from" )