diff --git a/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml b/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml index d64afb0a..49176751 100644 --- a/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml +++ b/charts/kagenti-operator/crds/agent.kagenti.dev_agentruntimes.yaml @@ -160,52 +160,6 @@ spec: - permissive - strict type: string - skills: - description: |- - Skills declares OCI skill images to mount into the agent pod as - Kubernetes ImageVolumes. Each skill is mounted read-only at - /agent/skills//. Requires the skillImageVolumes feature gate - and Kubernetes 1.31+ with the ImageVolume feature gate enabled. - items: - description: SkillImageRef identifies an OCI skill image to mount - into the agent pod. - properties: - image: - description: Image is the OCI image reference for the skill. - minLength: 1 - type: string - mountPath: - description: |- - MountPath is the absolute path where the skill image is mounted in - the container. Different agent frameworks expect skills in different - locations (e.g. /agent/skills/my-skill, /app/.claude/skills/my-skill). - minLength: 1 - pattern: ^/.* - type: string - name: - description: |- - Name is a unique identifier for this skill mount, used as the volume - name suffix (skill-). - maxLength: 58 - minLength: 1 - pattern: ^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$ - type: string - pullPolicy: - description: |- - PullPolicy for pulling the OCI skill image. Defaults to Always for - :latest tags and IfNotPresent otherwise (standard Kubernetes behavior). - enum: - - Always - - Never - - IfNotPresent - type: string - required: - - image - - mountPath - - name - type: object - maxItems: 20 - type: array targetRef: description: TargetRef identifies the workload backing this agent runtime (duck typing). @@ -523,6 +477,15 @@ spec: description: ConfiguredPods is the count of pods with expected labels/config format: int32 type: integer + linkedSkills: + description: |- + LinkedSkills lists skill names discovered from the kagenti.io/skills + annotation on the target workload. This annotation is set by the + kagenti backend (PR #1440) or manually by the user. The operator + reads but never sets this annotation. + items: + type: string + type: array phase: description: Phase is the high-level state of the AgentRuntime enum: diff --git a/charts/kagenti-operator/values.yaml b/charts/kagenti-operator/values.yaml index 0e670a8c..f9fcc90c 100644 --- a/charts/kagenti-operator/values.yaml +++ b/charts/kagenti-operator/values.yaml @@ -159,10 +159,10 @@ featureGates: # Default false — cached mode is faster and sufficient when namespace ConfigMaps # rarely change. Cache is cleared on webhook pod restart. perWorkloadConfigResolution: false - # skillImageVolumes controls whether AgentRuntime can mount OCI skill images - # as Kubernetes ImageVolumes into agent pods. Requires Kubernetes 1.31+ with - # the ImageVolume feature gate enabled. Default false. - skillImageVolumes: false + # skillDiscovery controls whether the AgentRuntime controller reads the + # kagenti.io/skills annotation from target workloads and populates + # status.linkedSkills. Default false. + skillDiscovery: false # Platform defaults for AuthBridge sidecar injection. # These are the lowest-priority layer — overridden by feature gates, diff --git a/kagenti-operator/api/v1alpha1/agentruntime_types.go b/kagenti-operator/api/v1alpha1/agentruntime_types.go index 72c81b11..b192be5a 100644 --- a/kagenti-operator/api/v1alpha1/agentruntime_types.go +++ b/kagenti-operator/api/v1alpha1/agentruntime_types.go @@ -122,14 +122,6 @@ type AgentRuntimeSpec struct { // +optional // +kubebuilder:validation:Enum=disabled;permissive;strict MTLSMode string `json:"mtlsMode,omitempty"` - - // Skills declares OCI skill images to mount into the agent pod as - // Kubernetes ImageVolumes. Each skill is mounted read-only at - // /agent/skills//. Requires the skillImageVolumes feature gate - // and Kubernetes 1.31+ with the ImageVolume feature gate enabled. - // +optional - // +kubebuilder:validation:MaxItems=20 - Skills []SkillImageRef `json:"skills,omitempty"` } // IdentitySpec configures workload identity for an AgentRuntime. @@ -194,41 +186,6 @@ type CardStatus struct { AttestedAgentSpiffeID string `json:"attestedAgentSpiffeID,omitempty"` } -// +kubebuilder:validation:Enum=Always;Never;IfNotPresent -type SkillPullPolicy string - -const ( - SkillPullAlways SkillPullPolicy = "Always" - SkillPullNever SkillPullPolicy = "Never" - SkillPullIfNotPresent SkillPullPolicy = "IfNotPresent" -) - -// SkillImageRef identifies an OCI skill image to mount into the agent pod. -type SkillImageRef struct { - // Name is a unique identifier for this skill mount, used as the volume - // name suffix (skill-). - // +kubebuilder:validation:MinLength=1 - // +kubebuilder:validation:MaxLength=58 - // +kubebuilder:validation:Pattern=`^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$` - Name string `json:"name"` - - // Image is the OCI image reference for the skill. - // +kubebuilder:validation:MinLength=1 - Image string `json:"image"` - - // MountPath is the absolute path where the skill image is mounted in - // the container. Different agent frameworks expect skills in different - // locations (e.g. /agent/skills/my-skill, /app/.claude/skills/my-skill). - // +kubebuilder:validation:MinLength=1 - // +kubebuilder:validation:Pattern=`^/.*` - MountPath string `json:"mountPath"` - - // PullPolicy for pulling the OCI skill image. Defaults to Always for - // :latest tags and IfNotPresent otherwise (standard Kubernetes behavior). - // +optional - PullPolicy SkillPullPolicy `json:"pullPolicy,omitempty"` -} - // AgentRuntimeStatus defines the observed state of AgentRuntime. type AgentRuntimeStatus struct { // Phase is the high-level state of the AgentRuntime @@ -243,6 +200,13 @@ type AgentRuntimeStatus struct { // +optional Card *CardStatus `json:"card,omitempty"` + // LinkedSkills lists skill names discovered from the kagenti.io/skills + // annotation on the target workload. This annotation is set by the + // kagenti backend (PR #1440) or manually by the user. The operator + // reads but never sets this annotation. + // +optional + LinkedSkills []string `json:"linkedSkills,omitempty"` + // Conditions represent the current state of the AgentRuntime // +optional Conditions []metav1.Condition `json:"conditions,omitempty"` diff --git a/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go b/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go index d49ba544..5a87d25b 100644 --- a/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/kagenti-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -377,11 +377,6 @@ func (in *AgentRuntimeSpec) DeepCopyInto(out *AgentRuntimeSpec) { *out = new(IdentitySpec) (*in).DeepCopyInto(*out) } - if in.Skills != nil { - in, out := &in.Skills, &out.Skills - *out = make([]SkillImageRef, len(*in)) - copy(*out, *in) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AgentRuntimeSpec. @@ -402,6 +397,11 @@ func (in *AgentRuntimeStatus) DeepCopyInto(out *AgentRuntimeStatus) { *out = new(CardStatus) (*in).DeepCopyInto(*out) } + if in.LinkedSkills != nil { + in, out := &in.LinkedSkills, &out.LinkedSkills + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) @@ -577,21 +577,6 @@ func (in *SignatureHeader) DeepCopy() *SignatureHeader { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SkillImageRef) DeepCopyInto(out *SkillImageRef) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SkillImageRef. -func (in *SkillImageRef) DeepCopy() *SkillImageRef { - if in == nil { - return nil - } - out := new(SkillImageRef) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SkillParameter) DeepCopyInto(out *SkillParameter) { *out = *in diff --git a/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml b/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml index d64afb0a..49176751 100644 --- a/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml +++ b/kagenti-operator/config/crd/bases/agent.kagenti.dev_agentruntimes.yaml @@ -160,52 +160,6 @@ spec: - permissive - strict type: string - skills: - description: |- - Skills declares OCI skill images to mount into the agent pod as - Kubernetes ImageVolumes. Each skill is mounted read-only at - /agent/skills//. Requires the skillImageVolumes feature gate - and Kubernetes 1.31+ with the ImageVolume feature gate enabled. - items: - description: SkillImageRef identifies an OCI skill image to mount - into the agent pod. - properties: - image: - description: Image is the OCI image reference for the skill. - minLength: 1 - type: string - mountPath: - description: |- - MountPath is the absolute path where the skill image is mounted in - the container. Different agent frameworks expect skills in different - locations (e.g. /agent/skills/my-skill, /app/.claude/skills/my-skill). - minLength: 1 - pattern: ^/.* - type: string - name: - description: |- - Name is a unique identifier for this skill mount, used as the volume - name suffix (skill-). - maxLength: 58 - minLength: 1 - pattern: ^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$ - type: string - pullPolicy: - description: |- - PullPolicy for pulling the OCI skill image. Defaults to Always for - :latest tags and IfNotPresent otherwise (standard Kubernetes behavior). - enum: - - Always - - Never - - IfNotPresent - type: string - required: - - image - - mountPath - - name - type: object - maxItems: 20 - type: array targetRef: description: TargetRef identifies the workload backing this agent runtime (duck typing). @@ -523,6 +477,15 @@ spec: description: ConfiguredPods is the count of pods with expected labels/config format: int32 type: integer + linkedSkills: + description: |- + LinkedSkills lists skill names discovered from the kagenti.io/skills + annotation on the target workload. This annotation is set by the + kagenti backend (PR #1440) or manually by the user. The operator + reads but never sets this annotation. + items: + type: string + type: array phase: description: Phase is the high-level state of the AgentRuntime enum: diff --git a/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_full.yaml b/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_full.yaml index b003652e..2e758753 100644 --- a/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_full.yaml +++ b/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_full.yaml @@ -1,6 +1,5 @@ # Full AgentRuntime: enroll a Deployment as an agent with per-workload overrides. -# Overrides the SPIFFE trust domain, and mounts OCI skill images -# (requires skillImageVolumes feature gate + K8s 1.31+). +# Overrides the SPIFFE trust domain for this workload. apiVersion: agent.kagenti.dev/v1alpha1 kind: AgentRuntime metadata: @@ -17,7 +16,3 @@ spec: identity: spiffe: trustDomain: custom.example.com - skills: - - name: weather-forecast - image: ghcr.io/redhat-et/skillimage/weather-forecast:v1.0.0 - mountPath: /agent/skills/weather-forecast diff --git a/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_skill_discovery.yaml b/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_skill_discovery.yaml new file mode 100644 index 00000000..2bdb5113 --- /dev/null +++ b/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_skill_discovery.yaml @@ -0,0 +1,76 @@ +# AgentRuntime with skill discovery: reads the kagenti.io/skills annotation +# from the target workload to discover linked skills. +# Requires: skillDiscovery feature gate enabled. +# +# Skills are declared on the Deployment, not the AgentRuntime. +# Two delivery mechanisms are supported: +# +# 1. OCI ImageVolumes (user-managed, GitOps-friendly, K8s 1.31+): +# volumes: +# - name: skill-openshift-review +# image: +# reference: quay.io/myorg/openshift-review:1.0.0 +# +# 2. ConfigMap volumes (kagenti backend, PR #1440): +# volumes: +# - name: skill-0 +# configMap: +# name: summarizer +# +# Both paths set SKILL_FOLDERS so the agent discovers skills at startup. +# The agent reports discovered skills in its A2A card. +# The operator reads the kagenti.io/skills annotation and populates +# status.linkedSkills. +--- +# Example Deployment with OCI skill ImageVolume +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-agent + labels: + app.kubernetes.io/name: my-agent + kagenti.io/type: agent + protocol.kagenti.io/a2a: "" + annotations: + kagenti.io/skills: '["openshift-review"]' +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/name: my-agent + template: + metadata: + labels: + app.kubernetes.io/name: my-agent + kagenti.io/type: agent + spec: + containers: + - name: agent + image: ghcr.io/kagenti/agent-examples/a2a_currency_converter:v0.1.0-alpha.1 + ports: + - containerPort: 8000 + env: + - name: SKILL_FOLDERS + value: /app/skills/openshift-review + volumeMounts: + - name: skill-openshift-review + mountPath: /app/skills/openshift-review + readOnly: true + volumes: + - name: skill-openshift-review + image: + reference: quay.io/myorg/openshift-review:1.0.0 +--- +# AgentRuntime observes the Deployment — no spec.skills needed +apiVersion: agent.kagenti.dev/v1alpha1 +kind: AgentRuntime +metadata: + name: my-agent + labels: + app.kubernetes.io/name: my-agent +spec: + type: agent + targetRef: + apiVersion: apps/v1 + kind: Deployment + name: my-agent diff --git a/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_skills.yaml b/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_skills.yaml deleted file mode 100644 index 5852bf4b..00000000 --- a/kagenti-operator/config/samples/agent_v1alpha1_agentruntime_skills.yaml +++ /dev/null @@ -1,23 +0,0 @@ -# AgentRuntime with OCI skill images: mounts skill OCI images as ImageVolumes. -# Requires: skillImageVolumes feature gate enabled, Kubernetes 1.31+. -apiVersion: agent.kagenti.dev/v1alpha1 -kind: AgentRuntime -metadata: - name: resume-agent-runtime - namespace: default - labels: - app.kubernetes.io/name: resume-agent -spec: - type: agent - targetRef: - apiVersion: apps/v1 - kind: Deployment - name: resume-agent - skills: - - name: resume-reviewer - image: ghcr.io/redhat-et/skillimage/resume-reviewer:v1.0.0 - mountPath: /agent/skills/resume-reviewer - - name: blog-writer - image: ghcr.io/redhat-et/skillimage/blog-writer:latest - mountPath: /agent/skills/blog-writer - pullPolicy: Always diff --git a/kagenti-operator/docs/api-reference.md b/kagenti-operator/docs/api-reference.md index fa4030a2..9f8d9807 100644 --- a/kagenti-operator/docs/api-reference.md +++ b/kagenti-operator/docs/api-reference.md @@ -404,7 +404,7 @@ The controller merges configuration from three layers (highest priority wins): 2. **Namespace defaults** — ConfigMap with `kagenti.io/defaults=true` label in the workload's namespace 3. **Cluster defaults** — `kagenti-platform-config` ConfigMap in `kagenti-system` -> **Note:** Feature gates (`kagenti-feature-gates`) are platform-wide policy and are **not** overrideable by namespace defaults or AgentRuntime CRs. They control which AuthBridge components (envoy proxy, SPIFFE helper, client registration) are enabled globally, and whether OCI skill image mounting (`skillImageVolumes`) is active. +> **Note:** Feature gates (`kagenti-feature-gates`) are platform-wide policy and are **not** overrideable by namespace defaults or AgentRuntime CRs. They control which AuthBridge components (envoy proxy, SPIFFE helper, client registration) are enabled globally, and whether skill discovery (`skillDiscovery`) is active. ### Spec Fields @@ -413,7 +413,6 @@ The controller merges configuration from three layers (highest priority wins): | `type` | string | Yes | Classifies the workload as `agent` or `tool` | | `targetRef` | [TargetRef](#targetref) | Yes | Identifies the workload backing this runtime (uses the same TargetRef type as AgentCard) | | `identity` | [IdentitySpec](#identityspec) | No | Optional per-workload identity overrides | -| `skills` | [][SkillImageRef](#skillimageref) | No | OCI skill images to mount into the agent pod as Kubernetes ImageVolumes. Requires the `skillImageVolumes` feature gate and Kubernetes 1.31+. Max 20 items. | #### IdentitySpec @@ -429,30 +428,6 @@ Configures workload identity for an AgentRuntime. |-------|------|----------|-------------| | `trustDomain` | string | No | Overrides the operator-level `--spire-trust-domain` for this workload. If empty, the operator flag value is used. Must match pattern: `^[a-zA-Z0-9]([a-zA-Z0-9\-\.]*[a-zA-Z0-9])?$` | -#### SkillImageRef - -Identifies an OCI skill image to mount into the agent pod as a Kubernetes [ImageVolume](https://kubernetes.io/docs/tasks/configure-pod-container/image-volumes/). Skills are packaged as OCI images following the [skillimage](https://github.com/redhat-et/skillimage) convention (`FROM scratch` with `skill.yaml` + `SKILL.md`). - -| Field | Type | Required | Description | -|-------|------|----------|-------------| -| `name` | string | Yes | Unique identifier for this skill mount. Used as the volume name suffix (`skill-`). Must be a valid DNS label (lowercase alphanumeric or hyphens, max 58 characters). | -| `image` | string | Yes | OCI image reference for the skill (e.g., `ghcr.io/redhat-et/skillimage/resume-reviewer:v1.0.0`) | -| `mountPath` | string | Yes | Absolute path where the skill image is mounted in the container. Different agent frameworks expect skills in different locations (e.g., `/agent/skills/my-skill`, `/app/.claude/skills/my-skill`). | -| `pullPolicy` | string | No | Image pull policy: `Always`, `Never`, or `IfNotPresent`. Defaults to `Always` for `:latest` tags, `IfNotPresent` otherwise (standard Kubernetes behavior). | - -**Prerequisites:** -- The `skillImageVolumes` feature gate must be enabled (defaults to `false`) -- Kubernetes 1.31+ with the `ImageVolume` feature gate enabled on the kubelet -- OpenShift 4.18+ (for OpenShift deployments) - -**Behavior:** -- Each skill is mounted as a read-only ImageVolume at the specified `mountPath` -- Skill changes (add, remove, image update, mount path change) trigger rolling updates via config-hash -- Skill volumes use the `skill-` prefix and do not interfere with existing ConfigMap, Secret, or CSI volumes -- The operator sets a `kagenti.io/skills` annotation on the target workload's metadata containing a JSON array of skill names (e.g., `["weather-forecast","resume-reviewer"]`). Downstream systems such as agent card controllers or the Kagenti UI can read this annotation to discover which skills are mounted without inspecting the pod spec. The annotation is removed when skills are cleared or the AgentRuntime is deleted. -- On AgentRuntime deletion, all skill volumes and the `kagenti.io/skills` annotation are removed from the target workload -- If the feature gate is disabled but skills are defined, a `SkillsMounted=False` condition is set with reason `FeatureGateDisabled` - ### Labels and Annotations Applied to Target Workloads The AgentRuntime controller applies the following labels and annotations to the target workload: @@ -468,7 +443,7 @@ The AgentRuntime controller applies the following labels and annotations to the | Annotation | Value | Description | |------------|-------|-------------| -| `kagenti.io/skills` | JSON array of skill names | Lists mounted skill names (e.g., `["weather-forecast","resume-reviewer"]`). Only set when the `skillImageVolumes` feature gate is enabled and skills are defined. Removed when skills are cleared or on AgentRuntime deletion. | +| `kagenti.io/skills` | JSON array of skill names | Read by the operator (not set by it) to discover linked skills. Set by the kagenti backend or the user. Value is a JSON array (e.g., `["weather-forecast","resume-reviewer"]`). Populates `status.linkedSkills` when the `skillDiscovery` feature gate is enabled. | **PodTemplateSpec labels:** @@ -501,9 +476,7 @@ The AgentRuntime controller applies the following labels and annotations to the | `Ready` | True | `Configured` | Labels and config-hash applied to the target workload | | `Ready` | False | `ConfigHashError` | Failed to compute the config hash | | `Ready` | False | `ConfigApplyError` | Failed to apply labels/annotations to the workload | -| `SkillsMounted` | True | `SkillsApplied` | OCI skill ImageVolumes applied to the target workload | -| `SkillsMounted` | False | `FeatureGateDisabled` | Skills defined but `skillImageVolumes` feature gate is disabled | -| `SkillsMounted` | False | `UnsupportedWorkloadKind` | Skills defined but the target workload kind (e.g., Sandbox) does not support skill ImageVolumes | +| `SkillsDiscovered` | True | `SkillsFound` | Linked skills discovered from `kagenti.io/skills` annotation on the target workload | ### Admission Validation @@ -569,9 +542,9 @@ spec: name: calculator-tool ``` -#### Agent Runtime with OCI Skill Images +#### Agent Runtime with Skill Discovery -Mount OCI-packaged skills into the agent pod. Requires the `skillImageVolumes` feature gate enabled in the `kagenti-feature-gates` ConfigMap and Kubernetes 1.31+. +When the `skillDiscovery` feature gate is enabled, the operator reads the `kagenti.io/skills` annotation from the target workload and populates `status.linkedSkills`. ```yaml apiVersion: agent.kagenti.dev/v1alpha1 @@ -585,23 +558,15 @@ spec: apiVersion: apps/v1 kind: Deployment name: resume-agent - skills: - - name: resume-reviewer - image: ghcr.io/redhat-et/skillimage/resume-reviewer:v1.0.0 - mountPath: /agent/skills/resume-reviewer - - name: blog-writer - image: ghcr.io/redhat-et/skillimage/blog-writer:latest - mountPath: /agent/skills/blog-writer - pullPolicy: Always ``` -To enable the feature gate: +To enable skill discovery: ```yaml # In the kagenti-feature-gates ConfigMap (kagenti-system namespace) # or via Helm values: featureGates: - skillImageVolumes: true + skillDiscovery: true ``` ### kubectl Usage Examples diff --git a/kagenti-operator/docs/architecture.md b/kagenti-operator/docs/architecture.md index 6c6a123d..4b3096ad 100644 --- a/kagenti-operator/docs/architecture.md +++ b/kagenti-operator/docs/architecture.md @@ -70,10 +70,9 @@ The Kagenti Operator is a Kubernetes controller that implements the [Operator Pa - Watches AgentRuntime CRs, Deployments, StatefulSets, and ConfigMaps - Applies `kagenti.io/type` label and `kagenti.io/config-hash` annotation to target workloads - Computes config hash from 3-layer merged configuration (cluster defaults → namespace defaults → CR overrides) -- Mounts OCI skill images as Kubernetes ImageVolumes when the `skillImageVolumes` feature gate is enabled (see [SkillImageRef](api-reference.md#skillimageref)) -- Sets `kagenti.io/skills` annotation on target workload metadata with mounted skill names for downstream discovery -- Triggers rolling updates when configuration changes (any layer, including skill additions/removals) -- On CR deletion: preserves type label, updates config-hash to defaults-only, removes managed-by label, skill volumes, and `kagenti.io/skills` annotation +- Discovers linked skills by reading the `kagenti.io/skills` annotation from target workloads when the `skillDiscovery` feature gate is enabled +- Triggers rolling updates when configuration changes +- On CR deletion: preserves type label, updates config-hash to defaults-only, removes managed-by label - Coordinates with the AuthBridge mutating webhook (in-process) which injects sidecars at Pod CREATE time ### Supporting Components diff --git a/kagenti-operator/internal/controller/agentruntime_config.go b/kagenti-operator/internal/controller/agentruntime_config.go index 15b0fb12..50567823 100644 --- a/kagenti-operator/internal/controller/agentruntime_config.go +++ b/kagenti-operator/internal/controller/agentruntime_config.go @@ -86,15 +86,7 @@ type resolvedConfig struct { // (single-digit agents) this is fine; in larger deployments, // formatting / whitespace edits to this CM during peak hours will // trigger a noticeable rollout fan-out. - AuthBridgeRuntime string `json:"authBridgeRuntime,omitempty"` - Skills []skillConfig `json:"skills,omitempty"` -} - -type skillConfig struct { - Name string `json:"name"` - Image string `json:"image"` - MountPath string `json:"mountPath"` - PullPolicy string `json:"pullPolicy,omitempty"` + AuthBridgeRuntime string `json:"authBridgeRuntime,omitempty"` } // ConfigResult holds the computed hash and any warnings from the config resolution. @@ -174,15 +166,6 @@ func resolveConfig(ctx context.Context, c client.Reader, namespace string, spec resolved.AuthBridgeMode = spec.AuthBridgeMode resolved.MTLSMode = spec.MTLSMode - for _, s := range spec.Skills { - resolved.Skills = append(resolved.Skills, skillConfig{ - Name: s.Name, - Image: s.Image, - MountPath: s.MountPath, - PullPolicy: string(s.PullPolicy), - }) - } - return resolved, warnings } diff --git a/kagenti-operator/internal/controller/agentruntime_controller.go b/kagenti-operator/internal/controller/agentruntime_controller.go index be824ffe..27a146e8 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller.go +++ b/kagenti-operator/internal/controller/agentruntime_controller.go @@ -58,8 +58,8 @@ const ( // AnnotationConfigHash is the annotation applied to PodTemplateSpec to trigger rolling updates. AnnotationConfigHash = "kagenti.io/config-hash" - // AnnotationSkills is the annotation applied to workload metadata to advertise - // which skill images are mounted. Value is a JSON array of skill names. + // AnnotationSkills is read from target workloads to discover linked skills. + // Value is a JSON array of skill names, set by the kagenti backend or the user. AnnotationSkills = "kagenti.io/skills" // AnnotationRestartPending marks a Sandbox that was scaled to 0 and needs @@ -68,10 +68,11 @@ const ( AnnotationRestartPending = "kagenti.io/restart-pending" // Condition types for AgentRuntime status. - ConditionTypeReady = "Ready" - ConditionTypeTargetResolved = "TargetResolved" - ConditionTypeConfigResolved = "ConfigResolved" - ConditionTypeCardFetched = "CardFetched" + ConditionTypeReady = "Ready" + ConditionTypeTargetResolved = "TargetResolved" + ConditionTypeConfigResolved = "ConfigResolved" + ConditionTypeCardFetched = "CardFetched" + ConditionTypeSkillsDiscovered = "SkillsDiscovered" // AnnotationLastCardFetchHash stores the change-detection key used to skip // redundant card fetches when the workload's pod template has not changed. @@ -223,29 +224,19 @@ func (r *AgentRuntimeReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } - // 6.5. Set SkillsMounted condition based on skills and feature gate state - if len(rt.Spec.Skills) > 0 { - fg := r.getFeatureGates() - if !fg.SkillImageVolumes { - r.setCondition(rt, ConditionTypeSkillsMounted, metav1.ConditionFalse, "FeatureGateDisabled", - "Skills defined but skillImageVolumes feature gate is disabled") - if r.Recorder != nil { - r.Recorder.Event(rt, corev1.EventTypeWarning, "SkillsNotMounted", - "skillImageVolumes feature gate is disabled; enable it to mount OCI skill images") - } - } else if rt.Spec.TargetRef.Kind == KindSandbox { - r.setCondition(rt, ConditionTypeSkillsMounted, metav1.ConditionFalse, "UnsupportedWorkloadKind", - "Sandbox workloads do not support skill ImageVolumes") - if r.Recorder != nil { - r.Recorder.Event(rt, corev1.EventTypeWarning, "SkillsNotMounted", - "Sandbox workloads do not support skill ImageVolumes") - } + // 6.5. Discover linked skills from workload annotation (set by kagenti backend or user) + fg := r.getFeatureGates() + if fg.SkillDiscovery { + rt.Status.LinkedSkills = r.readLinkedSkills(ctx, rt) + if len(rt.Status.LinkedSkills) > 0 { + r.setCondition(rt, ConditionTypeSkillsDiscovered, metav1.ConditionTrue, "SkillsFound", + fmt.Sprintf("%d linked skill(s) discovered from workload annotation", len(rt.Status.LinkedSkills))) } else { - r.setCondition(rt, ConditionTypeSkillsMounted, metav1.ConditionTrue, "SkillsApplied", - fmt.Sprintf("%d skill image(s) mounted", len(rt.Spec.Skills))) + meta.RemoveStatusCondition(&rt.Status.Conditions, ConditionTypeSkillsDiscovered) } } else { - meta.RemoveStatusCondition(&rt.Status.Conditions, ConditionTypeSkillsMounted) + rt.Status.LinkedSkills = nil + meta.RemoveStatusCondition(&rt.Status.Conditions, ConditionTypeSkillsDiscovered) } // 7. Count configured pods @@ -343,27 +334,6 @@ func (r *AgentRuntimeReconciler) applyWorkloadConfig(ctx context.Context, rt *ag workloadLabels[LabelManagedBy] = LabelManagedByValue acc.obj.SetLabels(workloadLabels) - // Advertise mounted skills on workload metadata - workloadAnnotations := acc.obj.GetAnnotations() - if workloadAnnotations == nil { - workloadAnnotations = make(map[string]string) - } - fg := r.getFeatureGates() - if fg.SkillImageVolumes && len(rt.Spec.Skills) > 0 { - names := make([]string, 0, len(rt.Spec.Skills)) - for _, s := range rt.Spec.Skills { - names = append(names, s.Name) - } - b, err := json.Marshal(names) - if err != nil { - logger.Error(err, "failed to marshal skill names") - } - workloadAnnotations[AnnotationSkills] = string(b) - } else { - delete(workloadAnnotations, AnnotationSkills) - } - acc.obj.SetAnnotations(workloadAnnotations) - // Apply labels to PodTemplateSpec podLabels := acc.getPodLabels(acc.obj) if podLabels == nil { @@ -380,13 +350,6 @@ func (r *AgentRuntimeReconciler) applyWorkloadConfig(ctx context.Context, rt *ag podAnnotations[AnnotationConfigHash] = configHash acc.setPodAnnotations(acc.obj, podAnnotations) - // Apply skill ImageVolumes when feature gate is enabled - if acc.getPodSpec != nil { - if fg.SkillImageVolumes { - reconcileSkillVolumes(acc.getPodSpec(acc.obj), rt.Spec.Skills) - } - } - logger.Info("Applying config to workload", "workload", ref.Name, "kind", ref.Kind, @@ -510,6 +473,47 @@ func (r *AgentRuntimeReconciler) countConfiguredPods(ctx context.Context, rt *ag return count, nil } +// readLinkedSkills reads the kagenti.io/skills annotation from the target +// workload and returns the skill names. This annotation is set by the kagenti +// backend (PR #1440) or manually by the user — the operator reads but never +// sets it. +func (r *AgentRuntimeReconciler) readLinkedSkills(ctx context.Context, rt *agentv1alpha1.AgentRuntime) []string { + logger := log.FromContext(ctx) + ref := rt.Spec.TargetRef + + acc, ok := newRuntimePodTemplateAccessor(ref.Kind) + if !ok { + return nil + } + + key := types.NamespacedName{Name: ref.Name, Namespace: rt.Namespace} + if err := r.Get(ctx, key, acc.obj); err != nil { + logger.V(1).Info("Failed to read workload for skill annotation", "error", err) + return nil + } + + annotations := acc.obj.GetAnnotations() + if annotations == nil { + return nil + } + + raw, ok := annotations[AnnotationSkills] + if !ok || raw == "" { + return nil + } + + var skills []string + if err := json.Unmarshal([]byte(raw), &skills); err != nil { + logger.V(1).Info("Failed to parse kagenti.io/skills annotation", "error", err, "raw", raw) + if r.Recorder != nil { + r.Recorder.Event(rt, corev1.EventTypeWarning, "SkillAnnotationParseError", + fmt.Sprintf("Failed to parse kagenti.io/skills annotation: %v", err)) + } + return nil + } + return skills +} + // resolveServiceForWorkload finds the Service that fronts the target workload. // It first tries a Service with the same name as the Deployment (standard convention), // then falls back to selector matching against the Deployment's pod template labels. @@ -689,20 +693,11 @@ func (r *AgentRuntimeReconciler) handleDeletion(ctx context.Context, rt *agentv1 podAnnotations[AnnotationConfigHash] = defaultsHash acc.setPodAnnotations(acc.obj, podAnnotations) - // Remove managed-by label and skills annotation from workload metadata + // Remove managed-by label from workload metadata workloadLabels := acc.obj.GetLabels() delete(workloadLabels, LabelManagedBy) acc.obj.SetLabels(workloadLabels) - workloadAnnotations := acc.obj.GetAnnotations() - delete(workloadAnnotations, AnnotationSkills) - acc.obj.SetAnnotations(workloadAnnotations) - - // Remove skill volumes on deletion - if acc.getPodSpec != nil { - reconcileSkillVolumes(acc.getPodSpec(acc.obj), nil) - } - logger.Info("Updated workload to defaults-only config on AgentRuntime deletion", "workload", ref.Name, "kind", ref.Kind) return r.Update(ctx, acc.obj) diff --git a/kagenti-operator/internal/controller/agentruntime_controller_test.go b/kagenti-operator/internal/controller/agentruntime_controller_test.go index 09c09a09..66f8b7ae 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller_test.go +++ b/kagenti-operator/internal/controller/agentruntime_controller_test.go @@ -182,96 +182,32 @@ var _ = Describe("AgentRuntime Controller", func() { }) }) - Context("When skills annotation is set on workload metadata", func() { - It("should set kagenti.io/skills when feature gate is enabled", func() { - dep := newDeployment("skills-anno-deploy", namespace) - Expect(k8sClient.Create(ctx, dep)).To(Succeed()) - defer func() { _ = k8sClient.Delete(ctx, dep) }() - - rt := newAgentRuntime("skills-anno-rt", namespace, "skills-anno-deploy", agentv1alpha1.RuntimeTypeAgent) - rt.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "weather-forecast", Image: "ghcr.io/example/weather:v1", MountPath: "/agent/skills/weather-forecast"}, - {Name: "resume-reviewer", Image: "ghcr.io/example/resume:v1", MountPath: "/agent/skills/resume-reviewer"}, + Context("When kagenti.io/skills annotation exists on workload", func() { + It("should read linked skills into status", func() { + dep := newDeployment("skills-read-deploy", namespace) + dep.Annotations = map[string]string{ + AnnotationSkills: `["summarizer","translator"]`, } - Expect(k8sClient.Create(ctx, rt)).To(Succeed()) - defer func() { _ = k8sClient.Delete(ctx, rt) }() - - r := newReconciler() - r.GetFeatureGates = func() *webhookconfig.FeatureGates { - return &webhookconfig.FeatureGates{SkillImageVolumes: true} - } - nn := types.NamespacedName{Name: "skills-anno-rt", Namespace: namespace} - - _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) - _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) - Expect(err).NotTo(HaveOccurred()) - - updatedDep := &appsv1.Deployment{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "skills-anno-deploy", Namespace: namespace}, updatedDep)).To(Succeed()) - - Expect(updatedDep.Annotations).To(HaveKey(AnnotationSkills)) - Expect(updatedDep.Annotations[AnnotationSkills]).To(Equal(`["weather-forecast","resume-reviewer"]`)) - }) - - It("should not set kagenti.io/skills when feature gate is disabled", func() { - dep := newDeployment("skills-anno-off-deploy", namespace) Expect(k8sClient.Create(ctx, dep)).To(Succeed()) defer func() { _ = k8sClient.Delete(ctx, dep) }() - rt := newAgentRuntime("skills-anno-off-rt", namespace, "skills-anno-off-deploy", agentv1alpha1.RuntimeTypeAgent) - rt.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "weather-forecast", Image: "ghcr.io/example/weather:v1", MountPath: "/agent/skills/weather-forecast"}, - } + rt := newAgentRuntime("skills-read-rt", namespace, "skills-read-deploy", agentv1alpha1.RuntimeTypeAgent) Expect(k8sClient.Create(ctx, rt)).To(Succeed()) defer func() { _ = k8sClient.Delete(ctx, rt) }() - r := newReconciler() - nn := types.NamespacedName{Name: "skills-anno-off-rt", Namespace: namespace} - - _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) - _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) - Expect(err).NotTo(HaveOccurred()) - - updatedDep := &appsv1.Deployment{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "skills-anno-off-deploy", Namespace: namespace}, updatedDep)).To(Succeed()) - - Expect(updatedDep.Annotations).NotTo(HaveKey(AnnotationSkills)) - }) - - It("should remove kagenti.io/skills on deletion", func() { - dep := newDeployment("skills-anno-del-deploy", namespace) - Expect(k8sClient.Create(ctx, dep)).To(Succeed()) - defer func() { _ = k8sClient.Delete(ctx, dep) }() - - rt := newAgentRuntime("skills-anno-del-rt", namespace, "skills-anno-del-deploy", agentv1alpha1.RuntimeTypeAgent) - rt.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "weather-forecast", Image: "ghcr.io/example/weather:v1", MountPath: "/agent/skills/weather-forecast"}, - } - Expect(k8sClient.Create(ctx, rt)).To(Succeed()) - r := newReconciler() r.GetFeatureGates = func() *webhookconfig.FeatureGates { - return &webhookconfig.FeatureGates{SkillImageVolumes: true} + return &webhookconfig.FeatureGates{SkillDiscovery: true} } - nn := types.NamespacedName{Name: "skills-anno-del-rt", Namespace: namespace} + nn := types.NamespacedName{Name: "skills-read-rt", Namespace: namespace} _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) - _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) - - // Verify annotation is present - depBefore := &appsv1.Deployment{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "skills-anno-del-deploy", Namespace: namespace}, depBefore)).To(Succeed()) - Expect(depBefore.Annotations).To(HaveKey(AnnotationSkills)) - - // Delete AgentRuntime - Expect(k8sClient.Delete(ctx, rt)).To(Succeed()) _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) Expect(err).NotTo(HaveOccurred()) - // Verify annotation is removed - depAfter := &appsv1.Deployment{} - Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "skills-anno-del-deploy", Namespace: namespace}, depAfter)).To(Succeed()) - Expect(depAfter.Annotations).NotTo(HaveKey(AnnotationSkills)) + updatedRT := &agentv1alpha1.AgentRuntime{} + Expect(k8sClient.Get(ctx, nn, updatedRT)).To(Succeed()) + Expect(updatedRT.Status.LinkedSkills).To(ConsistOf("summarizer", "translator")) }) }) diff --git a/kagenti-operator/internal/controller/agentruntime_skills.go b/kagenti-operator/internal/controller/agentruntime_skills.go deleted file mode 100644 index 2a7542dd..00000000 --- a/kagenti-operator/internal/controller/agentruntime_skills.go +++ /dev/null @@ -1,100 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controller - -import ( - "strings" - - corev1 "k8s.io/api/core/v1" - - agentv1alpha1 "github.com/kagenti/operator/api/v1alpha1" -) - -const ( - SkillVolumePrefix = "skill-" - - ConditionTypeSkillsMounted = "SkillsMounted" -) - -// reconcileSkillVolumes declaratively reconciles skill ImageVolumes in a PodSpec. -// It adds volumes/mounts for desired skills and removes any stale skill-prefixed -// volumes no longer in the desired list. Pass nil to remove all skill volumes. -func reconcileSkillVolumes(podSpec *corev1.PodSpec, desiredSkills []agentv1alpha1.SkillImageRef) { - desired := make(map[string]agentv1alpha1.SkillImageRef, len(desiredSkills)) - for _, s := range desiredSkills { - desired[SkillVolumePrefix+s.Name] = s - } - - var keptVolumes []corev1.Volume - for _, v := range podSpec.Volumes { - if !strings.HasPrefix(v.Name, SkillVolumePrefix) { - keptVolumes = append(keptVolumes, v) - continue - } - if skill, ok := desired[v.Name]; ok { - keptVolumes = append(keptVolumes, buildSkillVolume(skill)) - delete(desired, v.Name) - } - } - for _, skill := range desiredSkills { - volName := SkillVolumePrefix + skill.Name - if _, ok := desired[volName]; ok { - keptVolumes = append(keptVolumes, buildSkillVolume(skill)) - } - } - podSpec.Volumes = keptVolumes - - desiredMountNames := make(map[string]bool, len(desiredSkills)) - for _, s := range desiredSkills { - desiredMountNames[SkillVolumePrefix+s.Name] = true - } - if len(podSpec.Containers) > 0 { - reconcileSkillMounts(&podSpec.Containers[0], desiredSkills, desiredMountNames) - } -} - -func buildSkillVolume(skill agentv1alpha1.SkillImageRef) corev1.Volume { - return corev1.Volume{ - Name: SkillVolumePrefix + skill.Name, - VolumeSource: corev1.VolumeSource{ - Image: &corev1.ImageVolumeSource{ - Reference: skill.Image, - PullPolicy: corev1.PullPolicy(skill.PullPolicy), - }, - }, - } -} - -func reconcileSkillMounts(container *corev1.Container, desiredSkills []agentv1alpha1.SkillImageRef, desiredMountNames map[string]bool) { - var keptMounts []corev1.VolumeMount - for _, m := range container.VolumeMounts { - if strings.HasPrefix(m.Name, SkillVolumePrefix) && !desiredMountNames[m.Name] { - continue - } - if !strings.HasPrefix(m.Name, SkillVolumePrefix) { - keptMounts = append(keptMounts, m) - } - } - for _, s := range desiredSkills { - keptMounts = append(keptMounts, corev1.VolumeMount{ - Name: SkillVolumePrefix + s.Name, - MountPath: s.MountPath, - ReadOnly: true, - }) - } - container.VolumeMounts = keptMounts -} diff --git a/kagenti-operator/internal/controller/agentruntime_skills_test.go b/kagenti-operator/internal/controller/agentruntime_skills_test.go deleted file mode 100644 index d31c0e39..00000000 --- a/kagenti-operator/internal/controller/agentruntime_skills_test.go +++ /dev/null @@ -1,309 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controller - -import ( - "context" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - - agentv1alpha1 "github.com/kagenti/operator/api/v1alpha1" -) - -var _ = Describe("Skill Volume Reconciliation", func() { - Context("reconcileSkillVolumes", func() { - It("should add skill volumes to an empty PodSpec", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{Name: "agent", Image: "test:latest"}}, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "resume-reviewer", Image: "ghcr.io/example/resume-reviewer:v1.0.0", MountPath: "/agent/skills/resume-reviewer"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Volumes).To(HaveLen(1)) - Expect(podSpec.Volumes[0].Name).To(Equal("skill-resume-reviewer")) - Expect(podSpec.Volumes[0].VolumeSource.Image).NotTo(BeNil()) - Expect(podSpec.Volumes[0].VolumeSource.Image.Reference).To(Equal("ghcr.io/example/resume-reviewer:v1.0.0")) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(1)) - Expect(podSpec.Containers[0].VolumeMounts[0].Name).To(Equal("skill-resume-reviewer")) - Expect(podSpec.Containers[0].VolumeMounts[0].MountPath).To(Equal("/agent/skills/resume-reviewer")) - Expect(podSpec.Containers[0].VolumeMounts[0].ReadOnly).To(BeTrue()) - }) - - It("should preserve existing non-skill volumes", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{ - Name: "agent", - Image: "test:latest", - VolumeMounts: []corev1.VolumeMount{ - {Name: "config", MountPath: "/etc/config"}, - }, - }}, - Volumes: []corev1.Volume{ - {Name: "config", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: "my-config"}, - }}}, - }, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "blog-writer", Image: "ghcr.io/example/blog-writer:latest", MountPath: "/app/skills/blog-writer", PullPolicy: agentv1alpha1.SkillPullAlways}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Volumes).To(HaveLen(2)) - Expect(podSpec.Volumes[0].Name).To(Equal("config")) - Expect(podSpec.Volumes[1].Name).To(Equal("skill-blog-writer")) - Expect(podSpec.Volumes[1].VolumeSource.Image.PullPolicy).To(Equal(corev1.PullAlways)) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(2)) - Expect(podSpec.Containers[0].VolumeMounts[0].Name).To(Equal("config")) - Expect(podSpec.Containers[0].VolumeMounts[1].Name).To(Equal("skill-blog-writer")) - Expect(podSpec.Containers[0].VolumeMounts[1].MountPath).To(Equal("/app/skills/blog-writer")) - }) - - It("should remove stale skill volumes", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{ - Name: "agent", - Image: "test:latest", - VolumeMounts: []corev1.VolumeMount{ - {Name: "skill-old-skill", MountPath: "/agent/skills/old-skill", ReadOnly: true}, - {Name: "skill-keep-skill", MountPath: "/agent/skills/keep-skill", ReadOnly: true}, - }, - }}, - Volumes: []corev1.Volume{ - {Name: "skill-old-skill", VolumeSource: corev1.VolumeSource{Image: &corev1.ImageVolumeSource{Reference: "old:v1"}}}, - {Name: "skill-keep-skill", VolumeSource: corev1.VolumeSource{Image: &corev1.ImageVolumeSource{Reference: "keep:v1"}}}, - }, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "keep-skill", Image: "keep:v1", MountPath: "/agent/skills/keep-skill"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Volumes).To(HaveLen(1)) - Expect(podSpec.Volumes[0].Name).To(Equal("skill-keep-skill")) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(1)) - Expect(podSpec.Containers[0].VolumeMounts[0].Name).To(Equal("skill-keep-skill")) - }) - - It("should remove all skill volumes when desired is nil", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{ - Name: "agent", - Image: "test:latest", - VolumeMounts: []corev1.VolumeMount{ - {Name: "config", MountPath: "/etc/config"}, - {Name: "skill-a", MountPath: "/agent/skills/a", ReadOnly: true}, - {Name: "skill-b", MountPath: "/agent/skills/b", ReadOnly: true}, - }, - }}, - Volumes: []corev1.Volume{ - {Name: "config", VolumeSource: corev1.VolumeSource{ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: "my-config"}, - }}}, - {Name: "skill-a", VolumeSource: corev1.VolumeSource{Image: &corev1.ImageVolumeSource{Reference: "a:v1"}}}, - {Name: "skill-b", VolumeSource: corev1.VolumeSource{Image: &corev1.ImageVolumeSource{Reference: "b:v1"}}}, - }, - } - - reconcileSkillVolumes(podSpec, nil) - - Expect(podSpec.Volumes).To(HaveLen(1)) - Expect(podSpec.Volumes[0].Name).To(Equal("config")) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(1)) - Expect(podSpec.Containers[0].VolumeMounts[0].Name).To(Equal("config")) - }) - - It("should update skill image reference", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{Name: "agent", Image: "test:latest"}}, - Volumes: []corev1.Volume{ - {Name: "skill-my-skill", VolumeSource: corev1.VolumeSource{Image: &corev1.ImageVolumeSource{Reference: "old:v1"}}}, - }, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "my-skill", Image: "new:v2", MountPath: "/agent/skills/my-skill"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Volumes).To(HaveLen(1)) - Expect(podSpec.Volumes[0].VolumeSource.Image.Reference).To(Equal("new:v2")) - }) - - It("should only mount skills to the first container", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "agent", Image: "agent:latest"}, - {Name: "sidecar", Image: "sidecar:latest"}, - }, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "skill-a", Image: "a:v1", MountPath: "/app/.claude/skills/skill-a"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(1)) - Expect(podSpec.Containers[0].VolumeMounts[0].MountPath).To(Equal("/app/.claude/skills/skill-a")) - Expect(podSpec.Containers[1].VolumeMounts).To(BeEmpty()) - }) - - It("should not mount skills to injected sidecar containers", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "agent", Image: "my-agent:latest"}, - {Name: "envoy-proxy", Image: "envoyproxy/envoy:v1.30"}, - {Name: "spiffe-helper", Image: "ghcr.io/spiffe/spiffe-helper:latest"}, - {Name: "kagenti-client-registration", Image: "kagenti/client-reg:latest"}, - }, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "resume-reviewer", Image: "ghcr.io/example/resume-reviewer:v1.0.0", MountPath: "/agent/skills/resume-reviewer"}, - {Name: "blog-writer", Image: "ghcr.io/example/blog-writer:latest", MountPath: "/app/.claude/skills/blog-writer"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(2)) - Expect(podSpec.Containers[0].VolumeMounts[0].Name).To(Equal("skill-resume-reviewer")) - Expect(podSpec.Containers[0].VolumeMounts[1].Name).To(Equal("skill-blog-writer")) - for _, sidecar := range podSpec.Containers[1:] { - Expect(sidecar.VolumeMounts).To(BeEmpty(), - "sidecar %q should not have skill mounts", sidecar.Name) - } - }) - - It("should set pull policy when specified", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{Name: "agent", Image: "test:latest"}}, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "s1", Image: "img:latest", MountPath: "/skills/s1", PullPolicy: agentv1alpha1.SkillPullAlways}, - {Name: "s2", Image: "img:v1.0.0", MountPath: "/skills/s2", PullPolicy: agentv1alpha1.SkillPullIfNotPresent}, - {Name: "s3", Image: "img:v2.0.0", MountPath: "/skills/s3"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Volumes).To(HaveLen(3)) - Expect(podSpec.Volumes[0].VolumeSource.Image.PullPolicy).To(Equal(corev1.PullAlways)) - Expect(podSpec.Volumes[1].VolumeSource.Image.PullPolicy).To(Equal(corev1.PullIfNotPresent)) - Expect(podSpec.Volumes[2].VolumeSource.Image.PullPolicy).To(Equal(corev1.PullPolicy(""))) - }) - - It("should use different mount paths for different frameworks", func() { - podSpec := &corev1.PodSpec{ - Containers: []corev1.Container{{Name: "agent", Image: "test:latest"}}, - } - skills := []agentv1alpha1.SkillImageRef{ - {Name: "claude-skill", Image: "img:v1", MountPath: "/app/.claude/skills/my-skill"}, - {Name: "cursor-skill", Image: "img:v1", MountPath: "/app/.cursor/rules/my-skill"}, - } - - reconcileSkillVolumes(podSpec, skills) - - Expect(podSpec.Containers[0].VolumeMounts).To(HaveLen(2)) - Expect(podSpec.Containers[0].VolumeMounts[0].MountPath).To(Equal("/app/.claude/skills/my-skill")) - Expect(podSpec.Containers[0].VolumeMounts[1].MountPath).To(Equal("/app/.cursor/rules/my-skill")) - }) - }) - - Context("Config hash includes skills", func() { - ctx := context.Background() - const namespace = "default" - - It("should change when skills are added", func() { - specNoSkills := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-skills"}, - } - specWithSkills := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-skills"}, - Skills: []agentv1alpha1.SkillImageRef{ - {Name: "s1", Image: "img:v1", MountPath: "/skills/s1"}, - }, - } - - r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, specNoSkills) - r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, specWithSkills) - Expect(r1.Hash).NotTo(Equal(r2.Hash)) - }) - - It("should change when skill image changes", func() { - spec1 := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-img"}, - Skills: []agentv1alpha1.SkillImageRef{{Name: "s1", Image: "img:v1", MountPath: "/skills/s1"}}, - } - spec2 := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-img"}, - Skills: []agentv1alpha1.SkillImageRef{{Name: "s1", Image: "img:v2", MountPath: "/skills/s1"}}, - } - - r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec1) - r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec2) - Expect(r1.Hash).NotTo(Equal(r2.Hash)) - }) - - It("should change when skill pull policy changes", func() { - spec1 := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-pp"}, - Skills: []agentv1alpha1.SkillImageRef{{Name: "s1", Image: "img:v1", MountPath: "/skills/s1", PullPolicy: agentv1alpha1.SkillPullAlways}}, - } - spec2 := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-pp"}, - Skills: []agentv1alpha1.SkillImageRef{{Name: "s1", Image: "img:v1", MountPath: "/skills/s1", PullPolicy: agentv1alpha1.SkillPullIfNotPresent}}, - } - - r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec1) - r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec2) - Expect(r1.Hash).NotTo(Equal(r2.Hash)) - }) - - It("should change when skill mount path changes", func() { - spec1 := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-mp"}, - Skills: []agentv1alpha1.SkillImageRef{{Name: "s1", Image: "img:v1", MountPath: "/agent/skills/s1"}}, - } - spec2 := &agentv1alpha1.AgentRuntimeSpec{ - Type: agentv1alpha1.RuntimeTypeAgent, - TargetRef: agentv1alpha1.TargetRef{APIVersion: "apps/v1", Kind: "Deployment", Name: "hash-mp"}, - Skills: []agentv1alpha1.SkillImageRef{{Name: "s1", Image: "img:v1", MountPath: "/app/.claude/skills/s1"}}, - } - - r1, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec1) - r2, _ := ComputeConfigHash(ctx, k8sClient, namespace, spec2) - Expect(r1.Hash).NotTo(Equal(r2.Hash)) - }) - }) -}) diff --git a/kagenti-operator/internal/webhook/config/feature_gate_loader.go b/kagenti-operator/internal/webhook/config/feature_gate_loader.go index 2fac0177..069994f0 100644 --- a/kagenti-operator/internal/webhook/config/feature_gate_loader.go +++ b/kagenti-operator/internal/webhook/config/feature_gate_loader.go @@ -180,7 +180,7 @@ func logFeatureGates(fg *FeatureGates, source string) { "envoyProxy", fg.EnvoyProxy, "injectTools", fg.InjectTools, "perWorkloadConfigResolution", fg.PerWorkloadConfigResolution, - "skillImageVolumes", fg.SkillImageVolumes, + "skillDiscovery", fg.SkillDiscovery, ) log.Info("=============================================") } diff --git a/kagenti-operator/internal/webhook/config/feature_gates.go b/kagenti-operator/internal/webhook/config/feature_gates.go index f3e61801..8c8ea88c 100644 --- a/kagenti-operator/internal/webhook/config/feature_gates.go +++ b/kagenti-operator/internal/webhook/config/feature_gates.go @@ -22,10 +22,11 @@ type FeatureGates struct { // true → resolved path: webhook reads namespace ConfigMaps at // admission time and injects literal env var values. PerWorkloadConfigResolution bool `json:"perWorkloadConfigResolution" yaml:"perWorkloadConfigResolution"` - // SkillImageVolumes controls whether the AgentRuntime controller mounts OCI - // skill images as Kubernetes ImageVolumes into agent pods. Requires - // Kubernetes 1.31+ with the ImageVolume feature gate enabled. - SkillImageVolumes bool `json:"skillImageVolumes" yaml:"skillImageVolumes"` + // SkillDiscovery controls whether the AgentRuntime controller reads the + // kagenti.io/skills annotation from target workloads and populates + // status.linkedSkills. When disabled, skill discovery is skipped and + // the SkillsDiscovered condition is not set. + SkillDiscovery bool `json:"skillDiscovery" yaml:"skillDiscovery"` } // DefaultFeatureGates returns feature gates with sidecar injection enabled for @@ -36,6 +37,7 @@ func DefaultFeatureGates() *FeatureGates { EnvoyProxy: true, InjectTools: false, PerWorkloadConfigResolution: false, + SkillDiscovery: false, } } diff --git a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go index c5c6ad05..bf1a70ba 100644 --- a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go +++ b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook.go @@ -19,7 +19,6 @@ package v1alpha1 import ( "context" "fmt" - "strings" agentv1alpha1 "github.com/kagenti/operator/api/v1alpha1" ctrl "sigs.k8s.io/controller-runtime" @@ -53,10 +52,6 @@ func (v *AgentRuntimeValidator) ValidateCreate(ctx context.Context, rt *agentv1a if err := checkMTLSCompatibleWithMode(rt); err != nil { return nil, err } - if err := validateSkills(rt.Spec.Skills); err != nil { - return nil, err - } - return nil, nil } @@ -69,9 +64,6 @@ func (v *AgentRuntimeValidator) ValidateUpdate(ctx context.Context, _ *agentv1al if err := checkMTLSCompatibleWithMode(rt); err != nil { return nil, err } - if err := validateSkills(rt.Spec.Skills); err != nil { - return nil, err - } return nil, nil } @@ -139,35 +131,3 @@ func (v *AgentRuntimeValidator) checkDuplicateTargetRef(ctx context.Context, rt return nil } - -var deniedMountPrefixes = []string{"/proc", "/sys", "/dev", "/var/run/secrets"} - -func validateSkills(skills []agentv1alpha1.SkillImageRef) error { - if len(skills) == 0 { - return nil - } - - seenNames := make(map[string]bool, len(skills)) - seenPaths := make(map[string]bool, len(skills)) - for i, skill := range skills { - if seenNames[skill.Name] { - return fmt.Errorf("spec.skills[%d]: duplicate skill name %q", i, skill.Name) - } - seenNames[skill.Name] = true - - if seenPaths[skill.MountPath] { - return fmt.Errorf("spec.skills[%d]: duplicate mountPath %q", i, skill.MountPath) - } - seenPaths[skill.MountPath] = true - - if strings.Contains(skill.MountPath, "..") { - return fmt.Errorf("spec.skills[%d]: mountPath %q must not contain path traversal (..)", i, skill.MountPath) - } - for _, prefix := range deniedMountPrefixes { - if strings.HasPrefix(skill.MountPath, prefix) { - return fmt.Errorf("spec.skills[%d]: mountPath %q overlaps protected path %q", i, skill.MountPath, prefix) - } - } - } - return nil -} diff --git a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go index 319f637d..286ef638 100644 --- a/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go +++ b/kagenti-operator/internal/webhook/v1alpha1/agentruntime_webhook_test.go @@ -311,133 +311,3 @@ func TestAgentRuntimeValidator_MTLSCompatWithMode(t *testing.T) { }) } } - -func TestValidateSkills(t *testing.T) { - t.Run("empty skills is valid", func(t *testing.T) { - if err := validateSkills(nil); err != nil { - t.Errorf("unexpected error: %v", err) - } - }) - - t.Run("valid skills succeed", func(t *testing.T) { - skills := []agentv1alpha1.SkillImageRef{ - {Name: "resume-reviewer", Image: "ghcr.io/example/resume:v1", MountPath: "/agent/skills/resume-reviewer"}, - {Name: "blog-writer", Image: "ghcr.io/example/blog:v1", MountPath: "/agent/skills/blog-writer"}, - } - if err := validateSkills(skills); err != nil { - t.Errorf("unexpected error: %v", err) - } - }) - - t.Run("duplicate names rejected", func(t *testing.T) { - skills := []agentv1alpha1.SkillImageRef{ - {Name: "my-skill", Image: "img:v1", MountPath: "/skills/a"}, - {Name: "my-skill", Image: "img:v2", MountPath: "/skills/b"}, - } - err := validateSkills(skills) - if err == nil { - t.Fatal("expected error for duplicate skill names") - } - if !strings.Contains(err.Error(), "duplicate skill name") { - t.Errorf("unexpected error message: %v", err) - } - }) - - t.Run("duplicate mountPath rejected", func(t *testing.T) { - skills := []agentv1alpha1.SkillImageRef{ - {Name: "skill-a", Image: "img:v1", MountPath: "/agent/skills/shared"}, - {Name: "skill-b", Image: "img:v2", MountPath: "/agent/skills/shared"}, - } - err := validateSkills(skills) - if err == nil { - t.Fatal("expected error for duplicate mountPath") - } - if !strings.Contains(err.Error(), "duplicate mountPath") { - t.Errorf("unexpected error message: %v", err) - } - }) - - t.Run("path traversal rejected", func(t *testing.T) { - skills := []agentv1alpha1.SkillImageRef{ - {Name: "evil", Image: "img:v1", MountPath: "/agent/skills/../../etc/passwd"}, - } - err := validateSkills(skills) - if err == nil { - t.Fatal("expected error for path traversal") - } - if !strings.Contains(err.Error(), "path traversal") { - t.Errorf("unexpected error message: %v", err) - } - }) - - t.Run("protected path rejected", func(t *testing.T) { - for _, path := range []string{"/proc/self", "/sys/fs", "/dev/null", "/var/run/secrets/kubernetes.io/serviceaccount"} { - skills := []agentv1alpha1.SkillImageRef{ - {Name: "bad-mount", Image: "img:v1", MountPath: path}, - } - err := validateSkills(skills) - if err == nil { - t.Fatalf("expected error for protected path %q", path) - } - if !strings.Contains(err.Error(), "protected path") { - t.Errorf("unexpected error message for %q: %v", path, err) - } - } - }) -} - -func TestValidateSkills_CreateIntegration(t *testing.T) { - ctx := context.Background() - - t.Run("create with valid skills succeeds", func(t *testing.T) { - v := &AgentRuntimeValidator{} - rt := validAgentRuntime() - rt.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "s1", Image: "img:v1", MountPath: "/skills/s1"}, - } - _, err := v.ValidateCreate(ctx, rt) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - }) - - t.Run("create with duplicate skills rejected", func(t *testing.T) { - v := &AgentRuntimeValidator{} - rt := validAgentRuntime() - rt.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "s1", Image: "img:v1", MountPath: "/skills/a"}, - {Name: "s1", Image: "img:v2", MountPath: "/skills/b"}, - } - _, err := v.ValidateCreate(ctx, rt) - if err == nil { - t.Fatal("expected error for duplicate skill names") - } - }) - - t.Run("create with duplicate mountPath rejected", func(t *testing.T) { - v := &AgentRuntimeValidator{} - rt := validAgentRuntime() - rt.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "s1", Image: "img:v1", MountPath: "/skills/shared"}, - {Name: "s2", Image: "img:v2", MountPath: "/skills/shared"}, - } - _, err := v.ValidateCreate(ctx, rt) - if err == nil { - t.Fatal("expected error for duplicate mountPath") - } - }) - - t.Run("update with duplicate skills rejected", func(t *testing.T) { - v := &AgentRuntimeValidator{} - old := validAgentRuntime() - updated := validAgentRuntime() - updated.Spec.Skills = []agentv1alpha1.SkillImageRef{ - {Name: "s1", Image: "img:v1", MountPath: "/skills/a"}, - {Name: "s1", Image: "img:v2", MountPath: "/skills/b"}, - } - _, err := v.ValidateUpdate(ctx, old, updated) - if err == nil { - t.Fatal("expected error for duplicate skill names on update") - } - }) -} diff --git a/kagenti-operator/test/e2e/README.md b/kagenti-operator/test/e2e/README.md index 5dcf2da8..21531932 100644 --- a/kagenti-operator/test/e2e/README.md +++ b/kagenti-operator/test/e2e/README.md @@ -1,13 +1,12 @@ # E2E Tests -End-to-end tests for the kagenti-operator. The suite runs 32 specs: +End-to-end tests for the kagenti-operator. The suite runs 25 specs: - **Manager tests** (2 specs) — controller pod readiness and Prometheus metrics - **AuthBridge Injection tests** (4 specs) — sidecar injection, idempotency, opt-out, and HTTP validation - **AgentCard tests** (6 specs) — webhook validation, auto-discovery, duplicate prevention, audit mode, and SPIRE signature verification - **AgentRuntime tests** (8 specs) — label application, status lifecycle, idempotency, error handling, tool type, StatefulSet support, identity/trace overrides, and deletion cleanup - **Combined tests** (5 specs) — AgentRuntime + AgentCard + Auth Bridge integration: labels, auto-created card, sidecar injection, identity binding, and deletion cleanup -- **Skill Image Volumes tests** (7 specs) — feature gate disabled/enabled, volume mounting, update, removal, deletion cleanup, and webhook validation ## Prerequisites @@ -24,7 +23,7 @@ The test suite auto-detects Docker vs Podman. No env vars needed. AuthBridge sid # Create a fresh Kind cluster kind delete cluster 2>/dev/null; kind create cluster -# Run all 32 specs (~18 min) +# Run all 25 specs (~15 min) make test-e2e ``` @@ -59,8 +58,6 @@ go test ./test/e2e/ -v -ginkgo.v -ginkgo.focus="Manager" # AgentRuntime tests only (~3 min) go test ./test/e2e/ -v -ginkgo.v -ginkgo.focus="AgentRuntime E2E" -# Skill Image Volumes tests only (~4 min) -go test ./test/e2e/ -v -ginkgo.v -ginkgo.focus="Skill Image Volumes" ``` ## Cleanup @@ -91,14 +88,6 @@ kind delete cluster | Tool type label | Tool type | AgentRuntime with type=tool applies `kagenti.io/type=tool` label and no AgentCard is created | | StatefulSet target | StatefulSet target | AgentRuntime applies labels, config-hash, and reaches Active for a StatefulSet workload | | Identity/trace overrides | Identity and trace overrides | AgentRuntime with identity+trace spec produces a different config-hash than a minimal CR | -| Feature gate disabled | Skill Image Volumes | Skills declared but SkillsMounted=False with reason FeatureGateDisabled; no skill volumes on Deployment | -| Mount skill ImageVolumes | Skill Image Volumes (enabled) | AgentRuntime with skills adds Image volumes, read-only mounts, SkillsMounted=True, config-hash, and `kagenti.io/skills` annotation | -| Update skill image | Skill Image Volumes (enabled) | Changing skill image reference updates Deployment volume and config-hash | -| Remove skills | Skill Image Volumes (enabled) | Removing skills from CR removes all skill volumes, mounts, and `kagenti.io/skills` annotation from Deployment | -| Skill deletion cleanup | Skill Image Volumes (enabled) | AgentRuntime deletion removes skill volumes and `kagenti.io/skills` annotation from target Deployment | -| Duplicate skill names | Webhook validation | Webhook rejects AgentRuntime with duplicate skill names | -| Duplicate skill mountPaths | Webhook validation | Webhook rejects AgentRuntime with duplicate skill mountPaths | - ## Architecture ### What gets installed diff --git a/kagenti-operator/test/e2e/e2e_test.go b/kagenti-operator/test/e2e/e2e_test.go index 04af4704..9db48735 100644 --- a/kagenti-operator/test/e2e/e2e_test.go +++ b/kagenti-operator/test/e2e/e2e_test.go @@ -1941,7 +1941,7 @@ rules: }) }) -var _ = Describe("Skill Image Volumes E2E", Ordered, func() { +var _ = Describe("Skill Discovery E2E", Ordered, func() { const controllerNamespace = "kagenti-operator-system" const controllerDeployment = "kagenti-operator-controller-manager" @@ -1959,6 +1959,14 @@ rules: `) _, _ = utils.Run(clusterRoleCmd) + By("undeploying any stale controller to remove feature-gates volume from prior runs") + utils.UndeployController() + + By("cleaning up stale feature-gates ConfigMap") + cleanupCmd := exec.Command("kubectl", "delete", "configmap", "kagenti-feature-gates", + "-n", controllerNamespace, "--ignore-not-found") + _, _ = utils.Run(cleanupCmd) + Expect(utils.DeployController(controllerNamespace, projectImage)).To(Succeed(), "Failed to deploy controller") By("waiting for controller-manager to be ready") @@ -1981,12 +1989,12 @@ rules: g.Expect(output).NotTo(BeEmpty(), "webhook endpoint not yet populated") }, 2*time.Minute, 2*time.Second).Should(Succeed()) - By("creating skill test namespace") - cmd := exec.Command("kubectl", "create", "ns", skillTestNamespace) + By("creating skill discovery test namespace") + cmd := exec.Command("kubectl", "create", "ns", skillDiscoveryTestNamespace) _, err := utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - cmd = exec.Command("kubectl", "label", "--overwrite", "ns", skillTestNamespace, + cmd = exec.Command("kubectl", "label", "--overwrite", "ns", skillDiscoveryTestNamespace, "pod-security.kubernetes.io/enforce=restricted") _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) @@ -1999,15 +2007,17 @@ rules: _, err = utils.KubectlApplyStdin(runtimeClusterDefaultsConfigMapFixture(), "kagenti-system") Expect(err).NotTo(HaveOccurred()) - By("deploying skill agent target workload") - _, err = utils.KubectlApplyStdin(skillTargetDeploymentFixture(), skillTestNamespace) + By("deploying target Deployment with skills annotation") + _, err = utils.KubectlApplyStdin(skillDiscoveryDeploymentFixture(), skillDiscoveryTestNamespace) Expect(err).NotTo(HaveOccurred()) - Expect(utils.WaitForDeploymentReady("skill-agent-target", skillTestNamespace, 2*time.Minute)).To(Succeed()) + Expect(utils.WaitForDeploymentReady( + "skill-discovery-agent", skillDiscoveryTestNamespace, 2*time.Minute, + )).To(Succeed()) }) AfterAll(func() { - By("deleting skill test namespace") - cmd := exec.Command("kubectl", "delete", "ns", skillTestNamespace, "--ignore-not-found") + By("deleting skill discovery test namespace") + cmd := exec.Command("kubectl", "delete", "ns", skillDiscoveryTestNamespace, "--ignore-not-found") _, _ = utils.Run(cmd) By("cleaning up cluster defaults ConfigMap") @@ -2038,9 +2048,9 @@ rules: "logs", "-l", "control-plane=controller-manager", "-n", controllerNamespace, "--tail=100", }}, - {"Events", []string{"get", "events", "-n", skillTestNamespace, "--sort-by=.lastTimestamp"}}, - {"AgentRuntimes", []string{"get", "agentruntimes", "-n", skillTestNamespace, "-o", "yaml"}}, - {"Deployments", []string{"get", "deployments", "-n", skillTestNamespace, "-o", "yaml"}}, + {"Events", []string{"get", "events", "-n", skillDiscoveryTestNamespace, "--sort-by=.lastTimestamp"}}, + {"AgentRuntimes", []string{"get", "agentruntimes", "-n", skillDiscoveryTestNamespace, "-o", "yaml"}}, + {"Deployments", []string{"get", "deployments", "-n", skillDiscoveryTestNamespace, "-o", "yaml"}}, } { cmd := exec.Command("kubectl", diag.args...) out, err := utils.Run(cmd) @@ -2055,54 +2065,56 @@ rules: SetDefaultEventuallyPollingInterval(time.Second) Context("Feature gate disabled (default)", Ordered, func() { - It("should set SkillsMounted=False when feature gate is disabled", func() { - By("creating AgentRuntime with skills (feature gate disabled)") + It("should not populate linkedSkills when feature gate is disabled", func() { + By("creating AgentRuntime targeting Deployment with skills annotation") Eventually(func() error { - _, err := utils.KubectlApplyStdin(skillAgentRuntimeFixture(), skillTestNamespace) + _, err := utils.KubectlApplyStdin(skillDiscoveryAgentRuntimeFixture(), skillDiscoveryTestNamespace) return err }, 1*time.Minute, 5*time.Second).Should(Succeed()) By("waiting for AgentRuntime phase=Active") Eventually(func(g Gomega) { - phase, err := utils.KubectlGetJsonpath("agentruntime", "skill-agent-runtime", - skillTestNamespace, "{.status.phase}") + phase, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.phase}") g.Expect(err).NotTo(HaveOccurred()) g.Expect(phase).To(Equal("Active")) }).Should(Succeed()) - By("verifying SkillsMounted=False with reason FeatureGateDisabled") + By("verifying status.linkedSkills is empty") Eventually(func(g Gomega) { - status, err := utils.KubectlGetJsonpath("agentruntime", "skill-agent-runtime", - skillTestNamespace, - "{.status.conditions[?(@.type=='SkillsMounted')].status}") + skills, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(status).To(Equal("False")) + g.Expect(skills).To(BeEmpty()) + }).Should(Succeed()) - reason, err := utils.KubectlGetJsonpath("agentruntime", "skill-agent-runtime", - skillTestNamespace, - "{.status.conditions[?(@.type=='SkillsMounted')].reason}") + By("verifying SkillsDiscovered condition is absent") + Consistently(func(g Gomega) { + status, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].status}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(reason).To(Equal("FeatureGateDisabled")) - }).Should(Succeed()) + g.Expect(status).To(BeEmpty(), "SkillsDiscovered condition should not be set when feature gate is disabled") + }, 10*time.Second, 2*time.Second).Should(Succeed()) - By("verifying NO skill volumes on Deployment spec") + By("verifying operator did NOT mutate the Deployment (no skill volumes)") Eventually(func(g Gomega) { - volumes, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, + volumes, err := utils.KubectlGetJsonpath("deployment", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.spec.template.spec.volumes[*].name}") g.Expect(err).NotTo(HaveOccurred()) g.Expect(volumes).NotTo(ContainSubstring("skill-")) }).Should(Succeed()) By("cleaning up AgentRuntime for next context") - cmd := exec.Command("kubectl", "delete", "agentruntime", "skill-agent-runtime", - "-n", skillTestNamespace) + cmd := exec.Command("kubectl", "delete", "agentruntime", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace) _, err := utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "agentruntime", "skill-agent-runtime", - "-n", skillTestNamespace) + cmd := exec.Command("kubectl", "get", "agentruntime", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace) _, err := cmd.CombinedOutput() g.Expect(err).To(HaveOccurred(), "AgentRuntime should be deleted") }).Should(Succeed()) @@ -2110,11 +2122,9 @@ rules: }) Context("Feature gate enabled", Ordered, func() { - var initialConfigHash string - BeforeAll(func() { - By("enabling skillImageVolumes feature gate") - Expect(utils.EnableSkillImageVolumes(controllerNamespace, controllerDeployment)).To(Succeed()) + By("enabling skillDiscovery feature gate") + Expect(utils.EnableSkillDiscovery(controllerNamespace, controllerDeployment)).To(Succeed()) By("waiting for controller to be ready after feature gate patch") goTmpl := "{{ range .items }}" + @@ -2141,242 +2151,278 @@ rules: }, 2*time.Minute, 2*time.Second).Should(Succeed()) }) - It("should mount skill ImageVolumes to target Deployment", func() { - By("creating AgentRuntime with 2 skills") + It("should populate linkedSkills from annotation", func() { + By("creating AgentRuntime") Eventually(func() error { - _, err := utils.KubectlApplyStdin(skillAgentRuntimeFixture(), skillTestNamespace) + _, err := utils.KubectlApplyStdin(skillDiscoveryAgentRuntimeFixture(), skillDiscoveryTestNamespace) return err }, 1*time.Minute, 5*time.Second).Should(Succeed()) By("waiting for AgentRuntime phase=Active") Eventually(func(g Gomega) { - phase, err := utils.KubectlGetJsonpath("agentruntime", "skill-agent-runtime", - skillTestNamespace, "{.status.phase}") + phase, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.phase}") g.Expect(err).NotTo(HaveOccurred()) g.Expect(phase).To(Equal("Active")) }).Should(Succeed()) - By("verifying skill volumes on Deployment spec") + By("verifying status.linkedSkills contains discovered skills") Eventually(func(g Gomega) { - volumes, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.spec.volumes[*].name}") + raw, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(volumes).To(ContainSubstring("skill-resume-reviewer")) - g.Expect(volumes).To(ContainSubstring("skill-blog-writer")) + g.Expect(raw).To(ContainSubstring("summarizer")) + g.Expect(raw).To(ContainSubstring("openshift-review")) }).Should(Succeed()) - By("verifying skill volume image references") + By("verifying SkillsDiscovered condition is True with reason SkillsFound") Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "deployment", "skill-agent-target", - "-n", skillTestNamespace, - "-o", "jsonpath={.spec.template.spec.volumes}") - output, err := utils.Run(cmd) + status, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].status}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(output).To(ContainSubstring("registry.k8s.io/pause:3.9")) - g.Expect(output).To(ContainSubstring("registry.k8s.io/pause:3.10")) - }).Should(Succeed()) + g.Expect(status).To(Equal("True")) - By("verifying skill volume mounts on agent container") - Eventually(func(g Gomega) { - mounts, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.spec.containers[0].volumeMounts[*].name}") + reason, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].reason}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(mounts).To(ContainSubstring("skill-resume-reviewer")) - g.Expect(mounts).To(ContainSubstring("skill-blog-writer")) - }).Should(Succeed()) + g.Expect(reason).To(Equal("SkillsFound")) - By("verifying skill mount paths") - Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "deployment", "skill-agent-target", - "-n", skillTestNamespace, - "-o", "jsonpath={.spec.template.spec.containers[0].volumeMounts}") - output, err := utils.Run(cmd) + message, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].message}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(output).To(ContainSubstring("/agent/skills/resume-reviewer")) - g.Expect(output).To(ContainSubstring("/agent/skills/blog-writer")) + g.Expect(message).To(ContainSubstring("2 linked skill(s)")) }).Should(Succeed()) - By("verifying SkillsMounted=True condition") + By("verifying Deployment was NOT mutated (no skill volumes added)") Eventually(func(g Gomega) { - status, err := utils.KubectlGetJsonpath("agentruntime", "skill-agent-runtime", - skillTestNamespace, - "{.status.conditions[?(@.type=='SkillsMounted')].status}") - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(status).To(Equal("True")) - - reason, err := utils.KubectlGetJsonpath("agentruntime", "skill-agent-runtime", - skillTestNamespace, - "{.status.conditions[?(@.type=='SkillsMounted')].reason}") + volumes, err := utils.KubectlGetJsonpath("deployment", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.spec.template.spec.volumes[*].name}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(reason).To(Equal("SkillsApplied")) + g.Expect(volumes).NotTo(ContainSubstring("skill-")) }).Should(Succeed()) + }) + + It("should update linkedSkills when annotation changes", func() { + By("updating skills annotation on Deployment") + cmd := exec.Command("kubectl", "annotate", "--overwrite", + "deployment", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace, + `kagenti.io/skills=["summarizer","translator","openshift-review"]`) + _, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) - By("verifying kagenti.io/skills annotation on Deployment metadata") + By("verifying status.linkedSkills reflects the updated list") Eventually(func(g Gomega) { - ann, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.metadata.annotations['kagenti\\.io/skills']}") + raw, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(ann).To(ContainSubstring("resume-reviewer")) - g.Expect(ann).To(ContainSubstring("blog-writer")) + g.Expect(raw).To(ContainSubstring("summarizer")) + g.Expect(raw).To(ContainSubstring("translator")) + g.Expect(raw).To(ContainSubstring("openshift-review")) }).Should(Succeed()) - By("recording initial config-hash") + By("verifying SkillsDiscovered message reflects new count") Eventually(func(g Gomega) { - hash, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.metadata.annotations['kagenti\\.io/config-hash']}") + message, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].message}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(hash).To(HaveLen(64)) - initialConfigHash = hash + g.Expect(message).To(ContainSubstring("3 linked skill(s)")) }).Should(Succeed()) }) - It("should update volumes when skill image changes", func() { - By("updating AgentRuntime with changed skill image") - _, err := utils.KubectlApplyStdin(skillAgentRuntimeUpdatedFixture(), skillTestNamespace) + It("should clear linkedSkills when annotation is removed", func() { + By("removing skills annotation from Deployment") + cmd := exec.Command("kubectl", "annotate", + "deployment", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace, + "kagenti.io/skills-") + _, err := utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - By("verifying Deployment spec has new image reference") + By("verifying status.linkedSkills is empty") Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "deployment", "skill-agent-target", - "-n", skillTestNamespace, - "-o", "jsonpath={.spec.template.spec.volumes}") - output, err := utils.Run(cmd) + skills, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(output).NotTo(ContainSubstring("registry.k8s.io/pause:3.9")) - g.Expect(output).To(ContainSubstring("registry.k8s.io/pause:3.10")) + g.Expect(skills).To(BeEmpty()) }).Should(Succeed()) - By("verifying config-hash changed") + By("verifying SkillsDiscovered condition is removed") Eventually(func(g Gomega) { - hash, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.metadata.annotations['kagenti\\.io/config-hash']}") + status, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].status}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(hash).To(HaveLen(64)) - g.Expect(hash).NotTo(Equal(initialConfigHash)) - initialConfigHash = hash + g.Expect(status).To(BeEmpty()) }).Should(Succeed()) }) - It("should remove skill volumes when skills removed from CR", func() { - By("updating AgentRuntime to remove all skills") - _, err := utils.KubectlApplyStdin(skillAgentRuntimeNoSkillsFixture(), skillTestNamespace) + It("should update linkedSkills and rollout pods when an OCI skill volume is removed", func() { + By("deploying Deployment with two OCI skill ImageVolumes") + _, err := utils.KubectlApplyStdin(ociSkillDeploymentFixture(), skillDiscoveryTestNamespace) Expect(err).NotTo(HaveOccurred()) + Expect(utils.WaitForDeploymentReady("oci-skill-agent", skillDiscoveryTestNamespace, 2*time.Minute)).To(Succeed()) + + By("creating AgentRuntime targeting oci-skill-agent") + Eventually(func() error { + _, err := utils.KubectlApplyStdin(ociSkillAgentRuntimeFixture(), skillDiscoveryTestNamespace) + return err + }, 1*time.Minute, 5*time.Second).Should(Succeed()) - By("verifying no skill volumes remain on Deployment") + By("waiting for AgentRuntime phase=Active") Eventually(func(g Gomega) { - volumes, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.spec.volumes[*].name}") + phase, err := utils.KubectlGetJsonpath("agentruntime", "oci-skill-agent", + skillDiscoveryTestNamespace, "{.status.phase}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(volumes).NotTo(ContainSubstring("skill-")) + g.Expect(phase).To(Equal("Active")) }).Should(Succeed()) - By("verifying no skill mounts remain on container") + By("verifying both skills are discovered") Eventually(func(g Gomega) { - mounts, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.spec.containers[0].volumeMounts[*].name}") + raw, err := utils.KubectlGetJsonpath("agentruntime", "oci-skill-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(mounts).NotTo(ContainSubstring("skill-")) + g.Expect(raw).To(ContainSubstring("summarizer")) + g.Expect(raw).To(ContainSubstring("openshift-review")) }).Should(Succeed()) - By("verifying kagenti.io/skills annotation removed from Deployment") + By("capturing Deployment generation before skill removal") + genBefore, err := utils.KubectlGetJsonpath("deployment", "oci-skill-agent", + skillDiscoveryTestNamespace, "{.metadata.generation}") + Expect(err).NotTo(HaveOccurred()) + + By("removing openshift-review skill (ImageVolume + annotation update)") + _, err = utils.KubectlApplyStdin(ociSkillDeploymentOneSkillFixture(), skillDiscoveryTestNamespace) + Expect(err).NotTo(HaveOccurred()) + + By("verifying Deployment generation incremented (rollout triggered)") Eventually(func(g Gomega) { - ann, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.metadata.annotations['kagenti\\.io/skills']}") + genAfter, err := utils.KubectlGetJsonpath("deployment", "oci-skill-agent", + skillDiscoveryTestNamespace, "{.metadata.generation}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(ann).To(BeEmpty()) + g.Expect(genAfter).NotTo(Equal(genBefore), "Deployment generation should change after skill removal") }).Should(Succeed()) - By("verifying config-hash changed again") + By("waiting for Deployment rollout to complete") + Expect(utils.WaitForRollout("oci-skill-agent", skillDiscoveryTestNamespace, 2*time.Minute)).To(Succeed()) + + By("verifying AgentRuntime reflects only the remaining skill") Eventually(func(g Gomega) { - hash, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.metadata.annotations['kagenti\\.io/config-hash']}") + raw, err := utils.KubectlGetJsonpath("agentruntime", "oci-skill-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(hash).To(HaveLen(64)) - g.Expect(hash).NotTo(Equal(initialConfigHash)) - initialConfigHash = hash + g.Expect(raw).To(ContainSubstring("summarizer")) + g.Expect(raw).NotTo(ContainSubstring("openshift-review")) }).Should(Succeed()) - }) - It("should clean up skill volumes on AgentRuntime deletion", func() { - By("re-applying AgentRuntime with skills") - Eventually(func() error { - _, err := utils.KubectlApplyStdin(skillAgentRuntimeFixture(), skillTestNamespace) - return err - }, 1*time.Minute, 5*time.Second).Should(Succeed()) + By("verifying SkillsDiscovered message reflects 1 skill") + Eventually(func(g Gomega) { + message, err := utils.KubectlGetJsonpath("agentruntime", "oci-skill-agent", + skillDiscoveryTestNamespace, + "{.status.conditions[?(@.type=='SkillsDiscovered')].message}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(message).To(ContainSubstring("1 linked skill(s)")) + }).Should(Succeed()) - By("waiting for skill volumes to appear") + By("verifying Deployment only has the summarizer volume") Eventually(func(g Gomega) { - volumes, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, + volumes, err := utils.KubectlGetJsonpath("deployment", "oci-skill-agent", + skillDiscoveryTestNamespace, "{.spec.template.spec.volumes[*].name}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(volumes).To(ContainSubstring("skill-resume-reviewer")) + g.Expect(volumes).To(ContainSubstring("skill-summarizer")) + g.Expect(volumes).NotTo(ContainSubstring("skill-openshift-review")) }).Should(Succeed()) - By("deleting the AgentRuntime CR") - cmd := exec.Command("kubectl", "delete", "agentruntime", "skill-agent-runtime", - "-n", skillTestNamespace) + By("cleaning up oci-skill-agent resources") + cmd := exec.Command("kubectl", "delete", "agentruntime", "oci-skill-agent", + "-n", skillDiscoveryTestNamespace, "--ignore-not-found") + _, _ = utils.Run(cmd) + cmd = exec.Command("kubectl", "delete", "deployment", "oci-skill-agent", + "-n", skillDiscoveryTestNamespace, "--ignore-not-found") + _, _ = utils.Run(cmd) + }) + + It("should degrade gracefully with malformed skills annotation", func() { + By("setting a malformed kagenti.io/skills annotation") + cmd := exec.Command("kubectl", "annotate", "--overwrite", + "deployment", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace, + `kagenti.io/skills=not-valid-json`) _, err := utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - By("verifying AgentRuntime CR is gone") - Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "agentruntime", "skill-agent-runtime", - "-n", skillTestNamespace) - _, err := cmd.CombinedOutput() - g.Expect(err).To(HaveOccurred(), "AgentRuntime should be deleted") - }).Should(Succeed()) + By("triggering a reconcile via label touch") + cmd = exec.Command("kubectl", "label", "--overwrite", + "agentruntime", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace, + "malformed-test=trigger") + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) - By("verifying skill volumes removed from Deployment") + By("verifying linkedSkills is empty (no crash, graceful degradation)") Eventually(func(g Gomega) { - volumes, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.spec.template.spec.volumes[*].name}") + raw, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(volumes).NotTo(ContainSubstring("skill-")) + g.Expect(raw).To(SatisfyAny(BeEmpty(), Equal("[]"))) }).Should(Succeed()) - By("verifying kagenti.io/skills annotation removed after deletion") + By("verifying SkillAnnotationParseError event was emitted") Eventually(func(g Gomega) { - ann, err := utils.KubectlGetJsonpath("deployment", "skill-agent-target", - skillTestNamespace, - "{.metadata.annotations['kagenti\\.io/skills']}") + cmd := exec.Command("kubectl", "get", "events", + "-n", skillDiscoveryTestNamespace, + "--field-selector", "reason=SkillAnnotationParseError", + "-o", "jsonpath={.items[*].message}") + output, err := utils.Run(cmd) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(ann).To(BeEmpty()) + g.Expect(output).To(ContainSubstring("Failed to parse kagenti.io/skills annotation")) }).Should(Succeed()) + + By("restoring valid annotation") + cmd = exec.Command("kubectl", "annotate", "--overwrite", + "deployment", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace, + `kagenti.io/skills=["summarizer","openshift-review"]`) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) }) - }) - Context("Webhook validation", func() { - It("should reject AgentRuntime with duplicate skill names", func() { - By("attempting to apply AgentRuntime with duplicate skill names") + It("should clean up on AgentRuntime deletion", func() { + By("re-adding skills annotation to Deployment") + cmd := exec.Command("kubectl", "annotate", "--overwrite", + "deployment", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace, + `kagenti.io/skills=["summarizer","openshift-review"]`) + _, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) + + By("verifying linkedSkills is re-populated") Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "apply", "-f", "-", "-n", skillTestNamespace) - cmd.Stdin = strings.NewReader(skillDuplicateNamesAgentRuntimeFixture()) - output, err := cmd.CombinedOutput() - g.Expect(err).To(HaveOccurred(), "kubectl apply should fail for duplicate skill names") - g.Expect(string(output)).To(ContainSubstring("duplicate skill name")) - }, 1*time.Minute, 2*time.Second).Should(Succeed()) - }) + raw, err := utils.KubectlGetJsonpath("agentruntime", "skill-discovery-agent", + skillDiscoveryTestNamespace, "{.status.linkedSkills}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(raw).To(ContainSubstring("summarizer")) + }).Should(Succeed()) + + By("deleting the AgentRuntime CR") + cmd = exec.Command("kubectl", "delete", "agentruntime", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace) + _, err = utils.Run(cmd) + Expect(err).NotTo(HaveOccurred()) - It("should reject AgentRuntime with duplicate skill mountPaths", func() { - By("attempting to apply AgentRuntime with duplicate mountPaths") + By("verifying AgentRuntime CR is gone") Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "apply", "-f", "-", "-n", skillTestNamespace) - cmd.Stdin = strings.NewReader(skillDuplicateMountPathAgentRuntimeFixture()) - output, err := cmd.CombinedOutput() - g.Expect(err).To(HaveOccurred(), "kubectl apply should fail for duplicate mountPaths") - g.Expect(string(output)).To(ContainSubstring("duplicate mountPath")) - }, 1*time.Minute, 2*time.Second).Should(Succeed()) + cmd := exec.Command("kubectl", "get", "agentruntime", "skill-discovery-agent", + "-n", skillDiscoveryTestNamespace) + _, err := cmd.CombinedOutput() + g.Expect(err).To(HaveOccurred(), "AgentRuntime should be deleted") + }).Should(Succeed()) }) }) }) diff --git a/kagenti-operator/test/e2e/fixtures.go b/kagenti-operator/test/e2e/fixtures.go index 40d15719..9ce35247 100644 --- a/kagenti-operator/test/e2e/fixtures.go +++ b/kagenti-operator/test/e2e/fixtures.go @@ -1348,45 +1348,31 @@ data: ` } -// combinedClusterSPIFFEIDFixture returns YAML for a ClusterSPIFFEID matching -// the combined test namespace. -func combinedClusterSPIFFEIDFixture() string { - return `apiVersion: spire.spiffe.io/v1alpha1 -kind: ClusterSPIFFEID -metadata: - name: e2e-combined-test -spec: - spiffeIDTemplate: "spiffe://{{ .TrustDomain }}/ns/{{ .PodMeta.Namespace }}/sa/{{ .PodSpec.ServiceAccountName }}" - podSelector: - matchLabels: - kagenti.io/type: agent - namespaceSelector: - matchLabels: - kagenti-enabled: "true" -` -} - -// --- Skill Image Volumes E2E fixtures --- +// --- Skill Discovery E2E fixtures --- -const skillTestNamespace = "e2e-skills-test" +const skillDiscoveryTestNamespace = "e2e-skill-discovery-test" -func skillTargetDeploymentFixture() string { +// skillDiscoveryDeploymentFixture returns YAML for a Deployment with the +// kagenti.io/skills annotation set by the user (or kagenti backend). +func skillDiscoveryDeploymentFixture() string { return `apiVersion: apps/v1 kind: Deployment metadata: - name: skill-agent-target - namespace: ` + skillTestNamespace + ` + name: skill-discovery-agent + namespace: ` + skillDiscoveryTestNamespace + ` labels: - app.kubernetes.io/name: skill-agent-target + app.kubernetes.io/name: skill-discovery-agent + annotations: + kagenti.io/skills: '["summarizer","openshift-review"]' spec: replicas: 1 selector: matchLabels: - app.kubernetes.io/name: skill-agent-target + app.kubernetes.io/name: skill-discovery-agent template: metadata: labels: - app.kubernetes.io/name: skill-agent-target + app.kubernetes.io/name: skill-discovery-agent kagenti.io/inject: disabled spec: securityContext: @@ -1406,107 +1392,157 @@ spec: ` } -func skillAgentRuntimeFixture() string { +// skillDiscoveryAgentRuntimeFixture returns YAML for an AgentRuntime CR +// targeting the skill-discovery-agent Deployment. No spec.skills — the +// operator discovers skills from the Deployment's annotation. +func skillDiscoveryAgentRuntimeFixture() string { return `apiVersion: agent.kagenti.dev/v1alpha1 kind: AgentRuntime metadata: - name: skill-agent-runtime - namespace: ` + skillTestNamespace + ` + name: skill-discovery-agent + namespace: ` + skillDiscoveryTestNamespace + ` spec: type: agent targetRef: apiVersion: apps/v1 kind: Deployment - name: skill-agent-target - skills: - - name: resume-reviewer - image: registry.k8s.io/pause:3.9 - mountPath: /agent/skills/resume-reviewer - - name: blog-writer - image: registry.k8s.io/pause:3.10 - mountPath: /agent/skills/blog-writer - pullPolicy: Always + name: skill-discovery-agent ` } -func skillAgentRuntimeUpdatedFixture() string { - return `apiVersion: agent.kagenti.dev/v1alpha1 -kind: AgentRuntime +// ociSkillDeploymentFixture returns YAML for a Deployment with two OCI +// ImageVolume skills and the kagenti.io/skills annotation listing both. +func ociSkillDeploymentFixture() string { + return `apiVersion: apps/v1 +kind: Deployment metadata: - name: skill-agent-runtime - namespace: ` + skillTestNamespace + ` + name: oci-skill-agent + namespace: ` + skillDiscoveryTestNamespace + ` + labels: + app.kubernetes.io/name: oci-skill-agent + annotations: + kagenti.io/skills: '["summarizer","openshift-review"]' spec: - type: agent - targetRef: - apiVersion: apps/v1 - kind: Deployment - name: skill-agent-target - skills: - - name: resume-reviewer - image: registry.k8s.io/pause:3.10 - mountPath: /agent/skills/resume-reviewer - - name: blog-writer - image: registry.k8s.io/pause:3.10 - mountPath: /agent/skills/blog-writer - pullPolicy: Always + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/name: oci-skill-agent + template: + metadata: + labels: + app.kubernetes.io/name: oci-skill-agent + kagenti.io/inject: disabled + spec: + securityContext: + runAsNonRoot: true + runAsUser: 1000 + seccompProfile: + type: RuntimeDefault + containers: + - name: agent + image: registry.k8s.io/pause:3.9 + imagePullPolicy: IfNotPresent + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + volumeMounts: + - name: skill-summarizer + mountPath: /app/skills/summarizer + readOnly: true + - name: skill-openshift-review + mountPath: /app/skills/openshift-review + readOnly: true + volumes: + - name: skill-summarizer + image: + reference: registry.k8s.io/pause:3.9 + - name: skill-openshift-review + image: + reference: registry.k8s.io/pause:3.9 ` } -func skillAgentRuntimeNoSkillsFixture() string { - return `apiVersion: agent.kagenti.dev/v1alpha1 -kind: AgentRuntime +// ociSkillDeploymentOneSkillFixture returns the same Deployment with one skill +// removed (openshift-review), simulating an OCI skill removal. +func ociSkillDeploymentOneSkillFixture() string { + return `apiVersion: apps/v1 +kind: Deployment metadata: - name: skill-agent-runtime - namespace: ` + skillTestNamespace + ` + name: oci-skill-agent + namespace: ` + skillDiscoveryTestNamespace + ` + labels: + app.kubernetes.io/name: oci-skill-agent + annotations: + kagenti.io/skills: '["summarizer"]' spec: - type: agent - targetRef: - apiVersion: apps/v1 - kind: Deployment - name: skill-agent-target + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/name: oci-skill-agent + template: + metadata: + labels: + app.kubernetes.io/name: oci-skill-agent + kagenti.io/inject: disabled + spec: + securityContext: + runAsNonRoot: true + runAsUser: 1000 + seccompProfile: + type: RuntimeDefault + containers: + - name: agent + image: registry.k8s.io/pause:3.9 + imagePullPolicy: IfNotPresent + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + volumeMounts: + - name: skill-summarizer + mountPath: /app/skills/summarizer + readOnly: true + volumes: + - name: skill-summarizer + image: + reference: registry.k8s.io/pause:3.9 ` } -func skillDuplicateNamesAgentRuntimeFixture() string { +// ociSkillAgentRuntimeFixture returns YAML for an AgentRuntime CR +// targeting the oci-skill-agent Deployment. +func ociSkillAgentRuntimeFixture() string { return `apiVersion: agent.kagenti.dev/v1alpha1 kind: AgentRuntime metadata: - name: skill-duplicate-runtime - namespace: ` + skillTestNamespace + ` + name: oci-skill-agent + namespace: ` + skillDiscoveryTestNamespace + ` spec: type: agent targetRef: apiVersion: apps/v1 kind: Deployment - name: skill-agent-target - skills: - - name: my-skill - image: registry.k8s.io/pause:3.9 - mountPath: /agent/skills/my-skill - - name: my-skill - image: registry.k8s.io/pause:3.10 - mountPath: /agent/skills/my-skill-2 + name: oci-skill-agent ` } -func skillDuplicateMountPathAgentRuntimeFixture() string { - return `apiVersion: agent.kagenti.dev/v1alpha1 -kind: AgentRuntime +// combinedClusterSPIFFEIDFixture returns YAML for a ClusterSPIFFEID matching +// the combined test namespace. +func combinedClusterSPIFFEIDFixture() string { + return `apiVersion: spire.spiffe.io/v1alpha1 +kind: ClusterSPIFFEID metadata: - name: skill-dup-mount-runtime - namespace: ` + skillTestNamespace + ` + name: e2e-combined-test spec: - type: agent - targetRef: - apiVersion: apps/v1 - kind: Deployment - name: skill-agent-target - skills: - - name: skill-a - image: registry.k8s.io/pause:3.9 - mountPath: /agent/skills/shared - - name: skill-b - image: registry.k8s.io/pause:3.10 - mountPath: /agent/skills/shared + spiffeIDTemplate: "spiffe://{{ .TrustDomain }}/ns/{{ .PodMeta.Namespace }}/sa/{{ .PodSpec.ServiceAccountName }}" + podSelector: + matchLabels: + kagenti.io/type: agent + namespaceSelector: + matchLabels: + kagenti-enabled: "true" ` } diff --git a/kagenti-operator/test/utils/utils.go b/kagenti-operator/test/utils/utils.go index af4b84c0..7d744369 100644 --- a/kagenti-operator/test/utils/utils.go +++ b/kagenti-operator/test/utils/utils.go @@ -593,10 +593,10 @@ spec: return err == nil } -// EnableSkillImageVolumes creates a feature-gates ConfigMap with -// skillImageVolumes enabled and patches the controller Deployment to mount it. -func EnableSkillImageVolumes(namespace, deploy string) error { - By("creating feature-gates ConfigMap with skillImageVolumes enabled") +// EnableSkillDiscovery creates a feature-gates ConfigMap with +// skillDiscovery enabled and patches the controller Deployment to mount it. +func EnableSkillDiscovery(namespace, deploy string) error { + By("creating feature-gates ConfigMap with skillDiscovery enabled") cmd := exec.Command("kubectl", "apply", "-f", "-", "-n", namespace) cmd.Stdin = strings.NewReader(`apiVersion: v1 kind: ConfigMap @@ -606,11 +606,9 @@ data: feature-gates.yaml: | globalEnabled: true envoyProxy: true - spiffeHelper: true - clientRegistration: true injectTools: false perWorkloadConfigResolution: false - skillImageVolumes: true + skillDiscovery: true `) if _, err := Run(cmd); err != nil { return fmt.Errorf("failed to create feature-gates ConfigMap: %w", err) @@ -634,10 +632,10 @@ data: return WaitForRollout(deploy, namespace, 2*time.Minute) } -// DisableSkillImageVolumes updates the feature-gates ConfigMap to disable -// skillImageVolumes. The feature gate loader's file watcher picks up changes. -func DisableSkillImageVolumes(namespace string) error { - By("updating feature-gates ConfigMap to disable skillImageVolumes") +// DisableSkillDiscovery updates the feature-gates ConfigMap to disable +// skillDiscovery. The feature gate loader's file watcher picks up changes. +func DisableSkillDiscovery(namespace string) error { + By("updating feature-gates ConfigMap to disable skillDiscovery") cmd := exec.Command("kubectl", "apply", "-f", "-", "-n", namespace) cmd.Stdin = strings.NewReader(`apiVersion: v1 kind: ConfigMap @@ -647,11 +645,9 @@ data: feature-gates.yaml: | globalEnabled: true envoyProxy: true - spiffeHelper: true - clientRegistration: true injectTools: false perWorkloadConfigResolution: false - skillImageVolumes: false + skillDiscovery: false `) if _, err := Run(cmd); err != nil { return fmt.Errorf("failed to update feature-gates ConfigMap: %w", err)