Merge pull request #195 from docker/fix/linux-keychain-tests

fix: linux keychain tests

Author: Benehiko

.github/workflows/keychain.yml

@@ -20,9 +20,11 @@ jobs:
       matrix:
         subtest:
           - fedora-43-gnome-keyring
-          - fedora-43-kdewallet
           - ubuntu-24-gnome-keyring
-          - ubuntu-24-kdewallet
+          # disabled kdewallet tests since it prompts for a password in a
+          # headless environment... need to still fix this
+          # - fedora-43-kdewallet
+          # - ubuntu-24-kdewallet
     steps:
       - name: Checkout
         uses: actions/checkout@v4
@@ -39,7 +41,7 @@ jobs:
           endpoint: "docker/secrets-engine"
           install: true
       - name: Test
-        run: DOCKER_TARGET=${{ matrix.subtest }} make keychain-linux-unit-tests
+        run: DOCKER_TARGET=${{ matrix.subtest }} make keychain-linux-ci-unit-tests
   tests-windows:
     permissions:
       id-token: write

Makefile

@@ -70,9 +70,12 @@ unit-tests:
 		exit $$err; \
 	fi
 
-keychain-linux-unit-tests:
+keychain-linux-ci-unit-tests:
 	@docker buildx build $(DOCKER_BUILD_ARGS) --target=$(DOCKER_TARGET) --file store/Dockerfile .
 
+keychain-linux-unit-tests:
+	docker buildx bake --set '*.args.GO_VERSION=${GO_VERSION}' --file store/docker-bake.hcl
+
 keychain-unit-tests:
 	CGO_ENABLED=1 go test -v $$(go list ./store/keychain/...)
 

store/Dockerfile

@@ -3,7 +3,7 @@ ARG GO_VERSION=latest
 FROM --platform=${BUILDPLATFORM} golang:${GO_VERSION}-alpine AS go-base
 
 FROM --platform=${BUILDPLATFORM} fedora:43 AS fedora43
-RUN dnf install -y gnome-keyring kf6-kwallet dbus-daemon
+RUN dnf install -y gnome-keyring kf6-kwallet dbus-daemon bash
 COPY --from=go-base /usr/local/go /usr/local/go
 ENV PATH="/usr/local/go/bin:${PATH}"
 RUN useradd -ms /bin/bash user
@@ -12,7 +12,7 @@ WORKDIR /app
 RUN --mount=type=bind,target=.
 
 FROM --platform=${BUILDPLATFORM} ubuntu:24.04 AS ubuntu24
-RUN apt update && apt install -y --no-install-recommends libglib2.0-bin dbus gnome-keyring kwalletmanager
+RUN apt update && apt install -y --no-install-recommends libglib2.0-bin dbus gnome-keyring kwalletmanager libqca-qt5-2-plugins bash
 COPY --from=go-base /usr/local/go /usr/local/go
 ENV PATH="/usr/local/go/bin:${PATH}"
 RUN useradd -ms /bin/bash user
@@ -27,8 +27,7 @@ WORKDIR /app
 RUN --mount=type=bind,target=. \
     --mount=type=cache,target=/go/pkg/mod \
     --mount=type=cache,target=/root/.cache/go-build \
-    /app/store/scripts/gnome-keyring \
-    go test -v ./store/keychain/...
+    bash -c "set -euxo pipefail; /app/store/scripts/gnome-keyring"
 
 FROM fedora43 AS fedora-43-kdewallet
 ENV CGO_ENABLED=0
@@ -37,8 +36,7 @@ WORKDIR /app
 RUN --mount=type=bind,target=. \
     --mount=type=cache,target=/go/pkg/mod \
     --mount=type=cache,target=/root/.cache/go-build \
-    /app/store/scripts/kdewallet \
-    go test -v ./store/keychain/...
+    bash -c "set -euxo pipefail; /app/store/scripts/kdewallet"
 
 FROM ubuntu24 AS ubuntu-24-gnome-keyring
 ENV CGO_ENABLED=0
@@ -47,8 +45,7 @@ WORKDIR /app
 RUN --mount=type=bind,target=. \
     --mount=type=cache,target=/go/pkg/mod \
     --mount=type=cache,target=/root/.cache/go-build \
-    /app/store/scripts/gnome-keyring \
-    go test -v ./store/keychain/...
+    bash -c "set -euxo pipefail; /app/store/scripts/gnome-keyring"
 
 FROM ubuntu24 AS ubuntu-24-kdewallet
 ENV CGO_ENABLED=0
@@ -57,5 +54,4 @@ WORKDIR /app
 RUN --mount=type=bind,target=. \
     --mount=type=cache,target=/go/pkg/mod \
     --mount=type=cache,target=/root/.cache/go-build \
-    /app/store/scripts/kdewallet \
-    go test -v ./store/keychain/...
+    bash -c "set -euxo pipefail; /app/store/scripts/kdewallet"

store/docker-bake.hcl

@@ -0,0 +1,50 @@
+group "default" {
+  targets = [
+    "fedora_43_gnome_keyring",
+    # disabling kdewallet tests for now, it doesn't work in headless mode
+    # it just prompts anyway...
+    # "fedora_43_kdewallet",
+    # "ubuntu_24_kdewallet",
+    "ubuntu_24_gnome_keyring"
+  ]
+}
+
+variable "GO_VERSION" {
+  default = "1.24"
+}
+
+target "fedora_43_gnome_keyring" {
+  dockerfile = "store/Dockerfile"
+  target     = "fedora-43-gnome-keyring"
+  context    = "."
+  args       = {
+    GO_VERSION = GO_VERSION
+  }
+}
+
+target "fedora_43_kdewallet" {
+  dockerfile = "store/Dockerfile"
+  target     = "fedora-43-kdewallet"
+  context    = "."
+  args       = {
+    GO_VERSION = GO_VERSION
+  }
+}
+
+target "ubuntu_24_kdewallet" {
+  dockerfile = "store/Dockerfile"
+  target     = "ubuntu-24-kdewallet"
+  context    = "."
+  args       = {
+    GO_VERSION = GO_VERSION
+  }
+}
+
+target "ubuntu_24_gnome_keyring" {
+  dockerfile = "store/Dockerfile"
+  target     = "ubuntu-24-gnome-keyring"
+  context    = "."
+  args       = {
+    GO_VERSION = GO_VERSION
+  }
+}

store/docs/test.md

@@ -12,9 +12,12 @@ enabled to support macOS.
 The cross distro linux tests can be run via:
 
 ` ` `console
-DOCKER_TARGET=ubuntu-24-gnome-keyring make keychain-linux-unit-tests
+make keychain-linux-unit-tests
 ` ` `
 
+This test uses the store/docker-bake.hcl file to parallelize the test across
+multiple environments.
+
 For Linux keychain we have four sub-tests:
 
 ```mermaid
@@ -30,5 +33,11 @@ flowchart TD
 - `fedora-43-gnome-keyring`
 - `fedora-43-kdewallet`
 
+To run a targeted test, re-use the CI make target:
+
+```console
+DOCKER_TARGET=ubuntu-24-gnome-keyring make keychain-linux-ci-unit-tests
+```
+
 This will use `buildkit` to target only the `ubuntu-24-gnome-keyring` label inside
 the `store/Dockerfile`.

store/keychain/keychain.go

@@ -94,9 +94,14 @@ func (k *keychainStore[T]) safelyCleanMetadata(attributes map[string]string) {
 	keys := slices.Collect(maps.Keys(attributes))
 	for _, key := range keys {
 		after, found := strings.CutPrefix(key, "x_")
+		// this preserves metadata set by the caller.
+		// we are restoring it by stripping the "x_" prefix.
 		if found {
 			attributes[after] = attributes[key]
-			delete(attributes, key)
 		}
+		// delete should always happen since we also want to remove attributes
+		// there were never prefixed. In this case we are just dropping them
+		// entirely. e.g. "xdg:scheme" set by the linux keychain internally.
+		delete(attributes, key)
 	}
 }

store/keychain/keychain_test.go

@@ -305,6 +305,22 @@ func TestSafelyCleanMetadata(t *testing.T) {
 		kc.safelyCleanMetadata(attributes)
 		assert.Empty(t, attributes)
 	})
+
+	t.Run("underlying store attributes are always removed", func(t *testing.T) {
+		attributes := map[string]string{
+			secretIDKey:     "username",
+			serviceGroupKey: "com.test.test",
+			serviceNameKey:  "test",
+			"x_something":   "something",
+			// xdg:scheme is added by the underlying linux keychain after we
+			// have prefixed key's with 'x_'
+			"xdg:scheme": "org.freedesktop.Secret.Generic",
+		}
+		kc.safelyCleanMetadata(attributes)
+		assert.EqualValues(t, map[string]string{
+			"something": "something",
+		}, attributes)
+	})
 }
 
 func TestInternalMetadata(t *testing.T) {

store/scripts/gnome-keyring

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 # 1. Check relevant binaries available
 # 2. Creates the necessary files for the keyring daemon
@@ -8,7 +8,7 @@
 # 6. Waits for the keyring to be active by polling dbus over `gdbus`
 # 7. Check if the registered `org.freedesktop.secrets` backend matches what we expect.
 
-set -eux pipefail
+set -euxo pipefail
 
 if test -z $(command -v gnome-keyring-daemon); then
     echo "gnome-keyring-daemon is not installed"
@@ -20,8 +20,13 @@ if test -z $(command -v dbus-daemon); then
     exit 1
 fi
 
-mkdir -p ~/.local/share/keyrings
-touch ~/.local/share/keyrings/login.keyring
+if ! test -d ~/.local/share/keyrings; then 
+    mkdir -p ~/.local/share/keyrings
+fi
+
+if ! test -e ~/.local/share/keyrings/login.keyring; then
+    touch ~/.local/share/keyrings/login.keyring
+fi
 
 # create fake passwordless 'login' keyring
 echo '[keyring]
@@ -79,7 +84,11 @@ fi
 
 exe=$(readlink -f /proc/$pid/exe)
 
-if [[ "$exe" != *gnome-keyring-daemon* ]]; then
-    echo "dbus org.freedesktop.secrets is not using gnome-keyring-daemon"
-    exit 1
+if [[ "$exe" == *gnome-keyring-daemon* ]]; then
+    echo "dbus org.freedesktop.secrets is using gnome-keyring-daemon"
+    go test -v -count=1 ./store/keychain/...
+    exit 0
 fi
+
+echo "dbus org.freedesktop.secrets is not using gnome-keyring-daemon. Using ${exe}"
+exit 1

store/scripts/kdewallet

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 # 1. Check relevant binaries available
 # 2. Creates the necessary files for the keyring daemon
@@ -8,7 +8,7 @@
 # 6. Waits for the keyring to be active by polling dbus over `gdbus`
 # 7. Check if the registered `org.freedesktop.secrets` backend matches what we expect.
 
-set -eux pipefail
+set -euxo pipefail
 
 kwalletd=$(command -v kwalletd5 || command -v kwalletd6)
 
@@ -22,19 +22,33 @@ if test -z $(command -v dbus-daemon); then
     exit 1
 fi
 
-mkdir -p ~/.local/share/dbus-1/services
-touch ~/.local/share/dbus-1/services/org.freedesktop.secrets.service
+if ! test -d ~/.local/share/dbus-1/services; then
+    mkdir -p ~/.local/share/dbus-1/services
+fi
+
+if ! test -e ~/.local/share/dbus-1/services/org.freedesktop.secrets.service; then
+    touch ~/.local/share/dbus-1/services/org.freedesktop.secrets.service
+fi
 
-echo "[D-BUS Service]
+echo '[D-BUS Service]
 Name=org.freedesktop.secrets
-Exec=${kwalletd}" > ~/.local/share/dbus-1/services/org.freedesktop.secrets.service
+Exec=${kwalletd}' > ~/.local/share/dbus-1/services/org.freedesktop.secrets.service
 
 
-mkdir -p ~/.local/share/kwalletd
-mkdir ~/.config
+if ! test -d ~/.local/share/kwalletd; then
+    mkdir -p ~/.local/share/kwalletd
+fi
 
-echo -e "[Wallet]\nFirst Use=false\nDefault Wallet=kwallet" > ~/.config/kwalletrc
-echo -e "[Wallet]\nVersion=1" > ~/.local/share/kwalletd/kwallet.kwl
+if ! test -d ~/.config; then
+    mkdir ~/.config
+fi
+
+echo '[Wallet]
+First Use=false
+Default Wallet=kwallet' > ~/.config/kwalletrc
+
+echo '[Wallet]
+Version=1' > ~/.local/share/kwalletd/kwallet.kwl
 
 export QT_QPA_PLATFORM=minimal
 
@@ -86,7 +100,13 @@ fi
 
 exe=$(readlink -f /proc/$pid/exe)
 
-if [[ "$exe" != "/usr/bin/ksecretd" ]]; then
-    echo "dbus org.freedesktop.secrets is not using ${kwalletd}"
-    exit 1
+# ksecretd which is part of kwalletd6
+# kwalletd5 is part of kwalletd5
+if [[ "$exe" == "/usr/bin/ksecretd" || "$exe" == "/usr/bin/kwalletd5" ]]; then
+    echo "dbus org.freedesktop.secrets is using ${exe}"
+    go test -v -count=1 ./store/keychain/...
+    exit 0
 fi
+
+echo "dbus org.freedesktop.secrets is not using kwalletd5 or kwalletd6. Using: ${exe}"
+exit 1