Skip to content

Commit

Permalink
Merge pull request #5570 from thaJeztah/credentials_coverage
Browse files Browse the repository at this point in the history 
cli/config/credentials: add test for save being idempotent
  • Loading branch information
@thaJeztah
thaJeztah authored Oct 22, 2024
2 parents c34b80b + 3c78069 commit 61baf2a
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 27 deletions.
3 changes: 2 additions & 1 deletion cli/config/credentials/file_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ func NewFileStore(file store) Store {
return &fileStore{file: file}
}

// Erase removes the given credentials from the file store.
// Erase removes the given credentials from the file store.This function is
// idempotent and does not update the file if credentials did not change.
func (c *fileStore) Erase(serverAddress string) error {
if _, exists := c.file.GetAuthConfigs()[serverAddress]; !exists {
// nothing to do; no credentials found for the given serverAddress
Expand Down
93 changes: 83 additions & 10 deletions cli/config/credentials/file_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ import (

type fakeStore struct {
configs map[string]types.AuthConfig
saveFn func(*fakeStore) error
}

func (f *fakeStore) Save() error {
if f.saveFn != nil {
// Pass a reference to the fakeStore itself in case saveFn
// wants to access it.
return f.saveFn(f)
}
return nil
}

Expand All @@ -21,15 +27,82 @@ func (f *fakeStore) GetAuthConfigs() map[string]types.AuthConfig {
}

func (f *fakeStore) GetFilename() string {
return "/tmp/docker-fakestore"
return "no-config.json"
}

func newStore(auths map[string]types.AuthConfig) store {
return &fakeStore{configs: auths}
// TestFileStoreIdempotent verifies that the config-file isn't updated
// if nothing changed.
func TestFileStoreIdempotent(t *testing.T) {
var saveCount, expectedSaveCount int

s := NewFileStore(&fakeStore{
configs: map[string]types.AuthConfig{},
saveFn: func(*fakeStore) error {
saveCount++
return nil
},
})
authOne := types.AuthConfig{
Auth: "super_secret_token",
Email: "foo@example.com",
ServerAddress: "https://example.com",
}
authTwo := types.AuthConfig{
Auth: "also_super_secret_token",
Email: "bar@example.com",
ServerAddress: "https://other.example.com",
}

expectedSaveCount = 1
t.Run("store new credentials", func(t *testing.T) {
assert.NilError(t, s.Store(authOne))
retrievedAuth, err := s.Get(authOne.ServerAddress)
assert.NilError(t, err)
assert.Check(t, is.Equal(retrievedAuth, authOne))
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
})
t.Run("store same credentials is a no-op", func(t *testing.T) {
assert.NilError(t, s.Store(authOne))
retrievedAuth, err := s.Get(authOne.ServerAddress)
assert.NilError(t, err)
assert.Check(t, is.Equal(retrievedAuth, authOne))
assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed")
})
t.Run("store other credentials", func(t *testing.T) {
expectedSaveCount++
assert.NilError(t, s.Store(authTwo))
retrievedAuth, err := s.Get(authTwo.ServerAddress)
assert.NilError(t, err)
assert.Check(t, is.Equal(retrievedAuth, authTwo))
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
})
t.Run("erase credentials", func(t *testing.T) {
expectedSaveCount++
assert.NilError(t, s.Erase(authOne.ServerAddress))
retrievedAuth, err := s.Get(authOne.ServerAddress)
assert.NilError(t, err)
assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
})
t.Run("erase non-existing credentials is a no-op", func(t *testing.T) {
assert.NilError(t, s.Erase(authOne.ServerAddress))
retrievedAuth, err := s.Get(authOne.ServerAddress)
assert.NilError(t, err)
assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed")
})
t.Run("erase other credentials", func(t *testing.T) {
expectedSaveCount++
assert.NilError(t, s.Erase(authTwo.ServerAddress))
retrievedAuth, err := s.Get(authTwo.ServerAddress)
assert.NilError(t, err)
assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
})
}

func TestFileStoreAddCredentials(t *testing.T) {
f := newStore(make(map[string]types.AuthConfig))
f := &fakeStore{configs: map[string]types.AuthConfig{}}

s := NewFileStore(f)
auth := types.AuthConfig{
Expand All @@ -47,13 +120,13 @@ func TestFileStoreAddCredentials(t *testing.T) {
}

func TestFileStoreGet(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
"https://example.com": {
Auth: "super_secret_token",
Email: "foo@example.com",
ServerAddress: "https://example.com",
},
})
}}

s := NewFileStore(f)
a, err := s.Get("https://example.com")
Expand All @@ -71,7 +144,7 @@ func TestFileStoreGet(t *testing.T) {
func TestFileStoreGetAll(t *testing.T) {
s1 := "https://example.com"
s2 := "https://example2.example.com"
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
s1: {
Auth: "super_secret_token",
Email: "foo@example.com",
Expand All @@ -82,7 +155,7 @@ func TestFileStoreGetAll(t *testing.T) {
Email: "foo@example2.com",
ServerAddress: "https://example2.example.com",
},
})
}}

s := NewFileStore(f)
as, err := s.GetAll()
Expand All @@ -107,13 +180,13 @@ func TestFileStoreGetAll(t *testing.T) {
}

func TestFileStoreErase(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
"https://example.com": {
Auth: "super_secret_token",
Email: "foo@example.com",
ServerAddress: "https://example.com",
},
})
}}

s := NewFileStore(f)
err := s.Erase("https://example.com")
Expand Down
32 changes: 16 additions & 16 deletions cli/config/credentials/native_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func mockCommandFn(args ...string) client.Program {
}

func TestNativeStoreAddCredentials(t *testing.T) {
f := newStore(make(map[string]types.AuthConfig))
f := &fakeStore{configs: map[string]types.AuthConfig{}}
s := &nativeStore{
programFunc: mockCommandFn,
fileStore: NewFileStore(f),
Expand All @@ -116,7 +116,7 @@ func TestNativeStoreAddCredentials(t *testing.T) {
}

func TestNativeStoreAddInvalidCredentials(t *testing.T) {
f := newStore(make(map[string]types.AuthConfig))
f := &fakeStore{configs: map[string]types.AuthConfig{}}
s := &nativeStore{
programFunc: mockCommandFn,
fileStore: NewFileStore(f),
Expand All @@ -132,11 +132,11 @@ func TestNativeStoreAddInvalidCredentials(t *testing.T) {
}

func TestNativeStoreGet(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress: {
Email: "foo@example.com",
},
})
}}
s := &nativeStore{
programFunc: mockCommandFn,
fileStore: NewFileStore(f),
Expand All @@ -154,11 +154,11 @@ func TestNativeStoreGet(t *testing.T) {
}

func TestNativeStoreGetIdentityToken(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress2: {
Email: "foo@example2.com",
},
})
}}

s := &nativeStore{
programFunc: mockCommandFn,
Expand All @@ -176,11 +176,11 @@ func TestNativeStoreGetIdentityToken(t *testing.T) {
}

func TestNativeStoreGetAll(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress: {
Email: "foo@example.com",
},
})
}}

s := &nativeStore{
programFunc: mockCommandFn,
Expand Down Expand Up @@ -217,11 +217,11 @@ func TestNativeStoreGetAll(t *testing.T) {
}

func TestNativeStoreGetMissingCredentials(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress: {
Email: "foo@example.com",
},
})
}}

s := &nativeStore{
programFunc: mockCommandFn,
Expand All @@ -232,11 +232,11 @@ func TestNativeStoreGetMissingCredentials(t *testing.T) {
}

func TestNativeStoreGetInvalidAddress(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress: {
Email: "foo@example.com",
},
})
}}

s := &nativeStore{
programFunc: mockCommandFn,
Expand All @@ -247,11 +247,11 @@ func TestNativeStoreGetInvalidAddress(t *testing.T) {
}

func TestNativeStoreErase(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress: {
Email: "foo@example.com",
},
})
}}

s := &nativeStore{
programFunc: mockCommandFn,
Expand All @@ -263,11 +263,11 @@ func TestNativeStoreErase(t *testing.T) {
}

func TestNativeStoreEraseInvalidAddress(t *testing.T) {
f := newStore(map[string]types.AuthConfig{
f := &fakeStore{configs: map[string]types.AuthConfig{
validServerAddress: {
Email: "foo@example.com",
},
})
}}

s := &nativeStore{
programFunc: mockCommandFn,
Expand Down

0 comments on commit 61baf2a

Please sign in to comment.