Skip to content

Commit ebd03a9

Browse files
mergify[bot]mergify[bot]
authored
Merge branch 'main' into automated/sdk-version-bump
2 parents fa40447 + d37bc2c commit ebd03a9

6 files changed

Lines changed: 398 additions & 9 deletions

File tree

‎components/ambient-control-plane/internal/reconciler/kube_reconciler.go‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,8 @@ func (r *SimpleKubeReconciler) ensurePod(ctx context.Context, namespace string,
584584
credentialSidecarMode := false
585585
var credTmpVolumes []interface{}
586586
if r.cfg.CPTokenURL != "" && r.cfg.CPTokenPublicKey != "" {
587-
credSidecars, credMCPURLs, credTmpVols := r.buildCredentialSidecars(session.ID, namespace, credentialIDs)
587+
sessionEnvVars := parseSessionEnvVars(session.EnvironmentVariables)
588+
credSidecars, credMCPURLs, credTmpVols := r.buildCredentialSidecars(session.ID, namespace, credentialIDs, sessionEnvVars)
588589
credTmpVolumes = credTmpVols
589590
containers = append(containers, credSidecars...)
590591
if len(credMCPURLs) > 0 {
@@ -1193,7 +1194,21 @@ func (r *SimpleKubeReconciler) credentialSidecarImage(provider string) string {
11931194
}
11941195
}
11951196

1196-
func (r *SimpleKubeReconciler) buildCredentialSidecars(sessionID string, namespace string, credentialIDs map[string]string) ([]interface{}, map[string]string, []interface{}) {
1197+
// parseSessionEnvVars deserialises the JSON-encoded environment variables stored
1198+
// on a Session by the ambient-api-server into a plain map. Returns an empty
1199+
// map on any parse error so callers can proceed safely.
1200+
func parseSessionEnvVars(raw string) map[string]string {
1201+
if raw == "" {
1202+
return map[string]string{}
1203+
}
1204+
out := map[string]string{}
1205+
if err := json.Unmarshal([]byte(raw), &out); err != nil {
1206+
return map[string]string{}
1207+
}
1208+
return out
1209+
}
1210+
1211+
func (r *SimpleKubeReconciler) buildCredentialSidecars(sessionID string, namespace string, credentialIDs map[string]string, sessionEnvVars map[string]string) ([]interface{}, map[string]string, []interface{}) {
11971212
var sidecars []interface{}
11981213
var tmpVolumes []interface{}
11991214
mcpURLs := map[string]string{}
@@ -1242,6 +1257,17 @@ func (r *SimpleKubeReconciler) buildCredentialSidecars(sessionID string, namespa
12421257
env = append(env, envVar("MPP_CONFIG_NAMESPACE", r.cfg.MPPConfigNamespace))
12431258
}
12441259

1260+
// For the Jira sidecar, propagate JIRA_READ_ONLY_MODE from the session's
1261+
// environment variables. The runner sets this to "false" when the
1262+
// jira-write feature flag is enabled; the mcp-atlassian server honours
1263+
// READ_ONLY_MODE so write tools (e.g. jira_add_comment) are only
1264+
// exposed when explicitly enabled.
1265+
if provider == "jira" {
1266+
if v, ok := sessionEnvVars["JIRA_READ_ONLY_MODE"]; ok {
1267+
env = append(env, envVar("JIRA_READ_ONLY_MODE", v))
1268+
}
1269+
}
1270+
12451271
sidecar := map[string]interface{}{
12461272
"name": spec.Name,
12471273
"image": image,

‎components/ambient-control-plane/internal/reconciler/kube_reconciler_test.go‎

Lines changed: 162 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import (
55
"testing"
66
)
77

8+
// noSessionEnv is a convenience alias for tests that don't need session env vars.
9+
var noSessionEnv = map[string]string{}
10+
811
func TestBuildCredentialSidecars_NoCredentials(t *testing.T) {
912
r := &SimpleKubeReconciler{cfg: KubeReconcilerConfig{}}
10-
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", map[string]string{})
13+
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", map[string]string{}, noSessionEnv)
1114
if len(sidecars) != 0 {
1215
t.Errorf("expected 0 sidecars, got %d", len(sidecars))
1316
}
@@ -19,7 +22,7 @@ func TestBuildCredentialSidecars_NoCredentials(t *testing.T) {
1922
func TestBuildCredentialSidecars_NoImageConfigured(t *testing.T) {
2023
r := &SimpleKubeReconciler{cfg: KubeReconcilerConfig{}}
2124
credentialIDs := map[string]string{"github": "cred-123"}
22-
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs)
25+
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs, noSessionEnv)
2326
if len(sidecars) != 0 {
2427
t.Errorf("expected 0 sidecars (no image configured), got %d", len(sidecars))
2528
}
@@ -40,7 +43,7 @@ func TestBuildCredentialSidecars_GitHubSidecar(t *testing.T) {
4043
r.logger = r.logger.With().Logger()
4144

4245
credentialIDs := map[string]string{"github": "cred-123"}
43-
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs)
46+
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs, noSessionEnv)
4447

4548
if len(sidecars) != 1 {
4649
t.Fatalf("expected 1 sidecar, got %d", len(sidecars))
@@ -97,7 +100,7 @@ func TestBuildCredentialSidecars_MultipleSidecars(t *testing.T) {
97100
"kubeconfig": "cred-3",
98101
"google": "cred-4",
99102
}
100-
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs)
103+
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs, noSessionEnv)
101104

102105
if len(sidecars) != 4 {
103106
t.Fatalf("expected 4 sidecars, got %d", len(sidecars))
@@ -124,7 +127,7 @@ func TestBuildCredentialSidecars_UnknownProvider(t *testing.T) {
124127
r.logger = r.logger.With().Logger()
125128

126129
credentialIDs := map[string]string{"unknown-provider": "cred-999"}
127-
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs)
130+
sidecars, urls, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs, noSessionEnv)
128131

129132
if len(sidecars) != 0 {
130133
t.Errorf("expected 0 sidecars for unknown provider, got %d", len(sidecars))
@@ -143,7 +146,7 @@ func TestBuildCredentialSidecars_LocalImagePullPolicy(t *testing.T) {
143146
r.logger = r.logger.With().Logger()
144147

145148
credentialIDs := map[string]string{"github": "cred-123"}
146-
sidecars, _, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs)
149+
sidecars, _, _ := r.buildCredentialSidecars("test-session", "test-namespace", credentialIDs, noSessionEnv)
147150

148151
if len(sidecars) != 1 {
149152
t.Fatalf("expected 1 sidecar, got %d", len(sidecars))
@@ -176,3 +179,156 @@ func TestCredentialMCPURLsJSON(t *testing.T) {
176179
t.Error("round-trip failed for jira")
177180
}
178181
}
182+
183+
// ---------------------------------------------------------------------------
184+
// Tests for JIRA_READ_ONLY_MODE propagation (Issue #1506)
185+
// ---------------------------------------------------------------------------
186+
187+
func findEnvVar(env []interface{}, name string) (string, bool) {
188+
for _, item := range env {
189+
m, ok := item.(map[string]interface{})
190+
if !ok {
191+
continue
192+
}
193+
if m["name"] == name {
194+
val, _ := m["value"].(string)
195+
return val, true
196+
}
197+
}
198+
return "", false
199+
}
200+
201+
func TestBuildCredentialSidecars_JiraReadOnlyMode_SetToFalse(t *testing.T) {
202+
// When jira-write is enabled, JIRA_READ_ONLY_MODE=false must be passed to
203+
// the Jira sidecar so mcp-atlassian exposes write tools.
204+
r := &SimpleKubeReconciler{
205+
cfg: KubeReconcilerConfig{
206+
JiraMCPImage: "jira-mcp:latest",
207+
MCPAPIServerURL: "http://api.svc:8000",
208+
CPTokenURL: "http://cp.svc:8080",
209+
CPTokenPublicKey: "test-key",
210+
},
211+
}
212+
r.logger = r.logger.With().Logger()
213+
214+
credentialIDs := map[string]string{"jira": "cred-42"}
215+
sessionEnv := map[string]string{"JIRA_READ_ONLY_MODE": "false"}
216+
217+
sidecars, _, _ := r.buildCredentialSidecars("test-session", "test-ns", credentialIDs, sessionEnv)
218+
219+
if len(sidecars) != 1 {
220+
t.Fatalf("expected 1 sidecar, got %d", len(sidecars))
221+
}
222+
223+
sidecar := sidecars[0].(map[string]interface{})
224+
env := sidecar["env"].([]interface{})
225+
226+
val, found := findEnvVar(env, "JIRA_READ_ONLY_MODE")
227+
if !found {
228+
t.Fatal("JIRA_READ_ONLY_MODE not found in Jira sidecar env")
229+
}
230+
if val != "false" {
231+
t.Errorf("expected JIRA_READ_ONLY_MODE=false, got %q", val)
232+
}
233+
}
234+
235+
func TestBuildCredentialSidecars_JiraReadOnlyMode_NotSetByDefault(t *testing.T) {
236+
// When jira-write is not enabled, JIRA_READ_ONLY_MODE must NOT be injected
237+
// (mcp-atlassian defaults to read-only, which is the safe default).
238+
r := &SimpleKubeReconciler{
239+
cfg: KubeReconcilerConfig{
240+
JiraMCPImage: "jira-mcp:latest",
241+
MCPAPIServerURL: "http://api.svc:8000",
242+
CPTokenURL: "http://cp.svc:8080",
243+
CPTokenPublicKey: "test-key",
244+
},
245+
}
246+
r.logger = r.logger.With().Logger()
247+
248+
credentialIDs := map[string]string{"jira": "cred-42"}
249+
250+
sidecars, _, _ := r.buildCredentialSidecars("test-session", "test-ns", credentialIDs, noSessionEnv)
251+
252+
if len(sidecars) != 1 {
253+
t.Fatalf("expected 1 sidecar, got %d", len(sidecars))
254+
}
255+
256+
sidecar := sidecars[0].(map[string]interface{})
257+
env := sidecar["env"].([]interface{})
258+
259+
_, found := findEnvVar(env, "JIRA_READ_ONLY_MODE")
260+
if found {
261+
t.Error("JIRA_READ_ONLY_MODE should not be present when jira-write is disabled")
262+
}
263+
}
264+
265+
func TestBuildCredentialSidecars_JiraReadOnly_NotPropagatedToGitHub(t *testing.T) {
266+
// JIRA_READ_ONLY_MODE must only be injected into the Jira sidecar, never
267+
// into other provider sidecars (e.g. GitHub).
268+
r := &SimpleKubeReconciler{
269+
cfg: KubeReconcilerConfig{
270+
GitHubMCPImage: "github-mcp:latest",
271+
JiraMCPImage: "jira-mcp:latest",
272+
MCPAPIServerURL: "http://api.svc:8000",
273+
CPTokenURL: "http://cp.svc:8080",
274+
CPTokenPublicKey: "test-key",
275+
},
276+
}
277+
r.logger = r.logger.With().Logger()
278+
279+
credentialIDs := map[string]string{"github": "cred-1", "jira": "cred-2"}
280+
sessionEnv := map[string]string{"JIRA_READ_ONLY_MODE": "false"}
281+
282+
sidecars, _, _ := r.buildCredentialSidecars("test-session", "test-ns", credentialIDs, sessionEnv)
283+
284+
if len(sidecars) != 2 {
285+
t.Fatalf("expected 2 sidecars, got %d", len(sidecars))
286+
}
287+
288+
for _, item := range sidecars {
289+
sidecar := item.(map[string]interface{})
290+
name := sidecar["name"].(string)
291+
if name == "credential-github" {
292+
env := sidecar["env"].([]interface{})
293+
if _, found := findEnvVar(env, "JIRA_READ_ONLY_MODE"); found {
294+
t.Errorf("JIRA_READ_ONLY_MODE must not be injected into GitHub sidecar")
295+
}
296+
}
297+
}
298+
}
299+
300+
// ---------------------------------------------------------------------------
301+
// Tests for parseSessionEnvVars
302+
// ---------------------------------------------------------------------------
303+
304+
func TestParseSessionEnvVars_ValidJSON(t *testing.T) {
305+
raw := `{"JIRA_READ_ONLY_MODE":"false","FOO":"bar"}`
306+
got := parseSessionEnvVars(raw)
307+
if got["JIRA_READ_ONLY_MODE"] != "false" {
308+
t.Errorf("expected false, got %q", got["JIRA_READ_ONLY_MODE"])
309+
}
310+
if got["FOO"] != "bar" {
311+
t.Errorf("expected bar, got %q", got["FOO"])
312+
}
313+
}
314+
315+
func TestParseSessionEnvVars_Empty(t *testing.T) {
316+
got := parseSessionEnvVars("")
317+
if len(got) != 0 {
318+
t.Errorf("expected empty map, got %v", got)
319+
}
320+
}
321+
322+
func TestParseSessionEnvVars_InvalidJSON(t *testing.T) {
323+
got := parseSessionEnvVars("not-json")
324+
if len(got) != 0 {
325+
t.Errorf("expected empty map on parse error, got %v", got)
326+
}
327+
}
328+
329+
func TestParseSessionEnvVars_EmptyObject(t *testing.T) {
330+
got := parseSessionEnvVars("{}")
331+
if len(got) != 0 {
332+
t.Errorf("expected empty map, got %v", got)
333+
}
334+
}

‎components/backend/handlers/sessions.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,10 @@ func CreateSession(c *gin.Context) {
984984
}
985985

986986
// When the jira-write flag is enabled for this workspace, let the Jira MCP server write.
987-
overrides, err := getWorkspaceOverrides(c.Request.Context(), reqK8s, project)
987+
// Use the backend service-account client (K8sClient) — not the user's token — because
988+
// reading the feature-flag ConfigMap is a server-side concern and the user's token may
989+
// lack configmaps.get permission, causing intermittent RBAC failures.
990+
overrides, err := getWorkspaceOverrides(c.Request.Context(), K8sClient, project)
988991
if err != nil {
989992
log.Printf("WARNING: failed to read feature flag overrides for project %s: %v", project, err)
990993
}

‎components/runners/ambient-runner/ambient_runner/endpoints/workflow.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,12 @@ async def clone_workflow_at_runtime(
149149
logger.warning(f"Subpath '{subpath}' not found, using entire repo")
150150
if workflow_final.exists():
151151
shutil.rmtree(workflow_final)
152+
workflow_final.parent.mkdir(parents=True, exist_ok=True)
152153
shutil.move(str(temp_dir), str(workflow_final))
153154
else:
154155
if workflow_final.exists():
155156
shutil.rmtree(workflow_final)
157+
workflow_final.parent.mkdir(parents=True, exist_ok=True)
156158
shutil.move(str(temp_dir), str(workflow_final))
157159

158160
logger.info(f"Workflow '{workflow_name}' ready at {workflow_final}")

0 commit comments

Comments
 (0)