Skip to content

Add extra containers entry to inject side cars into deployment #3486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ab-andresc
Copy link
Contributor

@ab-andresc ab-andresc commented Jun 10, 2025

Proposed changes

This feature adds an extra variable to the deployment template to add extra containers.

Problem: Users would like to inject additional sidecars, these can do a variety of things.

Example: Install sigsci agent as a sidecar

https://www.fastly.com/documentation/guides/next-gen-waf/setup-and-configuration/kubernetes/kubernetes-agent-module/

Use case 2:

Do JWT validation with a separate container

Solution: Adds an entry to the template.

Testing: Manually added a sidecar, rendering looks correct.

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Copy link

nginx-bot bot commented Jun 10, 2025

Hi @ab-andresc! Welcome to the project! 🎉

Thanks for opening this pull request!
Be sure to check out our Contributing Guidelines while you wait for someone on the team to review this.

Please make sure to include the issue number in the PR description to automatically close the issue when the PR is merged.
See Linking a pull request to an issue and our Pull Request Guidelines for more information.

@nginx-bot nginx-bot bot added the community label Jun 10, 2025
Copy link
Contributor

github-actions bot commented Jun 10, 2025

✅ All required contributors have signed the F5 CLA for this PR. Thank you!
Posted by the CLA Assistant Lite bot.

@github-actions github-actions bot added the helm-chart Relates to helm chart label Jun 10, 2025
@ab-andresc ab-andresc changed the title add extra containers Add extra containers entry to inject side cars into deployment Jun 10, 2025
@ab-andresc ab-andresc force-pushed the patch-1 branch 3 times, most recently from 20fcf9f to 2040ee0 Compare June 10, 2025 11:19
@ab-andresc
Copy link
Contributor Author

I have hereby read the F5 CLA and agree to its terms

@ab-andresc ab-andresc force-pushed the patch-1 branch 2 times, most recently from e1b1ab5 to 66936a3 Compare June 10, 2025 11:26
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jun 10, 2025
@sjberman
Copy link
Collaborator

Hi @ab-andresc, I'm curious about the use cases you mention. What benefit does the sigsci-agent or JWT validation do for the control plane? What connections are these sidecars handling?

@sjberman
Copy link
Collaborator

Per offline conversation, it seems the intention is to actually configure containers for the NGINX deployment, not the NGF control plane deployment. Since the control plane now provisions the NGINX data plane, the latter is no longer exposed directly in the helm chart. Instead, we include these configurable fields in the NginxProxy CRD and then the control plane uses that to set the deployment config when provisioning. An example of how we add a new field for this type of use case can be seen here: #3319.

@sindhushiv sindhushiv added this to the v2.0.1 milestone Jun 10, 2025
@sindhushiv sindhushiv moved this from 🆕 New to External Pull Requests in NGINX Gateway Fabric Jun 10, 2025
@@ -410,6 +416,11 @@ type DaemonSetSpec struct {
//
// +optional
Container ContainerSpec `json:"container"`
// ExtraContainers defines additional containers to be added to the NGINX Pod.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// ExtraContainers defines additional containers to be added to the NGINX Pod.
// ExtraContainers defines additional containers to be added to the NGINX Pod.

Comment on lines +401 to +402
// ExtraContainers defines additional containers to be added to the NGINX Pod.
// Uses upstream corev1.Container to allow arbitrary sidecars.
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be great if you could specify one example of the sidecars users can use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community documentation Improvements or additions to documentation helm-chart Relates to helm chart
Projects
Status: External Pull Requests
Development

Successfully merging this pull request may close these issues.

4 participants