From 11f28e43848da31fc9ef1066f60e8f033c4abaf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 26 Sep 2024 14:39:27 +0200 Subject: [PATCH 1/3] ci: Increase the tests timeout --- .github/workflows/qa.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/qa.yaml b/.github/workflows/qa.yaml index 9893549f9..1522c91b4 100644 --- a/.github/workflows/qa.yaml +++ b/.github/workflows/qa.yaml @@ -9,6 +9,7 @@ on: env: DEBIAN_FRONTEND: noninteractive + GO_TESTS_TIMEOUT: 20m apt_deps: >- libpam-dev libglib2.0-dev @@ -192,7 +193,7 @@ jobs: # Overriding the default coverage directory is not an exported flag of go test (yet), so # we need to override it using the test.gocoverdir flag instead. #TODO: Update when https://go-review.googlesource.com/c/go/+/456595 is merged. - go test -cover -covermode=set ./... -coverpkg=./... -shuffle=on -args -test.gocoverdir="${raw_cov_dir}" + go test -timeout ${GO_TESTS_TIMEOUT} -cover -covermode=set ./... -coverpkg=./... -shuffle=on -args -test.gocoverdir="${raw_cov_dir}" # Convert the raw coverage data into textfmt so we can merge the Rust one into it go tool covdata textfmt -i="${raw_cov_dir}" -o="${cov_dir}/coverage.out" @@ -208,7 +209,7 @@ jobs: - name: Run tests (with race detector) run: | - go test -race ./... + go test -timeout ${GO_TESTS_TIMEOUT} -race ./... - name: Run PAM tests (with Address Sanitizer) env: @@ -218,10 +219,10 @@ jobs: G_DEBUG: "fatal-criticals" run: | # Use `-dwarflocationlists` to give ASAN a better time to unwind the stack trace - go test -C ./pam/internal -asan -gcflags="-dwarflocationlists=true" ./... + go test -C ./pam/internal -asan -gcflags="-dwarflocationlists=true" -timeout ${GO_TESTS_TIMEOUT} ./... pushd ./pam/integration-tests - go test -asan -gcflags="-dwarflocationlists=true" -c + go test -asan -gcflags="-dwarflocationlists=true" -timeout ${GO_TESTS_TIMEOUT} -c # FIXME: Suppression may be removed with newer libpam, as the one we ship in ubuntu as some leaks env LSAN_OPTIONS=suppressions=$(pwd)/lsan.supp \ ./integration-tests.test \ From e7aaa584ce2d4654ca7cb6c085e3d4016a9a5789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 26 Sep 2024 15:10:57 +0200 Subject: [PATCH 2/3] pam/integration-tests: Save generated services file artifacts --- pam/integration-tests/exec_test.go | 3 ++- pam/integration-tests/gdm_test.go | 12 +++++++----- pam/integration-tests/helpers_test.go | 5 +++++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pam/integration-tests/exec_test.go b/pam/integration-tests/exec_test.go index b9c9f52b5..003c3d807 100644 --- a/pam/integration-tests/exec_test.go +++ b/pam/integration-tests/exec_test.go @@ -843,7 +843,7 @@ func getModuleArgs(t *testing.T, clientPath string, args []string) []string { logFile := os.Stderr.Name() if !testutils.IsVerbose() { logFile = prepareFileLogging(t, "exec-module.log") - saveArtifactsForDebug(t, []string{logFile}) + saveArtifactsForDebugOnCleanup(t, []string{logFile}) } moduleArgs = append(moduleArgs, "--exec-log", logFile) @@ -891,6 +891,7 @@ func preparePamTransactionForServiceFile(t *testing.T, serviceFile string, user } else { tx, err = pam.StartConfDir(filepath.Base(serviceFile), user, nil, filepath.Dir(serviceFile)) } + saveArtifactsForDebugOnCleanup(t, []string{serviceFile}) require.NoError(t, err, "PAM: Error to initialize module") require.NotNil(t, tx, "PAM: Transaction is not set") t.Cleanup(func() { require.NoError(t, tx.End(), "PAM: can't end transaction") }) diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 88567e69c..163a1e515 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -673,11 +673,11 @@ func TestGdmModule(t *testing.T) { moduleArgs := []string{"socket=" + socketPath} gdmLog := prepareFileLogging(t, "authd-pam-gdm.log") - t.Cleanup(func() { saveArtifactsForDebug(t, []string{gdmLog}) }) + saveArtifactsForDebugOnCleanup(t, []string{gdmLog}) moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog) - serviceFile := createServiceFile(t, "gdm-authd", libPath, - moduleArgs) + serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) + saveArtifactsForDebugOnCleanup(t, []string{serviceFile}) pamUser := "user-integration-" + strings.ReplaceAll(filepath.Base(t.Name()), "_", "-") if tc.pamUser != nil { @@ -788,10 +788,11 @@ func TestGdmModuleAuthenticateWithoutGdmExtension(t *testing.T) { moduleArgs = append(moduleArgs, "socket="+socketPath) gdmLog := prepareFileLogging(t, "authd-pam-gdm.log") - t.Cleanup(func() { saveArtifactsForDebug(t, []string{gdmLog}) }) + saveArtifactsForDebugOnCleanup(t, []string{gdmLog}) moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog) serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) + saveArtifactsForDebugOnCleanup(t, []string{serviceFile}) pamUser := "user-integration-auth-no-gdm-extension" gh := newGdmTestModuleHandler(t, serviceFile, pamUser) t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") }) @@ -830,10 +831,11 @@ func TestGdmModuleAcctMgmtWithoutGdmExtension(t *testing.T) { moduleArgs = append(moduleArgs, "socket="+socketPath) gdmLog := prepareFileLogging(t, "authd-pam-gdm.log") - t.Cleanup(func() { saveArtifactsForDebug(t, []string{gdmLog}) }) + saveArtifactsForDebugOnCleanup(t, []string{gdmLog}) moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog) serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) + saveArtifactsForDebugOnCleanup(t, []string{serviceFile}) pamUser := "user-integration-acctmgmt-no-gdm-extension" gh := newGdmTestModuleHandler(t, serviceFile, pamUser) t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") }) diff --git a/pam/integration-tests/helpers_test.go b/pam/integration-tests/helpers_test.go index 0ba18d277..9b4d7eb60 100644 --- a/pam/integration-tests/helpers_test.go +++ b/pam/integration-tests/helpers_test.go @@ -51,3 +51,8 @@ func requirePreviousBrokerForUser(t *testing.T, socketPath string, brokerName st } require.Equal(t, prevBroker.PreviousBroker, prevBrokerID) } + +func saveArtifactsForDebugOnCleanup(t *testing.T, artifacts []string) { + t.Helper() + t.Cleanup(func() { saveArtifactsForDebug(t, artifacts) }) +} From 1f50dd157a954339e40cdaf298a76d11aaad7816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 26 Sep 2024 16:39:38 +0200 Subject: [PATCH 3/3] pam/integration-tests/exec: Use a file to pass very long arguments --- pam/integration-tests/cmd/exec-client/client.go | 11 +++++++++++ pam/integration-tests/exec_test.go | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/pam/integration-tests/cmd/exec-client/client.go b/pam/integration-tests/cmd/exec-client/client.go index 050ebc892..a39835305 100644 --- a/pam/integration-tests/cmd/exec-client/client.go +++ b/pam/integration-tests/cmd/exec-client/client.go @@ -11,6 +11,7 @@ import ( "os" "reflect" "regexp" + "strings" "github.com/godbus/dbus/v5" "github.com/msteinert/pam/v2" @@ -23,6 +24,7 @@ var ( pamFlags = flag.Int64("flags", 0, "pam flags") serverAddress = flag.String("server-address", "", "the dbus connection address to use to communicate with module") logFile = flag.String("client-log", "", "the file where to save logs") + argsFile = flag.String("client-args-file", "", "the file where arguments are saved") ) func main() { @@ -78,6 +80,15 @@ func mainFunc() error { log.SetOutput(f) } + if argsFile != nil && *argsFile != "" { + log.Debug(context.TODO(), "Reading arguments from %s", *argsFile) + ca, err := os.ReadFile(*argsFile) + if err != nil { + return err + } + args = append(args, strings.Split(string(ca), "\t")...) + } + actionFlags := pam.Flags(0) if pamFlags != nil { actionFlags = pam.Flags(*pamFlags) diff --git a/pam/integration-tests/exec_test.go b/pam/integration-tests/exec_test.go index 003c3d807..19aeaef1f 100644 --- a/pam/integration-tests/exec_test.go +++ b/pam/integration-tests/exec_test.go @@ -850,6 +850,17 @@ func getModuleArgs(t *testing.T, clientPath string, args []string) []string { if clientPath != "" { moduleArgs = append(moduleArgs, "--", clientPath) moduleArgs = append(moduleArgs, "-client-log", logFile) + + if len(strings.Join(append(moduleArgs, args...), " ")) > 768 { + // FIXME: If the number of arguments is too big, we may break old PAM. + // This is not required anymore when we can use libpam 1.6.0 in CI: + // https://github.com/linux-pam/linux-pam/pull/658 + clientArgsPath := filepath.Join(t.TempDir(), "client-args-file") + require.NoError(t, os.WriteFile(clientArgsPath, []byte(strings.Join(args, "\t")), 0600), + "Setup: Creation of client args file failed") + saveArtifactsForDebugOnCleanup(t, []string{clientArgsPath}) + return append(moduleArgs, "-client-args-file", clientArgsPath) + } } return append(moduleArgs, args...) }