CA Paths Fixes by shirady · Pull Request #1570 · noobaa/noobaa-operator

shirady · 2025-03-24T15:21:56Z

Explain the changes

In pkg/utilutil.go - Change the /etc/ocp-injected-ca-bundle.crt to /etc/ocp-injected-ca-bundle/ca-bundle.crt.
In pkg/system/reconciler.go - Rename r.CaBundleConf.Name from r.Request.Name + "-ca-inject" to "ocp-injected-ca-bundle" to align with the rename of configmap-ca-inject.yaml from noobaa-ca-inject to ocp-injected-ca-bundle in PR Modify the handling of injected OCP CA bundles #1328.
In pkg/system/phase2_creating.go and pkg/system/phase4_configuring.go - the core and the endpoint change the mount path from /etc/ocp-injected-ca-bundle.crt (with file extension) to a directory (remove the extension).
Rename AddToRootCAs to CombineCaBundle, add the const InjectedBundleCertCAFile which is /etc/ocp-injected-ca-bundle/ca-bundle.crt, and assign r.ApplyCAsToPods to be the new constant (will be NODE_EXTRA_CA_CERTS eventually).
Change the condition of checking only the existence of the config map of the CA (configmap-ca-inject) to check this and also that we have data in it.
The reason is that when we test the OLM (in the CI), we have the OLM and then we have the operator so the config map as it is created by the OLM, but in the first step it is empty.
When manually looking at the config map in a real cluster there is data, that probably comes from additional needed configuration in the cluster (which is not in the scope of the test).
Add a comment about ApplyCAsToPods so it would be clear that it is relevant to the endpoint and core pods.
in CombineCaBundle use const instead of hard-coded string (to avoid circular dependency had to move the consts from options to util file).
(this change might be temporary - need to decide) Change the ApplyCAsToPods to ServiceServingCertCAFile (previous InjectedBundleCertCAFile in comment).

Issues:

Currently, when I tried to deploy with the cluster-bot I saw that NODE_EXTRA_CA_CERTS is empty, while it used to be
NODE_EXTRA_CA_CERTS=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt.

GAPs:

Tests end-to-end with certificates.

Testing Instructions:

Basic Manual Tests (more details in the comment below)

Check the value of NODE_EXTRA_CA_CERTS:

I used the cluster-bot with launch 4.17 aws
Installed ODF operator and MCG standalone according to the docs here.
Check the env NODE_EXTRA_CA_CERTS:

oc exec statefulset/noobaa-core -c core -n openshift-storage -- printenv | grep NODE_EXTRA_CA_CERTS (was empty)
oc exec noobaa-endpoint-<characters> -n openshift-storage -- env | grep NODE_EXTRA_CA_CERTS (was empty)

same steps with launch 4.14 aws in the cluster-bot (the version was chosen as a version before the changes of PR Modify the handling of injected OCP CA bundles #1328) we could see NODE_EXTRA_CA_CERTS=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt (both on core and endpoint).
After the change we would see (both on core and endpoint pods):

NODE_EXTRA_CA_CERTS=/etc/ocp-injected-ca-bundle/ca-bundle.crt
NODE_EXTRA_CA_CERTS=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt (with the commit mentioned in change number 8).
Doc added/updated
Tests added

shirady · 2025-03-24T15:23:52Z

Additional Testing Instructions:

The tests were done on the code changes in version 5.17.

1) Code change - path of cert

In pkg/utilutil.go - Change the /etc/ocp-injected-ca-bundle.crt to /etc/ocp-injected-ca-bundle/ca-bundle.crt(tested on the operator pod)

Explanation:

NODE_EXTRA_CA_CERTS is empty, since the value was assigned from:
(in the core pod)

noobaa-operator/pkg/system/phase2_creating.go

Lines 467 to 468 in 3f43b19

    
           case "NODE_EXTRA_CA_CERTS": 
        
           	c.Env[j].Value = r.ApplyCAsToPods

(in the endpoint pod)

noobaa-operator/pkg/system/phase4_configuring.go

Lines 407 to 408 in 3f43b19

    
           case "NODE_EXTRA_CA_CERTS": 
        
           	c.Env[j].Value = r.ApplyCAsToPods

and r.ApplyCAsToPods is assigned after:

noobaa-operator/pkg/system/reconciler.go

Lines 407 to 414 in 3f43b19

    
           err = util.AddToRootCAs(options.ServiceServingCertCAFile) 
        
           if err == nil { 
        
           	r.ApplyCAsToPods = options.ServiceServingCertCAFile 
        
           } else if !os.IsNotExist(err) { 
        
           	log.Errorf("❌ NooBaa %q failed to add root CAs to system default", r.NooBaa.Name) 
        
           	res.RequeueAfter = 3 * time.Second 
        
           	return res, nil 
        
           }

but in the function AddToRootCAs the hard-coded path does not exist in the operator pod:

noobaa-operator/pkg/util/util.go

Lines 151 to 154 in 3f43b19

    
           var certFiles = []string{ 
        
           	"/etc/ocp-injected-ca-bundle.crt", 
        
           	localCertFile, 
        
           }

Before code changes:

In version 4.17 I run:

connect to the operator pod: oc rsh noobaa-operator-<characters>
check the previous path: ls -al /etc/pki/ca-trust/extracted/pem (it's a directory and it is generated from OCP - not NooBaa):

total 904
drwxr-xr-x. 3 root root    123 Sep 18  2024 .
drwxr-xr-x. 6 root root     70 Sep 18  2024 ..
-rw-r--r--. 1 root root    898 Aug 19  2024 README
dr-xr-xr-x. 2 root root  16384 Sep 18  2024 directory-hash
-r--r--r--. 1 root root 165521 Sep 18  2024 email-ca-bundle.pem
-r--r--r--. 1 root root 502506 Sep 18  2024 objsign-ca-bundle.pem
-r--r--r--. 1 root root 226489 Sep 18  2024 tls-ca-bundle.pem

check the code path in file level: ls -al /etc/ocp-injected-ca-bundle.crt

ls: cannot access '/etc/ocp-injected-ca-bundle.crt': No such file or directory

check the path in the directory level: ls -al /etc/ocp-injected-ca-bundle

total 0
drwxrwsrwx. 3 root 1000710000 81 Mar 24 10:06 .
drwxr-xr-x. 1 root root       36 Mar 24 10:06 ..
drwxr-sr-x. 2 root 1000710000 27 Mar 24 10:06 ..2025_03_24_10_06_38.1871780206
lrwxrwxrwx. 1 root 1000710000 32 Mar 24 10:06 ..data -> ..2025_03_24_10_06_38.1871780206
lrwxrwxrwx. 1 root 1000710000 20 Mar 24 10:06 ca-bundle.crt -> ..data/ca-bundle.crt

from this I took the file path: ls -al /etc/ocp-injected-ca-bundle/ca-bundle.crt

lrwxrwxrwx. 1 root 1000710000 20 Mar 24 10:06 /etc/ocp-injected-ca-bundle/ca-bundle.crt -> ..data/ca-bundle.crt

After code changes:

We can see the printing in the operator logs:
kubectl logs noobaa-operator-<characters>

time="2025-03-31T07:35:22Z" level=info msg="Successfuly appended \"/etc/ocp-injected-ca-bundle/ca-bundle.crt\" to RootCAs"
time="2025-03-31T07:35:22Z" level=info msg="Successfuly appended \"/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt\" to RootCAs"

2) Code change - rename the r.CaBundleConf.Name

In pkg/system/reconciler.go - Rename r.CaBundleConf.Name from r.Request.Name + "-ca-inject" to "ocp-injected-ca-bundle"to align with the rename of configmap-ca-inject.yaml from noobaa-ca-inject to ocp-injected-ca-bundle in PR #1328

since CaBundleConf is:

noobaa-operator/pkg/system/reconciler.go

Line 190 in 3f43b19

    
           CaBundleConf:              util.KubeObject(bundle.File_deploy_internal_configmap_ca_inject_yaml).(*corev1.ConfigMap),

and the name is: "ocp-injected-ca-bundle"

noobaa-operator/pkg/bundle/deploy.go

Lines 3918 to 3925 in 3f43b19

    
           const File_deploy_internal_configmap_ca_inject_yaml = `apiVersion: v1 
        
           kind: ConfigMap 
        
           metadata: 
        
             labels: 
        
               config.openshift.io/inject-trusted-cabundle: "true" 
        
             name: ocp-injected-ca-bundle 
        
           data: {} 
        
           `

and it doesn't match:

noobaa-operator/pkg/system/reconciler.go

Line 285 in 3f43b19

r.CaBundleConf.Name = r.Request.Name + "-ca-inject"

Therefore, whenever we check the name (for example):

noobaa-operator/pkg/system/phase2_creating.go

Line 544 in 3f43b19

if util.KubeCheckQuiet(r.CaBundleConf) {

it will not find it.

After code changes:

I added printing to validate that the value is true, for example (in SetDesiredCoreApp):

r.Logger.Info("SDSD in SetDesiredCoreApp - case core")
r.Logger.Info("SDSD r.CaBundleConf.Name ", r.CaBundleConf.Name)
myCheck := util.KubeCheckQuiet(r.CaBundleConf)
r.Logger.Info("SDSD myCheck in SetDesiredCoreApp - case core util.KubeCheckQuiet(r.CaBundleConf) ", myCheck)

We can see the printing in the operator logs:
kubectl logs noobaa-operator-<characters>

time="2025-03-31T07:33:23Z" level=info msg="SDSD in SetDesiredCoreApp - case core" sys=openshift-storage/noobaa
time="2025-03-31T07:33:23Z" level=info msg="SDSD r.CaBundleConf.Name ocp-injected-ca-bundle" sys=openshift-storage/noobaa
time="2025-03-31T07:33:23Z" level=info msg="SDSD myCheck in SetDesiredCoreApp - case core util.KubeCheckQuiet(r.CaBundleConf) true" sys=openshift-storage/noobaa

Note: all cases were true (checked with: kubectl logs noobaa-operator-7f6c74dd69-qqk6k | grep "util.KubeCheckQuiet(r.CaBundleConf) false" - empty output).

3) Code change - mount path of directory (don't show file extension)

In pkg/system/phase2_creating.go and pkg/system/phase4_configuring.go - the core and the endpoint change the mount path from /etc/ocp-injected-ca-bundle.crt (with file extension) to a directory (remove the extension).

seems like a file (extension):

noobaa-operator/pkg/system/phase2_creating.go

Line 547 in 3f43b19

MountPath: "/etc/ocp-injected-ca-bundle.crt",

It was (directory level - no extension):

noobaa-operator/pkg/system/phase2_creating.go

Line 493 in 2c86789

MountPath: "/etc/pki/ca-trust/extracted/pem",

Therefore, it is suggested to remove the file extension.

Before code changes:

connect to the endpoint pod: oc rsh noobaa-endpoint-<characters>
check the previous path: ls -al /etc/pki/ca-trust/extracted/pem

total 904
drwxrwxr-x. 1 root root    123 Oct 24 12:50 .
drwxrwxr-x. 1 root root     70 Oct 24 12:50 ..
-rw-rw-r--. 1 root root    898 Aug 19  2024 README
dr-xr-xr-x. 1 root root  16384 Oct 24 12:50 directory-hash
-r--r--r--. 1 root root 165521 Oct 24 12:50 email-ca-bundle.pem
-r--r--r--. 1 root root 502506 Oct 24 12:50 objsign-ca-bundle.pem
-r--r--r--. 1 root root 226489 Oct 24 12:50 tls-ca-bundle.pem
sh-5.1# ls -al /etc/ocp-injected-ca-bundle

check the path in the directory level: ls -al /etc/ocp-injected-ca-bundle

ls: cannot access '/etc/ocp-injected-ca-bundle': No such file or directory

In the operator yaml we have the configmap property:

noobaa-operator/deploy/operator.yaml

Lines 34 to 40 in 3f43b19

    
           - name: ocp-injected-ca-bundle 
        
             configMap: 
        
               name: ocp-injected-ca-bundle 
        
               items: 
        
               - key: ca-bundle.crt 
        
                 path: ca-bundle.crt 
        
               optional: true

but we don't have this either in the endpoint deployment or the core statefulset:
oc get statefulset noobaa-core -o yaml
oc get deployment noobaa-endpoint -o yaml

After code changes:

connect to the endpoint pod: oc rsh noobaa-endpoint-<characters>
ls -al /etc/ocp-injected-ca-bundle

total 0
drwxrwsrwx. 3 root 1000710000 80 Mar 31 07:25 .
drwxrwxr-x. 1 root root      136 Mar 31 07:25 ..
drwxr-sr-x. 2 root 1000710000 27 Mar 31 07:25 ..2025_03_31_07_25_58.320495734
lrwxrwxrwx. 1 root 1000710000 31 Mar 31 07:25 ..data -> ..2025_03_31_07_25_58.320495734
lrwxrwxrwx. 1 root 1000710000 20 Mar 31 07:25 ca-bundle.crt -> ..data/ca-bundle.crt

same in the core pod: oc rsh noobaa-core-0

4) Code change - path of cert

Rename AddToRootCAs to CombineCaBundle, add the const InjectedBundleCertCAFile which is /etc/ocp-injected-ca-bundle/ca-bundle.crt, and assign r.ApplyCAsToPods to be the new constant (will be NODE_EXTRA_CA_CERTS eventually).

Explanation:

NODE_EXTRA_CA_CERTS is assigned from:
(in the core pod)

noobaa-operator/pkg/system/phase2_creating.go

Lines 467 to 468 in 3f43b19

    
           case "NODE_EXTRA_CA_CERTS": 
        
           	c.Env[j].Value = r.ApplyCAsToPods

(in the endpoint pod)

noobaa-operator/pkg/system/phase4_configuring.go

Lines 407 to 408 in 3f43b19

    
           case "NODE_EXTRA_CA_CERTS": 
        
           	c.Env[j].Value = r.ApplyCAsToPods

and r.ApplyCAsToPods is assigned after:

noobaa-operator/pkg/system/reconciler.go

Lines 408 to 414 in 3f43b19

    
           if err == nil { 
        
           	r.ApplyCAsToPods = options.ServiceServingCertCAFile 
        
           } else if !os.IsNotExist(err) { 
        
           	log.Errorf("❌ NooBaa %q failed to add root CAs to system default", r.NooBaa.Name) 
        
           	res.RequeueAfter = 3 * time.Second 
        
           	return res, nil 
        
           }

now we added the constant, and the env NODE_EXTRA_CA_CERTS will be assigned to /etc/ocp-injected-ca-bundle/ca-bundle.crt.

Before code changes:

oc exec statefulset/noobaa-core -c core -n openshift-storage -- printenv | grep NODE_EXTRA_CA_CERTS (was empty)
oc exec noobaa-endpoint-<characters> -n openshift-storage -- env | grep NODE_EXTRA_CA_CERTS (was empty)

After code changes:

oc exec noobaa-core-0 -n openshift-storage -- env | grep NODE_EXTRA_CA_CERTS

Defaulted container "core" out of: core, noobaa-log-processor
NODE_EXTRA_CA_CERTS=/etc/ocp-injected-ca-bundle/ca-bundle.crt

oc exec noobaa-endpoint-56979564f4-pstkr -n openshift-storage -- env | grep NODE_EXTRA_CA_CERTS

NODE_EXTRA_CA_CERTS=/etc/ocp-injected-ca-bundle/ca-bundle.crt

5) Code change - Change the condition existence of the config map of the CA (`configmap-ca-inject`) + data in it.

The reason is that when we test the OLM (in the CI), we have the OLM and then we have the operator so the config map as it is created by the OLM, but in the first step it is empty.
When manually looking at the config map in a real cluster there is data, that probably comes from additional needed configuration in the cluster (which is not in the scope of the test).

The config map ocp-injected-ca-bundle is created by the olm after the operator is installed and gets its CA injected by the network-operator (in namespace openshift-network-operator).

I could see in a cluster:
oc logs -n openshift-network-operator deploy/network-operator

I0406 07:30:35.257033       1 log.go:245] reconciling (/v1, Kind=ConfigMap) openshift-storage/ocp-injected-ca-bundle
I0406 07:30:35.300056       1 log.go:245] Apply / Create of (/v1, Kind=ConfigMap) openshift-storage/ocp-injected-ca-bundle was successful

later this controller reconciles and there no changes, for example:

I0406 07:30:35.309980       1 log.go:245] Reconciling configmap from  openshift-storage/ocp-injected-ca-bundle
I0406 07:30:35.311865       1 log.go:245] ConfigMap openshift-storage/ocp-injected-ca-bundle ca-bundle.crt unchanged, skipping
I0406 07:30:35.326456       1 log.go:245] Reconciling configmap from  openshift-storage/ocp-injected-ca-bundle
I0406 07:30:35.328750       1 log.go:245] ConfigMap openshift-storage/ocp-injected-ca-bundle ca-bundle.crt unchanged, skipping

To summarize what is happening from my understanding:

The customer adds its private CAs using open shift.
When he installed ODF operator it also installed the noobaa-operator which triggers the OLM to create an empty config map ocp-injected-ca-bundle
The configmap has the label config.openshift.io/inject-trusted-cabundle: "true" which the pod of the network-operator watched and reconciled (we saw it in the logs + references - upstream docs, official docs).
The endpoint and core pods should be created after the operator pod and therefore will have the mount path for the CA (the condition checks that the config map exists and has data in it).

…ndle/ca-bundle.crt (tested on operator pod) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

…-injected-ca-bundle" to align with the rename of configmap-ca-inject.yaml from noobaa-ca-inject to ocp-injected-ca-bundle in PR 1328 Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

…CertCAFile which is /etc/ocp-injected-ca-bundle/ca-bundle.crt + assign r.ApplyCAsToPods to be the new constant (will be NODE_EXTRA_CA_CERTS eventually) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com> (cherry picked from commit 41215ae64ada1a5c7cd5a85322201e64a2945fda)

…t the data in the core and endpoint pods Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

(to avoid circular dependency had to move the consts from options to util) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

…jectedBundleCertCAFile in comment) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

shirady · 2025-04-10T05:19:14Z

Updates

This PR moved to draft (might be closed and a new PR / revert PR will be open):
After the current changes, the core and the endpoint have the env NODE_EXTRA_CA_CERTS with /var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt (“ServiceServingCertCAFile” in the code which contains the openshift-service-serving as an issuer) - but this cert is partial. In version 5.14 we also had the certs that were mounted in the pods by overriding the existing path:
oc get pod noobaa-core-0 -o yaml | less

    - mountPath: /etc/pki/ca-trust/extracted/pem
      name: noobaa-ca-inject

The NODE_EXTRA_CA_CERTS is a single path (we cannot pass a couple of paths), and this env cannot be set and changed while the system is running (so we cannot create a workaround for updating when the cluster is up) -Node js documentation.
Therefore, it was overridden in the past (until version 5.14) - but returning the mount path as it was might recreate the race that we used to have that PR #1328 tries to solve.

Recommendation of what the testing end-to-end can include:

tests with certificates of 2 kinds: internal to the cluster (like with RGW backingstore) and external to the cluster (for a server outside of the cluster).
The test plan must also include 2 cases:

When a (correct) certificate is added - expected to create a backingstore successfully (also must look at the logs of noobaa-core and see that "request_unsecured true protocol https"), continue to send requests from the endpoint pod and check that as well.
When a wrong certificate is added - expect the backingstore to fail upon creation.
Updating certificate - should we delete the pod and the new certificate is loaded?

shirady · 2025-04-21T06:01:38Z

I'm closing this PR as it continues in PR #1587

pull-request-size bot added the size/S label Mar 24, 2025

shirady requested review from Neon-White and jackyalbo March 24, 2025 15:57

shirady self-assigned this Mar 24, 2025

pull-request-size bot added size/XS and removed size/S labels Mar 25, 2025

shirady force-pushed the check-certs branch 4 times, most recently from 47a7b8c to 1ccce7a Compare March 30, 2025 11:06

pull-request-size bot added size/L and removed size/XS labels Mar 30, 2025

shirady force-pushed the check-certs branch 3 times, most recently from 6af0112 to c671827 Compare April 3, 2025 08:30

pull-request-size bot added size/S size/M and removed size/L size/S labels Apr 3, 2025

nimrod-becker requested a review from dannyzaken April 3, 2025 13:29

shirady marked this pull request as ready for review April 6, 2025 09:05

shirady added 8 commits April 7, 2025 18:21

change the /etc/ocp-injected-ca-bundle.crt to /etc/ocp-injected-ca-bu…

855bfea

…ndle/ca-bundle.crt (tested on operator pod) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

rename r.CaBundleConf.Name from r.Request.Name + "-ca-inject" to "ocp…

1d90687

…-injected-ca-bundle" to align with the rename of configmap-ca-inject.yaml from noobaa-ca-inject to ocp-injected-ca-bundle in PR 1328 Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

in the core and the endpoint change the mountpath - remove the extension

3dfb6f4

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

change the cm check to check for existance and data of cm before moun…

4844e25

…t the data in the core and endpoint pods Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

add a comment about ApplyCAsToPods

2062511

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

in CombineCaBundle use const instead of hard-coded string

bb01895

(to avoid circular dependency had to move the consts from options to util) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

change the ApplyCAsToPods to be ServiceServingCertCAFile (previous In…

9622fd5

…jectedBundleCertCAFile in comment) Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

shirady force-pushed the check-certs branch from 20d830f to 9622fd5 Compare April 7, 2025 15:21

shirady marked this pull request as draft April 7, 2025 15:28

Neon-White mentioned this pull request Apr 14, 2025

CA Paths Fixes #1587

Merged

shirady closed this Apr 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CA Paths Fixes#1570

CA Paths Fixes#1570
shirady wants to merge 8 commits intonoobaa:masterfrom
shirady:check-certs

shirady commented Mar 24, 2025 •

edited

Loading

Uh oh!

shirady commented Mar 24, 2025 •

edited

Loading

Uh oh!

shirady commented Apr 10, 2025

Uh oh!

shirady commented Apr 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

shirady commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explain the changes

Issues:

Testing Instructions:

Basic Manual Tests (more details in the comment below)

Uh oh!

shirady commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional Testing Instructions:

1) Code change - path of cert

Explanation:

Before code changes:

After code changes:

2) Code change - rename the r.CaBundleConf.Name

After code changes:

3) Code change - mount path of directory (don't show file extension)

Before code changes:

After code changes:

4) Code change - path of cert

Explanation:

Before code changes:

After code changes:

5) Code change - Change the condition existence of the config map of the CA (configmap-ca-inject) + data in it.

Uh oh!

shirady commented Apr 10, 2025

Updates

Recommendation of what the testing end-to-end can include:

Uh oh!

shirady commented Apr 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

shirady commented Mar 24, 2025 •

edited

Loading

shirady commented Mar 24, 2025 •

edited

Loading

5) Code change - Change the condition existence of the config map of the CA (`configmap-ca-inject`) + data in it.