Skip to content

Stop overriding system defaults in SSL_CERT_DIR#549

Merged
tekton-robot merged 1 commit intotektoncd:mainfrom
concaf:concaf/fix/ssl-cert-dir-mount
Dec 8, 2021
Merged

Stop overriding system defaults in SSL_CERT_DIR#549
tekton-robot merged 1 commit intotektoncd:mainfrom
concaf:concaf/fix/ssl-cert-dir-mount

Conversation

@concaf
Copy link
Contributor

@concaf concaf commented Dec 8, 2021

Changes

Prior to this commit, we used to set default value of SSL_CERT_DIR in
our TaskRun, etc pods to /tekton-custom-certs; while this worked for
certs mounted via the bundle config maps, this broke use cases where users
relied on default locations (like /etc/ssl/certs,
/etc/pki/tls/certs, etc) for already existing or user-mounted certificates.

This commit fixes that by setting SSL_CERT_DIR as a list of directories
which contains /tekton-custom-certs and the system default locations
as well.

Release Notes

System default locations for certificates are no longer overridden by SSL_CERT_DIR

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Dec 8, 2021
@tekton-robot tekton-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 8, 2021
@tekton-robot
Copy link
Contributor

tekton-robot commented Dec 8, 2021

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/proxy/proxy.go 10.1% 10.7% 0.6

@concaf concaf force-pushed the concaf/fix/ssl-cert-dir-mount branch from 2cd981b to d5de419 Compare December 8, 2021 13:10
@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 8, 2021
@tekton-robot
Copy link
Contributor

tekton-robot commented Dec 8, 2021

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/proxy/proxy.go 10.1% 10.7% 0.6

@concaf
Stop overriding system defaults in SSL_CERT_DIR
65c7712 
Prior to this commit, we used to set default value of SSL_CERT_DIR in
our TaskRun, etc pods to `/tekton-custom-certs`; while this worked for
certs mounted via the bundle config maps, this broke use cases where users
relied on default locations (like `/etc/ssl/certs`,
`/etc/pki/tls/certs`, etc) for already existing or user-mounted certificates.

This commit fixes that by setting SSL_CERT_DIR as a list of directories
which contains `/tekton-custom-certs` and the system default locations
as well.
@concaf concaf force-pushed the concaf/fix/ssl-cert-dir-mount branch from d5de419 to 65c7712 Compare December 8, 2021 13:17
@tekton-robot
Copy link
Contributor

tekton-robot commented Dec 8, 2021

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/reconciler/proxy/proxy.go 10.1% 10.7% 0.6

@concaf
Copy link
Contributor Author

concaf commented Dec 8, 2021

/test pull-tekton-operator-integration-tests

@nikhil-thomas
Copy link
Member

nikhil-thomas commented Dec 8, 2021

/approve
✔️ lgtm

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2021
vdemeester
vdemeester approved these changes Dec 8, 2021
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 8, 2021
@tekton-robot
Copy link
Contributor

tekton-robot commented Dec 8, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhil-thomas, vdemeester

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [nikhil-thomas,vdemeester]

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

@tekton-robot tekton-robot merged commit e0d55e6 into tektoncd:main Dec 8, 2021
@aThorp96 aThorp96 mentioned this pull request Jul 29, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdemeester vdemeester vdemeester approved these changes

@pradeepitm12 pradeepitm12 Awaiting requested review from pradeepitm12

@sm43 sm43 Awaiting requested review from sm43

Assignees

@vdemeester vdemeester

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@concaf @tekton-robot @nikhil-thomas @vdemeester