Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .ai/spec/what/sandbox-execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes
7. **[OLS-3066] Input delivery — ConfigMap**: For each step invocation, the operator MUST create a namespaced `ConfigMap` in the operator namespace with owner reference to the `AgenticRun` (`controller: true`, `blockOwnerDeletion: true`). Name pattern `ls-input-{step}-{run}` truncated to 63 chars. The ConfigMap MUST contain four keys: `query` (rendered prompt text from the step template), `output-schema` (JSON schema computed by `outputSchemaForStep`), `context` (JSON object with targetNamespaces, previousAttempts, approvedOption, executionResult as applicable), and `result-template` (pre-filled Result CR JSON — see rule 7a). The ConfigMap MUST be mounted read-only into the sandbox pod at `/input/`. Creation MUST be idempotent (`AlreadyExists` = no-op).
7a. **[OLS-3066] Result template**: The `result-template` key MUST contain a JSON object with complete `apiVersion`, `kind`, `metadata` (name, namespace, labels, ownerReferences including AgenticRun UID), and `spec` (agenticRunName, retryIndex for execution/verification). The operator pre-computes all metadata — the sandbox only fills in `status` fields. CR naming MUST use the existing `resultCRName` convention. Owner references MUST use the current AgenticRun UID.
8. **[OLS-3066] Output delivery — Result CR via `oc`**: The sandbox MUST create the Result CR in two steps: (a) `oc create -f <result.json>` using the pre-filled template with `spec` fields, then (b) `oc patch <resultCR> --type=merge --subresource=status` with the agent output in `status` fields (options, diagnosis, actionRequired, actionsTaken, checks, conditions, failureReason as applicable per step). The Result CR `status.conditions` MUST include a `Completed` condition set to `True` as part of the status patch — this is the operator's readiness signal (see rule 8b). On agent failure, the sandbox MUST still create the Result CR with `status.failureReason` populated and exit 0 (the sandbox succeeded; the agent failed). On sandbox failure (cannot read input, `oc create` fails, etc.), the sandbox MUST write an error message to `/dev/termination-log` (max 4096 bytes) and exit non-zero.
8a. **[OLS-3066] Sandbox RBAC for Result CRs**: The sandbox ServiceAccount (`lightspeed-agent` for analysis/verification/escalation, per-run SA for execution) MUST have `create` and `patch` (with `status` subresource) permissions on `AnalysisResult`, `ExecutionResult`, `VerificationResult`, and `EscalationResult` in the AgenticRun namespace.
8a. **[OLS-3066] Sandbox RBAC for Result CRs**: Verification, escalation, and execution sandbox identities MUST have the narrow Result permissions required by their phases in the AgenticRun namespace. The per-run analysis identity MUST NOT receive Result write permission. Before asynchronous analysis is enabled, its Result delivery MUST use a separately enforced expected-result-name publisher path as required by rule 20a.
8b. **[OLS-3066] Result CR readiness signal**: The operator MUST only process a Result CR when its `status.conditions` includes `Completed=True`. A Result CR without this condition indicates the sandbox has called `oc create` but has not yet patched the status — the operator MUST wait for the status update (which triggers another `Owns()` watch event). This guards against the race between `oc create` and `oc patch --subresource=status`.
9. **[OLS-3066] Watch-driven async**: The controller MUST use watch-based event delivery instead of synchronous polling. `SetupWithManager` MUST `Owns()` Pods (bare-pod mode), SandboxClaims (sandbox-claim mode), ConfigMaps, and all Result CR types (AnalysisResult, ExecutionResult, VerificationResult, EscalationResult). Pod watches are for **failure detection only** (Pod `Failed`, `ImagePullBackOff`). Result CR watches are for **completion detection** (Result CR created with `Completed` condition). Every in-progress step MUST return `RequeueAfter(30s)` as a safety net for missed watch events.
10. **Output schema selection**: The `output-schema` key in the input ConfigMap MUST be the step-specific JSON schema computed by the operator: analysis schema depends on `spec.analysisOutput.mode`, whether execution/verification steps exist in the run, and optional injected `components` sub-schema from `spec.analysisOutput.schema`; other steps use fixed schemas for their response shapes.
Expand Down Expand Up @@ -52,7 +52,8 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes
19b. [PLANNED: OLS-3594] **OLSConfig / classic-operator handoff** for introspection and connection details is deferred with 19a (includes OLS-3572).
19c. [PLANNED: OLS-3697] **RHOKP endpoint and CA** are included in the handoff ConfigMap (`lightspeed-sandbox-config`) when OKP is enabled. The RHOKP CA cert volume/mount is part of the base `sandbox-pod-spec` PodSpec; the `rhokp-endpoint` key provides the HTTPS URL. Sandbox pods can query OKP documentation directly via the standalone RHOKP Service.
20. **Sandbox observability patch**: Immediately after creating a claim (or Pod in `bare-pod` mode), the controller MUST patch `AgenticRun.status.steps.<step>.sandbox` with the resource name and operator namespace so consoles/CLIs can tail logs before the sandbox is ready. In `bare-pod` mode, `status.steps.<step>.sandbox.claimName` MUST be set to the Pod name (same field used for SandboxClaim names in `sandbox-claim` mode).
21. **Execution RBAC materialization**: When the approved remediation option includes RBAC requests, before execution the controller MUST create a **per-run ServiceAccount** named `ls-exec-{run-namespace}-{run-name}` in the operator namespace (truncated to 63 chars), then create namespace-scoped `Role`+`RoleBinding` pairs in each target namespace and `ClusterRole`+`ClusterRoleBinding` for cluster-scoped rules, binding subjects to **this per-run SA** (not the shared `lightspeed-agent` SA). The per-run SA MUST NOT carry an owner reference — cross-namespace owner refs are unsupported by Kubernetes GC (the SA lives in the operator namespace while the AgenticRun may be in a different namespace). Cleanup is handled explicitly by `cleanupExecutionRBAC` via the AgenticRun's finalizer. Idempotent create MUST tolerate existing objects (`AlreadyExists` is a no-op). Only execution sandbox pods use this SA — analysis and verification pods continue using `lightspeed-agent`. The operator's `cluster-admin` privilege (external prerequisite) ensures no RBAC escalation issues when creating arbitrary Roles/SAs. See the workspace-level agentic security spec at `ols/.ai/spec/what/agentic-security.md` rules 7-15 for full specification.
20a. **Analysis identity isolation**: Before analysis, the controller MUST create a UID-qualified per-run ServiceAccount in the operator namespace. A RoleBinding to the built-in `view` ClusterRole MUST grant that identity read access only in the AgenticRun namespace; `spec.targetNamespaces` MUST NOT widen analysis access because the AgenticRun API does not carry a verified creator identity. A supplemental Role MAY grant only `pods/log` get. The identity MUST receive no ClusterRoleBinding, Secret permission, or namespace-wide Result write permission. A future batch sandbox MUST use a separately enforced, expected-result-name write path rather than widening this identity. It MUST be deleted after analysis and on failure, suspension, terminal reconciliation, or AgenticRun deletion.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
21. **Execution RBAC materialization**: When the approved remediation option includes RBAC requests, before execution the controller MUST create a **per-run ServiceAccount** named `ls-exec-{run-namespace}-{run-name}` in the operator namespace (truncated to 63 chars), then create namespace-scoped `Role`+`RoleBinding` pairs in each target namespace and `ClusterRole`+`ClusterRoleBinding` for cluster-scoped rules, binding subjects to **this per-run SA** (not the shared `lightspeed-agent` SA). The per-run SA MUST NOT carry an owner reference — cross-namespace owner refs are unsupported by Kubernetes GC (the SA lives in the operator namespace while the AgenticRun may be in a different namespace). Cleanup is handled explicitly by `cleanupExecutionRBAC` via the AgenticRun's finalizer. Idempotent create MUST tolerate existing objects (`AlreadyExists` is a no-op). Only execution sandbox pods use this SA — analysis uses its own per-run identity, while verification continues using `lightspeed-agent`. The operator's `cluster-admin` privilege (external prerequisite) ensures no RBAC escalation issues when creating arbitrary Roles/SAs. See the workspace-level agentic security spec at `ols/.ai/spec/what/agentic-security.md` rules 7-15 for full specification.
21a. **Read RBAC multi-binding resolution**: The shared `lightspeed-agent` SA may have **multiple** `ClusterRoleBinding`s granting read access (e.g. `cluster-reader` and `cluster-monitoring-view`). When granting read permissions to a per-run execution SA, the controller MUST discover **all** `ClusterRoleBinding`s where `lightspeed-agent` is a subject and add the per-run SA to every one of them — not just the first match. Discovery results MUST be cached for the lifetime of the process (the infrastructure CRBs are static). On cleanup, the per-run SA MUST be removed from all discovered bindings. [OLS-3712]
21b. **Execution outcome override**: When the execution agent reports `success=false` but **all mutating actions succeeded**, the controller MUST NOT hard-fail the run. Instead it MUST override the execution outcome to `Succeeded` and proceed to the verification step (or trust-mode completion if verification is absent). The controller determines mutation success by inspecting `ExecutionAction` entries: observation action types (`pre-check`, `post-check`, `verification`, `check`, `wait`) are excluded; all remaining action types are considered mutations. The run MUST hard-fail only when (a) no mutating actions exist (agent never attempted the fix), or (b) any mutating action has `outcome=Failed`. [OLS-3558]
22. **RBAC subjects namespace**: RoleBindings MUST reference the **per-run service account** in the **operator namespace** (where sandbox pods run), even when roles live in target namespaces.
Expand Down Expand Up @@ -96,9 +97,9 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes
34. **[OLS-3066] No endpoint construction**: With the batch execution model, the operator does not construct agent HTTP URLs. There is no HTTP communication between operator and sandbox. Input is delivered via ConfigMap mount (rule 7); output is delivered via Result CR creation (rule 8).
35. **Sandbox release**: In `bare-pod` mode, `Release` deletes the Pod by name. In `sandbox-claim` mode, `Release` deletes both the SandboxClaim and its SandboxTemplate (same name). Both paths are idempotent — NotFound is treated as success.
36. **PodSpecBuilder**: `PodSpecBuilder` takes a base `*corev1.PodSpec` (from the config cache) and overlays agent-specific configuration: LLM env vars, credential mounts, skills volumes, MCP config, required secrets, input ConfigMap volume mount [OLS-3066], SA token mounting, security context. It produces a single typed `corev1.PodSpec`. In `bare-pod` mode, this PodSpec is used directly to create a Pod. In `sandbox-claim` mode, it is converted to an unstructured map and embedded in a `SandboxTemplate`. [OLS-3066] HTTP readiness/liveness probes are no longer set (see rule 30).
36a. **SA token mounting**: `PodSpecBuilder` MUST set `AutomountServiceAccountToken=true` for **all** steps. The correct ServiceAccount is already resolved upstream (per-run SA for execution, shared `lightspeed-agent` for analysis/verification/escalation), and the pod needs the projected token to authenticate `oc`/`kubectl` commands against the cluster API regardless of step.
36a. **SA token mounting**: `PodSpecBuilder` MUST set `AutomountServiceAccountToken=true` for **all** steps. The correct ServiceAccount is already resolved upstream: analysis uses its per-run SA; execution uses a per-run SA only when requested RBAC rules are materialized and otherwise uses `lightspeed-agent`; verification and escalation use `lightspeed-agent`. The pod needs the projected token to authenticate `oc`/`kubectl` commands against the cluster API regardless of step.
37. **RBAC**: The operator's ClusterRole MUST include Pod create/delete/get/list/watch verbs (bare-pod mode) AND SandboxClaim, SandboxTemplate, Sandbox resource verbs (sandbox-claim mode).
38. **Bootstrap**: The operator MUST unconditionally create the `lightspeed-agent` ServiceAccount in the operator namespace at startup (idempotent). This SA is used by analysis/verification/escalation steps — execution uses per-run SAs per rule 21. The `lightspeed-agent` SA MUST NOT have any execution-level Roles or ClusterRoles bound to it. No base `SandboxTemplate` is created at bootstrap — templates are built per-run by `SandboxManager`.
38. **Bootstrap**: The operator MUST unconditionally create the `lightspeed-agent` ServiceAccount in the operator namespace at startup (idempotent). This SA is used by verification and escalation, plus execution when no requested RBAC rules require a per-run SA. Analysis always uses its per-run SA; execution with requested RBAC uses a separate per-run SA per rules 20a and 21. The `lightspeed-agent` SA MUST NOT have any execution-level Roles or ClusterRoles bound to it. No base `SandboxTemplate` is created at bootstrap — templates are built per-run by `SandboxManager`.

## Configuration Surface

Expand Down
14 changes: 14 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ rules:
- get
- list
- watch
- apiGroups:
- ""
resources:
- pods/log
verbs:
- get
- apiGroups:
- ""
resources:
Expand Down Expand Up @@ -134,6 +140,14 @@ rules:
- get
- list
- update
- apiGroups:
- rbac.authorization.k8s.io
resourceNames:
- view
resources:
- clusterroles
verbs:
- bind
- apiGroups:
- rbac.authorization.k8s.io
resources:
Expand Down
125 changes: 125 additions & 0 deletions controller/agenticrun/analysis_identity.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package agenticrun

import (
"context"
"crypto/sha256"
"fmt"
"strings"

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"

agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1"
)

const (
analysisViewClusterRole = "view"

ErrCreateAnalysisSA = "create analysis SA"
ErrCreateAnalysisViewBinding = "create analysis view RoleBinding"
ErrCreateAnalysisLogRole = "create analysis log Role"
ErrCreateAnalysisLogBinding = "create analysis log RoleBinding"
ErrDeleteAnalysisViewBinding = "delete analysis view RoleBinding"
ErrDeleteAnalysisLogBinding = "delete analysis log RoleBinding"
ErrDeleteAnalysisLogRole = "delete analysis log Role"
ErrDeleteAnalysisSA = "delete analysis SA"
)

func analysisIdentityName(run *agenticv1alpha1.AgenticRun) string {
digest := fmt.Sprintf("%x", sha256.Sum256([]byte(run.Namespace+"\x00"+run.Name+"\x00"+string(run.UID))))[:8]
prefix := strings.TrimRight(fmt.Sprintf("ls-analysis-%s-%s", run.Namespace, run.Name), "-._")
const maxPrefixLength = 47 // 47 + '-' + 8 leaves room for RoleBinding suffixes.
if len(prefix) > maxPrefixLength {
prefix = strings.TrimRight(prefix[:maxPrefixLength], "-._")
}
return prefix + "-" + digest
}

// ensureAnalysisIdentity creates a per-run ServiceAccount in the operator
// namespace. It can read only the AgenticRun namespace through the built-in
// view ClusterRole, plus failed-step pod logs.
func ensureAnalysisIdentity(ctx context.Context, c client.Client, run *agenticv1alpha1.AgenticRun, operatorNS string) (string, error) {
name := analysisIdentityName(run)
labels := rbacLabels(run.Name, "analysis-identity")
subjects := []rbacv1.Subject{{
Kind: rbacv1.ServiceAccountKind,
Name: name,
Namespace: operatorNS,
}}

resources := []struct {
object client.Object
errMsg string
}{
{
object: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: operatorNS, Labels: labels},
AutomountServiceAccountToken: ptr.To(false),
},
errMsg: ErrCreateAnalysisSA,
},
{
object: &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: name + "-view", Namespace: run.Namespace, Labels: labels},
RoleRef: rbacv1.RoleRef{
APIGroup: rbacv1.GroupName,
Kind: "ClusterRole",
Name: analysisViewClusterRole,
},
Subjects: subjects,
},
errMsg: ErrCreateAnalysisViewBinding,
},
{
object: &rbacv1.Role{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: run.Namespace, Labels: labels},
Rules: []rbacv1.PolicyRule{
{APIGroups: []string{""}, Resources: []string{"pods/log"}, Verbs: []string{"get"}},
},
},
errMsg: ErrCreateAnalysisLogRole,
},
{
object: &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: name + "-logs", Namespace: run.Namespace, Labels: labels},
RoleRef: rbacv1.RoleRef{
APIGroup: rbacv1.GroupName,
Kind: "Role",
Name: name,
},
Subjects: subjects,
},
errMsg: ErrCreateAnalysisLogBinding,
},
}

for _, resource := range resources {
if err := c.Create(ctx, resource.object); err != nil && !apierrors.IsAlreadyExists(err) {
return "", fmt.Errorf("%s %s: %w", resource.errMsg, name, err)
}
}
return name, nil
}

func cleanupAnalysisIdentity(ctx context.Context, c client.Client, run *agenticv1alpha1.AgenticRun, operatorNS string) error {
name := analysisIdentityName(run)
resources := []struct {
object client.Object
errMsg string
}{
{&rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: name + "-view", Namespace: run.Namespace}}, ErrDeleteAnalysisViewBinding},
{&rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: name + "-logs", Namespace: run.Namespace}}, ErrDeleteAnalysisLogBinding},
{&rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: run.Namespace}}, ErrDeleteAnalysisLogRole},
{&corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: operatorNS}}, ErrDeleteAnalysisSA},
}
for _, resource := range resources {
if err := deleteIfExists(ctx, c, resource.object); err != nil {
return fmt.Errorf("%s %s: %w", resource.errMsg, name, err)
}
}
return nil
}
Loading