-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add global certificate management for ZTVP applications #83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
fd1ada7 to
6407ec3
Compare
sabre1041
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good. A few comments
| memory: 128Mi | ||
| limits: | ||
| cpu: 200m | ||
| memory: 256Mi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend increasing these values. My cluster OOMKilled the container
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
increased, this size mismatch was due to the addition of proxy CA extraction.
.github/workflows/superlinter.yml
Outdated
| # These are the validation we disable atm | ||
| VALIDATE_ANSIBLE: false | ||
| VALIDATE_BASH: false | ||
| VALIDATE_BASH_EXEC: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this because we do not set the execute bit on the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shell script was a helm template, which has the helm syntax inside, bash_exec cannot pass it. I've updated the filename to avoid this single file's bash_exec linter execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I usually do in these cases is have a simple script and control the variables and conditions using environment variables. Another option is to have source files and import them depending on the environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FILTER_REGEX_EXCLUDE option also works for me
cd77935 to
4349ad5
Compare
|
I think it might be interesting to extend the functionality to 2 additional components External Secrets OperatorCurrently, the caProvider:
key: ca.crt
name: kube-root-ca.crt
namespace: golang-external-secrets
type: ConfigMapThis can be configured via an override in golang-external-secrets:
name: golang-external-secrets
namespace: golang-external-secrets
project: hub
chart: golang-external-secrets
chartVersion: 0.1.*
overrides:
- name: golangExternalSecrets.caProvider.hostCluster.name
value: ztvp-trusted-ca
- name: golangExternalSecrets.caProvider.hostCluster.key
value: tls-ca-bundle.pem
- name: golangExternalSecrets.caProvider.hostCluster.namespace
value: openshift-configShould we change it permanently if we adopt this solution? Image registries (Quay)The cluster can use image registries that use internal certificates. In this case, the configuration is done in the apiVersion: config.openshift.io/v1
kind: Image
name: cluster
spec:
additionalTrustedCA:
name: ztvp-registry-trusted-caThe apiVersion: v1
data:
quay-registry-quay-quay-enterprise.apps.example.domain: |
-----BEGIN CERTIFICATE-----
... ...
kind: ConfigMap
metadata:
name: ztvp-registry-trusted-ca
namespace: openshift-configThis functionality would allow configuring CRI-O on the nodes to download images from registries with internal certificates. |
…Files Implements comprehensive certificate management for ZTVP: Certificate Sources: - Primary custom CA via secretRef (customCA.secretRef) - Additional certificates via extraValueFiles (overrides/values-ztvp-certificates.yaml) - Auto-detected proxy CA from trusted-ca-bundle (openshift-config-managed) - Auto-detected ingress CA from all IngressControllers (not just default) - Auto-detected service CA from openshift-service-ca Features: - Initial Job for immediate certificate extraction on install - CronJob for periodic certificate rotation (daily at 2 AM) - Warning and continue behavior for missing additional certificates - Automatic rollout restart for consuming applications (labeled strategy) - ACM Policy distribution to target namespaces Configuration: - Use extraValueFiles for complex nested structures (additionalCertificates, rollout) - Simple overrides via values-hub.yaml for flat key-value pairs Signed-off-by: Min Zhang <[email protected]>
4349ad5 to
73e20c0
Compare
| # Configure Java to use custom truststore via system properties | ||
| # This applies to ALL Java SSL connections including Vert.x/Netty | ||
| - name: JAVA_TOOL_OPTIONS | ||
| value: "-Djavax.net.ssl.trustStore=/truststore/cacerts -Djavax.net.ssl.trustStoreType=JKS -Djavax.net.ssl.trustStorePassword=$(TRUSTSTORE_PASSWORD)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of configuring the variables at the JVM level, I would do it using the environment variables that Quarkus has for this: QUARKUS_OIDC_TLS_TRUST_STORE_FILE and QUARKUS_OIDC_TLS_TRUST_STORE_PASSWORD.
We could add them to the charts/qtodo/templates/app-config-env.yaml file, like other Quarkus variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mlorenzofr , updated based on your suggestion that using Quarkus env variables is cleaner.
| - name: db-credentials | ||
| mountPath: /run/secrets/db-credentials | ||
| - name: truststore | ||
| mountPath: /truststore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps I would create it under /run/secrets or /etc/pki instead of creating a directory in the root
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mlorenzofr , updated the truststore to a more standard location based on your suggestion.
mlorenzofr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, it works properly.
I'm adding a couple of small changes that I think would better align the solution with the style of other qtodo components.
73e20c0 to
ac3a0ce
Compare
| vaultPath: "secret/data/global/oidc-client-secret" | ||
|
|
||
| # Truststore configuration for Java CA certificates | ||
| truststore: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| truststore: | |
| truststore: | |
| enabled: true |
a364831 to
cbb9415
Compare
- Add init container to convert CA bundle to Java JKS truststore - Add ExternalSecret for truststore password from Vault - Configure JAVA_TOOL_OPTIONS for JVM-level SSL truststore - Mount ztvp-trusted-ca ConfigMap for CA certificates - Enable truststore by default when SPIRE is enabled Signed-off-by: Min Zhang <[email protected]>
cbb9415 to
9c5a3ef
Compare
Replace deprecated JKS truststore with modern PKCS12 format: - Use keytool with -storetype PKCS12 to create truststore - Configure via JAVA_TOOL_OPTIONS (Quarkus TLS registry doesn't work with OIDC) - Update Hibernate ORM env var to non-deprecated name - Add wait-for-keycloak init container to prevent OIDC 503 warnings PKCS12 is the modern standard recommended over Java-specific JKS format. Signed-off-by: Min Zhang <[email protected]>
Add ztvp-certificates chart for managing trusted CA certificates across all ZTVP applications with support for custom certificates via Kubernetes secret references.
Key features:
Certificate Sources:
Configuration:
Chart deploys:
Applications can consume the CA bundle by: