Skip to content

Conversation

@danielerez
Copy link
Contributor

This is a cherry-pick of #7448

When generating the install config, it should not include the
entire system CA bundle. I.e. when setting the AdditionalTrustBundle[1].
However, when using a mirror regsitry (e.g. with MirrorRegistryRef[2] in ASC),
the installConfigBuilder is adding the content of tls-ca-bundle.pem[3].
This pem file is created[4] by ASC controller, and includes the full system CA bundle
since the bundle[5] is injected into 'cluster-trusted-ca-bundle'.

Therefore, the suggestion solution is to create a separate pem file
just for the certificates specified in MirrorRegistryRef.
I.e. generate a new 'user-registry-ca-bundle.pem' file, that will be included
as part of AdditionalTrustBundle. These certificates will propagate into the
'user-ca-bundle' CM during cluster installation.
This will ensure that 'user-ca-bundle' CM indeed includes only custom certificates
mandatory for the user, instead of the system CA bundle.

Note: as backwards compatibility, for flows as ABI, we keep a fallback
to the current behaviour (i.e. include 'tls-ca-bundle.pem' only when
'user-registry-ca-bundle.pem' doesn't exist).

[1] https://github.com/openshift/assisted-service/blob/40ab10db5e872e519ab0a97e82fc318423feeaba/internal/installcfg/builder/builder.go#L275
[2] https://github.com/openshift/assisted-service/blob/cb169a2d2c97bb3dccd06ad4b75f2937e01f78f4/vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.go#L82
[3] https://github.com/openshift/assisted-service/blob/a1c3229afee1f0f774b286283fb0d0098b9eac03/internal/common/common.go#L35
[4] https://github.com/openshift/assisted-service/blob/341f9860c455cccc42741f350024d05aa72755f8/internal/controller/controllers/agentserviceconfig_controller.go#L1848
[5] https://github.com/openshift/assisted-service/blob/341f9860c455cccc42741f350024d05aa72755f8/internal/controller/controllers/agentserviceconfig_controller.go#L1059
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 8, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 8, 2025

@danielerez: This pull request references MGMT-20207 which is a valid jira issue.

Details

In response to this:

This is a cherry-pick of #7448

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 8, 2025
@openshift-ci openshift-ci bot requested review from pastequo and romfreiman April 8, 2025 09:01
@danielerez
Copy link
Contributor Author

/cc @carbonin
/cc @gamli75
/cc @AlonaKaplan

@danielerez danielerez changed the title MGMT-20207: avoid adding system CA bundle to AdditionalTrustBundle [release-ocm-2.12] MGMT-20207: avoid adding system CA bundle to AdditionalTrustBundle Apr 8, 2025
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2025
@openshift-ci
Copy link

openshift-ci bot commented Apr 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danielerez, gamli75

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 8, 2025
@codecov
Copy link

codecov bot commented Apr 8, 2025

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.36%. Comparing base (097e8e3) to head (c12798d).
Report is 1 commits behind head on release-ocm-2.12.

Files with missing lines Patch % Lines
pkg/mirrorregistries/generator.go 80.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##           release-ocm-2.12    #7520      +/-   ##
====================================================
+ Coverage             68.34%   68.36%   +0.01%     
====================================================
  Files                   257      257              
  Lines                 37668    37690      +22     
====================================================
+ Hits                  25743    25765      +22     
- Misses                 9638     9639       +1     
+ Partials               2287     2286       -1     
Files with missing lines Coverage Δ
internal/common/common.go 27.69% <ø> (ø)
...oller/controllers/agentserviceconfig_controller.go 84.05% <100.00%> (+0.14%) ⬆️
internal/controller/controllers/common.go 78.77% <ø> (ø)
pkg/mirrorregistries/generator.go 65.38% <80.00%> (+0.80%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-ci
Copy link

openshift-ci bot commented Apr 8, 2025

@danielerez: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit a7a1ec4 into openshift:release-ocm-2.12 Apr 8, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants