feat(skills-init): add securityContext + resources configuration to i…

[jholt96]

…nitContainer

there are issues with the initContainer where if a limitRanger is set in the namespace then it can add a limit that is too low for the skills to be added. There is also an issue where the securityContext is not set for enterprise controls and slows adoption

[Copilot]

Pull request overview

This PR adds user-configurable resources and securityContext for the skills-init init container so agents can (a) avoid too-low defaults injected by namespace LimitRanger and (b) satisfy enterprise security controls without slowing adoption.

Changes:

• Extends the Agent CRD schema to support spec.skills.initContainer.{resources,securityContext}.
• Adds new API type SkillsInitContainer (plus deepcopy generation) to represent the init container config.
• Updates the agent translator to apply the init container’s resources/securityContext (and to pass env/resources into the init container).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
helm/kagent-crds/templates/kagent.dev_agents.yaml	Adds CRD OpenAPI schema for skills.initContainer in the Helm-distributed CRDs.
go/api/config/crd/bases/kagent.dev_agents.yaml	Adds the same CRD schema in the controller-generated CRD bases.
go/api/v1alpha2/agent_types.go	Introduces SkillsInitContainer and wires it into SkillForAgent.
go/api/v1alpha2/zz_generated.deepcopy.go	Adds deepcopy support for the new init container config type.
go/core/internal/controller/translator/agent/adk_api_translator.go	Applies init container resources/securityContext and passes env/resources into skills-init.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

initEnv := append(dep.Env, sharedEnv...) propagates all user-specified deployment env vars (including potential SecretKeyRefs) into the skills-init init container. Since init containers often run with different permissions/attack surface, consider limiting init container env to only what it needs (e.g., shared/proxy-related env) or adding an explicit skills.initContainer.env field rather than inheriting dep.Env wholesale.

Suggested change

	initEnv := append(dep.Env, sharedEnv...)
	initEnv := make([]corev1.EnvVar, 0, len(sharedEnv))
	initEnv = append(initEnv, sharedEnv...)

[jholt96]

Just to add more context on this, We have strict securityContext requirements for all containers and this was blocking us from being able to properly deploy this out for our clusters. The same for the resources block being a requirement. Lmk if there are any changes!

[EItanya]

So I think a much simpler fix here could just use a restricted security context and small resources as the default rather than making this configurable. What do you think?

[jholt96]

So I think a much simpler fix here could just use a restricted security context and small resources as the default rather than making this configurable. What do you think?

I think a standard securityContext would be totally fine with me if thats the direction we want. I was trying to keep it configurable as that was the behavior before. I can update the PR with that!

For the resources, I actually saw the initContainer go OOM from the limit ranger setting too low of limits. We were having it pull about 10 skills folders which is when we were seeing that happen. Granted they were unnecessarily bloated folders but I think it might make sense to keep this configurable because eventually someone will hit the limits we set.

The other change in here is populating the envs for the initContainer. I can break that out into a separate PR if you want but we have github access behind corporate proxy so we needed to set those envs.

Lmk what you think and I can adjust the PR from there!

[EItanya]

So I think a much simpler fix here could just use a restricted security context and small resources as the default rather than making this configurable. What do you think?

I think a standard securityContext would be totally fine with me if thats the direction we want. I was trying to keep it configurable as that was the behavior before. I can update the PR with that!

For the resources, I actually saw the initContainer go OOM from the limit ranger setting too low of limits. We were having it pull about 10 skills folders which is when we were seeing that happen. Granted they were unnecessarily bloated folders but I think it might make sense to keep this configurable because eventually someone will hit the limits we set.

The other change in here is populating the envs for the initContainer. I can break that out into a separate PR if you want but we have github access behind corporate proxy so we needed to set those envs.

Lmk what you think and I can adjust the PR from there!

Ok, I buy that, but I do think we should probably split the env vars, and maybe have separate vars because I don't think we necessarily need to set all of the env vars from the main container, what do you think?

[jholt96]

So I think a much simpler fix here could just use a restricted security context and small resources as the default rather than making this configurable. What do you think?

I think a standard securityContext would be totally fine with me if thats the direction we want. I was trying to keep it configurable as that was the behavior before. I can update the PR with that!
For the resources, I actually saw the initContainer go OOM from the limit ranger setting too low of limits. We were having it pull about 10 skills folders which is when we were seeing that happen. Granted they were unnecessarily bloated folders but I think it might make sense to keep this configurable because eventually someone will hit the limits we set.
The other change in here is populating the envs for the initContainer. I can break that out into a separate PR if you want but we have github access behind corporate proxy so we needed to set those envs.
Lmk what you think and I can adjust the PR from there!

Ok, I buy that, but I do think we should probably split the env vars, and maybe have separate vars because I don't think we necessarily need to set all of the env vars from the main container, what do you think?

Makes sense to me will get the PR updated