Skip to content

[Contour Gateway Provisioner] add imagepullsecret for envoy and contour (fixes #7138) #7141

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Contour Gateway Provisioner] add imagepullsecret for envoy and contour (fixes #7138) #7141

wants to merge 1 commit into from

Conversation

@flbla
Copy link

@flbla flbla commented Aug 4, 2025 •
edited
Loading

add a new image-pull-secret args for Contour Gateway Provisioner to be able to use an existing secret as image-pull-secret for envoy and contour

fixes #7138

@flbla flbla requested a review from a team as a code owner August 4, 2025 09:40
@flbla flbla requested review from sunjayBhatia and tsaarni and removed request for a team August 4, 2025 09:40
@sunjayBhatia sunjayBhatia requested review from a team, rajatvig and wilsonwu and removed request for a team August 4, 2025 09:40
@github-actions
Copy link

github-actions bot commented Aug 4, 2025

Hi @flbla! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@tsaarni tsaarni added the release-note/small A small change that needs one line of explanation in the release notes. label Aug 4, 2025
@codecov
Copy link

codecov bot commented Aug 4, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 74.35897% with 10 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1db83a7). Learn more about missing BASE report.
⚠️ Report is 45 commits behind head on main.

Files with missing lines Patch % Lines
...nternal/provisioner/objects/dataplane/dataplane.go 63.15% 6 Missing and 1 partial ⚠️
internal/provisioner/controller/gateway.go 50.00% 2 Missing ⚠️
cmd/contour/gatewayprovisioner.go 83.33% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7141   +/-   ##
=======================================
  Coverage        ?   81.04%           
=======================================
  Files           ?      130           
  Lines           ?    19696           
  Branches        ?        0           
=======================================
  Hits            ?    15962           
  Misses          ?     3449           
  Partials        ?      285
Files with missing lines Coverage Δ
...ernal/provisioner/objects/deployment/deployment.go 90.41% <100.00%> (ø)
cmd/contour/gatewayprovisioner.go 46.60% <83.33%> (ø)
internal/provisioner/controller/gateway.go 63.85% <50.00%> (ø)
...nternal/provisioner/objects/dataplane/dataplane.go 84.63% <63.15%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tsaarni
Copy link
Member

tsaarni commented Aug 13, 2025

thank you @flbla! Can you add changelog file changelogs/unreleased/7141-flbla-small.md with short description about the new command line flag in like this.

@flbla
Copy link
Author

flbla commented Aug 13, 2025

hi @tsaarni , okay, I added it

sunjayBhatia
sunjayBhatia reviewed Aug 14, 2025
View reviewed changes
Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

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

Question to consider:

Should we validate the image pull secret is present/valid in the cluster before reconciling the dataplane? or leave it up to the user to know that if their pods fail to deploy, it may be because the secret is invalid/not present?

sunjayBhatia
sunjayBhatia reviewed Aug 14, 2025
View reviewed changes
cmd/contour/gatewayprovisioner.go Outdated
Default(provisionerConfig.gatewayControllerName).
StringVar(&provisionerConfig.gatewayControllerName)

cmd.Flag("image-pull-secret", "The image pull secret for the managed Envoy and Contour.").
Copy link
Member

Choose a reason for hiding this comment

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

nit: change flag to image-pull-secret-name

sunjayBhatia
sunjayBhatia reviewed Aug 14, 2025
View reviewed changes
changelogs/unreleased/7141-flbla-small.md Outdated
@@ -0,0 +1 @@
add a new flag: --image-pull-secret, which allows users to specify a secret in the same namespace as the gatewayprovisionner for pulling images from private registries. when set, it's used to pull Envoy and Contour images. No newline at end of file
Copy link
Member

@sunjayBhatia sunjayBhatia Aug 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

shouldnt this specify that the secret lives in the same namespace as the deployed data/control plane? (not the provisioner, since it is a local object reference on the data/controlplane pod spec)

sunjayBhatia
sunjayBhatia reviewed Aug 14, 2025
View reviewed changes
internal/provisioner/objects/deployment/deployment.go
}

if imagePullSecret != "" {
deploy.Spec.Template.Spec.ImagePullSecrets = []core_v1.LocalObjectReference{
Copy link
Member

Choose a reason for hiding this comment

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

lets add some more test coverage (here and the dataplane tests) for the case when imagepullsecret is provided

@flbla
Copy link
Author

flbla commented Aug 18, 2025

Hi @sunjayBhatia, thank you for the code review, I've updated based on your feedback.

@github-actions
Copy link

The Contour project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 30d of inactivity, lifecycle/stale is applied
  • After 60d of inactivity since lifecycle/stale was applied, the PR is closed

You can:

  • Ensure your PR is passing all CI checks. PRs that are fully green are more likely to be reviewed. If you are having trouble with CI checks, reach out to the #contour channel in the Kubernetes Slack workspace.
  • Mark this PR as fresh by commenting or pushing a commit
  • Close this PR
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 26, 2025
@flbla
Copy link
Author

flbla commented Sep 26, 2025

Keep it open please

@tsaarni tsaarni removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunjayBhatia sunjayBhatia sunjayBhatia left review comments

@tsaarni tsaarni Awaiting requested review from tsaarni tsaarni is a code owner automatically assigned from projectcontour/maintainers

@rajatvig rajatvig Awaiting requested review from rajatvig rajatvig was automatically assigned from projectcontour/contour-reviewers

@wilsonwu wilsonwu Awaiting requested review from wilsonwu wilsonwu was automatically assigned from projectcontour/contour-reviewers

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

release-note/small A small change that needs one line of explanation in the release notes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Contour Gateway Provisioner] how to set an imagepullsecret for envoy daemonset?

3 participants

@flbla @tsaarni @sunjayBhatia