Adding option to enable Back End HTTPS for Prow Ingress #573

NiJuFirenzia · 2025-12-10T19:16:14Z

Prow currently requires SSL connections to be terminated before connecting to the deck and hook pods. This does not enable users to run prow with backend https enabled on their ingress. This PR adds the option to enable backend https on ingresses by passing in specific flags to the hook and deck pods. The argument flags will look like as below:

hook:

- --enable-ssl=true
- --server-cert-file=/etc/prow/tls.crt
- --server-key-file=/etc/prow/tls.key

deck:

- --enable-ssl=true
- --server-cert-file=/etc/prow/tls.crt
- --server-key-file=/etc/prow/tls.key
- --client-cert-file=etc/prow/deck-client.crt

In the example above, a secret containing the cert file and key file was mounted to the deck and hook pods. Additionally this has been tested to continue working as is with ingress backend to be set to just HTTP if the flags are not passed in.

netlify · 2025-12-10T19:16:21Z

✅ Deploy Preview for k8s-prow ready!

Name	Link
🔨 Latest commit	`2ad6fa1`
🔍 Latest deploy log	https://app.netlify.com/projects/k8s-prow/deploys/696a3c89be577c00085419a4
😎 Deploy Preview	https://deploy-preview-573--k8s-prow.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

linux-foundation-easycla · 2025-12-10T19:16:22Z

The committers listed above are authorized under a signed CLA.

✅ login: NiJuFirenzia / name: Nikhil Jiju (2ad6fa1)

k8s-ci-robot · 2025-12-10T19:16:23Z

Welcome @NiJuFirenzia!

It looks like this is your first PR to kubernetes-sigs/prow 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/prow has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-12-10T19:16:27Z

Hi @NiJuFirenzia. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

NiJuFirenzia · 2025-12-10T19:21:29Z

This PR addresses Issue #328

matthyx · 2025-12-10T20:21:11Z

@NiJuFirenzia thanks for your PR, can you check the CLA and sign it?

matthyx · 2025-12-10T21:38:35Z

/ok-to-test

petr-muller · 2025-12-17T15:27:46Z

cmd/hook/main.go

+	enableSSL bool
+	certFile  string
+	keyFile   string


let's group this into a small struct that we can embed at both places, we can share validation etc. similarly to how the ThingOptions bits above

petr-muller · 2025-12-17T15:29:38Z

cmd/hook/main.go

+		httpServer := &http.Server{
+			Addr:    ":" + strconv.Itoa(o.port),
+			Handler: hookMux,
+		}


this seems identical in both branches, so can be extracted before the condition

petr-muller · 2025-12-17T16:12:21Z

cmd/deck/pluginhelp.go

 // cacheLife is the time that we keep a pluginhelp.Help struct before considering it stale.
 // We consider help valid for a minute to prevent excessive calls to hook.
 const cacheLife = time.Minute
+const tlsEnabledScehma = "https"


Suggested change

const tlsEnabledScehma = "https"

const tlsEnabledSchema = "https"

petr-muller · 2025-12-17T16:16:15Z

cmd/deck/pluginhelp.go

+			transport.TLSClientConfig = &tls.Config{
+				RootCAs: caCertPool,
+			}
+			http.DefaultTransport = transport


helpAgent should get a custom client member with this transport instead of modifying the global default transport (this would affect anything else in deck that is a http client and relies on the default)

petr-muller · 2025-12-17T16:29:03Z

cmd/deck/pluginhelp.go

+		caCert, err := os.ReadFile(ha.cert)
+		if err != nil {
+			return nil, fmt.Errorf("error decoding cert file: %w", err)
+		}
+		caCertPool := x509.NewCertPool()
+		caCertPool.AppendCertsFromPEM(caCert)


I am not entirely sure I like this coupling. This essentially reuses Deck's server cert as a CA cert to establish trust with Hook's server cert for this behavior where Deck is a client to hook's pluginhelp endpoint? That's a bit hacky and suprising.

That would require having to add another option flag to take in a client cert right? I had been trying to limit the options that we would need

k8s-ci-robot · 2026-01-15T13:31:53Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: NiJuFirenzia
Once this PR has been reviewed and has the lgtm label, please ask for approval from petr-muller. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Nikhil Jiju <Nikhil.Jiju@fmr.com>

NiJuFirenzia · 2026-01-16T14:32:44Z

All change requests have been addressed and this PR is ready for a re-review

petr-muller

There's a flag name mismatch between error message and actual names. I have pointed out some opportunities to reduce stuttering (sslEnablement.EnableSSL and verbosity sslEnablement vs ssl) but treat them as non-blocking nits.

petr-muller · 2026-01-24T18:17:26Z

pkg/flagutil/ssl_enablement.go

+func (o *SSLEnablementOptions) Validate(_ bool) error {
+	if o.EnableSSL {
+		if o.ServerCertFile == "" {
+			return errors.New("flag --enable-ssl was set to true but required flag --cert-file was not set")


Suggested change

return errors.New("flag --enable-ssl was set to true but required flag --cert-file was not set")

return errors.New("flag --enable-ssl was set to true but required flag --server-cert-file was not set")

actual flags are different

petr-muller · 2026-01-24T18:18:16Z

pkg/flagutil/ssl_enablement.go

+			return errors.New("flag --enable-ssl was set to true but required flag --cert-file was not set")
+		}
+		if o.ServerKeyFile == "" {
+			return errors.New("flag --enable-ssl was set to true but required flag --key-file was not set")


Suggested change

return errors.New("flag --enable-ssl was set to true but required flag --key-file was not set")

return errors.New("flag --enable-ssl was set to true but required flag --server-key-file was not set")

petr-muller · 2026-01-24T18:21:54Z

pkg/flagutil/ssl_enablement.go

+}
+
+func (o *SSLEnablementOptions) Validate(_ bool) error {
+	if o.EnableSSL {


We should either check also the opposite case (enabled is false but cert was passed, possibly confusing the admin they are using SSL when they are not), or make this an implied field from the presence of at least one file flags (if at least one is set, assume the user wants to have it enabled and validate both are set).

petr-muller · 2026-01-24T18:23:24Z

cmd/hook/main.go

 	bugzilla               prowflagutil.BugzillaOptions
 	instrumentationOptions prowflagutil.InstrumentationOptions
 	jira                   prowflagutil.JiraOptions
+	sslEnablement          prowflagutil.SSLEnablementOptions


Suggested change

sslEnablement prowflagutil.SSLEnablementOptions

ssl prowflagutil.SSLEnablementOptions

petr-muller · 2026-01-24T18:25:34Z

pkg/flagutil/ssl_enablement.go

+// HTTPS backend. If enableSSL is true, both certFile and keyFile must be set
+// to the location of the cert and key files respectively.
+
+type SSLEnablementOptions struct {


tbh I'd call this just SSLOptions or SSLServerOptions and the package flagutil/ssl

petr-muller · 2026-01-24T18:27:41Z

pkg/flagutil/ssl_enablement.go

+	EnableSSL      bool
+	ServerCertFile string
+	ServerKeyFile  string


Suggested change

EnableSSL bool

ServerCertFile string

ServerKeyFile string

Enabled bool

CertFile string

KeyFile string

the instance of this struct will typically called ssl or something so meaning is typically provided by that context

k8s-ci-robot added area/deck Issues or PRs related to prow's deck component area/hook Issues or PRs related to prow's hook component labels Dec 10, 2025

k8s-ci-robot requested review from cjwagner and smg247 December 10, 2025 19:16

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 10, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 10, 2025

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 10, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 10, 2025

kayhern force-pushed the add-option-for-ssl branch 2 times, most recently from b32381d to 2cfbae6 Compare December 16, 2025 15:12

petr-muller requested changes Dec 17, 2025

View reviewed changes

k8s-ci-robot assigned petr-muller Dec 17, 2025

kayhern force-pushed the add-option-for-ssl branch from 2cfbae6 to 9865cdd Compare January 15, 2026 13:31

Adding option to enable Back End HTTPS for Prow Ingress

2ad6fa1

Signed-off-by: Nikhil Jiju <Nikhil.Jiju@fmr.com>

kayhern force-pushed the add-option-for-ssl branch from 9865cdd to 2ad6fa1 Compare January 16, 2026 13:26

NiJuFirenzia requested a review from petr-muller January 16, 2026 14:51

petr-muller reviewed Jan 24, 2026

View reviewed changes

	const tlsEnabledScehma = "https"
	const tlsEnabledSchema = "https"

	return errors.New("flag --enable-ssl was set to true but required flag --cert-file was not set")
	return errors.New("flag --enable-ssl was set to true but required flag --server-cert-file was not set")

	sslEnablement prowflagutil.SSLEnablementOptions
	ssl prowflagutil.SSLEnablementOptions

Adding option to enable Back End HTTPS for Prow Ingress #573

Are you sure you want to change the base?

Adding option to enable Back End HTTPS for Prow Ingress #573

Conversation

NiJuFirenzia commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for k8s-prow ready!

Uh oh!

linux-foundation-easycla bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Dec 10, 2025

Uh oh!

k8s-ci-robot commented Dec 10, 2025

Uh oh!

NiJuFirenzia commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthyx commented Dec 10, 2025

Uh oh!

matthyx commented Dec 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 15, 2026

Uh oh!

NiJuFirenzia commented Jan 16, 2026

Uh oh!

petr-muller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

NiJuFirenzia commented Dec 10, 2025 •

edited

Loading

netlify bot commented Dec 10, 2025 •

edited

Loading

linux-foundation-easycla bot commented Dec 10, 2025 •

edited

Loading

NiJuFirenzia commented Dec 10, 2025 •

edited

Loading