Skip to content

Reload cert in conversion webhook #399#410

Merged
jfaltermeier merged 3 commits intomainfrom
jf/certificate-reload
Apr 15, 2025
Merged

Reload cert in conversion webhook #399#410
jfaltermeier merged 3 commits intomainfrom
jf/certificate-reload

Conversation

@jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Mar 14, 2025

Allow to set conversion webhook certificate reload period

Related change: eclipse-theia/theia-cloud-helm#91

@jfaltermeier jfaltermeier marked this pull request as ready for review March 14, 2025 15:42
@jfaltermeier
Copy link
Contributor Author

jfaltermeier commented Mar 14, 2025

Note: The webhook performs checks based on the timeframe.

For the webhook to reload, the certificate actually needs to have changed in the meantime.

Then we get (debug) logs like this:

2025-03-14 15:41:00,953 DEBUG [jdk.eve.security] (vert.x-internal-blocking-1) X509Certificate: Alg:SHA256withRSA, Serial:4e:e6:c5:33:0f:4b:82:65:a1:22:0a:8a:b6:6a:9d:17, Subject:CN=crd.conversion.cert, Issuer:CN=Theia Cloud CA, Key type:RSA, Length:2048, Cert Id:778408355, Valid from:3/14/25, 3:40 PM, Valid until:3/14/25, 4:40 PM
2025-03-14 15:41:00,983 DEBUG [io.qua.ver.htt.run.opt.TlsCertificateReloader] (vert.x-eventloop-thread-0) TLS certificates updated

If the cert hasn't changed, nothing will be logged.

So if you want to verify this, you need to set the default log level to debug for the conversion webhook and generate certs that will renew during your testing time.

@jfaltermeier
Copy link
Contributor Author

@xai This one still needs a review, please have a look when you get a chance.

Copy link
Contributor

@xai xai left a comment

Choose a reason for hiding this comment

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

At least with newer docker versions, the change triggers two warnings:

 2 warnings found (use docker --debug to expand):
 - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 17)
 - JSONArgsRecommended: JSON arguments recommended for ENTRYPOINT to prevent unintended behavior related to OS signals (line 19)

@xai
Copy link
Contributor

xai commented Mar 26, 2025

Apart from the build warnings, everything works as expected 🎉

After setting the log level to debug (e.g. by running kubectl -n theia-cloud patch deployment conversion-webhook --type='json' -p='[{"op":"add","path":"/spec/template/spec/containers/0/env/-","value":{"name":"QUARKUS_LOG_LEVEL","value":"DEBUG"}}]'), re-creating the cert results in the expected

2025-03-26 13:46:02,681 DEBUG [jdk.eve.security] (vert.x-internal-blocking-1) X509Certificate: Alg:SHA256withRSA, Serial:73:41:35:34:d5:12:5e:9b:f2:d1:c6:37:17:11:c3:f5, Subject:CN=crd.conversion.cert, Issuer:CN=Theia Cloud CA, Key type:RSA, Length:2048, Cert Id:3082424124, Valid from:3/26/25, 1:44 PM, Valid until:6/24/25, 1:44 PM
2025-03-26 13:46:02,702 DEBUG [io.qua.ver.htt.run.opt.TlsCertificateReloader] (vert.x-eventloop-thread-3) TLS certificates updated

@jfaltermeier
Copy link
Contributor Author

I committed your suggestions and ran our e2e tests on this branch: https://github.com/eclipse-theia/theia-cloud/actions/runs/14125714948
I also fixed the Helm chart repository link, which was still pointing to eclipsesource instead of eclipse-theia.

@jfaltermeier jfaltermeier requested a review from xai March 28, 2025 09:43
Copy link
Contributor

@xai xai left a comment

Choose a reason for hiding this comment

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

Thanks!

@jfaltermeier jfaltermeier force-pushed the jf/certificate-reload branch from e10b7e5 to 047d0dd Compare April 15, 2025 08:56
@jfaltermeier
Copy link
Contributor Author

@lucas-koehler Could you please approve as well so we can go ahead and merge this? We need one approval with write access

@jfaltermeier jfaltermeier merged commit 55813e2 into main Apr 15, 2025
4 checks passed
@jfaltermeier jfaltermeier deleted the jf/certificate-reload branch April 15, 2025 09:38
Mtze pushed a commit to EduIDE/EduIDE-Cloud that referenced this pull request May 6, 2025
* Update to Theia IDE 1.53.200

Contributed on behalf of STMicroelectronics

* Updated Helm URLs and added devcontainer definition (eclipse-theia#353)

* Changed helm URLs to new eclipse-theia location.
* Created devcontainer config for GKE Getting Started with Terraform

Co-authored-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>

* Improve and document debugging of operator and service

* Minor improvement: Local testing values should only pull IfNotPresent

I would assume that this is the most common use case.

* Fix lastActivity not being initialized properly

We need to set it after the resource has been created.
Therefore it is now handled by the operator not the service.
Add warning in case resources are edited before being applied to the cluster.
Only check HANDLED sessions from the activity tracking.
Other resources should be cleaned up differently.

* Add review suggestions

* Fix maven repository

This should fix the maven builds.

* Update old eclipsesource urls to eclipse-theia urls

* Value update for configuration snippet feature

eclipse-theia/theia-cloud-helm#76

* eclipse-theiaGH-349: Extend service with a list app definitions endpoint

### App definition endpoint
- Add AppDefinitionResource with endpoint to list all app definitions.
  The endpoint is restricted to authenticated users but is still available in anonymous mode

### Sensitive data redaction
- Add `SensitiveData` annotation to mark properties that should not be serialized publicly by Jackson.
- Add a corresponding serializer and serializer modifiert for Jackson and register the modifier in the service
- Add unit tests for the serializer

### Javascript API
- Update openapi.json from service
- Regenerate api code
- Add AppDefinitions namespace with function to list app definitions

### Testing Page
- Add button to get app definitions
- Apply formatting rules to App.tsx by saving it
- Minor fix in example keycloak URL to remove obsolete `/auth`

### Misc
- Add mockito dependency to the common maven module

* Add REST API markdown docs based on openapi definition

Use the openapi-generator-cli to generate markdown docs from the openapi.json.
Also document how this works and add a hint that the generator supports
various doc and code languages.

* Document naming conventions & Naming alignment (eclipse-theia#368)

Contributed on behalf of STMicroelectronics

Co-authored-by: Simon Graband <sgraband@eclipsesource.com>
Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>

* Prepare 0.12.0 Release (eclipse-theia#369)

* move devcontainer to gke directory
* create changelog
* update versions
* update vsix

* Update versions to 1.0.0-next/SNAPSHOT

Contributed on behalf of STMicroelectronics

* Update Demos to Theia 1.55.0

Contributed on behalf of STMicroelectronics

* Add Automated License Check Workflow

Contributed on behalf of STMicroelectronics

* Improve session pod labels (eclipse-theia#362)

org.eclipse.theia.cloud.common.util.LabelsUtil is added to encapsulate
creation of labels.

To indicate that a pod is a user session, its
'app.kubernetes.io/component' label is set to 'session'.

For custom labels, we use the prefix 'theia-cloud.io'.

This is currently used for:
- Identifying a session pods associated with a user:
  'theia-cloud.io/user' = '$username'
- Identifying a session pod by appdefinition name:
  'theia-cloud.io/app-definition' = $appdefinitionname

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>
Co-authored-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>
Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>

* Update dependencies (eclipse-theia#371)

* Update dependencies
  * rollup
  * vite
  * body-parser
  * express
  * path-to-regexp
  * send
  * serve-static
  * micromatch
  * webpack
  * cookie
  * junit in demos
* update workflow actions (use sha for non github actions)
* update helm dependencies to latest version
  * except keycloak
* add license information to internal pages

Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>

* Prepare 1.0.0 Release

* update changelog
* update to release versions
* update terraform helm charts

Contributed on behalf of STMicroelectronics

* Prepare 1.0.0 Release

pin inversify in demo application.
eclipse-theia/theia#14431 (comment)

* Prepare 1.0.0 Release

pin inversify in demo application.
eclipse-theia/theia#14431 (comment)

* Update versions to 1.1.0-next/SNAPSHOT

Contributed on behalf of STMicroelectronics

* Update Demos to Theia 1.56.1

* Remove unneccessary files from demos

Contributed on behalf of STMicroelectronics

Signed-off-by: Simon Graband <sgraband@eclipsesource.com>

* Cleanup dockerfiles and update monitor-theia demo to 1.56.0

Contributed on behalf of STMicroelectronics

Signed-off-by: Simon Graband <sgraband@eclipsesource.com>

* Update dependencies

* update npm dependencies
  * cross-spawn
  * express
  * socket.io
  * engine.io
  * theia
  * vite
  * webpack
* update maven dependencies
* install libsecret-1-dev in workflows that need it

Contributed on behalf of STMicroelectronics

* Fix NPM Publish

* Fix name handling for eager start

- Fix endless loop and a NPE in the NamingUtil's methods for eager start name creation (based on an app definition)
- Fix id extraction from generated names for the adapted name generation in an earlier PR.

* Trigger instance pod email ConfigMap update via annotation

Add an annotation to an eagerly started instance pod after the email config map was updated.
This is necessary to trigger a sync with the Kubelet.
Otherwise, the pod might not be updated with the new email list for the OAuth proxy in time.
This is the case because ConfigMap changes are not propagated to the pod immediately but during a periodic sync.

* Add terraform test configuration for eager start

* Implement session delete handling for eager start

- Rename EagerSessionStartHandler to EagerSessionHandler
- Implement `sessionDeleted` method to clean up and make the instance available for another user.
  This includes restarting the pod to get a new temporary workspace.

* Fix service max instances and session pod checks in K8sUtil

Check whether max instances are reached by counting sessions relevant to the app definition that are not in error state.
This makes it unnecessary to analyze all pods belonging to the session. Thus, performance for large amounts of pods is increased.

K8sUtil#getPodForSession now finds a pod belonging to a session via owner references instead of ENV variables.
With this, the pod can also be resolved in eager mode where the session name cannot be injected as an env variable.

Also adds some logging to K8sUtil#reportPerformance and increases the log level for exceptions
in the performance REST handler method.

* Extend testing page with getting session performance data

* Fix eager session delete handling for already removed owner references

- Extend LabelsUtil to add the session name to session labels
- Find service from session via labels instead of owner references in eager session delete handler
  With this, the handling also works if the owner reference has been removed automatically
- Add minor clarifying comments

* Setup theia folder with example app and config-store skeleton (eclipse-theia#389)

## Setup new theia folder with example app and config-store extension skeleton

- Create a new root-level folder `theia` for all theia extensions,
  corresponding example extensions and a test browser app.
- The theia folder uses a copied version of the tsconfig and eslint settings of the node folder.
- Initial setup of config-store extension, service and client
- Add license check for theia folder
- Depend on the exact version in the example app to avoid pulling older versions from the registry
- Add Theia CI workflow to build all Theia extensions and the example app
- Add reusable theia extension publish workflow

## Move monitor-theia extension to theia folder and setup Theia CI

- Move monitor-theia extension from node/ to theia/extensions/
- Add monitor-theia to Theia example app
- Adapt monitor theia Demo docker file
- Adapt monitor extension workflow to only do the publishing

* Fix reusable-theia-extension workflow

Remove hardcoded extensions/ prefix to align with the other commands using workflow input package_workspace

* Stabilize service cleanup for eager start

Removing the labels and owner reference fails for rare cases when cleaning an eager instance's service after a session was removed.
This adds retries to stabilize the sessionDeleted handling.

* service: Add admin user concept and app def scaling endpoint (eclipse-theia#400)

* service: Add admin user support via annotation

- Introduce the `@AdminOnly` annotation to mark REST resources or methods as accessible only to admin users.
- Implement AdminOnlyFilter that intercepts requests and aborts non-admin users with a 403 Forbidden response.
- Update ApplicationProperties to include a configurable admin group name property ("theia.cloud.auth.admin.group")
  with a default value of "theia-cloud/admin".
- Enhance TheiaCloudUser by adding an admin flag.
- Modify TheiaCloudUserProducer to derive the admin status from the MicroProfile JWT's groups claim.
- Add tests for the new admin-only filter, properties, and user producer functionality.
- Extend service Dockerfile to allow configuring the admin group name via environment variable.

* service: Add admin endpoint to update app def's min/max instances

- Add new resource AdminAppDefinitionAdminResource for all admin endpoints regarding app definitions
- Minor extensions in K8SUtil, AppDefinitionSpec to allow editing min/max instances
- Add tests for AdminAppDefintionAdminResource
- Add RootAdminResource with a ping endpoint that only returns if the user is an admin
- Regenerate OpenAPI definition, docs and common code

* terrafom: Add admin user group and outputs for Keycloak module

Extend Keycloak terraform module:
- Define an admin group with the default name
- Export realm, admin group and test users via outputs

Test setup:
- Add test user foo to the admin group
- Document how to get a Keycloak access token on the command line

* fix: Update Keycloak URL in tasks.json to include realm path

This is required for the Quarkus dev server to discover the OIDC config.
Also see the official documentation:
https://quarkus.io/guides/security-oidc-configuration-properties-reference#quarkus-oidc_quarkus-oidc-auth-server-url

* Theia Cloud Image for ARM64 eclipse-theia#403

* Theia Cloud Image for ARM64 eclipse-theia#403

* use JRE/JDK 21 base images because image are available for arm64

* Truncate labels longer than 63 chars

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>

* Truncate labels longer than 63 chars

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>

* Prepare 1.0.1 Release

* Prepare 1.0.1 Release

* Initial E2E Tests (eclipse-theia#387)

* add initial E2E tests
* Dont' add workspace as owner reference on persistent volume
* Fix theia monitor demo build: include yarn.lock to make build more reproducible

Contributed on behalf of STMicroelectronics

* Initial E2E Tests (eclipse-theia#387)

* fix branch

* Update NginX

* Add session-uuid label + accept AppDefinition instead of AppDefinitionSpec

* Add the session-specific label theia-cloud.io/session-uuid
* Pass the AppDefinition object instead of its Spec
* Add missing license header in LabelsUtil

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>

* Reload cert in conversion webhook (eclipse-theia#410)

Co-authored-by: Olaf Lessenich <olessenich@eclipsesource.com>

* allow blank issues

---------

Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>
Signed-off-by: Simon Graband <sgraband@eclipsesource.com>
Co-authored-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>
Co-authored-by: Harmen Wessels <97173058+harmen-xb@users.noreply.github.com>
Co-authored-by: Simon Graband <sgraband@eclipsesource.com>
Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>
Co-authored-by: Olaf Lessenich <olessenich@eclipsesource.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants