Skip to content

chore: Improve mounting certs into users containers#1980

Merged
tolusha merged 4 commits intomainfrom
CRW-8316
Mar 24, 2025
Merged

chore: Improve mounting certs into users containers#1980
tolusha merged 4 commits intomainfrom
CRW-8316

Conversation

@tolusha
Copy link
Contributor

@tolusha tolusha commented Mar 17, 2025

What does this PR do?

  1. Removing the che-trusted-ca-certs ConfigMap from user namespaces. These ConfigMaps were used to mount certificates into the /public-certs directory.
  2. The ca-certs-merged ConfigMap is created in the user namespace and is merged either into the /public-certs directory or /etc/pki/ca-trust/extracted/pem, depending on the value of spec.devEnvironments.trustedCerts.disableWorkspaceCaBundleMount

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

https://issues.redhat.com/browse/CRW-8316

How to test this PR?

  1. Deploy a next Eclipse Che version
chectl update next
chectl server:deploy -p openshift 
  1. Login Eclipse Che (open dashboard)

  2. Observe a size of two ConfigMaps in a user namespace (pretty big)

~ oc get configmaps -n <..> ca-certs-merged -o json | jq '.data."tls-ca-bundle.pem" | length'
238783
~ oc get configmaps -n <..> che-trusted-ca-certs -o json | jq '.data."tls-ca-bundle.pem" | length'
~ 238783
  1. Update Eclipse Che with a new changes
chectl server:update --che-operator-image=quay.io/abazko/operator:CRW-8316
  1. Wait until operator completes reconciliation

  2. Check that che-trusted-ca-certs doesn't exist in a user namespace

~ oc get configmaps -n <..> che-trusted-ca-certs 
configmaps "che-trusted-ca-certs" not found
  1. Start an empty workspace

  2. Ensure there is no anymore /public-certs directory mounted

oc exec -n <user_namespace> <workspace_pod> -c universal-developer-image -- ls /public-certs
  1. Ensure that tls-ca-bundle.pem file is a new file in the directory
oc exec -n <user_namespace> <workspace_pod> -c universal-developer-image -- ls /etc/pki/ca-trust/extracted/pem -l
  1. Check it content, ensure it contains content from kube-root-ca.crt, self-signed-certificate and ca-certs ConfigMaps
oc exec -n <user_namespace> <workspace_pod> -c universal-developer-image -- cat /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem | less
  1. Stop a user workspace

  2. Add custom certificate into OpenShift following the doc:
    https://docs.openshift.com/container-platform/4.17/security/certificates/updating-ca-bundle.html#ca-bundle-replacing_updating-ca-bundle

  3. After a while start a user workspace again

  4. Recheck step 11, ensure tls-ca-bundle.pem contains recently added certificate from step 13

  5. Disable workspace mounting certs

oc patch checluster eclipse-che -n eclipse-che --patch '{"spec": {"devEnvironments": {"trustedCerts": {"disableWorkspaceCaBundleMount": true}}}}' --type=merge
  1. Wait workspace is restarted

  2. Step 10 is not correct anymore, all files have the same creation date

  3. The size of ca-certs-merged is pretty small:

~ oc get configmaps -n <..> ca-certs-merged -o json | jq '.data."tls-ca-bundle.pem" | length'
10986
  1. Check the content of tls-ca-bundle.pem in /public-certs dir. Ensure it contains content from kube-root-ca.crt, self-signed-certificate and ca-certs ConfigMaps
oc exec -n <user_namespace> <workspace-pod> -c universal-developer-image -- cat /public-certs/tls-ca-bundle.pem
  1. Ensure that ca-certs ConfigMap contains only certs from step 3
oc get configmaps -n eclipse-che ca-certs -o yaml
  1. Do any editor tests

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Mar 17, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@codecov
Copy link

codecov bot commented Mar 17, 2025

Codecov Report

Attention: Patch coverage is 63.63636% with 48 lines in your changes missing coverage. Please review.

Project coverage is 60.07%. Comparing base (e58ad4a) to head (e4fdd7e).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
pkg/deploy/configmap.go 42.10% 22 Missing ⚠️
pkg/deploy/sync.go 0.00% 12 Missing ⚠️
pkg/deploy/labels.go 0.00% 11 Missing ⚠️
pkg/deploy/tls/certificates.go 95.65% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1980      +/-   ##
==========================================
- Coverage   60.08%   60.07%   -0.02%     
==========================================
  Files          76       76              
  Lines        9824     9895      +71     
==========================================
+ Hits         5903     5944      +41     
- Misses       3517     3547      +30     
  Partials      404      404              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha tolusha marked this pull request as ready for review March 18, 2025 11:53
@ibuziuk ibuziuk requested a review from dmytro-ndp March 18, 2025 15:04
@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Mar 18, 2025

@tolusha : thank you for detailed test scenario!

Could you please explain where to get a valid custom-ca for the next step ? :

  1. Add custom certificate into OpenShift following the doc:
    https://docs.openshift.com/container-platform/4.17/security/certificates/updating-ca-bundle.html#ca-bundle-replacing_updating-ca-bundle

And it's not clear how long to wait—minutes, ten minutes, or more—when executing the followup step: 14. After a while start a user workspace again.

@tolusha
Copy link
Contributor Author

tolusha commented Mar 18, 2025

The file appears /tmp/cheCA.crt after deploying Eclipse Che with chectl
After a while for me sounds like 5m

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Mar 18, 2025

@tolusha: I ran test scenario expressed in the PR description against Eclipse Che deployed to test OCP 4.17.
There was expected results at step 1-16, only content of /tmp/cheCA.crt was doubled at the step 15.

At the same time I faced different behavior after disabling workspace mounting certs at step 16.

  1. Wait workspace is restarted

Actually, workspace hadn't restarted within 10 minutes, then I restarted it manually.

  1. Step 10 is not correct anymore, all files have the same creation date

The actual creation date of tls-ca-bundle.pem file was different to other files.

$ oc exec -n  admin-che workspacec2c09bc0db8446cd-68c6b5d9fc-kxqz7 -c universal-developer-image -- ls /etc/pki/ca-trust/extracted/pem -l
total 892
-rw-rw-r--. 1 root root    898 Jul 24  2024 README
-r--r--r--. 1 root root 165521 Oct 31 00:03 email-ca-bundle.pem
-r--r--r--. 1 root root 502506 Oct 31 00:03 objsign-ca-bundle.pem
-rw-r-----. 1 root user 236259 Mar 18 19:21 tls-ca-bundle.pem
  1. The size of ca-certs-merged is pretty small:

There was the same size of ca-certs-merged as before.

  1. Ensure that ca-certs ConfigMap contains only certs from step 3

There were no any certs in the ca-certs ConfigMap:

$ oc get configmaps -n eclipse-che ca-certs -o yaml
apiVersion: v1
kind: ConfigMap
metadata:
  annotations:
    openshift.io/owning-component: Networking / cluster-network-operator
  creationTimestamp: "2025-03-18T17:24:25Z"
  labels:
    app.kubernetes.io/component: ca-bundle
    app.kubernetes.io/instance: che
    app.kubernetes.io/managed-by: che-operator
    app.kubernetes.io/name: che
    app.kubernetes.io/part-of: che.eclipse.org
  name: ca-certs
  namespace: eclipse-che
  ownerReferences:
  - apiVersion: org.eclipse.che/v2
    blockOwnerDeletion: true
    controller: true
    kind: CheCluster
    name: eclipse-che
    uid: f1863a11-e533-4df0-8a26-3f194d485329
  resourceVersion: "51051003"
  uid: 0fd501c4-7346-4ee3-8080-0a90221b0323

Then I deleted CheCluster and created it again from scratch with property devEnvironments: trustedCerts: "disableWorkspaceCaBundleMount": true.

But che-operator failed to install Eclipse Che.

In the "che-operator" pod logs I found the next text: che-operator.logs.txt

I would appreciate it if you could take a look at the test results.

@tolusha
Copy link
Contributor Author

tolusha commented Mar 19, 2025

From what I can see, step 16 wasn't completed successfully.
Please ensure, that CheCluster CR has been updated

@dmytro-ndp
Copy link
Contributor

@tolusha : thanks for the comment.
I ensured, that CheCluster CR had been updated when executed step 16.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
v1 "k8s.io/api/core/v1"
"k8s.io/api/core/v1"
Copy link
Member

Choose a reason for hiding this comment

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

if I remember correctly, this change needs to be reverted every time after update-dev-resources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right

// Remove trusted-ca-certs ConfigMap from the target namespace to reduce the number of ConfigMaps
// and avoid mounting the same certificates under different paths.
// See cerificates#syncCheCABundleCerts
trustedCACertsCMKey := client.ObjectKey{Name: prefixedName("trusted-ca-certs"), Namespace: targetNs}
Copy link
Member

Choose a reason for hiding this comment

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

on dogfooding, I see there is che-trusted-ca-certs, is it going to be removed by prefix?

Copy link
Member

Choose a reason for hiding this comment

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

Also, testing configuration of 3.19, am I correct that che-trusted-ca-certs and ca-certs-merged are pretty much identical?

Screenshot 2025-03-19 at 14 21 37

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. che-trusted-ca-certs will be removed completedly
  2. yes, those CMs are identical

Comment on lines 227 to 229
//if m == nil {
// continue
//}
Copy link
Member

Choose a reason for hiding this comment

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

leftover?

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Mar 20, 2025

@dmytro-ndp
It is ready to review again

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Mar 20, 2025

@tolusha : seems like container image quay.io/abazko/operator:CRW-8316 mentioned in step 5 of test scenario from PR description doesn't include latest commit 28b87c4

Could you please update that image?

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

The test scenario from the PR description passed when run using Eclipse Che Next with the dashboard image quay.io/abazko/operator@sha256:265f6fb39412b5a4df067bba95baf24ca121a5f580927603d63843483b40747b

Test details:

  • workspace was restarted automatically at step 17, after setting up "devEnvironments.trustedCerts.disableWorkspaceCaBundleMount: true"

  • result of step "18. Step 10 is not correct anymore, all files have the same creation date":

$ oc exec -n admin-che workspace30fc90bb823e42dd-d79b5f65d-rj585 -c universal-developer-image -- ls /etc/pki/ca-trust/extracted/pem -l
total 884
-rw-rw-r--. 1 root root    898 Jul 24  2024 README
-r--r--r--. 1 root root 165521 Oct 31 00:03 email-ca-bundle.pem
-r--r--r--. 1 root root 502506 Oct 31 00:03 objsign-ca-bundle.pem
-r--r--r--. 1 root root 226489 Oct 31 00:03 tls-ca-bundle.pem
  • result of step "19. The size of ca-certs-merged is pretty small":
$ oc get configmaps -n admin-che ca-certs-merged -o json | jq '.data."tls-ca-bundle.pem" | length'
10888

$ oc get configmaps -n admin-che che-trusted-ca-certs -o json | jq '.data."tls-ca-bundle.pem" | length'
Error from server (NotFound): configmaps "che-trusted-ca-certs" not found
  • "Step 22. Do any editor tests"
    A few sample workspaces - .NET 5.0, React - started successfully. VS Code Editors opened, and I was able to start the React application and open it in a separate tab.

Well done, @tolusha !

@openshift-ci
Copy link

openshift-ci bot commented Mar 21, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dmytro-ndp, tolusha

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

@tolusha tolusha merged commit ae7054e into main Mar 24, 2025
21 checks passed
@tolusha tolusha deleted the CRW-8316 branch March 24, 2025 07:43
@devspacesbuild
Copy link

Build 3.21 :: operator_3.x/450: Console, Changes, Git Data

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9127: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67114466 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9128: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67116633 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9129: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67119459 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9131: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67123288 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9135: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67126362 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9136: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67128187 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9137: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67130972 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

@devspacesbuild
Copy link

Build 3.21 :: get-sources-rhpkg-container-build_3.x/9142: FAILURE

devspaces-operator-bundle : 3.x :: Failed in 67131361 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants