Add Agent CRD and reconciler for remote AI agents#464
Open
nickmisasi wants to merge 17 commits into
Open
Conversation
mm-cloud-bot
added
the
release-note
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a new Estimated code review effort: 5 (Critical) | ~120 minutes ChangesMattermost Agent CR and LiteLLM Gateway Support
Sequence Diagram(s)sequenceDiagram
participant AgentReconciler
participant Kubernetes as Kubernetes API
participant MattermostCR as Mattermost CR
participant LiteLLMDeployment
participant AgentResources as Agent Resources
AgentReconciler->>Kubernetes: Get Agent
AgentReconciler->>Kubernetes: Get referenced Mattermost CR
Kubernetes-->>AgentReconciler: MattermostCR status
alt Mattermost not Stable
AgentReconciler->>Kubernetes: Update Agent status (Reconciling)
AgentReconciler-->>AgentReconciler: Requeue after dependencyRequeueDelay
else Mattermost Stable
AgentReconciler->>LiteLLMDeployment: checkLiteLLMGateway (readiness)
LiteLLMDeployment-->>AgentReconciler: ready/not-ready
AgentReconciler->>AgentResources: checkAgent (ServiceAccount, Secret, NetworkPolicy, Service, PVC, Deployment)
AgentResources-->>AgentReconciler: reconciled resources
AgentReconciler->>Kubernetes: checkAgentHealth (Deployment rollout status)
Kubernetes-->>AgentReconciler: readyReplicas/rollout state
AgentReconciler->>Kubernetes: Update Agent status (Stable/Error)
end
sequenceDiagram
participant MattermostReconciler
participant DBSecret as DB Credentials Secret
participant MasterKeySecret as Master Key Secret
participant LiteLLMDeployment
participant LiteLLMService
MattermostReconciler->>MattermostReconciler: cleanupLiteLLMIfDisabled
MattermostReconciler->>MattermostReconciler: checkLiteLLM (gateway enabled?)
MattermostReconciler->>DBSecret: Get DB credentials
DBSecret-->>MattermostReconciler: connectionString or error
MattermostReconciler->>MasterKeySecret: Get/Create master key (unowned)
MattermostReconciler->>LiteLLMDeployment: Create/Update Deployment (owned)
MattermostReconciler->>LiteLLMService: Create/Update Service (owned)
alt gateway disabled
MattermostReconciler->>LiteLLMDeployment: Delete (if owned)
MattermostReconciler->>LiteLLMService: Delete (if owned)
end
MattermostReconciler->>MattermostReconciler: Update Mattermost status (error/cleared)
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
apis/mattermost/v1beta1/agent_utils.go (1)
94-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent use of secret-name-prefix constant.
BotTokenSecretNamereusesAgentBotTokenSecretNamePrefix, butLiteLLMKeySecretNameandHookSecretNamehardcode the literal"agent-"instead. Minor inconsistency; not a functional issue since the values match today, but future prefix changes would only update one of the three names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apis/mattermost/v1beta1/agent_utils.go` around lines 94 - 112, The secret-name helpers are inconsistent because BotTokenSecretName uses AgentBotTokenSecretNamePrefix while LiteLLMKeySecretName and HookSecretName hardcode "agent-". Update LiteLLMKeySecretName and HookSecretName in Agent to reuse the same prefix constant (or a shared helper) so all secret-name builders stay in sync if the prefix changes.apis/mattermost/v1beta1/agent_types.go (1)
21-35: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNo minimum size validation on storage.
Sizeis a required field with no+kubebuilder:validationlower bound, so a PVC request of"0"would pass API validation and only fail (or behave oddly) at PVC creation. Consider adding a CEL/format constraint if zero-size storage should be rejected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apis/mattermost/v1beta1/agent_types.go` around lines 21 - 35, Add minimum-size validation to AgentStorageConfig.Size so zero or invalid storage requests are rejected at API validation time rather than during PVC creation. Update the AgentStorageConfig struct in agent_types.go with an appropriate kubebuilder/CEL validation constraint on the Size field, and keep the existing storage-related fields unchanged.controllers/mattermost/agent/controller_test.go (1)
29-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the
testLogger()helper.These four tests inline the same 4-line blubr logger construction, while later tests already use the
testLogger()helper (e.g. Line 449). Consider replacing the inline setup withlogger := testLogger()for consistency and less duplication.Also applies to: 81-84, 188-191, 260-263
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/mattermost/agent/controller_test.go` around lines 29 - 32, The tests are duplicating the same blubr/logger setup inline instead of reusing the existing testLogger() helper. Replace the repeated logger initialization in the affected tests with logger := testLogger() so the controller_test.go cases stay consistent with the later tests and avoid duplication.pkg/mattermost/litellm.go (3)
131-140: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueLiteLLM Deployment resource requests/limits are hardcoded.
CPU/memory for the shared gateway are fixed values with no way to size them via
OperatorManagedLLMGateway. For a namespace-shared component whose load scales with the number of agents/models, exposing this via the CR spec would be useful down the line.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/mattermost/litellm.go` around lines 131 - 140, The LiteLLM deployment in `litellm.go` is using hardcoded `Resources` values, so `OperatorManagedLLMGateway` cannot tune CPU/memory for different workloads. Update the deployment wiring to source requests/limits from the CR spec instead of fixed literals, using the existing LiteLLM deployment construction path so the shared gateway can be sized per namespace.
55-168: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winLiteLLM pod also lacks a
SecurityContext.Same concern as the Agent Deployment — no non-root/
AllowPrivilegeEscalation: false/dropped-capabilities hardening on the gateway container, which handles master keys and provider API keys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/mattermost/litellm.go` around lines 55 - 168, GenerateLiteLLMDeployment currently builds the litellm container without any SecurityContext hardening. Update the container spec in GenerateLiteLLMDeployment to add a SecurityContext like the Agent Deployment, ensuring the litellm container runs non-root, has AllowPrivilegeEscalation disabled, and drops capabilities. Keep the change localized to the container definition inside GenerateLiteLLMDeployment so the gateway pod is hardened consistently with the rest of the deployments.
97-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePull-policy computation duplicated with
pkg/mattermost/agent.go.The
imageTagNeedsAlwaysPull-based pull-policy selection (lines 97-100) is identical to the block inGenerateAgentDeployment(agent.go lines 181-184). Extracting a small shared helper, e.g.pullPolicyForImage(image string) corev1.PullPolicy, would remove the duplication.♻️ Suggested consolidation
func pullPolicyForImage(image string) corev1.PullPolicy { if imageTagNeedsAlwaysPull(image) { return corev1.PullAlways } return corev1.PullIfNotPresent }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/mattermost/litellm.go` around lines 97 - 100, The pull-policy selection logic in the liteLLM deployment setup is duplicated with GenerateAgentDeployment, so extract the shared image-based policy decision into a small helper such as pullPolicyForImage(image string) corev1.PullPolicy and use it from both the liteLLM code path and GenerateAgentDeployment instead of repeating the imageTagNeedsAlwaysPull check inline.pkg/mattermost/agent.go (2)
262-269: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated magic port number
8065instead of the localMattermostServerPortconstant.
mmServerURL(line 68-69) already uses theMattermostServerPortconstant, butGenerateAgentNetworkPolicyhardcodesintstr.FromInt32(8065)again. Reusing the constant avoids drift if the port is ever changed.♻️ Suggested fix
- mmPort := intstr.FromInt32(8065) + mmPort := intstr.FromInt32(MattermostServerPort)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/mattermost/agent.go` around lines 262 - 269, GenerateAgentNetworkPolicy currently duplicates the Mattermost server port as a magic number instead of reusing the existing MattermostServerPort constant. Update the mmPort setup in GenerateAgentNetworkPolicy to use the same constant already used by mmServerURL so the network policy stays aligned if the port changes. Keep the fix localized to the GenerateAgentNetworkPolicy function and remove the hardcoded 8065 value.
186-225: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo liveness/readiness probes on the agent container.
Unlike
GenerateLiteLLMDeployment(which defines both), the agent container has no probes. Without a readiness probe, the Service can route traffic to the pod before it's actually ready to serve, and without a liveness probe a hung process won't be restarted automatically.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/mattermost/agent.go` around lines 186 - 225, The agent Deployment built in the agent manifest currently defines the container in the deployment spec without any health probes. Update the container definition in the deployment निर्माण path to add both readiness and liveness probes, following the same pattern used by GenerateLiteLLMDeployment, so the agent container only receives traffic when ready and can be restarted if it becomes unhealthy.controllers/mattermost/agent/agent.go (1)
19-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the repeated create→get→update pattern.
checkAgentServiceAccount,checkAgentService,checkAgentDeployment, andcheckAgentNetworkPolicyeach repeat the same "CreateXIfNotExists → Get → Update" sequence with only the type and CreateIfNotExists call varying. A small generic helper (parameterized by client.Object and the create func) would remove this duplication while keepingcheckAgentService's extraCopyServiceEmptyAutoAssignedFieldsstep as a hook.♻️ Example consolidation sketch
func (r *AgentReconciler) ensureOwnedResource(ctx context.Context, owner metav1.Object, desired client.Object, current client.Object, create func() error, preUpdate func(), reqLogger logr.Logger) error { if err := create(); err != nil { return errors.Wrap(err, "failed to create resource") } if err := r.Client.Get(ctx, types.NamespacedName{Name: desired.GetName(), Namespace: desired.GetNamespace()}, current); err != nil { return errors.Wrap(err, "failed to get resource") } if preUpdate != nil { preUpdate() } return r.Resources.Update(current, desired, reqLogger) }Also applies to: 61-78, 110-125, 127-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/mattermost/agent/agent.go` around lines 19 - 34, The repeated create→get→update flow in checkAgentServiceAccount should be consolidated into a shared helper, since checkAgentService, checkAgentDeployment, and checkAgentNetworkPolicy follow the same pattern. Add a generic ensureOwnedResource-style helper in AgentReconciler that accepts the desired/current client.Object and a create callback, then performs the Get and Update steps with consistent error wrapping. Keep checkAgentService’s CopyServiceEmptyAutoAssignedFields logic as an optional preUpdate hook so the existing service-specific behavior remains intact.controllers/mattermost/agent/litellm_test.go (1)
1-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for the shared-resource lifecycle functions.
This file only tests
checkLiteLLMMasterKeyandcheckLiteLLMDBCredentials. The more intricate logic —cleanupLiteLLMIfLast's sibling-counting (excluding self by UID, excluding Agents mid-deletion),deleteLiteLLMResources, andensureUnownedResource's create/update branching incheckLiteLLMDeployment/checkLiteLLMService— has no direct tests here. Given this is the mechanism gating shared-resource teardown across sibling Agents, it's worth covering the "last agent" vs. "sibling remains" vs. "sibling being deleted concurrently" cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/mattermost/agent/litellm_test.go` around lines 1 - 93, This test file only covers LiteLLM secret checks and is missing coverage for the shared-resource lifecycle logic in cleanupLiteLLMIfLast, deleteLiteLLMResources, and ensureUnownedResource. Add tests that exercise the “last agent” path, the “sibling agent still exists” path, and the “sibling is being deleted” path, and verify the create/update behavior indirectly through checkLiteLLMDeployment and checkLiteLLMService. Use the existing reconciliation helpers and symbols like checkLiteLLMIfLast, deleteLiteLLMResources, ensureUnownedResource, checkLiteLLMDeployment, and checkLiteLLMService to locate the relevant code.controllers/mattermost/agent/litellm.go (1)
34-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo handling for
AlreadyExistson the create path for shared resources.These LiteLLM resources are explicitly shared across sibling Agents and created from independent Reconcile invocations.
ensureUnownedResourceandcheckLiteLLMMasterKeyboth do a Get-then-Create without handling a concurrentAlreadyExistsonCreate, which is a real possibility if two sibling Agents' reconciles race (e.g., both created at once, orMaxConcurrentReconciles > 1). The failure is self-healing on the next reconcile, but it's cheap to make idempotent up front.♻️ Example hardening for ensureUnownedResource
if createErr := r.Client.Create(ctx, desired); createErr != nil { - return pkgerrors.Wrap(createErr, "failed to create resource") + if k8sErrors.IsAlreadyExists(createErr) { + return nil + } + return pkgerrors.Wrap(createErr, "failed to create resource") }Also applies to: 100-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/mattermost/agent/litellm.go` around lines 34 - 50, The create path in ensureUnownedResource (and the similar Get-then-Create flow in checkLiteLLMMasterKey) is not idempotent under concurrent reconciles, so handle a Create returning AlreadyExists by re-reading the resource and continuing instead of failing. Update the logic around r.Client.Create and the shared-resource setup path so sibling AgentReconciler instances racing on the same LiteLLM object can safely converge without surfacing a transient error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controllers/mattermost/agent/litellm.go`:
- Around line 141-155: The LiteLLM credentials check only verifies that the
Secret exists, so `checkLiteLLMDBCredentials` should also validate that
`existing.Data` contains the required `connectionString` key. Update the logic
in `checkLiteLLMDBCredentials` to inspect the fetched `corev1.Secret` after
`r.Client.Get`, and return a clear error when the key is missing, matching the
existing `mmv1beta.AgentLiteLLMDBCredentialsSecret`/`connectionString` messaging
so the reconciler surfaces the issue early.
In `@pkg/mattermost/agent.go`:
- Around line 100-225: The Agent Deployment in GenerateAgentDeployment currently
creates a pod with no PodSecurityContext or container SecurityContext, so harden
the pod spec by adding security settings on the PodTemplateSpec and the
mmv1beta.Agent container. Update the Deployment to run as non-root, disable
privilege escalation, enforce a read-only root filesystem, and drop unnecessary
capabilities while keeping the existing volume mounts and env handling intact.
---
Nitpick comments:
In `@apis/mattermost/v1beta1/agent_types.go`:
- Around line 21-35: Add minimum-size validation to AgentStorageConfig.Size so
zero or invalid storage requests are rejected at API validation time rather than
during PVC creation. Update the AgentStorageConfig struct in agent_types.go with
an appropriate kubebuilder/CEL validation constraint on the Size field, and keep
the existing storage-related fields unchanged.
In `@apis/mattermost/v1beta1/agent_utils.go`:
- Around line 94-112: The secret-name helpers are inconsistent because
BotTokenSecretName uses AgentBotTokenSecretNamePrefix while LiteLLMKeySecretName
and HookSecretName hardcode "agent-". Update LiteLLMKeySecretName and
HookSecretName in Agent to reuse the same prefix constant (or a shared helper)
so all secret-name builders stay in sync if the prefix changes.
In `@controllers/mattermost/agent/agent.go`:
- Around line 19-34: The repeated create→get→update flow in
checkAgentServiceAccount should be consolidated into a shared helper, since
checkAgentService, checkAgentDeployment, and checkAgentNetworkPolicy follow the
same pattern. Add a generic ensureOwnedResource-style helper in AgentReconciler
that accepts the desired/current client.Object and a create callback, then
performs the Get and Update steps with consistent error wrapping. Keep
checkAgentService’s CopyServiceEmptyAutoAssignedFields logic as an optional
preUpdate hook so the existing service-specific behavior remains intact.
In `@controllers/mattermost/agent/controller_test.go`:
- Around line 29-32: The tests are duplicating the same blubr/logger setup
inline instead of reusing the existing testLogger() helper. Replace the repeated
logger initialization in the affected tests with logger := testLogger() so the
controller_test.go cases stay consistent with the later tests and avoid
duplication.
In `@controllers/mattermost/agent/litellm_test.go`:
- Around line 1-93: This test file only covers LiteLLM secret checks and is
missing coverage for the shared-resource lifecycle logic in
cleanupLiteLLMIfLast, deleteLiteLLMResources, and ensureUnownedResource. Add
tests that exercise the “last agent” path, the “sibling agent still exists”
path, and the “sibling is being deleted” path, and verify the create/update
behavior indirectly through checkLiteLLMDeployment and checkLiteLLMService. Use
the existing reconciliation helpers and symbols like checkLiteLLMIfLast,
deleteLiteLLMResources, ensureUnownedResource, checkLiteLLMDeployment, and
checkLiteLLMService to locate the relevant code.
In `@controllers/mattermost/agent/litellm.go`:
- Around line 34-50: The create path in ensureUnownedResource (and the similar
Get-then-Create flow in checkLiteLLMMasterKey) is not idempotent under
concurrent reconciles, so handle a Create returning AlreadyExists by re-reading
the resource and continuing instead of failing. Update the logic around
r.Client.Create and the shared-resource setup path so sibling AgentReconciler
instances racing on the same LiteLLM object can safely converge without
surfacing a transient error.
In `@pkg/mattermost/agent.go`:
- Around line 262-269: GenerateAgentNetworkPolicy currently duplicates the
Mattermost server port as a magic number instead of reusing the existing
MattermostServerPort constant. Update the mmPort setup in
GenerateAgentNetworkPolicy to use the same constant already used by mmServerURL
so the network policy stays aligned if the port changes. Keep the fix localized
to the GenerateAgentNetworkPolicy function and remove the hardcoded 8065 value.
- Around line 186-225: The agent Deployment built in the agent manifest
currently defines the container in the deployment spec without any health
probes. Update the container definition in the deployment निर्माण path to add
both readiness and liveness probes, following the same pattern used by
GenerateLiteLLMDeployment, so the agent container only receives traffic when
ready and can be restarted if it becomes unhealthy.
In `@pkg/mattermost/litellm.go`:
- Around line 131-140: The LiteLLM deployment in `litellm.go` is using hardcoded
`Resources` values, so `OperatorManagedLLMGateway` cannot tune CPU/memory for
different workloads. Update the deployment wiring to source requests/limits from
the CR spec instead of fixed literals, using the existing LiteLLM deployment
construction path so the shared gateway can be sized per namespace.
- Around line 55-168: GenerateLiteLLMDeployment currently builds the litellm
container without any SecurityContext hardening. Update the container spec in
GenerateLiteLLMDeployment to add a SecurityContext like the Agent Deployment,
ensuring the litellm container runs non-root, has AllowPrivilegeEscalation
disabled, and drops capabilities. Keep the change localized to the container
definition inside GenerateLiteLLMDeployment so the gateway pod is hardened
consistently with the rest of the deployments.
- Around line 97-100: The pull-policy selection logic in the liteLLM deployment
setup is duplicated with GenerateAgentDeployment, so extract the shared
image-based policy decision into a small helper such as pullPolicyForImage(image
string) corev1.PullPolicy and use it from both the liteLLM code path and
GenerateAgentDeployment instead of repeating the imageTagNeedsAlwaysPull check
inline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7710f81-3b63-4a6a-9135-c01c67cee467
📒 Files selected for processing (30)
apis/mattermost/v1beta1/agent_types.goapis/mattermost/v1beta1/agent_utils.goapis/mattermost/v1beta1/mattermost_types.goapis/mattermost/v1beta1/mattermost_utils.goapis/mattermost/v1beta1/zz_generated.deepcopy.goapis/mattermost/v1beta1/zz_generated.openapi.goconfig/crd/bases/installation.mattermost.com_agents.yamlconfig/crd/bases/installation.mattermost.com_mattermosts.yamlconfig/crd/kustomization.yamlconfig/rbac/role.yamlconfig/samples/installation.mattermost.com_v1beta1_agent.yamlconfig/samples/kustomization.yamlcontrollers/mattermost/agent/agent.gocontrollers/mattermost/agent/agent_test.gocontrollers/mattermost/agent/controller.gocontrollers/mattermost/agent/controller_test.gocontrollers/mattermost/agent/litellm.gocontrollers/mattermost/agent/litellm_test.gocontrollers/mattermost/agent/random.gocontrollers/mattermost/agent/utils.godocs/mattermost-operator/mattermost-operator.yamldocs/mattermost_v1beta1_crd.mdmain.gopkg/mattermost/agent.gopkg/mattermost/agent_test.gopkg/mattermost/litellm.gopkg/mattermost/litellm_test.gopkg/mattermost/mattermost_v1beta.gopkg/mattermost/mattermost_v1beta_test.gopkg/resources/create_resources.go
nickmisasi
force-pushed
the
the-trail-ship
branch
from
July 9, 2026 19:11
f360cbb to
d805277
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apis/mattermost/v1beta1/agent_utils.go (1)
94-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent use of the secret-name prefix constant.
AgentBotTokenSecretNamePrefix("agent-") is only used inBotTokenSecretName.LiteLLMKeySecretName,HookSecretName, andStoragePVCNameall hardcode the literal"agent-"instead of reusing the constant, so a future prefix change would need updating in four places instead of one.♻️ Proposed refactor
func (a *Agent) LiteLLMKeySecretName() string { - return "agent-" + a.Name + "-litellm-key" + return AgentBotTokenSecretNamePrefix + a.Name + "-litellm-key" } func (a *Agent) HookSecretName() string { - return "agent-" + a.Name + "-hook-secret" + return AgentBotTokenSecretNamePrefix + a.Name + "-hook-secret" } func (a *Agent) StoragePVCName() string { - return "agent-" + a.Name + "-storage" + return AgentBotTokenSecretNamePrefix + a.Name + "-storage" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apis/mattermost/v1beta1/agent_utils.go` around lines 94 - 112, The name-building helpers on Agent are inconsistently hardcoding the same prefix, so update LiteLLMKeySecretName, HookSecretName, and StoragePVCName to reuse AgentBotTokenSecretNamePrefix instead of the literal "agent-". Keep the existing BotTokenSecretName pattern as the reference and make all secret/PVC name helpers derive from the shared constant so future prefix changes only require one update.controllers/mattermost/agent/controller.go (1)
126-134: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTreat a missing Mattermost CR as a transient dependency, not a hard error.
An
Agentcan legitimately be created before (or briefly out of sync with) its referencedMattermost. Here any error fromGet— includingNotFound— is returned as a reconcile error, which produces error-level log noise and exponential backoff, whereas the very next branch (Line 135-138) treats a not-yet-StableMattermost as a normaldependencyRequeueDelayrequeue. HandlingNotFoundthe same way is more consistent and quieter.♻️ Suggested handling
mm := &mmv1beta.Mattermost{} err = r.Client.Get(ctx, types.NamespacedName{ Name: agent.Spec.MattermostRef.Name, Namespace: agent.Namespace, }, mm) if err != nil { + if k8sErrors.IsNotFound(err) { + reqLogger.Info("Referenced Mattermost not found, requeuing", "mattermost", agent.Spec.MattermostRef.Name) + return reconcile.Result{RequeueAfter: dependencyRequeueDelay}, nil + } r.updateStatusReconcilingAndLogError(ctx, agent, status, reqLogger, err) return reconcile.Result{}, err }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/mattermost/agent/controller.go` around lines 126 - 134, Treat a missing Mattermost CR as a transient dependency in the Agent reconcile flow. In controller/mattermost/agent/controller.go, update the r.Client.Get path in the Agent controller so that a NotFound from fetching the referenced Mattermost is handled the same way as the existing not-Stable branch: log at a non-error level if needed, update status/reconcile with dependencyRequeueDelay, and return a normal requeue instead of bubbling the error. Keep the behavior in the Agent reconcile logic around updateStatusReconcilingAndLogError and the Mattermost lookup consistent with the later dependency check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apis/mattermost/v1beta1/agent_utils.go`:
- Around line 94-112: The name-building helpers on Agent are inconsistently
hardcoding the same prefix, so update LiteLLMKeySecretName, HookSecretName, and
StoragePVCName to reuse AgentBotTokenSecretNamePrefix instead of the literal
"agent-". Keep the existing BotTokenSecretName pattern as the reference and make
all secret/PVC name helpers derive from the shared constant so future prefix
changes only require one update.
In `@controllers/mattermost/agent/controller.go`:
- Around line 126-134: Treat a missing Mattermost CR as a transient dependency
in the Agent reconcile flow. In controller/mattermost/agent/controller.go,
update the r.Client.Get path in the Agent controller so that a NotFound from
fetching the referenced Mattermost is handled the same way as the existing
not-Stable branch: log at a non-error level if needed, update status/reconcile
with dependencyRequeueDelay, and return a normal requeue instead of bubbling the
error. Keep the behavior in the Agent reconcile logic around
updateStatusReconcilingAndLogError and the Mattermost lookup consistent with the
later dependency check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4821efe-1b00-4f8e-bee5-8016b6926a33
📒 Files selected for processing (30)
apis/mattermost/v1beta1/agent_types.goapis/mattermost/v1beta1/agent_utils.goapis/mattermost/v1beta1/mattermost_types.goapis/mattermost/v1beta1/mattermost_utils.goapis/mattermost/v1beta1/zz_generated.deepcopy.goapis/mattermost/v1beta1/zz_generated.openapi.goconfig/crd/bases/installation.mattermost.com_agents.yamlconfig/crd/bases/installation.mattermost.com_mattermosts.yamlconfig/crd/kustomization.yamlconfig/rbac/role.yamlconfig/samples/installation.mattermost.com_v1beta1_agent.yamlconfig/samples/kustomization.yamlcontrollers/mattermost/agent/agent.gocontrollers/mattermost/agent/agent_test.gocontrollers/mattermost/agent/controller.gocontrollers/mattermost/agent/controller_test.gocontrollers/mattermost/agent/litellm.gocontrollers/mattermost/agent/litellm_test.gocontrollers/mattermost/agent/random.gocontrollers/mattermost/agent/utils.godocs/mattermost-operator/mattermost-operator.yamldocs/mattermost_v1beta1_crd.mdmain.gopkg/mattermost/agent.gopkg/mattermost/agent_test.gopkg/mattermost/litellm.gopkg/mattermost/litellm_test.gopkg/mattermost/mattermost_v1beta.gopkg/mattermost/mattermost_v1beta_test.gopkg/resources/create_resources.go
✅ Files skipped from review due to trivial changes (4)
- config/samples/kustomization.yaml
- config/crd/kustomization.yaml
- apis/mattermost/v1beta1/zz_generated.openapi.go
- apis/mattermost/v1beta1/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (23)
- controllers/mattermost/agent/random.go
- apis/mattermost/v1beta1/mattermost_utils.go
- config/rbac/role.yaml
- controllers/mattermost/agent/utils.go
- main.go
- pkg/mattermost/litellm.go
- config/samples/installation.mattermost.com_v1beta1_agent.yaml
- controllers/mattermost/agent/agent.go
- apis/mattermost/v1beta1/mattermost_types.go
- pkg/resources/create_resources.go
- config/crd/bases/installation.mattermost.com_mattermosts.yaml
- controllers/mattermost/agent/litellm_test.go
- pkg/mattermost/mattermost_v1beta.go
- apis/mattermost/v1beta1/agent_types.go
- pkg/mattermost/agent_test.go
- controllers/mattermost/agent/litellm.go
- config/crd/bases/installation.mattermost.com_agents.yaml
- docs/mattermost-operator/mattermost-operator.yaml
- pkg/mattermost/agent.go
- pkg/mattermost/litellm_test.go
- pkg/mattermost/mattermost_v1beta_test.go
- controllers/mattermost/agent/agent_test.go
- controllers/mattermost/agent/controller_test.go
Introduce the Agent CRD, resource generators, and reconciler for ServiceAccount, hook Secret, Service, Deployment, and deny-egress NetworkPolicy. Includes MM server RBAC for Agent CRs, sample manifest, and regenerated CRD/docs/YAML. Agent RBAC for the Mattermost server is opt-in via spec.agents.enabled. Co-authored-by: Cursor <cursoragent@cursor.com>
Add optional AgentStorageConfig with PVC generation, volume mounts, and create-only reconciler wiring before Deployment creation. Regenerates CRD, deepcopy/openapi, and docs for the new Storage field.
Support deny/allowWeb/allow egress NetworkPolicy modes with deny as the default, and auto-select PullAlways for :dev/:latest/untagged images. Updates the sample manifest and regenerates CRD/docs.
Add LLMGateway API types, LiteLLM resource generators, reconciler provisioning/readiness checks, finalizer teardown, and NetworkPolicy peer rules. Regenerates CRD/docs to complete the Agent feature tree.
nickmisasi
force-pushed
the
the-trail-ship
branch
from
July 9, 2026 19:52
d805277 to
8bb3ec5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apis/mattermost/v1beta1/agent_utils.go (1)
100-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the shared prefix constant in the other name helpers.
LiteLLMKeySecretName,HookSecretName, andStoragePVCNamehardcode"agent-"whileBotTokenSecretNameusesAgentBotTokenSecretNamePrefix. Consider aligning all four for consistency (and consider renaming the constant to something more generic since it is no longer just for bot tokens).♻️ Proposed fix
func (a *Agent) LiteLLMKeySecretName() string { - return "agent-" + a.Name + "-litellm-key" + return AgentBotTokenSecretNamePrefix + a.Name + "-litellm-key" } func (a *Agent) HookSecretName() string { - return "agent-" + a.Name + "-hook-secret" + return AgentBotTokenSecretNamePrefix + a.Name + "-hook-secret" } func (a *Agent) StoragePVCName() string { - return "agent-" + a.Name + "-storage" + return AgentBotTokenSecretNamePrefix + a.Name + "-storage" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apis/mattermost/v1beta1/agent_utils.go` around lines 100 - 112, The name helpers currently hardcode the shared "agent-" prefix in LiteLLMKeySecretName, HookSecretName, and StoragePVCName while BotTokenSecretName already uses AgentBotTokenSecretNamePrefix, so make all four helpers build names from the same shared prefix constant for consistency. Update the relevant helper methods on Agent to reference that constant, and if the constant name is too specific, rename it to something generic and use the new name everywhere these identifiers are constructed.apis/mattermost/v1beta1/agent_types.go (1)
60-66: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDocumented default for
EgressPolicyisn't backed by a CRD default.The comment states
"deny"is the default, but there's no+kubebuilder:defaultmarker, so the generated schema leaves the field empty when omitted rather than showing/enforcing"deny". Since this field governs network egress (a security-relevant setting), making the default explicit at the schema level improves auditability and avoids relying solely on reconciler-side fallback logic.♻️ Proposed fix
// +kubebuilder:validation:Enum=deny;allowWeb;allow + // +kubebuilder:default=deny // +optional EgressPolicy string `json:"egressPolicy,omitempty"`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apis/mattermost/v1beta1/agent_types.go` around lines 60 - 66, Add an explicit CRD default for EgressPolicy in the Agent type so the schema matches the documented "deny" default. Update the EgressPolicy field in AgentSpec/Agent type to include a kubebuilder default marker alongside the existing validation enum, keeping the comment and schema aligned for the generated CRD.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apis/mattermost/v1beta1/agent_types.go`:
- Around line 141-151: LLMGatewayConfig currently allows both External and
OperatorManaged to be set, but they must be mutually exclusive while still
allowing both to be nil as the default. Add validation in the LLMGatewayConfig
handling path to reject any config where both fields are non-nil, and make sure
the reconcile logic for the agent’s gateway selection continues to treat nil as
unset/default rather than an error. Use the LLMGatewayConfig, External, and
OperatorManaged symbols to locate the change.
---
Nitpick comments:
In `@apis/mattermost/v1beta1/agent_types.go`:
- Around line 60-66: Add an explicit CRD default for EgressPolicy in the Agent
type so the schema matches the documented "deny" default. Update the
EgressPolicy field in AgentSpec/Agent type to include a kubebuilder default
marker alongside the existing validation enum, keeping the comment and schema
aligned for the generated CRD.
In `@apis/mattermost/v1beta1/agent_utils.go`:
- Around line 100-112: The name helpers currently hardcode the shared "agent-"
prefix in LiteLLMKeySecretName, HookSecretName, and StoragePVCName while
BotTokenSecretName already uses AgentBotTokenSecretNamePrefix, so make all four
helpers build names from the same shared prefix constant for consistency. Update
the relevant helper methods on Agent to reference that constant, and if the
constant name is too specific, rename it to something generic and use the new
name everywhere these identifiers are constructed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dae00a3-3e7b-49f5-84d7-598398b58e97
📒 Files selected for processing (30)
apis/mattermost/v1beta1/agent_types.goapis/mattermost/v1beta1/agent_utils.goapis/mattermost/v1beta1/mattermost_types.goapis/mattermost/v1beta1/mattermost_utils.goapis/mattermost/v1beta1/zz_generated.deepcopy.goapis/mattermost/v1beta1/zz_generated.openapi.goconfig/crd/bases/installation.mattermost.com_agents.yamlconfig/crd/bases/installation.mattermost.com_mattermosts.yamlconfig/crd/kustomization.yamlconfig/rbac/role.yamlconfig/samples/installation.mattermost.com_v1beta1_agent.yamlconfig/samples/kustomization.yamlcontrollers/mattermost/agent/agent.gocontrollers/mattermost/agent/agent_test.gocontrollers/mattermost/agent/controller.gocontrollers/mattermost/agent/controller_test.gocontrollers/mattermost/agent/litellm.gocontrollers/mattermost/agent/litellm_test.gocontrollers/mattermost/agent/random.gocontrollers/mattermost/agent/utils.godocs/mattermost-operator/mattermost-operator.yamldocs/mattermost_v1beta1_crd.mdmain.gopkg/mattermost/agent.gopkg/mattermost/agent_test.gopkg/mattermost/litellm.gopkg/mattermost/litellm_test.gopkg/mattermost/mattermost_v1beta.gopkg/mattermost/mattermost_v1beta_test.gopkg/resources/create_resources.go
✅ Files skipped from review due to trivial changes (4)
- config/crd/kustomization.yaml
- config/samples/kustomization.yaml
- apis/mattermost/v1beta1/zz_generated.openapi.go
- apis/mattermost/v1beta1/zz_generated.deepcopy.go
🚧 Files skipped from review as they are similar to previous changes (23)
- controllers/mattermost/agent/random.go
- config/rbac/role.yaml
- config/samples/installation.mattermost.com_v1beta1_agent.yaml
- apis/mattermost/v1beta1/mattermost_utils.go
- main.go
- apis/mattermost/v1beta1/mattermost_types.go
- pkg/mattermost/mattermost_v1beta.go
- controllers/mattermost/agent/litellm_test.go
- config/crd/bases/installation.mattermost.com_mattermosts.yaml
- pkg/mattermost/litellm.go
- controllers/mattermost/agent/utils.go
- controllers/mattermost/agent/agent.go
- config/crd/bases/installation.mattermost.com_agents.yaml
- docs/mattermost-operator/mattermost-operator.yaml
- pkg/mattermost/agent.go
- controllers/mattermost/agent/controller.go
- controllers/mattermost/agent/controller_test.go
- pkg/mattermost/agent_test.go
- controllers/mattermost/agent/litellm.go
- pkg/mattermost/mattermost_v1beta_test.go
- pkg/mattermost/litellm_test.go
- controllers/mattermost/agent/agent_test.go
- pkg/resources/create_resources.go
Gateway config (image, resources) now lives on Mattermost spec.agents.llmGateway and the Mattermost controller reconciles the LiteLLM Deployment/Service with owner references. The Agent-side operatorManaged field becomes an empty opt-in marker with CEL exactly-one validation on the gateway union. Deletes the per-Agent finalizer, sibling-counting cleanup, and unowned-resource machinery that compensated for the old ownership model; drops the LiteLLM ConfigMap (its only setting already exists as an env var); wires the previously dead resources field through to the deployment; replaces the two new copy-paste CreateXIfNotExists helpers with one generic ResourceHelper.CreateIfNotExists and re-hides the last-applied annotation constant. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Agents get a dedicated installation.mattermost.com/agent label; ClusterLabel now truthfully points at the referenced installation, and NetworkPolicy peers match the full Mattermost selector labels so an agent can no longer satisfy server peer selectors by name collision. External gateways get a port-scoped egress rule derived from the URL (with CEL URL validation) instead of meaningless LiteLLM pod selectors. EgressPolicy becomes a typed enum dispatched via a single switch, removing the build-then-discard flow; the two web rules collapse into one. Adds a single Agent.GatewayEndpoint() resolver, secret-key and service-URL constants/helpers, a TCP readiness probe for the agent container, safer resource defaulting, and drops the impossible error return from Agent.SetDefaults. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Reconcile now runs one phase per concern (fetch, defaults persistence, Mattermost gate, LiteLLM gate, checkAgent, health, status) instead of eleven inline checks with nine cloned error stanzas. Config issues are marked with a sentinel error so quiet 60s requeues apply only to confirmed misconfiguration while transient API errors keep controller-runtime backoff. Secret checks now validate required keys, ObservedGeneration is recorded up front, NetworkPolicy is ensured before the Deployment, rollout health requires the observed generation and updated replicas, and Mattermost/LiteLLM events now enqueue dependent Agents via field-indexed watches. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Deduplicates the copy-pasted fixture stacks behind shared builders and one setup entry point, makes generator tests the single owner of shape assertions (egress/env matrices) while controller tests assert orchestration only, replaces slice-index NetworkPolicy assertions with semantic rule lookups, deletes fossil comments referencing code that never existed, and adds coverage for PVC create-only semantics, drift repair, the external-gateway happy path, LiteLLM unready-to-ready gating, and gateway disable transitions. Net -794 test lines with coverage unchanged (82.7% agent controller, 92.2% pkg/mattermost). Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
The Agent type carried +genclient with no generated client, lister, or informer. Regenerated with code-generator v0.26.3 (matching the style of the committed pkg/client tree) so the v1beta1 clientset now includes Agents. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Embeds Mattermost owner references in the LiteLLM generators so updates no longer strip ownership, guards disable-path deletion with an IsControlledBy check so a second installation in the namespace cannot destroy the gateway, and reorders reconciliation so gateway config problems surface in status.Error while the installation stays Stable instead of freezing every dependent Agent. Also: tightens external gateway URL validation to http/https so the egress port rule cannot be silently dropped, persists Reconciling status on dependency waits, adds secret watches for unowned prerequisite secrets in both controllers, rolls LiteLLM pods on DB credential rotation via a pod-template hash annotation, completes the rollout check with Status.Replicas, rejects llmGateway without agents.enabled and agent names colliding with the installation name at admission, and threads ctx through the new mattermost-controller code. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
checkLiteLLMDeployment/Service now refuse to update a fixed-name gateway object controlled by another Mattermost installation, so two installations in one namespace surface a status error instead of stealing the gateway from each other on every reconcile. The external gateway URL rule now requires a host and an http/https scheme via the CEL URL library, closing the scheme-less case (litellm.example.com:4000 parses as a scheme) that silently dropped the egress rule. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apis/mattermost/v1beta1/agent_types.go`:
- Around line 159-166: Add a CEL validation rule to the Agent markers near the
existing metadata-name XValidation, requiring self.metadata.name to match RFC
1035 label requirements: lowercase alphanumeric characters or hyphens, starting
and ending with an alphanumeric character, and no more than 63 characters.
Preserve the existing rule requiring the agent name to differ from
self.spec.mattermostRef.name.
In `@apis/mattermost/v1beta1/mattermost_utils.go`:
- Around line 91-110: Update AgentsLLMGateway.SetDefaults to default
Resources.Requests and Resources.Limits together only when both are nil,
mirroring Agent.SetDefaults. Preserve any user-provided partial resource
configuration and avoid injecting default requests or limits that could conflict
with the supplied counterpart.
In `@controllers/mattermost/agent/health_check.go`:
- Around line 23-30: Update the Deployment lookup error path in the health-check
function to clear or reset status.ReadyReplicas before returning the wrapped Get
error, ensuring failed lookups cannot preserve prior replica counts. Keep the
existing error wrapping and successful Deployment handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c218ea32-cf60-494c-83ff-dfca07f37330
📒 Files selected for processing (41)
apis/mattermost/v1beta1/agent_types.goapis/mattermost/v1beta1/agent_utils.goapis/mattermost/v1beta1/mattermost_types.goapis/mattermost/v1beta1/mattermost_utils.goapis/mattermost/v1beta1/zz_generated.deepcopy.goapis/mattermost/v1beta1/zz_generated.openapi.goconfig/crd/bases/installation.mattermost.com_agents.yamlconfig/crd/bases/installation.mattermost.com_mattermosts.yamlconfig/samples/installation.mattermost.com_v1beta1_agent.yamlcontrollers/mattermost/agent/agent.gocontrollers/mattermost/agent/agent_test.gocontrollers/mattermost/agent/controller.gocontrollers/mattermost/agent/controller_test.gocontrollers/mattermost/agent/health_check.gocontrollers/mattermost/agent/litellm.gocontrollers/mattermost/agent/litellm_test.gocontrollers/mattermost/agent/status.gocontrollers/mattermost/mattermost/controller.gocontrollers/mattermost/mattermost/controller_test.gocontrollers/mattermost/mattermost/litellm.gocontrollers/mattermost/mattermost/litellm_test.godocs/mattermost-operator/mattermost-operator.yamldocs/mattermost_v1beta1_crd.mdpkg/client/v1beta1/clientset/versioned/typed/mattermost/v1beta1/agent.gopkg/client/v1beta1/clientset/versioned/typed/mattermost/v1beta1/fake/fake_agent.gopkg/client/v1beta1/clientset/versioned/typed/mattermost/v1beta1/fake/fake_mattermost_client.gopkg/client/v1beta1/clientset/versioned/typed/mattermost/v1beta1/generated_expansion.gopkg/client/v1beta1/clientset/versioned/typed/mattermost/v1beta1/mattermost_client.gopkg/client/v1beta1/informers/externalversions/generic.gopkg/client/v1beta1/informers/externalversions/mattermost/v1beta1/agent.gopkg/client/v1beta1/informers/externalversions/mattermost/v1beta1/interface.gopkg/client/v1beta1/listers/mattermost/v1beta1/agent.gopkg/client/v1beta1/listers/mattermost/v1beta1/expansion_generated.gopkg/mattermost/agent.gopkg/mattermost/agent_test.gopkg/mattermost/litellm.gopkg/mattermost/litellm_test.gopkg/mattermost/mattermost_v1beta.gopkg/mattermost/mattermost_v1beta_test.gopkg/resources/create_resources.gopkg/utils/random.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/mattermost/mattermost_v1beta.go
- apis/mattermost/v1beta1/zz_generated.openapi.go
- docs/mattermost-operator/mattermost-operator.yaml
- config/crd/bases/installation.mattermost.com_agents.yaml
Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
The Agent and Mattermost CRDs now carry x-kubernetes-validations (CEL) rules, which require Kubernetes >= 1.25; the e2e kind cluster was pinned to v1.22.9 so 'make deploy' failed applying the CRDs. Bump the node image and kubectl to v1.32.5 (shipped with the pinned kind v0.29.0) and move mysql-operator to v0.6.3, whose v0.6.2 predecessor relied on PodSecurityPolicy and batch/v1beta1 APIs removed in 1.25. Also switch kubectl downloads to dl.k8s.io since the legacy GCS bucket no longer serves new releases. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
The mattermosts CRD exceeds the 256KiB cap on the client-side last-applied-configuration annotation, so plain kubectl apply is rejected by the API server. Co-authored-by: Nick Misasi <nick13misasi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
AgentCRD (installation.mattermost.com/v1beta1) and reconciler that manage the infrastructure for remote AI agent pods integrating with a Mattermost installation. The operator is pure infrastructure — bot provisioning and all LiteLLM business logic (model registration, virtual-key creation) are owned by the mattermost-plugin-agents plugin.The work is structured as four incremental, independently building commits (suitable for commit-by-commit review):
minLength, CEL rule onmattermostRef.name, egress enum), resource generators, and reconciliation of ServiceAccount, hook Secret (generated), Service, Deployment, and a deny-egress NetworkPolicy (Mattermost server + DNS only). Mattermost server RBAC for Agent CRs is opt-in viaspec.agents.enabledon the Mattermost CR. Includes RBAC markers, sample manifest, and regenerated CRD/docs/install YAML.spec.storage(AgentStorageConfig) with PVC generation, volume mount, create-only reconcile, and cascade delete via owner references.deny(default) /allowWeb(outbound TCP 80/443, port-based) /allow(all egress) NetworkPolicy modes;PullAlwaysauto-selected for:dev/:latest/untagged images.status.errorwith a 60s requeue instead of creating pods with unresolvable secret refs.Test coverage is roughly 1:1 with production code, including controller-level tests for the missing-secret paths, all three egress modes, finalizer/teardown behavior, and generator unit tests.
Ticket Link
NONE
Release Note
Made with Cursor