Skip to content

feat(filter): support envFrom for ConfigMap/Secret env injection#63

Open
nikitos wants to merge 6 commits into
PlainsightAI:mainfrom
nikitos:pr/envfrom
Open

feat(filter): support envFrom for ConfigMap/Secret env injection#63
nikitos wants to merge 6 commits into
PlainsightAI:mainfrom
nikitos:pr/envfrom

Conversation

@nikitos

@nikitos nikitos commented May 26, 2026

Copy link
Copy Markdown

Summary

Adds support for the standard Kubernetes envFrom mechanism on filters, so a filter can populate its container environment variables in bulk from ConfigMaps and/or Secrets, in addition to the existing per-variable env field.

What changed

  • API: New optional EnvFrom []corev1.EnvFromSource field on the Filter spec (api/v1alpha1/pipeline_types.go), using the standard Kubernetes EnvFromSource type for full compatibility (ConfigMap refs, Secret refs, and prefix).
  • Reconciliation: filter.EnvFrom is propagated to the container on both filter build paths: batch jobs (buildJob) and streaming deployments (buildStreamingDeployment).
  • Generated artifacts: Regenerated zz_generated.deepcopy.go and both CRD YAMLs (config/crd/bases/... and deployment/.../crds/...), which stay in sync.

Precedence

Values defined in env (including those injected by the controller) take precedence over values sourced through envFrom, regardless of ordering. This follows standard kubelet behavior, where a container's explicit env entries override any matching variables brought in by envFrom.

Testing

Added unit tests verifying that envFrom entries (a ConfigMap ref and a Secret ref) are propagated to the resulting container on both paths:

  • TestBuildJob_EnvFrom_Propagated (batch)
  • TestBuildStreamingDeployment_EnvFrom_Propagated (streaming)

@lucasmundim lucasmundim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! The change is clean and the generated artifacts (deepcopy + both CRD YAMLs) are properly in sync.

A few asks before merging — left inline. Summary: please add unit tests for the new field on both the batch and streaming paths, and consider documenting the precedence interaction with env.

Comment thread api/v1alpha1/pipeline_types.go Outdated
Comment thread internal/controller/pipelineinstance_controller_batch.go
Comment thread internal/controller/pipelineinstance_controller_streaming.go

@lucasmundim lucasmundim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the feedback — docstring is clear, the precedence note is in place, and both the batch and streaming paths now have unit tests. Looks good to merge.

@shingonoide shingonoide left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, @nikitos. This is a clean, well-scoped addition. We verified that the new EnvFrom reaches the container on both the batch (buildJob) and streaming (buildStreamingDeployment) paths, which are the only two filter-to-container build sites, so nothing is missed. The generated deepcopy and both CRD YAMLs are in sync, and the precedence note (controller-injected env wins over envFrom, regardless of order) is an accurate description of kubelet behavior. Tests for both paths pass locally. Looks good to us.

@shingonoide shingonoide changed the title add envfrom to filter spec and pass it to k8s resources feat: add envFrom to filter spec and propagate to k8s workloads May 29, 2026
@shingonoide shingonoide changed the title feat: add envFrom to filter spec and propagate to k8s workloads feat(filter): support envFrom for ConfigMap/Secret env injection May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants