From 218be7f9c512da0f0f3e967b06f61b1cfb6a589f Mon Sep 17 00:00:00 2001 From: Bastian Hofmann Date: Wed, 20 Sep 2023 12:16:19 +0200 Subject: [PATCH 1/4] Fix probes when Qdrant is served over TLS This also improves the integration test runtime and reliability by not installing a fresh cluster for every test but upgrading it with the new config. Fixes https://github.com/qdrant/qdrant-helm/issues/77 --- charts/qdrant/templates/statefulset.yaml | 9 ++ .../templates/tests/test-db-interaction.yaml | 85 ++++++++++++------- test/integration/api_key.bats | 14 +-- test/integration/assets/.gitignore | 6 ++ test/integration/assets/tls-values.yaml | 19 +++++ test/integration/default_installation.bats | 13 +-- test/integration/https.bats | 24 ++++++ test/integration/random_api_key.bats | 14 +-- test/integration/security_context.bats | 13 +-- test/integration/setup_suite.bash | 2 + test/integration/unprivileged_image.bats | 13 +-- 11 files changed, 121 insertions(+), 91 deletions(-) create mode 100644 test/integration/assets/.gitignore create mode 100644 test/integration/assets/tls-values.yaml create mode 100644 test/integration/https.bats diff --git a/charts/qdrant/templates/statefulset.yaml b/charts/qdrant/templates/statefulset.yaml index cdc7790..086590c 100644 --- a/charts/qdrant/templates/statefulset.yaml +++ b/charts/qdrant/templates/statefulset.yaml @@ -100,6 +100,9 @@ spec: httpGet: path: /livez port: {{ .targetPort }} + {{- if and $values.config.service $values.config.service.enable_tls }} + scheme: HTTPS + {{- end }} {{- end }} initialDelaySeconds: {{ $values.livenessProbe.initialDelaySeconds }} timeoutSeconds: {{ $values.livenessProbe.timeoutSeconds }} @@ -117,6 +120,9 @@ spec: httpGet: path: /readyz port: {{ .targetPort }} + {{- if and $values.config.service $values.config.service.enable_tls }} + scheme: HTTPS + {{- end }} {{- end }} initialDelaySeconds: {{ $values.readinessProbe.initialDelaySeconds }} timeoutSeconds: {{ $values.readinessProbe.timeoutSeconds }} @@ -134,6 +140,9 @@ spec: httpGet: path: /readyz port: {{ .targetPort }} + {{- if and $values.config.service $values.config.service.enable_tls }} + scheme: HTTPS + {{- end }} {{- end }} initialDelaySeconds: {{ $values.startupProbe.initialDelaySeconds }} timeoutSeconds: {{ $values.startupProbe.timeoutSeconds }} diff --git a/charts/qdrant/templates/tests/test-db-interaction.yaml b/charts/qdrant/templates/tests/test-db-interaction.yaml index 7520888..2fc0da3 100644 --- a/charts/qdrant/templates/tests/test-db-interaction.yaml +++ b/charts/qdrant/templates/tests/test-db-interaction.yaml @@ -11,15 +11,21 @@ metadata: spec: containers: - name: test-script - image: registry.suse.com/bci/python:3.11 + image: registry.suse.com/bci/bci-base:latest args: ['bash', '/app/entrypoint.sh'] volumeMounts: - mountPath: /app name: test-script + {{- if .Values.additionalVolumeMounts }} +{{- toYaml .Values.additionalVolumeMounts | default "" | nindent 8 }} + {{- end}} volumes: - name: test-script configMap: name: "{{ include "qdrant.fullname" . }}-test-db-interaction" + {{- if .Values.additionalVolumes }} +{{- toYaml .Values.additionalVolumes | default "" | nindent 4 }} + {{- end}} restartPolicy: Never serviceAccountName: {{ include "qdrant.fullname" . }} --- @@ -35,43 +41,56 @@ data: entrypoint.sh: | #!/bin/bash set -xe - zypper install -y python311-dbm - pip3 install qdrant-client=={{ .Chart.AppVersion }} - python3 /app/test.py - test.py: | - from qdrant_client import QdrantClient - from qdrant_client.http.models import Distance, VectorParams - from qdrant_client.http.models import PointStruct + echo 'connect-timeout = 5' > ~/.curlrc + if [ -d /mnt/secrets/certs ]; then + cp /mnt/secrets/certs/ca.pem /usr/share/pki/trust/anchors/private-ca.pem + update-ca-certificates + fi + + QDRANT_COLLECTION="test_collection" {{- range .Values.service.ports }} {{- if eq .name "http" }} - print("Connecting to {{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}") - client = QdrantClient("{{ include "qdrant.fullname" $root }}.{{ $namespace }}", port={{ .port }}) + echo "Connecting to {{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}" + QDRANT_URL="http://{{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}" + {{- if and $root.Values.config.service $root.Values.config.service.enable_tls }} + echo "Using https" + QDRANT_URL="https://{{ include "qdrant.fullname" $root }}.{{ $namespace }}:{{ .port }}" + {{- end }} {{- end }} {{- end }} + API_KEY_HEADER="" + {{- if .Values.apiKey }} + API_KEY_HEADER="Api-key: {{ .Values.apiKey }}" + {{- end }} + + # Delete collection if exists + curl -X DELETE -H "${API_KEY_HEADER}" $QDRANT_URL/collections/${QDRANT_COLLECTION} - client.recreate_collection( - collection_name="test_collection", - vectors_config=VectorParams(size=4, distance=Distance.DOT), - ) + # Create collection + curl -X PUT \ + -H 'Content-Type: application-json' \ + -d '{"vectors":{"size":4,"distance":"Dot"}}' \ + -H "${API_KEY_HEADER}" \ + $QDRANT_URL/collections/${QDRANT_COLLECTION} --fail-with-body - operation_info = client.upsert( - collection_name="test_collection", - wait=True, - points=[ - PointStruct(id=1, vector=[0.05, 0.61, 0.76, 0.74], payload={"city": "Berlin"}), - PointStruct(id=2, vector=[0.19, 0.81, 0.75, 0.11], payload={"city": "London"}), - PointStruct(id=3, vector=[0.36, 0.55, 0.47, 0.94], payload={"city": "Moscow"}), - PointStruct(id=4, vector=[0.18, 0.01, 0.85, 0.80], payload={"city": "New York"}), - PointStruct(id=5, vector=[0.24, 0.18, 0.22, 0.44], payload={"city": "Beijing"}), - PointStruct(id=6, vector=[0.35, 0.08, 0.11, 0.44], payload={"city": "Mumbai"}), - ] - ) - print(operation_info) + # Insert points + curl -X PUT \ + -H 'Content-Type: application-json' \ + -d '{"points":[ + {"id":1,"vector":[0.05, 0.61, 0.76, 0.74],"payload":{"city":"Berlin"}}, + {"id":2,"vector":[0.19, 0.81, 0.75, 0.11],"payload":{"city":"London"}}, + {"id":3,"vector":[0.36, 0.55, 0.47, 0.94],"payload":{"city":"Moscow"}}, + {"id":4,"vector":[0.18, 0.01, 0.85, 0.80],"payload":{"city":"New York"}}, + {"id":5,"vector":[0.24, 0.18, 0.22, 0.44],"payload":{"city":"Beijing"}}, + {"id":6,"vector":[0.35, 0.08, 0.11, 0.44],"payload":{"city":"Mumbai"}} + ]}' \ + -H "${API_KEY_HEADER}" \ + $QDRANT_URL/collections/${QDRANT_COLLECTION}/points --fail-with-body - search_result = client.search( - collection_name="test_collection", - query_vector=[0.2, 0.1, 0.9, 0.7], - limit=3 - ) - print(search_result) + # Run query + curl -X POST \ + -H 'Content-Type: application-json' \ + -d '{"vector":[0.2, 0.1, 0.9, 0.7],"limit":3}' \ + -H "${API_KEY_HEADER}" \ + $QDRANT_URL/collections/${QDRANT_COLLECTION}/points/search --fail-with-body diff --git a/test/integration/api_key.bats b/test/integration/api_key.bats index 389a793..4998111 100644 --- a/test/integration/api_key.bats +++ b/test/integration/api_key.bats @@ -1,20 +1,12 @@ setup_file() { - kubectl create namespace qdrant-helm-integration - kubectl create serviceaccount default -n qdrant-helm-integration || true - helm install qdrant charts/qdrant --set apiKey=foobar -n qdrant-helm-integration --wait + helm upgrade --install qdrant charts/qdrant --set apiKey=foobar -n qdrant-helm-integration --wait kubectl rollout status statefulset qdrant -n qdrant-helm-integration } -teardown_file() { - helm uninstall qdrant -n qdrant-helm-integration - kubectl delete serviceaccount default -n qdrant-helm-integration - kubectl delete namespace qdrant-helm-integration -} - @test "api key authentication works" { - run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H 'api-key: foobar' + run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H 'api-key: foobar' --fail-with-body [ $status -eq 0 ] - [ "${output}" != "Invalid api-key" ] + [[ "${output}" =~ .*\"status\":\"ok\".* ]] } @test "api key authentication fails with key" { diff --git a/test/integration/assets/.gitignore b/test/integration/assets/.gitignore new file mode 100644 index 0000000..a11c596 --- /dev/null +++ b/test/integration/assets/.gitignore @@ -0,0 +1,6 @@ +*.key +*.crt +*.csr +*.pem +*.ext +*.srl \ No newline at end of file diff --git a/test/integration/assets/tls-values.yaml b/test/integration/assets/tls-values.yaml new file mode 100644 index 0000000..0a63061 --- /dev/null +++ b/test/integration/assets/tls-values.yaml @@ -0,0 +1,19 @@ +additionalVolumes: + - secret: + secretName: test-tls + name: tls + +additionalVolumeMounts: + - name: tls + mountPath: /mnt/secrets/certs + +config: + service: + enable_tls: true + tls: + cert: "/mnt/secrets/certs/tls.crt" + key: "/mnt/secrets/certs/tls.key" + ca_cert: "/mnt/secrets/certs/ca.pem" + cluster: + p2p: + enable_tls: true \ No newline at end of file diff --git a/test/integration/default_installation.bats b/test/integration/default_installation.bats index 39c144e..d663d1d 100644 --- a/test/integration/default_installation.bats +++ b/test/integration/default_installation.bats @@ -1,17 +1,6 @@ setup_file() { - kubectl create namespace qdrant-helm-integration - kubectl create serviceaccount default -n qdrant-helm-integration || true - helm install qdrant charts/qdrant -n qdrant-helm-integration --wait + helm upgrade --install qdrant charts/qdrant -n qdrant-helm-integration --wait kubectl rollout status statefulset qdrant -n qdrant-helm-integration - # wait a bit to ensure that qdrant is really up, and we don't get connection refused - # errors in Github Actions - sleep 10 -} - -teardown_file() { - helm uninstall qdrant -n qdrant-helm-integration - kubectl delete serviceaccount default -n qdrant-helm-integration - kubectl delete namespace qdrant-helm-integration } @test "helm test - default values" { diff --git a/test/integration/https.bats b/test/integration/https.bats new file mode 100644 index 0000000..c034769 --- /dev/null +++ b/test/integration/https.bats @@ -0,0 +1,24 @@ +setup_file() { + rm test/integration/assets/ca.* + rm test/integration/assets/tls.* + openssl genrsa -des3 -passout pass:insecure -out test/integration/assets/ca.key 2048 + openssl req -x509 -passin pass:insecure -new -nodes -key test/integration/assets/ca.key -sha256 -days 1825 -out test/integration/assets/ca.pem -subj "/C=DE/ST=Berlin/L=Berlin/O=Qdrant/OU=Cloud/CN=qdrant" + openssl genrsa -out test/integration/assets/tls.key 2048 + openssl req -new -key test/integration/assets/tls.key -out test/integration/assets/tls.csr -subj "/C=DE/ST=Berlin/L=Berlin/O=Qdrant/OU=Cloud/CN=qdrant.qdrant-helm-integration" + openssl x509 -req -passin pass:insecure -in test/integration/assets/tls.csr -CA test/integration/assets/ca.pem -CAkey test/integration/assets/ca.key -CAcreateserial -out test/integration/assets/tls.crt -days 825 -sha256 + + kubectl -n qdrant-helm-integration create secret generic test-tls --from-file=tls.key="test/integration/assets/tls.key" --from-file=tls.crt="test/integration/assets/tls.crt" --from-file=ca.pem="test/integration/assets/ca.pem" + helm upgrade --install qdrant charts/qdrant -n qdrant-helm-integration --wait -f test/integration/assets/tls-values.yaml + kubectl rollout status statefulset qdrant -n qdrant-helm-integration +} + +@test "Connection with https" { + run kubectl exec -n default curl -- curl -k -s https://qdrant.qdrant-helm-integration:6333/collections --fail-with-body + [ $status -eq 0 ] + [[ "${output}" =~ .*\"status\":\"ok\".* ]] +} + +@test "helm test - with https" { + run helm test qdrant -n qdrant-helm-integration --logs + [ $status -eq 0 ] +} \ No newline at end of file diff --git a/test/integration/random_api_key.bats b/test/integration/random_api_key.bats index 530a287..56198d1 100644 --- a/test/integration/random_api_key.bats +++ b/test/integration/random_api_key.bats @@ -1,22 +1,14 @@ setup_file() { - kubectl create namespace qdrant-helm-integration - kubectl create serviceaccount default -n qdrant-helm-integration || true - helm install qdrant charts/qdrant --set apiKey=true -n qdrant-helm-integration --wait + helm upgrade --install qdrant charts/qdrant --set apiKey=true -n qdrant-helm-integration --wait kubectl rollout status statefulset qdrant -n qdrant-helm-integration } -teardown_file() { - helm uninstall qdrant -n qdrant-helm-integration - kubectl delete serviceaccount default -n qdrant-helm-integration - kubectl delete namespace qdrant-helm-integration -} - @test "random api key works" { apiKey=$(kubectl -n qdrant-helm-integration get secret qdrant-apikey -o jsonpath="{.data.api-key}" | base64 --decode) [ ${#apiKey} -eq 32 ] - run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H "api-key: ${apiKey}" + run kubectl exec -n default curl -- curl -s http://qdrant.qdrant-helm-integration:6333/collections -H "api-key: ${apiKey}" --fail-with-body [ $status -eq 0 ] - [ "${output}" != "Invalid api-key" ] + [[ "${output}" =~ .*\"status\":\"ok\".* ]] } @test "random api key stays the same after upgrade" { diff --git a/test/integration/security_context.bats b/test/integration/security_context.bats index 2fc62f1..d1ffdd3 100644 --- a/test/integration/security_context.bats +++ b/test/integration/security_context.bats @@ -1,16 +1,5 @@ -setup_file() { - kubectl create namespace qdrant-helm-integration - kubectl create serviceaccount default -n qdrant-helm-integration || true -} - -teardown_file() { - helm uninstall qdrant -n qdrant-helm-integration - kubectl delete serviceaccount default -n qdrant-helm-integration - kubectl delete namespace qdrant-helm-integration -} - @test "update without security context to security context corrects file permissions" { - helm install qdrant charts/qdrant --set-json 'podSecurityContext=false,containerSecurityContext=false' -n qdrant-helm-integration --wait + helm upgrade --install qdrant charts/qdrant --set-json 'podSecurityContext=false,containerSecurityContext=false' -n qdrant-helm-integration --wait kubectl rollout status statefulset qdrant -n qdrant-helm-integration user=$(kubectl exec qdrant-0 -n qdrant-helm-integration -- id -u) [ "${user}" = "0" ] diff --git a/test/integration/setup_suite.bash b/test/integration/setup_suite.bash index 800b4c2..829f9d2 100644 --- a/test/integration/setup_suite.bash +++ b/test/integration/setup_suite.bash @@ -3,6 +3,8 @@ setup_suite() { kubectl create serviceaccount default -n default kubectl -n default run curl --image=curlimages/curl --command -- sh -c "sleep 3600" kubectl wait --for=condition=Ready pod/curl -n default --timeout=300s + kubectl create namespace qdrant-helm-integration + kubectl create serviceaccount default -n qdrant-helm-integration || true } teardown_suite() { diff --git a/test/integration/unprivileged_image.bats b/test/integration/unprivileged_image.bats index 5493bc7..59d4baf 100644 --- a/test/integration/unprivileged_image.bats +++ b/test/integration/unprivileged_image.bats @@ -1,17 +1,6 @@ setup_file() { - kubectl create namespace qdrant-helm-integration - kubectl create serviceaccount default -n qdrant-helm-integration || true - helm install qdrant charts/qdrant -n qdrant-helm-integration --wait --set image.useUnprivilegedImage=true + helm upgrade --install qdrant charts/qdrant -n qdrant-helm-integration --wait --set image.useUnprivilegedImage=true kubectl rollout status statefulset qdrant -n qdrant-helm-integration - # wait a bit to ensure that qdrant is really up, and we don't get connection refused - # errors in Github Actions - sleep 10 -} - -teardown_file() { - helm uninstall qdrant -n qdrant-helm-integration - kubectl delete serviceaccount default -n qdrant-helm-integration - kubectl delete namespace qdrant-helm-integration } @test "helm test - with unprivileged image" { From 9e8b89a6ab242e5800b71d68acaa5eb9e4fde233 Mon Sep 17 00:00:00 2001 From: Bastian Hofmann Date: Tue, 19 Sep 2023 11:45:44 +0200 Subject: [PATCH 2/4] Deactivate liveness and startup probes by default Since these probes restart the pod on failure, they can disrupt the health of the Qdrant cluster unnecessarily and cause more problems then actually help. Having a readinessProbe is usually enough. --- charts/qdrant/values.yaml | 4 ++-- test/qdrant_probes_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/qdrant/values.yaml b/charts/qdrant/values.yaml index 6a92448..5f51114 100644 --- a/charts/qdrant/values.yaml +++ b/charts/qdrant/values.yaml @@ -54,7 +54,7 @@ ingress: # secretName: tls-secret-name livenessProbe: - enabled: true + enabled: false initialDelaySeconds: 5 periodSeconds: 5 timeoutSeconds: 1 @@ -70,7 +70,7 @@ readinessProbe: successThreshold: 1 startupProbe: - enabled: true + enabled: false initialDelaySeconds: 10 periodSeconds: 5 timeoutSeconds: 1 diff --git a/test/qdrant_probes_test.go b/test/qdrant_probes_test.go index c3962ec..ea478a3 100644 --- a/test/qdrant_probes_test.go +++ b/test/qdrant_probes_test.go @@ -38,7 +38,7 @@ func TestDefaultProbesOnStatefulset(t *testing.T) { return container.Name == "qdrant" }) - require.Equal(t, "/readyz", container.StartupProbe.HTTPGet.Path) + require.Empty(t, container.StartupProbe) require.Equal(t, "/readyz", container.ReadinessProbe.HTTPGet.Path) - require.Equal(t, "/livez", container.LivenessProbe.HTTPGet.Path) + require.Empty(t, container.LivenessProbe) } From 6ebd522e6c01a13a7e5845c16a2b48c8bbfca67c Mon Sep 17 00:00:00 2001 From: Bastian Hofmann Date: Wed, 20 Sep 2023 12:31:32 +0200 Subject: [PATCH 3/4] Fix tests --- test/integration/https.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/https.bats b/test/integration/https.bats index c034769..e3cb437 100644 --- a/test/integration/https.bats +++ b/test/integration/https.bats @@ -1,6 +1,6 @@ setup_file() { - rm test/integration/assets/ca.* - rm test/integration/assets/tls.* + rm test/integration/assets/ca.* || true + rm test/integration/assets/tls.* || true openssl genrsa -des3 -passout pass:insecure -out test/integration/assets/ca.key 2048 openssl req -x509 -passin pass:insecure -new -nodes -key test/integration/assets/ca.key -sha256 -days 1825 -out test/integration/assets/ca.pem -subj "/C=DE/ST=Berlin/L=Berlin/O=Qdrant/OU=Cloud/CN=qdrant" openssl genrsa -out test/integration/assets/tls.key 2048 From ab7684a100aad98eec3210f2a7478654f99d17c4 Mon Sep 17 00:00:00 2001 From: Bastian Hofmann Date: Wed, 20 Sep 2023 14:15:46 +0200 Subject: [PATCH 4/4] Try to fix connection issues --- charts/qdrant/templates/tests/test-db-interaction.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/charts/qdrant/templates/tests/test-db-interaction.yaml b/charts/qdrant/templates/tests/test-db-interaction.yaml index 2fc0da3..e1883d5 100644 --- a/charts/qdrant/templates/tests/test-db-interaction.yaml +++ b/charts/qdrant/templates/tests/test-db-interaction.yaml @@ -41,7 +41,8 @@ data: entrypoint.sh: | #!/bin/bash set -xe - echo 'connect-timeout = 5' > ~/.curlrc + echo 'connect-timeout = 5' > $HOME/.curlrc + echo 'retry = 5' >> $HOME/.curlrc if [ -d /mnt/secrets/certs ]; then cp /mnt/secrets/certs/ca.pem /usr/share/pki/trust/anchors/private-ca.pem