Skip to content
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

CLIP-1826: Add user provided certificates to the default Java truststore #663

Merged
merged 4 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions docs/docs/userguide/CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,53 @@ readinessProbe:
!!!warning "`startupProbe` and `livenessProbe`"

Both `startupProbe` and `livenessProbe` are disabled by default. Make sure you go through the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/){.external} before enabling such probes. Misconfiguration can result in unwanted container restarts and failed "cold" starts.

## :material-certificate: Self Signed Certificates

Accessing applications or websites that are encrypted with SSL using certificates not signed by a public authority will result in a connection error. The stacktrace will contain the following:

```shell
caused by: javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: PKIX path building failed:sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
```

To fix the issue, self signed certificates need to be added to Java truststore. First off, you need to create a [Kubernetes secret](https://kubernetes.io/docs/concepts/configuration/secret/){.external} containing base64-encoded certificate(s). Here's an example [kubectl command](https://kubernetes.io/docs/tasks/configmap-secret/managing-secret-using-kubectl/#use-source-files){.external} to create a secret from 2 local files:

```shell
kubectl create secret generic dev-certificates \
--from-file=stg.crt=./stg.crt \
--from-file=dev.crt=./dev.crt -n $namespace
```

The resulting secret will have the following data:

```yaml
data:
stg.crt: base64encodedstgcrt
dev.crt: base64encodeddevcrt
```

!!!info "You can have as many keys (certificates) in the secret as required. All keys will be mounted as files to `/tmp/crt` in the container and imported into Java truststore. In the example above, certificates will be mounted as `/tmp/crt/stg.crt` and `/tmp/crt/dev.crt`. File extension in the secret keys does not matter as long as the file is a valid certificate."

Once done, provide the secret name in Helm values:

```yaml
jira:
additionalCertificates:
secretName: dev-certificates
```

Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:
The product Helm chart will add additional `volumeMounts` and `volumes` to the pod(s), as well as an extra init container that will:


* copy the default Java cacerts to a runtime volume shared between the init container and the main container at `/var/ssl`
* run [keytool -import](https://docs.oracle.com/javase/8/docs/technotes/tools/unix/keytool.html){.external} to import all certificates in `/tmp/crt` mounted from `dev-certificates` secret to `/var/ssl/cacerts`

`-Djavax.net.ssl.trustStore=/var/ssl/cacerts` system property will be automatically added to `JVM_SUPPORT_RECOMMENDED_ARGS` environment variable.

If necessary, it is possible to override the default `keytool -import` command:

```yaml
jira:
additionalCertificates:
secretName: dev-certificates
customCmd: keytool -import ...
```
Comment on lines +522 to +571
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bianchi2 Im just wondering if it makes more sense for this to go under the TROUBLESHOOTING section? Maybe not, but as a compromise a section in the troubleshooter page talking to PKIX exceptions with a link to the fix might be good?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good idea! I'll raise another PR with docs changes as this one has been merged.

1 change: 1 addition & 0 deletions src/main/charts/bamboo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Kubernetes: `>=1.21.x-0`
| bamboo.accessLog.localHomeSubPath | string | `"log"` | The subdirectory within the local-home volume where access logs should be stored. |
| bamboo.accessLog.mountPath | string | `"/opt/atlassian/bamboo/logs"` | The path within the Bamboo container where the local-home volume should be mounted in order to capture access logs. |
| bamboo.additionalBundledPlugins | list | `[]` | Specifies a list of additional Bamboo plugins that should be added to the Bamboo container. Note plugins installed via this method will appear as bundled plugins rather than user plugins. These should be specified in the same manner as the 'additionalLibraries' property. Additional details: https://atlassian.github.io/data-center-helm-charts/examples/external_libraries/EXTERNAL_LIBS/ NOTE: only .jar files can be loaded using this approach. OBR's can be extracted (unzipped) to access the associated .jar An alternative to this method is to install the plugins via "Manage Apps" in the product system administration UI. |
| bamboo.additionalCertificates | object | `{"customCmd":null,"secretName":null}` | Certificates to be added to Java truststore. Provide reference to a secret that contains the certificates |
| bamboo.additionalEnvironmentVariables | list | `[]` | Defines any additional environment variables to be passed to the Bamboo container. See https://hub.docker.com/r/atlassian/bamboo-server for supported variables. |
| bamboo.additionalJvmArgs | list | `[]` | Specifies a list of additional arguments that can be passed to the Bamboo JVM, e.g. system properties. |
| bamboo.additionalLibraries | list | `[]` | Specifies a list of additional Java libraries that should be added to the Bamboo container. Each item in the list should specify the name of the volume that contains the library, as well as the name of the library file within that volume's root directory. Optionally, a subDirectory field can be included to specify which directory in the volume contains the library file. Additional details: https://atlassian.github.io/data-center-helm-charts/examples/external_libraries/EXTERNAL_LIBS/ |
Expand Down
19 changes: 19 additions & 0 deletions src/main/charts/bamboo/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ on Tomcat's logs directory. THis ensures that Tomcat+Bamboo logs get captured in
{{- if .Values.volumes.sharedHome.subPath }}
subPath: {{ .Values.volumes.sharedHome.subPath | quote }}
{{- end }}
{{- if .Values.bamboo.additionalCertificates.secretName }}
- name: keystore
mountPath: /var/ssl
{{- end }}
{{- end }}

{{/*
Expand Down Expand Up @@ -205,6 +209,13 @@ For each additional plugin declared, generate a volume mount that injects that l
{{- with .Values.volumes.additional }}
{{- toYaml . | nindent 0 }}
{{- end }}
{{- if .Values.bamboo.additionalCertificates.secretName }}
- name: keystore
emptyDir: {}
- name: certs
secret:
secretName: {{ .Values.bamboo.additionalCertificates.secretName }}
{{- end }}
{{- end }}

{{- define "bamboo.volumes.localHome" -}}
Expand Down Expand Up @@ -307,3 +318,11 @@ Define additional hosts here to allow template overrides when used as a sub char
{{- toYaml . }}
{{- end }}
{{- end }}

{{- define "bamboo.addCrtToKeystoreCmd" }}
{{- if .Values.bamboo.additionalCertificates.customCmd}}
{{ .Values.bamboo.additionalCertificates.customCmd}}
{{- else }}
set -e; for crt in /tmp/crt/*.*; do echo "Adding $crt to keystore"; keytool -import -cacerts -storepass changeit -noprompt -alias $(echo $(basename $crt)) -file $crt; done; cp $JAVA_HOME/lib/security/cacerts /var/ssl/cacerts
Copy link
Member

Choose a reason for hiding this comment

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

What happens when customer has or wants to define different passkey than the default changeit?

Copy link
Collaborator Author

@bianchi2 bianchi2 Sep 16, 2023

Choose a reason for hiding this comment

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

We assume that the official image is used. In case of a custom truststore, it should be already mounted as a secret. So, if a user already mounts a custom truststore, importing new certs happens outside helm then.

Copy link
Member

Choose a reason for hiding this comment

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

Right and if customer wants to do something specific in this command, they can always redefine the customCmd.

{{- end }}
{{- end }}
3 changes: 3 additions & 0 deletions src/main/charts/bamboo/templates/config-jvm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ data:
{{ . }}
{{- end }}
-XX:ActiveProcessorCount={{ include "flooredCPU" .Values.bamboo.resources.container.requests.cpu }}
{{- if .Values.bamboo.additionalCertificates.secretName }}
-Djavax.net.ssl.trustStore=/var/ssl/cacerts
{{- end }}
{{ include "common.jmx.javaagent" . | indent 4 | trim }}
max_heap: {{ .Values.bamboo.resources.jvm.maxHeap }}
min_heap: {{ .Values.bamboo.resources.jvm.minHeap }}
12 changes: 12 additions & 0 deletions src/main/charts/bamboo/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ spec:
command: ["sh", "-c", {{ include "bamboo.sharedHome.permissionFix.command" . | quote }}]
{{- end }}
{{- include "common.jmx.initContainer" . | nindent 8 }}
{{- if .Values.bamboo.additionalCertificates.secretName }}
- name: import-certs
image: {{ include "bamboo.image" . | quote }}
Copy link
Member

Choose a reason for hiding this comment

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

I guess we are running this image because we want to run the keytool util? I am just wondering whether it is necessary to spin up the large product image to run a simple command.

Copy link
Collaborator Author

@bianchi2 bianchi2 Sep 16, 2023

Choose a reason for hiding this comment

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

Great question :) In fact, it does not really matter if we use temurin image for keytool init container. This won't save us any time since product image needs to be pulled anyways. It's a matter of which phase takes longer - init or containers start. If we pull product image in the init phase, container runtime will have it available when the product container starts. No gains/benefits with either of the approaches, imho.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for confirming this. I was thinking that because we pull the image anyway, it is faster on the pull time but for a very short period we take more resources than we need but as the execution is super quick, it is negligible.

Last thing, if this fails, does it stop container execution, and how will customers get to the error logs (are they sufficient to understand why it failed?)

imagePullPolicy: {{ .Values.image.pullPolicy }}
volumeMounts:
- name: keystore
mountPath: /var/ssl
- name: certs
mountPath: /tmp/crt
command: ["/bin/bash"]
args: ["-c", {{ include "bamboo.addCrtToKeystoreCmd" . }}]
{{- end }}
containers:
- name: {{ if .Values.bamboo.useHelmReleaseNameAsContainerName}}{{ include "common.names.fullname" . }}{{ else }}{{ .Chart.Name }}{{ end }}
image: {{ include "bamboo.image" . | quote }}
Expand Down
6 changes: 6 additions & 0 deletions src/main/charts/bamboo/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,12 @@ bamboo:
# labelSelector:
# matchLabels:

# -- Certificates to be added to Java truststore. Provide reference to a secret that contains the certificates
#
additionalCertificates:
secretName:
customCmd:

# Monitoring
#
monitoring:
Expand Down
2 changes: 2 additions & 0 deletions src/main/charts/bitbucket/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Kubernetes: `>=1.21.x-0`
| additionalLabels | object | `{}` | Additional labels that should be applied to all resources |
| affinity | object | `{}` | Standard Kubernetes affinities that will be applied to all Bitbucket pods Due to the performance requirements it is highly recommended running all Bitbucket pods in the same availability zone as your dedicated NFS server. To achieve this, you can define `affinity` and `podAffinity` rules that will place all pods into the same zone, and therefore minimise the real distance between the application pods and the shared storage. More specific documentation can be found in the official Affinity and Anti-affinity documentation: https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity This is an example on how to ensure the pods are in the same zone as NFS server that is labeled with `role=nfs-server`: podAffinity: requiredDuringSchedulingIgnoredDuringExecution: - labelSelector: matchExpressions: - key: role operator: In values: - nfs-server # needs to be the same value as NFS server deployment topologyKey: topology.kubernetes.io/zone |
| bitbucket.additionalBundledPlugins | list | `[]` | Specifies a list of additional Bitbucket plugins that should be added to the Bitbucket container. Note plugins installed via this method will appear as bundled plugins rather than user plugins. These should be specified in the same manner as the 'additionalLibraries' property. Additional details: https://atlassian.github.io/data-center-helm-charts/examples/external_libraries/EXTERNAL_LIBS/ NOTE: only .jar files can be loaded using this approach. OBR's can be extracted (unzipped) to access the associated .jar An alternative to this method is to install the plugins via "Manage Apps" in the product system administration UI. |
| bitbucket.additionalCertificates | object | `{"customCmd":null,"secretName":null}` | Certificates to be added to Java truststore. Provide reference to a secret that contains the certificates |
| bitbucket.additionalEnvironmentVariables | list | `[]` | Defines any additional environment variables to be passed to the Bitbucket container. See https://hub.docker.com/r/atlassian/bitbucket-server for supported variables. |
| bitbucket.additionalJvmArgs | list | `[]` | Specifies a list of additional arguments that can be passed to the Bitbucket JVM, e.g. system properties. |
| bitbucket.additionalLibraries | list | `[]` | Specifies a list of additional Java libraries that should be added to the Bitbucket container. Each item in the list should specify the name of the volume that contains the library, as well as the name of the library file within that volume's root directory. Optionally, a subDirectory field can be included to specify which directory in the volume contains the library file. Additional details: https://atlassian.github.io/data-center-helm-charts/examples/external_libraries/EXTERNAL_LIBS/ |
Expand All @@ -55,6 +56,7 @@ Kubernetes: `>=1.21.x-0`
| bitbucket.livenessProbe.initialDelaySeconds | int | `60` | Time to wait before starting the first probe |
| bitbucket.livenessProbe.periodSeconds | int | `5` | How often (in seconds) the Bitbucket container liveness probe will run |
| bitbucket.livenessProbe.timeoutSeconds | int | `1` | Number of seconds after which the probe times out |
| bitbucket.mesh.additionalCertificates | object | `{"customCmd":null,"secretName":null}` | Certificates to be added to Java truststore. Provide reference to a secret that contains the certificates |
| bitbucket.mesh.additionalEnvironmentVariables | object | `{}` | Defines any additional environment variables to be passed to the Bitbucket mesh containers. |
| bitbucket.mesh.additionalFiles | string | `nil` | Additional existing ConfigMaps and Secrets not managed by Helm that should be mounted into service container |
| bitbucket.mesh.additionalInitContainers | object | `{}` | Additional initContainer definitions that will be added to all Bitbucket pods |
Expand Down
23 changes: 23 additions & 0 deletions src/main/charts/bitbucket/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,13 @@ Define additional hosts here to allow template overrides when used as a sub char
{{- with .Values.volumes.additional }}
{{- toYaml . | nindent 0 }}
{{- end }}
{{- if .Values.bitbucket.additionalCertificates.secretName }}
- name: keystore
emptyDir: {}
- name: certs
secret:
secretName: {{ .Values.bitbucket.additionalCertificates.secretName }}
{{- end }}
{{- end }}

{{- define "bitbucket.volumes.localHome" -}}
Expand Down Expand Up @@ -445,3 +452,19 @@ volumeClaimTemplates:
{{- . }}
{{- end }}
{{- end}}

{{- define "bitbucket.addCrtToKeystoreCmd" }}
{{- if .Values.bitbucket.additionalCertificates.customCmd}}
{{ .Values.bitbucket.additionalCertificates.customCmd}}
{{- else }}
set -e; for crt in /tmp/crt/*.*; do echo "Adding $crt to keystore"; keytool -import -cacerts -storepass changeit -noprompt -alias $(echo $(basename $crt)) -file $crt; done; cp $JAVA_HOME/lib/security/cacerts /var/ssl/cacerts
{{- end }}
{{- end }}

{{- define "bitbucketMesh.addCrtToKeystoreCmd" }}
{{- if .Values.bitbucket.mesh.additionalCertificates.customCmd}}
{{ .Values.bitbucket.mesh.additionalCertificates.customCmd}}
{{- else }}
set -e; for crt in /tmp/crt/*.*; do echo "Adding $crt to keystore"; keytool -import -cacerts -storepass changeit -noprompt -alias $(echo $(basename $crt)) -file $crt; done; cp $JAVA_HOME/lib/security/cacerts /var/ssl/cacerts
{{- end }}
{{- end }}
3 changes: 3 additions & 0 deletions src/main/charts/bitbucket/templates/config-jvm-mesh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ data:
{{- if .Values.monitoring.exposeJmxMetrics }}
-javaagent:{{ .Values.monitoring.jmxExporterCustomJarLocation | default (printf "%s/jmx_prometheus_javaagent.jar" ( .Values.bitbucket.mesh.volume.mountPath)) }}={{ .Values.monitoring.jmxExporterPort}}:/opt/atlassian/jmx/jmx-config.yaml
{{- end }}
{{- if .Values.bitbucket.mesh.additionalCertificates.secretName }}
-Djavax.net.ssl.trustStore=/var/ssl/cacerts
{{- end }}
max_heap: {{ .Values.bitbucket.mesh.resources.jvm.maxHeap }}
min_heap: {{ .Values.bitbucket.mesh.resources.jvm.minHeap }}
3 changes: 3 additions & 0 deletions src/main/charts/bitbucket/templates/config-jvm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,8 @@ data:
{{- if .Values.monitoring.exposeJmxMetrics }}
-Dplugin.bitbucket-git.mesh.sidecar.jvmArgs=-javaagent:{{ .Values.monitoring.jmxExporterCustomJarLocation | default (printf "%s/jmx_prometheus_javaagent.jar" .Values.volumes.sharedHome.mountPath) }}=9998:/opt/atlassian/jmx/jmx-config.yaml
{{- end }}
{{- if .Values.bitbucket.additionalCertificates.secretName }}
-Djavax.net.ssl.trustStore=/var/ssl/cacerts
{{- end }}
max_heap: {{ .Values.bitbucket.resources.jvm.maxHeap }}
min_heap: {{ .Values.bitbucket.resources.jvm.minHeap }}
23 changes: 23 additions & 0 deletions src/main/charts/bitbucket/templates/statefulset-mesh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ spec:
securityContext:
runAsUser: 0 # make sure we run as root to avoid permission denied
{{- end }}
{{- if .Values.bitbucket.mesh.additionalCertificates.secretName }}
- name: import-certs
image: {{ .Values.bitbucket.mesh.image.repository }}:{{ .Values.bitbucket.mesh.image.tag }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
volumeMounts:
- name: keystore
mountPath: /var/ssl
- name: certs
mountPath: /tmp/crt
command: ["/bin/bash"]
args: ["-c", {{ include "bitbucketMesh.addCrtToKeystoreCmd" . }}]
{{- end }}
containers:
- name: {{ if .Values.bitbucket.useHelmReleaseNameAsContainerName}}{{ include "common.names.fullname" . }}-mesh{{ else }}{{ .Chart.Name }}-mesh{{ end }}
image: {{ .Values.bitbucket.mesh.image.repository }}:{{ .Values.bitbucket.mesh.image.tag }}
Expand Down Expand Up @@ -74,6 +86,10 @@ spec:
mountPath: {{ .mountPath }}/{{ .key }}
subPath: {{ .key }}
{{ end }}
{{- if .Values.bitbucket.mesh.additionalCertificates.secretName }}
- name: keystore
mountPath: /var/ssl
{{- end }}
{{- include "common.jmx.config.volumeMounts" . | nindent 12 }}
{{- with .Values.bitbucket.mesh.resources.container }}
resources:
Expand Down Expand Up @@ -147,6 +163,13 @@ spec:
- key: {{ .key }}
path: {{ .key }}
{{ end }}
{{- if .Values.bitbucket.mesh.additionalCertificates.secretName }}
- name: keystore
emptyDir: {}
- name: certs
secret:
secretName: {{ .Values.bitbucket.mesh.additionalCertificates.secretName }}
{{- end }}
{{ include "common.jmx.config.volume" . | nindent 8 }}
{{ include "bitbucket.mesh.volumeClaimTemplates" . | nindent 2 }}
{{ end }}
16 changes: 16 additions & 0 deletions src/main/charts/bitbucket/templates/statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ spec:
command: ["sh", "-c", {{ include "bitbucket.sharedHome.permissionFix.command" . | quote }}]
{{- end }}
{{- include "common.jmx.initContainer" . | nindent 8 }}
{{- if .Values.bitbucket.additionalCertificates.secretName }}
- name: import-certs
image: {{ include "bitbucket.image" . | quote }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
volumeMounts:
- name: keystore
mountPath: /var/ssl
- name: certs
mountPath: /tmp/crt
command: ["/bin/bash"]
args: ["-c", {{ include "bitbucket.addCrtToKeystoreCmd" . }}]
{{- end }}
containers:
- name: {{ if .Values.bitbucket.useHelmReleaseNameAsContainerName}}{{ include "common.names.fullname" . }}{{ else }}{{ .Chart.Name }}{{ end }}
image: {{ include "bitbucket.image" . | quote }}
Expand Down Expand Up @@ -131,6 +143,10 @@ spec:
subPath: {{ .Values.volumes.sharedHome.subPath | quote }}
{{- end }}
{{- end }}
{{- if .Values.bitbucket.additionalCertificates.secretName }}
- name: keystore
mountPath: /var/ssl
{{- end }}
{{- include "bitbucket.additionalVolumeMounts" . | nindent 12 }}
{{- include "common.jmx.config.volumeMounts" . | nindent 12 }}
{{- include "bitbucket.additionalLibraries" . | nindent 12 }}
Expand Down
12 changes: 12 additions & 0 deletions src/main/charts/bitbucket/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,12 @@ bitbucket:
#
terminationGracePeriodSeconds: 35

# -- Certificates to be added to Java truststore. Provide reference to a secret that contains the certificates
#
additionalCertificates:
secretName:
customCmd:

# -- Specifies a list of additional arguments that can be passed to the Bitbucket JVM, e.g.
# system properties.
#
Expand Down Expand Up @@ -1102,6 +1108,12 @@ bitbucket:
# labelSelector:
# matchLabels:

# -- Certificates to be added to Java truststore. Provide reference to a secret that contains the certificates
#
additionalCertificates:
secretName:
customCmd:

# Monitoring
#
monitoring:
Expand Down
Loading
Loading