Feat: Add corporate certificates to Build APIs by sayan-biswas · Pull Request #2107 · shipwright-io/build

sayan-biswas · 2026-02-18T07:10:18Z

Changes:

Added APIs to allow user to define custom Root CAs in Build and BuildRun APIs

Submitter Checklist

Includes tests if functionality changed/was added
Includes docs if changes are user-facing
Set a kind label on this PR
Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Custom CA certificated can now be provided in `Build` and `BuildRun` under `CABundle` property.

openshift-ci · 2026-02-18T07:10:24Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign saschaschwarze0 for approval. 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:

OWNERS

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

sayan-biswas · 2026-02-24T10:30:42Z

/retest

Copilot

Pull request overview

This PR adds support for corporate/custom Root CA certificates to the Build and BuildRun APIs. The feature allows users to specify CA bundles via Secret or ConfigMap references at both the Build and BuildRun levels, with the BuildRun specification taking precedence. The certificates are mounted into workload containers and relevant environment variables are configured.

Changes:

Added CA bundle API types (CABundle, SourceObjectKeySelector) for Build/BuildRun specifications
Implemented CA bundle validation logic in the validate package
Implemented CA bundle volume and environment variable injection for TaskRun/PipelineRun execution
Added CA bundle parameter definitions to task specifications
Enhanced error handling in taskrun_generator.go to properly check errors from called functions

Reviewed changes

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

Show a summary per file

File	Description
pkg/apis/build/v1beta1/cabundle.go	New API types for CA bundle references
pkg/apis/build/v1beta1/build_types.go	Added CABundle field to BuildSpec and new BuildReason constant
pkg/apis/build/v1beta1/buildrun_types.go	Added CABundle field to BuildRunSpec
pkg/cabundle/cabundle.go	Core CA bundle validation and volume/environment variable generation
pkg/cabundle/cabundle_test.go	Comprehensive tests for CA bundle handling
pkg/cabundle/suite_test.go	Test suite setup
pkg/validate/cabundle.go	Validation logic for Build CA bundle references
pkg/validate/validate.go	Added Certificates constant and validation case
pkg/reconciler/buildrun/resources/cabundle.go	TaskRun/PipelineRun certificate injection
pkg/reconciler/buildrun/resources/taskrun.go	Added paramCABundle constant
pkg/reconciler/buildrun/resources/resource_builders.go	CA bundle parameter specifications and values
pkg/reconciler/buildrun/resources/taskrun_generator.go	Enhanced error handling and certificate injection
pkg/reconciler/buildrun/buildrun.go	Added CA bundle validation to reconciliation
deploy/crds/shipwright.io_builds.yaml	Updated Build CRD
deploy/crds/shipwright.io_buildruns.yaml	Updated BuildRun CRD
test/utils/v1beta1/pipelinerun.go	Added copyright header

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

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

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

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 12 comments.

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

avinal

In my testing, builds using TaskRuns ran successfully, but builds using PipelineRuns failed due to nil pointer error.

Apart from that, it seems deepcopygen was not rerun.

avinal · 2026-04-28T11:56:30Z

+	switch o := object.(type) {
+	case *corev1.Secret:
+		data = o.Data[ca.Secret.Key]
+	case *corev1.ConfigMap:
+		data = []byte(o.Data[ca.ConfigMap.Key])
+	}
+
+	// Parse and validate each certificate in the PEM data
+	block, rest := pem.Decode(data)
+	if block == nil {
+		return fmt.Errorf("no certificate present in CA bundle")
+	}


I believe in case the key doesn't exist, we should return a proper error and not rely on Decode() to finally return that "Certificate is not present". This way we will know what exactly is wrong.

In the API the key field is not optional.

Are we validating that the provided key is present in the actual Secret/ConfigMap? Probably not up until this point.

This would be completely valid API:

caBundle: configMap: name: my-cas key: this-is-not-a-real-file.txt

And yet the ConfigMap (from say, cert-manager) would have the following:

kind: ConfigMap metadata: name: my-cas data: ca.crt: | ...

We are validating a valid CA to be present in the Secret or ConfigMap, so the key is implicit here.
But I'll test this out.

sgaist · 2026-05-03T19:16:59Z

-	return generateTaskSpecVolumes(
+	if err = generateTaskSpecVolumes(
 		g.taskRun.Spec.TaskSpec,
 		execCtx.volumeMounts,
 		execCtx.strategyVolumes,
 		execCtx.buildVolumes,
 		execCtx.buildRunVolumes,
-	)
+	); err != nil {
+		return err
+	}
+
+	return nil


The original code can be left as is.

if err is nil, nil is returned and if err is not nil, err is returned, so the value of err is returned in all cases. Therefore, return generateTaskSpecVolumes( is already doing the right thing.

Suggested change

return generateTaskSpecVolumes(

if err = generateTaskSpecVolumes(

g.taskRun.Spec.TaskSpec,

execCtx.volumeMounts,

execCtx.strategyVolumes,

execCtx.buildVolumes,

execCtx.buildRunVolumes,

)

); err != nil {

return err

}

return nil

return generateTaskSpecVolumes(

Yes.. This part of the code was already correct. Made this change to improve the code readability and match the other function.

I think this is a matter of style preference. As a community I don't think we've established any hard guideline with respect to error returns (return nil vs. return value of an error-return function).

Don't we need to add the CA bundles to the set of task spec volumes?

Yes.
Its added separately in pkg/reconciler/buildrun/resource/cabundle.go

sgaist · 2026-05-03T19:19:16Z

+	if err := applyParameters(g.taskRun, g.build, g.buildRun, g.strategy); err != nil {
+		return err
+	}
+
+	return nil


Same as above, the original return statement should stay as is.

Do we need to apply the CA bundle parameter here?

Parameter for CA bundle is not needed for it functioning. But it was given with the intention of allowing user to dynamically use it in TaskRun steps. Not sure what use case that would be.

adambkaplan

I think this is a good start to implementing SHIP-0042.

The one thing that stands out which isn't implemented here Java keytool support (jks and pkcs12 formats). These are separate files that are provided in addition to (not separately from) the standard PEM-encoded cert bundles.

adambkaplan · 2026-05-11T14:01:36Z

+// +kubebuilder:validation:ExactlyOneOf=configMap;secret
+type CABundle struct {


I think we need a Type field in this object as well, to act as a union discriminator. This solidifies the user's intent, and follows the same API pattern we have for source code.

Kubernetes has go tags which can be used to generate the correct OpenAPI schema (unionDiscriminator and unionMember). I am pretty sure that controller-gen does the right thing with these. https://kubernetes.io/docs/reference/using-api/declarative-validation/#tag-unionDiscriminator

@adambkaplan What do we want use the type field for?
Holding Secret or ConfigMap type?

adambkaplan · 2026-05-11T15:18:12Z

-	return generateTaskSpecVolumes(
+	if err = generateTaskSpecVolumes(
 		g.taskRun.Spec.TaskSpec,
 		execCtx.volumeMounts,
 		execCtx.strategyVolumes,
 		execCtx.buildVolumes,
 		execCtx.buildRunVolumes,
-	)
+	); err != nil {
+		return err
+	}
+
+	return nil


I think this is a matter of style preference. As a community I don't think we've established any hard guideline with respect to error returns (return nil vs. return value of an error-return function).

Don't we need to add the CA bundles to the set of task spec volumes?

adambkaplan · 2026-05-11T15:18:48Z

+	if err := applyParameters(g.taskRun, g.build, g.buildRun, g.strategy); err != nil {
+		return err
+	}
+
+	return nil


Do we need to apply the CA bundle parameter here?

Reference: shipwright-io/community#281 # Changes: - Added APIs to allow user to define custom Root CAs in Build and BuildRun APIs - Add unit tests Signed-off-by: Sayan Biswas <sabiswas@redhat.com>

sayan-biswas · 2026-05-15T11:12:54Z

I think this is a good start to implementing SHIP-0042.

The one thing that stands out which isn't implemented here Java keytool support (jks and pkcs12 formats). These are separate files that are provided in addition to (not separately from) the standard PEM-encoded cert bundles.

@adambkaplan I have few queries related to few comments for the TLS development.

Do we intend to use the Type field to differentiate between Secret and ConfigMap?

spec:
  caBundle:
    type: Secret
    name: ca-bundle
    key: ca.crt

For jks and pkcs12 , do we want the user to provide the references object key in the API like ?

spec:
  caBundle:
    type: Secret
    name: ca-bundle
    ca:
      key: ca.crt
    jks:
      key: ca.jks
    pkcs12:
      key: ca.pkcs

or have them generated dynamically from the PEM data of CA during runtime when enabled in the API like?

spec:
  caBundle:
    type: Secret
    name: ca-bundle
    key: ca.crt
    jks: true
    pkcs12: true

openshift-ci Bot added release-note Label for when a PR has specified a release note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Feb 18, 2026

pull-request-size Bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 18, 2026

shipwright-ci-bot added this to Issues Feb 18, 2026

sayan-biswas mentioned this pull request Feb 18, 2026

SHIP-0042: Trusted certificates shipwright-io/community#281

Merged

4 tasks

sayan-biswas force-pushed the certificates branch 2 times, most recently from fe07158 to b4af1ae Compare February 23, 2026 15:22

sayan-biswas marked this pull request as ready for review April 6, 2026 05:41

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 6, 2026

openshift-ci Bot requested review from hasanawad94 and karanibm6 April 6, 2026 05:41

sayan-biswas requested review from adambkaplan and Copilot April 6, 2026 05:41

Copilot started reviewing on behalf of sayan-biswas April 6, 2026 05:42 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

Comment thread pkg/reconciler/buildrun/resources/resource_builders.go Outdated

Comment thread pkg/validate/validate.go Outdated

Comment thread pkg/cabundle/cabundle.go Outdated

sayan-biswas force-pushed the certificates branch from b4af1ae to 5da79ab Compare April 13, 2026 15:30

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2026

sayan-biswas requested a review from Copilot April 13, 2026 15:30

Copilot started reviewing on behalf of sayan-biswas April 13, 2026 15:31 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

sayan-biswas force-pushed the certificates branch from 5da79ab to 8df886b Compare April 16, 2026 10:58

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 16, 2026

sayan-biswas force-pushed the certificates branch from 8df886b to 584410e Compare April 20, 2026 03:09

sayan-biswas requested a review from Copilot April 20, 2026 03:10

Copilot started reviewing on behalf of sayan-biswas April 20, 2026 03:11 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

sayan-biswas force-pushed the certificates branch from 584410e to eea9bd4 Compare April 20, 2026 17:32

pull-request-size Bot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 20, 2026

pull-request-size Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 20, 2026

sayan-biswas force-pushed the certificates branch 2 times, most recently from 57586d0 to e912647 Compare April 27, 2026 10:56

avinal suggested changes Apr 28, 2026

View reviewed changes

sayan-biswas force-pushed the certificates branch from e912647 to 36532b6 Compare May 3, 2026 18:01

sgaist suggested changes May 3, 2026

View reviewed changes

adambkaplan added this to the release-v0.20.0 milestone May 11, 2026

adambkaplan requested changes May 11, 2026

View reviewed changes

sayan-biswas force-pushed the certificates branch 2 times, most recently from 90a9dfa to 1504859 Compare May 13, 2026 23:41

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2026

sayan-biswas force-pushed the certificates branch from 1504859 to a618966 Compare May 14, 2026 00:32

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2026

Feat: Add corporate certificates to Build APIs

c247c2b

Reference: shipwright-io/community#281 # Changes: - Added APIs to allow user to define custom Root CAs in Build and BuildRun APIs - Add unit tests Signed-off-by: Sayan Biswas <sabiswas@redhat.com>

sayan-biswas force-pushed the certificates branch from a618966 to c247c2b Compare May 14, 2026 01:15

		// +kubebuilder:validation:ExactlyOneOf=configMap;secret
		type CABundle struct {

Conversation

sayan-biswas commented Feb 18, 2026 • edited by adambkaplan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes:

Submitter Checklist

Release Notes

Uh oh!

openshift-ci Bot commented Feb 18, 2026

Uh oh!

sayan-biswas commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

avinal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adambkaplan 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!

sayan-biswas commented Feb 18, 2026 •

edited by adambkaplan

Loading

sayan-biswas commented May 15, 2026 •

edited

Loading