Skip to content

Commit e5e8d00

Browse files
Ashvin Sharmasaracen
authored and
GitLab
committed
Merge branch 'ajwalker/refactor-cache' into 'main'
Refactor cache functionality to remove common package dependency Closes #39238 See merge request https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/6366 Merged-by: Ashvin Sharma <ashsharma@gitlab.com> Approved-by: Cameron Swords <cswords@gitlab.com> Approved-by: Ashvin Sharma <ashsharma@gitlab.com> Reviewed-by: Ashvin Sharma <ashsharma@gitlab.com> Co-authored-by: Arran Walker <ajwalker@gitlab.com>
2 parents 9bbb245 + 22946d5 commit e5e8d00

43 files changed

Lines changed: 944 additions & 985 deletions

Some content is hidden

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

‎cache/adapter.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"sync"
99
"time"
1010

11-
"gitlab.com/gitlab-org/gitlab-runner/common"
11+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
1212
)
1313

1414
type PresignedURL struct {
@@ -29,7 +29,7 @@ type Adapter interface {
2929
GetGoCloudURL(ctx context.Context, upload bool) (GoCloudURL, error)
3030
}
3131

32-
type Factory func(config *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error)
32+
type Factory func(config *cacheconfig.Config, timeout time.Duration, objectName string) (Adapter, error)
3333

3434
type FactoriesMap struct {
3535
internal map[string]Factory
@@ -72,7 +72,7 @@ func Factories() *FactoriesMap {
7272
return factories
7373
}
7474

75-
func getCreateAdapter(cacheConfig *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error) {
75+
func getCreateAdapter(cacheConfig *cacheconfig.Config, timeout time.Duration, objectName string) (Adapter, error) {
7676
create, err := Factories().Find(cacheConfig.Type)
7777
if err != nil {
7878
return nil, fmt.Errorf("cache factory not found: %w", err)

‎cache/adapter_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"time"
99

1010
"github.com/stretchr/testify/assert"
11-
"gitlab.com/gitlab-org/gitlab-runner/common"
11+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
1212
)
1313

1414
var defaultTimeout = 1 * time.Hour
@@ -30,7 +30,7 @@ func prepareMockedFactoriesMap() func() {
3030
}
3131

3232
func makeTestFactory(test factorizeTestCase) Factory {
33-
return func(config *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error) {
33+
return func(config *cacheconfig.Config, timeout time.Duration, objectName string) (Adapter, error) {
3434
if test.errorOnFactorize != nil {
3535
return nil, test.errorOnFactorize
3636
}
@@ -77,11 +77,11 @@ func TestCreateAdapter(t *testing.T) {
7777

7878
_ = factories.Register(
7979
"additional-adapter",
80-
func(config *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error) {
80+
func(config *cacheconfig.Config, timeout time.Duration, objectName string) (Adapter, error) {
8181
return NewMockAdapter(t), nil
8282
})
8383

84-
config := &common.CacheConfig{
84+
config := &cacheconfig.Config{
8585
Type: adapterTypeName,
8686
}
8787

@@ -100,7 +100,7 @@ func TestCreateAdapter(t *testing.T) {
100100

101101
func TestDoubledRegistration(t *testing.T) {
102102
adapterTypeName := "test"
103-
fakeFactory := func(config *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error) {
103+
fakeFactory := func(config *cacheconfig.Config, timeout time.Duration, objectName string) (Adapter, error) {
104104
return nil, nil
105105
}
106106

‎cache/azure/adapter.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ import (
1111
"github.com/sirupsen/logrus"
1212

1313
"gitlab.com/gitlab-org/gitlab-runner/cache"
14-
"gitlab.com/gitlab-org/gitlab-runner/common"
14+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
1515
)
1616

1717
type signedURLGenerator func(ctx context.Context, name string, options *signedURLOptions) (*url.URL, error)
1818
type blobTokenGenerator func(ctx context.Context, name string, options *signedURLOptions) (string, error)
1919

2020
type azureAdapter struct {
2121
timeout time.Duration
22-
config *common.CacheAzureConfig
22+
config *cacheconfig.CacheAzureConfig
2323
objectName string
2424

2525
generateSignedURL signedURLGenerator
@@ -130,7 +130,7 @@ func (a *azureAdapter) getSigner() sasSigner {
130130
return signer
131131
}
132132

133-
func New(config *common.CacheConfig, timeout time.Duration, objectName string) (cache.Adapter, error) {
133+
func New(config *cacheconfig.Config, timeout time.Duration, objectName string) (cache.Adapter, error) {
134134
azure := config.Azure
135135
if azure == nil {
136136
return nil, fmt.Errorf("missing Azure configuration")

‎cache/azure/adapter_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
"github.com/stretchr/testify/require"
1616

1717
"gitlab.com/gitlab-org/gitlab-runner/cache"
18-
"gitlab.com/gitlab-org/gitlab-runner/common"
18+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
1919
)
2020

2121
var (
@@ -27,11 +27,11 @@ var (
2727
defaultTimeout = 1 * time.Hour
2828
)
2929

30-
func defaultAzureCache() *common.CacheConfig {
31-
return &common.CacheConfig{
30+
func defaultAzureCache() *cacheconfig.Config {
31+
return &cacheconfig.Config{
3232
Type: "azure",
33-
Azure: &common.CacheAzureConfig{
34-
CacheAzureCredentials: common.CacheAzureCredentials{
33+
Azure: &cacheconfig.CacheAzureConfig{
34+
CacheAzureCredentials: cacheconfig.CacheAzureCredentials{
3535
AccountName: accountName,
3636
AccountKey: accountKey,
3737
},
@@ -56,7 +56,7 @@ type adapterOperationInvalidConfigTestCase struct {
5656

5757
func prepareMockedCredentialsResolverInitializer(tc adapterOperationInvalidConfigTestCase) func() {
5858
oldCredentialsResolverInitializer := credentialsResolverInitializer
59-
credentialsResolverInitializer = func(config *common.CacheAzureConfig) (*defaultCredentialsResolver, error) {
59+
credentialsResolverInitializer = func(config *cacheconfig.CacheAzureConfig) (*defaultCredentialsResolver, error) {
6060
if tc.errorOnCredentialsResolverInitialization {
6161
return nil, errors.New("test error")
6262
}

‎cache/azure/azure.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
1414
"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/sas"
1515
"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/service"
16-
"gitlab.com/gitlab-org/gitlab-runner/common"
16+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
1717
)
1818

1919
const DefaultAzureServer = "blob.core.windows.net"
@@ -109,15 +109,15 @@ func getSASToken(ctx context.Context, name string, o *signedURLOptions) (string,
109109
return sas.Encode(), nil
110110
}
111111

112-
func getBlobServiceURL(config *common.CacheAzureConfig) string {
112+
func getBlobServiceURL(config *cacheconfig.CacheAzureConfig) string {
113113
domain := DefaultAzureServer
114114
if config.StorageDomain != "" {
115115
domain = config.StorageDomain
116116
}
117117
return fmt.Sprintf("https://%s.%s", config.CacheAzureCredentials.AccountName, domain)
118118
}
119119

120-
func newAccountKeySigner(config *common.CacheAzureConfig) (sasSigner, error) {
120+
func newAccountKeySigner(config *cacheconfig.CacheAzureConfig) (sasSigner, error) {
121121
credentials := config.CacheAzureCredentials
122122
if credentials.AccountName == "" {
123123
return nil, fmt.Errorf("missing Azure storage account name")
@@ -138,7 +138,7 @@ func newAccountKeySigner(config *common.CacheAzureConfig) (sasSigner, error) {
138138
return &accountKeySigner{blobServiceURL: blobServiceURL, credential: credential}, nil
139139
}
140140

141-
func newUserDelegationKeySigner(config *common.CacheAzureConfig, options ...userDelegationKeyOption) (sasSigner, error) {
141+
func newUserDelegationKeySigner(config *cacheconfig.CacheAzureConfig, options ...userDelegationKeyOption) (sasSigner, error) {
142142
if config.AccountName == "" {
143143
return nil, fmt.Errorf("no Azure storage account name provided")
144144
}

‎cache/azure/azure_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616

1717
"github.com/stretchr/testify/assert"
1818
"github.com/stretchr/testify/require"
19-
"gitlab.com/gitlab-org/gitlab-runner/common"
19+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
2020
)
2121

2222
type azureSigningTest struct {
@@ -103,11 +103,11 @@ func TestAccountKeySigning(t *testing.T) {
103103

104104
for tn, tt := range tests {
105105
t.Run(tn, func(t *testing.T) {
106-
credentials := &common.CacheAzureCredentials{
106+
credentials := &cacheconfig.CacheAzureCredentials{
107107
AccountName: tt.accountName,
108108
AccountKey: tt.accountKey,
109109
}
110-
config := &common.CacheAzureConfig{
110+
config := &cacheconfig.CacheAzureConfig{
111111
CacheAzureCredentials: *credentials,
112112
ContainerName: tt.containerName,
113113
StorageDomain: tt.storageDomain,
@@ -207,11 +207,11 @@ func TestUserDelegationSigning(t *testing.T) {
207207

208208
for tn, tt := range tests {
209209
t.Run(tn, func(t *testing.T) {
210-
credentials := &common.CacheAzureCredentials{
210+
credentials := &cacheconfig.CacheAzureCredentials{
211211
AccountName: tt.accountName,
212212
AccountKey: tt.accountKey,
213213
}
214-
config := &common.CacheAzureConfig{
214+
config := &cacheconfig.CacheAzureConfig{
215215
CacheAzureCredentials: *credentials,
216216
}
217217
opts := &signedURLOptions{

‎cache/azure/credentials_resolver.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import (
44
"errors"
55
"fmt"
66

7-
"gitlab.com/gitlab-org/gitlab-runner/common"
7+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
88
)
99

1010
type credentialsResolver interface {
@@ -13,14 +13,14 @@ type credentialsResolver interface {
1313
}
1414

1515
type defaultCredentialsResolver struct {
16-
config *common.CacheAzureConfig
16+
config *cacheconfig.CacheAzureConfig
1717
}
1818

1919
func (cr *defaultCredentialsResolver) Resolve() error {
2020
return cr.readCredentialsFromConfig()
2121
}
2222

23-
func (cr *defaultCredentialsResolver) Credentials() *common.CacheAzureCredentials {
23+
func (cr *defaultCredentialsResolver) Credentials() *cacheconfig.CacheAzureCredentials {
2424
return &cr.config.CacheAzureCredentials
2525
}
2626

@@ -46,7 +46,7 @@ func (cr *defaultCredentialsResolver) readCredentialsFromConfig() error {
4646
return nil
4747
}
4848

49-
func newDefaultCredentialsResolver(config *common.CacheAzureConfig) (*defaultCredentialsResolver, error) {
49+
func newDefaultCredentialsResolver(config *cacheconfig.CacheAzureConfig) (*defaultCredentialsResolver, error) {
5050
if config == nil {
5151
return nil, fmt.Errorf("config can't be nil")
5252
}

‎cache/azure/credentials_resolver_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,34 +7,34 @@ import (
77

88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
10-
"gitlab.com/gitlab-org/gitlab-runner/common"
10+
"gitlab.com/gitlab-org/gitlab-runner/cache/cacheconfig"
1111
)
1212

1313
type credentialsResolverTestCase struct {
14-
config *common.CacheAzureConfig
14+
config *cacheconfig.CacheAzureConfig
1515
errorExpectedOnInitialization bool
1616
errorExpectedOnResolve bool
17-
expectedCredentials *common.CacheAzureCredentials
17+
expectedCredentials *cacheconfig.CacheAzureCredentials
1818
}
1919

2020
type signerTestCase struct {
21-
config *common.CacheAzureConfig
21+
config *cacheconfig.CacheAzureConfig
2222
errorExpectedOnSigner bool
2323
expectedSignerType string
2424
}
2525

26-
func getCredentialsConfig(accountName string, accountKey string) *common.CacheAzureConfig {
27-
return &common.CacheAzureConfig{
28-
CacheAzureCredentials: common.CacheAzureCredentials{
26+
func getCredentialsConfig(accountName string, accountKey string) *cacheconfig.CacheAzureConfig {
27+
return &cacheconfig.CacheAzureConfig{
28+
CacheAzureCredentials: cacheconfig.CacheAzureCredentials{
2929
AccountName: accountName,
3030
AccountKey: accountKey,
3131
},
3232
ContainerName: "test-container",
3333
}
3434
}
3535

36-
func getExpectedCredentials(accountName string, accountKey string) *common.CacheAzureCredentials {
37-
return &common.CacheAzureCredentials{
36+
func getExpectedCredentials(accountName string, accountKey string) *cacheconfig.CacheAzureCredentials {
37+
return &cacheconfig.CacheAzureCredentials{
3838
AccountName: accountName,
3939
AccountKey: accountKey,
4040
}
@@ -47,7 +47,7 @@ func TestDefaultCredentialsResolver(t *testing.T) {
4747
errorExpectedOnInitialization: true,
4848
},
4949
"credentials not set": {
50-
config: &common.CacheAzureConfig{},
50+
config: &cacheconfig.CacheAzureConfig{},
5151
errorExpectedOnResolve: true,
5252
},
5353
"credentials direct in config": {

0 commit comments

Comments
 (0)