Skip to content

Commit 7a3fcfa

Browse files
committed
tests: backward-compatibility with v0.8.2
We recently introduced bad-looking regressions in v0.9.0. These were not caught by our test suite. This commit adds a new `backward_compatibility` suite to make sure it doesn't happen again. Signed-off-by: Albin Kerouanton <albinker@gmail.com>
1 parent 833d2c3 commit 7a3fcfa

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+7699
-1
lines changed

.github/workflows/build.yml

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,15 @@ jobs:
5050
- macOS-14
5151
- macOS-13
5252
- windows-2022
53+
env:
54+
TEST_BC_BASE_VERSION: 0.8.2
5355
steps:
5456
-
5557
name: Checkout
5658
uses: actions/checkout@v4
59+
with:
60+
# fetch-tags is required for backward compatibility tests
61+
fetch-tags: true
5762
-
5863
name: Set up Go
5964
uses: actions/setup-go@v5
@@ -106,6 +111,72 @@ jobs:
106111
run: |
107112
make test COVERAGEDIR=${{ env.DESTDIR }}
108113
shell: bash
114+
-
115+
name: Test backward compatibility (pass)
116+
if: always() && (startsWith(matrix.os, 'ubuntu-') || startsWith(matrix.os, 'macOS-'))
117+
run: |
118+
export TMPDIR=${TMPDIR:-/tmp}
119+
export helper=pass
120+
121+
wget -O ${TMPDIR}/docker-credential-${helper} https://github.com/docker/docker-credential-helpers/releases/download/v${TEST_BC_BASE_VERSION}/docker-credential-${helper}-v${TEST_BC_BASE_VERSION}.$(go env GOOS)-$(go env GOARCH)
122+
chmod +x ${TMPDIR}/docker-credential-${helper}
123+
124+
make build-${helper}
125+
126+
export TEST_BC_PREVIOUS_COMMAND=${TMPDIR}/docker-credential-${helper}
127+
export TEST_BC_COMMAND=$(realpath ${DESTDIR}/docker-credential-${helper})
128+
129+
make test-backward-compatibility
130+
shell: bash
131+
-
132+
name: Test backward compatibility (secretservice)
133+
if: always() && startsWith(matrix.os, 'ubuntu-')
134+
run: |
135+
export TMPDIR=${TMPDIR:-/tmp}
136+
export helper=secretservice
137+
138+
wget -O ${TMPDIR}/docker-credential-${helper} https://github.com/docker/docker-credential-helpers/releases/download/v${TEST_BC_BASE_VERSION}/docker-credential-${helper}-v${TEST_BC_BASE_VERSION}.$(go env GOOS)-$(go env GOARCH)
139+
chmod +x ${TMPDIR}/docker-credential-${helper}
140+
141+
make build-${helper}
142+
143+
export TEST_BC_PREVIOUS_COMMAND=${TMPDIR}/docker-credential-${helper}
144+
export TEST_BC_COMMAND=$(realpath ${DESTDIR}/docker-credential-${helper})
145+
146+
make test-backward-compatibility
147+
shell: bash
148+
-
149+
name: Test backward compatibility (osxkeychain)
150+
if: always() && startsWith(matrix.os, 'macOS-')
151+
run: |
152+
export TMPDIR=${TMPDIR:-/tmp}
153+
export helper=osxkeychain
154+
155+
wget -O ${TMPDIR}/docker-credential-${helper} https://github.com/docker/docker-credential-helpers/releases/download/v${TEST_BC_BASE_VERSION}/docker-credential-${helper}-v${TEST_BC_BASE_VERSION}.$(go env GOOS)-$(go env GOARCH)
156+
chmod +x ${TMPDIR}/docker-credential-${helper}
157+
158+
make build-${helper}
159+
160+
export TEST_BC_PREVIOUS_COMMAND=${TMPDIR}/docker-credential-${helper}
161+
export TEST_BC_COMMAND=$(realpath ${DESTDIR}/docker-credential-${helper})
162+
163+
security unlock-keychain
164+
165+
make test-backward-compatibility
166+
shell: bash
167+
-
168+
name: Test backward compatibility (wincred)
169+
if: always() && startsWith(matrix.os, 'windows-')
170+
run: |
171+
export helper=wincred
172+
173+
Invoke-WebRequest -OutFile ${TMPDIR}/docker-credential-${helper}.exe https://github.com/docker/docker-credential-helpers/releases/download/v${TEST_BC_BASE_VERSION}/docker-credential-${helper}-v${TEST_BC_BASE_VERSION}.$(go env GOOS)-$(go env GOARCH).exe
174+
175+
export TEST_BC_PREVIOUS_COMMAND=${TMPDIR}/docker-credential-${helper}.exe
176+
export TEST_BC_COMMAND=$(Resolve-Path ${DESTDIR}/docker-credential-${helper}.exe)
177+
178+
make test-backward-compatibility
179+
shell: bash
109180
-
110181
name: Upload coverage
111182
uses: codecov/codecov-action@v5

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ test:
6262
go test -short -v -coverprofile=$(COVERAGEDIR)/coverage.txt -covermode=atomic ./...
6363
go tool cover -func=$(COVERAGEDIR)/coverage.txt
6464

65+
.PHONY: test-backward-compatibility
66+
test-backward-compatibility:
67+
go test -short -v ./tests/backward_compatibility
68+
6569
.PHONY: lint
6670
lint:
6771
$(BUILDX_CMD) bake lint

go.mod

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ retract v0.9.0 // osxkeychain: a regression caused backward-incompatibility with
77
require (
88
github.com/danieljoos/wincred v1.2.2
99
github.com/keybase/go-keychain v0.0.1
10+
gotest.tools/v3 v3.5.2
1011
)
1112

12-
require golang.org/x/sys v0.20.0 // indirect
13+
require (
14+
github.com/google/go-cmp v0.5.9 // indirect
15+
golang.org/x/sys v0.20.0 // indirect
16+
)

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ
22
github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8=
33
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
44
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
5+
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
6+
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
57
github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU=
68
github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k=
79
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -14,3 +16,5 @@ golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
1416
golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
1517
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
1618
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
19+
gotest.tools/v3 v3.5.2 h1:7koQfIKdy+I8UTetycgUqXWSDwpgv193Ka+qRsmBY8Q=
20+
gotest.tools/v3 v3.5.2/go.mod h1:LtdLGcnqToBH83WByAAi/wiwSFCArdFIUV/xxN4pcjA=
Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
package client
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"strings"
7+
"testing"
8+
9+
"github.com/docker/docker-credential-helpers/client"
10+
"github.com/docker/docker-credential-helpers/credentials"
11+
"gotest.tools/v3/assert"
12+
is "gotest.tools/v3/assert/cmp"
13+
"gotest.tools/v3/skip"
14+
)
15+
16+
func newShellCommands(t *testing.T) (string, client.ProgramFunc, client.ProgramFunc) {
17+
cmd := os.Getenv("TEST_BC_COMMAND")
18+
previousCmd := os.Getenv("TEST_BC_PREVIOUS_COMMAND")
19+
skip.If(t, cmd == "", "TEST_BC_COMMAND not set, skipping backward compatibility test")
20+
skip.If(t, previousCmd == "", "TEST_BC_PREVIOUS_COMMAND not set, skipping backward compatibility test")
21+
22+
oldP := client.NewShellProgramFunc(previousCmd)
23+
newP := client.NewShellProgramFunc(cmd)
24+
25+
oldName, oldErr := version(oldP)
26+
newName, newErr := version(newP)
27+
28+
assert.NilError(t, oldErr)
29+
assert.NilError(t, newErr)
30+
assert.Equal(t, oldName, newName)
31+
32+
return oldName, oldP, newP
33+
}
34+
35+
func version(program client.ProgramFunc) (string, error) {
36+
cmd := program(credentials.ActionVersion)
37+
out, err := cmd.Output()
38+
t := strings.TrimSpace(string(out))
39+
if err != nil {
40+
return "", fmt.Errorf("error getting version - err: %v, out: `%s`", err, t)
41+
}
42+
parts := strings.Split(t, " ")
43+
return parts[0], nil
44+
}
45+
46+
// TestGetFromNewVersion tests that a new version of the helper can read
47+
// credentials stored by an older version of the helper.
48+
func TestGetFromNewVersion(t *testing.T) {
49+
helperName, oldP, newP := newShellCommands(t)
50+
51+
skip.If(t, helperName == "docker-credential-secretservice", "test requires gnome-keyring but CI doesn't have it")
52+
53+
testcases := []struct {
54+
name string
55+
serverURL string
56+
}{
57+
{
58+
name: "with no path",
59+
serverURL: "https://registry.example.com/",
60+
},
61+
{
62+
name: "with path",
63+
serverURL: "https://registry.example.com/v1/",
64+
},
65+
{
66+
name: "with port",
67+
serverURL: "https://registry.example.com:5000/",
68+
},
69+
{
70+
name: "with path and port",
71+
serverURL: "https://registry.example.com:5000/v1/",
72+
},
73+
}
74+
75+
for _, tc := range testcases {
76+
t.Run(tc.name, func(t *testing.T) {
77+
err := client.Store(oldP, &credentials.Credentials{
78+
ServerURL: tc.serverURL,
79+
Username: "testuser",
80+
Secret: "testsecret",
81+
})
82+
defer client.Erase(newP, tc.serverURL)
83+
assert.NilError(t, err)
84+
85+
creds, err := client.Get(newP, tc.serverURL)
86+
assert.NilError(t, err)
87+
88+
assert.Check(t, is.Equal(creds.Username, "testuser"))
89+
assert.Check(t, is.Equal(creds.Secret, "testsecret"))
90+
})
91+
}
92+
}
93+
94+
// TestListFromNewVersion tests that a new version of the helper can list
95+
// credentials stored by an older version of the helper.
96+
func TestListFromNewVersion(t *testing.T) {
97+
helperName, oldP, newP := newShellCommands(t)
98+
99+
skip.If(t, helperName == "docker-credential-secretservice", "test requires gnome-keyring but CI doesn't have it")
100+
101+
priorCreds, err := client.List(newP)
102+
assert.NilError(t, err)
103+
// Capture the number of credentials before we store any new ones. When
104+
// tests are run on a developer's machine, they may have credentials
105+
// already stored in the helper, and we don't want to count those
106+
// against the helper's ability to list credentials.
107+
priorList := len(priorCreds)
108+
109+
testCreds := []struct {
110+
credentials.Credentials
111+
skip bool
112+
skipReason string
113+
}{
114+
{
115+
Credentials: credentials.Credentials{
116+
ServerURL: "https://registry.example.com/",
117+
Username: "testuser1",
118+
Secret: "testsecret1",
119+
},
120+
},
121+
{
122+
Credentials: credentials.Credentials{
123+
ServerURL: "https://registry.example.com/v1/",
124+
Username: "testuser1",
125+
Secret: "testsecret1",
126+
},
127+
},
128+
{
129+
Credentials: credentials.Credentials{
130+
ServerURL: "https://registry.example.com/v2/",
131+
Username: "testuser2",
132+
Secret: "testsecret2",
133+
},
134+
},
135+
{
136+
Credentials: credentials.Credentials{
137+
ServerURL: "https://registry.example.com:5000",
138+
Username: "testuser1",
139+
Secret: "testsecret1",
140+
},
141+
},
142+
{
143+
Credentials: credentials.Credentials{
144+
ServerURL: "https://registry.example.com:5000/v1/",
145+
Username: "testuser1",
146+
Secret: "testsecret1",
147+
},
148+
skip: helperName == "docker-credential-osxkeychain",
149+
skipReason: "docker-credential-osxkeychain returns malformed URI when a port is specified",
150+
},
151+
}
152+
for i := range testCreds {
153+
creds := testCreds[i]
154+
155+
err = client.Store(oldP, &creds.Credentials)
156+
defer client.Erase(oldP, creds.ServerURL)
157+
assert.NilError(t, err)
158+
}
159+
160+
oldCreds, err := client.List(oldP)
161+
assert.NilError(t, err)
162+
t.Logf("credentials found by old version: %+v", oldCreds)
163+
assert.Check(t, is.Equal(len(oldCreds), priorList+len(testCreds)))
164+
165+
newCreds, err := client.List(newP)
166+
assert.NilError(t, err)
167+
t.Logf("credentials found by new version: %+v", newCreds)
168+
assert.Check(t, is.Equal(len(newCreds), priorList+len(testCreds)))
169+
170+
for _, tc := range testCreds {
171+
t.Run(tc.ServerURL, func(t *testing.T) {
172+
skip.If(t, tc.skip, tc.skipReason)
173+
if _, ok := oldCreds[tc.ServerURL]; !ok {
174+
t.Errorf("ServerURL=%q: no credentials stored in oldCreds, want one", tc.ServerURL)
175+
return
176+
}
177+
if _, ok := newCreds[tc.ServerURL]; !ok {
178+
t.Errorf("ServerURL=%q: no credentials stored in newCreds, want one", tc.ServerURL)
179+
return
180+
}
181+
assert.Check(t, is.Equal(newCreds[tc.ServerURL], tc.Username), "ServerURL=%q: got username=%q, want username %q", tc.ServerURL, newCreds[tc.ServerURL], tc.Username)
182+
assert.Check(t, is.Equal(newCreds[tc.ServerURL], oldCreds[tc.ServerURL]), "ServerURL=%q: got username=%q, want username %q", tc.ServerURL, newCreds[tc.ServerURL], tc.Username)
183+
})
184+
}
185+
}
186+
187+
// TestReplaceFromNewVersion tests that a new version of the helper can
188+
// credentials stored by an older version of the helper, and that both the old
189+
// and new helpers can read the credentials.
190+
func TestReplaceFromNewVersion(t *testing.T) {
191+
helperName, oldP, newP := newShellCommands(t)
192+
193+
skip.If(t, helperName == "docker-credential-secretservice", "test requires gnome-keyring but CI doesn't have it")
194+
195+
const serverURL = "https://registry.example.com/"
196+
err := client.Store(oldP, &credentials.Credentials{
197+
ServerURL: serverURL,
198+
Username: "testuser1",
199+
Secret: "testsecret1",
200+
})
201+
// defer client.Erase(oldP, serverURL)
202+
assert.NilError(t, err)
203+
204+
err = client.Store(newP, &credentials.Credentials{
205+
ServerURL: serverURL,
206+
Username: "testuser2",
207+
Secret: "testsecret2",
208+
})
209+
// defer client.Erase(newP, serverURL)
210+
assert.NilError(t, err)
211+
212+
creds, err := client.Get(oldP, serverURL)
213+
assert.NilError(t, err)
214+
assert.Check(t, is.Equal(creds.Username, "testuser2"))
215+
assert.Check(t, is.Equal(creds.Secret, "testsecret2"))
216+
217+
creds, err = client.Get(newP, serverURL)
218+
assert.NilError(t, err)
219+
assert.Check(t, is.Equal(creds.Username, "testuser2"))
220+
assert.Check(t, is.Equal(creds.Secret, "testsecret2"))
221+
}
222+
223+
// TestEraseFromNewVersion tests that a new version of the helper can erase
224+
// credentials stored by an older version of the helper.
225+
func TestEraseFromNewVersion(t *testing.T) {
226+
helperName, oldP, newP := newShellCommands(t)
227+
228+
skip.If(t, helperName == "docker-credential-secretservice", "test requires gnome-keyring but CI doesn't have it")
229+
230+
const serverURL = "https://registry.example.com/"
231+
err := client.Store(oldP, &credentials.Credentials{
232+
ServerURL: serverURL,
233+
Username: "testuser1",
234+
Secret: "testsecret1",
235+
})
236+
defer client.Erase(oldP, serverURL)
237+
assert.NilError(t, err)
238+
239+
err = client.Erase(newP, serverURL)
240+
assert.NilError(t, err)
241+
242+
creds, err := client.Get(oldP, serverURL)
243+
assert.ErrorIs(t, err, credentials.NewErrCredentialsNotFound(), "expected err %q; got err = nil, creds = %+v", credentials.NewErrCredentialsNotFound(), creds)
244+
assert.Check(t, is.Nil(creds))
245+
246+
creds, err = client.Get(newP, serverURL)
247+
assert.ErrorIs(t, err, credentials.NewErrCredentialsNotFound(), "expected err %q; got err = nil, creds = %+v", credentials.NewErrCredentialsNotFound(), creds)
248+
assert.Check(t, is.Nil(creds))
249+
}

0 commit comments

Comments
 (0)