Unverified Commit 5f3b523e authored by Pedro Pombeiro's avatar Pedro Pombeiro
Browse files

Enable line limit linter

Closes #25884
parent 675ec0cb
......@@ -25,6 +25,9 @@ linters-settings:
min-complexity: 10
govet:
check-shadowing: false
lll:
line-length: 120
tab-width: 4
nestif:
min-complexity: 5
......@@ -43,6 +46,7 @@ linters:
- gosimple
- govet
- ineffassign
- lll
- misspell
- nakedret
- staticcheck
......
......@@ -74,9 +74,11 @@ func TestCreateAdapter(t *testing.T) {
assert.NoError(t, err)
}
factories.Register("additional-adapter", func(config *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error) {
return new(MockAdapter), nil
})
factories.Register(
"additional-adapter",
func(config *common.CacheConfig, timeout time.Duration, objectName string) (Adapter, error) {
return new(MockAdapter), nil
})
config := &common.CacheConfig{
Type: adapterTypeName,
......
......@@ -69,7 +69,12 @@ func prepareFakeBuild(tc cacheOperationTest) *common.Build {
return build
}
func testCacheOperation(t *testing.T, operationName string, operation func(build *common.Build, key string) *url.URL, tc cacheOperationTest) {
func testCacheOperation(
t *testing.T,
operationName string,
operation func(build *common.Build, key string) *url.URL,
tc cacheOperationTest,
) {
t.Run(operationName, func(t *testing.T) {
hook := test.NewGlobal()
......
......@@ -105,7 +105,13 @@ func prepareMockedCredentialsResolverForInvalidConfig(adapter *gcsAdapter, tc ad
adapter.credentialsResolver = cr
}
func testAdapterOperationWithInvalidConfig(t *testing.T, name string, tc adapterOperationInvalidConfigTestCase, adapter *gcsAdapter, operation func() *url.URL) {
func testAdapterOperationWithInvalidConfig(
t *testing.T,
name string,
tc adapterOperationInvalidConfigTestCase,
adapter *gcsAdapter,
operation func() *url.URL,
) {
t.Run(name, func(t *testing.T) {
prepareMockedCredentialsResolverForInvalidConfig(adapter, tc)
hook := test.NewGlobal()
......@@ -213,7 +219,13 @@ func prepareMockedCredentialsResolver(adapter *gcsAdapter) func(t *testing.T) {
}
}
func prepareMockedSignedURLGenerator(t *testing.T, tc adapterOperationTestCase, expectedMethod string, expectedContentType string, adapter *gcsAdapter) {
func prepareMockedSignedURLGenerator(
t *testing.T,
tc adapterOperationTestCase,
expectedMethod string,
expectedContentType string,
adapter *gcsAdapter,
) {
adapter.generateSignedURL = func(bucket string, name string, opts *storage.SignedURLOptions) (string, error) {
require.Equal(t, accessID, opts.GoogleAccessID)
require.Equal(t, privateKey, string(opts.PrivateKey))
......@@ -224,7 +236,15 @@ func prepareMockedSignedURLGenerator(t *testing.T, tc adapterOperationTestCase,
}
}
func testAdapterOperation(t *testing.T, tc adapterOperationTestCase, name string, expectedMethod string, expectedContentType string, adapter *gcsAdapter, operation func() *url.URL) {
func testAdapterOperation(
t *testing.T,
tc adapterOperationTestCase,
name string,
expectedMethod string,
expectedContentType string,
adapter *gcsAdapter,
operation func() *url.URL,
) {
t.Run(name, func(t *testing.T) {
cleanupCredentialsResolverMock := prepareMockedCredentialsResolver(adapter)
defer cleanupCredentialsResolverMock(t)
......@@ -248,6 +268,7 @@ func testAdapterOperation(t *testing.T, tc adapterOperationTestCase, name string
}
func TestAdapterOperation(t *testing.T) {
//nolint:lll
tests := map[string]adapterOperationTestCase{
"error-on-URL-signing": {
returnedURL: "",
......@@ -276,8 +297,24 @@ func TestAdapterOperation(t *testing.T) {
adapter, ok := a.(*gcsAdapter)
require.True(t, ok, "Adapter should be properly casted to *adapter type")
testAdapterOperation(t, tc, "GetDownloadURL", http.MethodGet, "", adapter, a.GetDownloadURL)
testAdapterOperation(t, tc, "GetUploadURL", http.MethodPut, "application/octet-stream", adapter, a.GetUploadURL)
testAdapterOperation(
t,
tc,
"GetDownloadURL",
http.MethodGet,
"",
adapter,
a.GetDownloadURL,
)
testAdapterOperation(
t,
tc,
"GetUploadURL",
http.MethodPut,
"application/octet-stream",
adapter,
a.GetUploadURL,
)
})
}
}
......@@ -44,8 +44,12 @@ func onFakeMinioURLGenerator(tc cacheOperationTest) func() {
err = errors.New("test error")
}
client.On("PresignedGetObject", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.presignedURL, err)
client.On("PresignedPutObject", mock.Anything, mock.Anything, mock.Anything).Return(tc.presignedURL, err)
client.
On("PresignedGetObject", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(tc.presignedURL, err)
client.
On("PresignedPutObject", mock.Anything, mock.Anything, mock.Anything).
Return(tc.presignedURL, err)
oldNewMinioURLGenerator := newMinioClient
newMinioClient = func(s3 *common.CacheS3Config) (minioClient, error) {
......@@ -60,7 +64,11 @@ func onFakeMinioURLGenerator(tc cacheOperationTest) func() {
}
}
func testCacheOperation(t *testing.T, operationName string, operation func(adapter cache.Adapter) *url.URL, tc cacheOperationTest) {
func testCacheOperation(
t *testing.T,
operationName string,
operation func(adapter cache.Adapter) *url.URL, tc cacheOperationTest,
) {
t.Run(operationName, func(t *testing.T) {
cleanupMinioURLGeneratorMock := onFakeMinioURLGenerator(tc)
defer cleanupMinioURLGeneratorMock()
......@@ -102,8 +110,16 @@ func TestCacheOperation(t *testing.T) {
for testName, test := range tests {
t.Run(testName, func(t *testing.T) {
testCacheOperation(t, "GetDownloadURL", func(adapter cache.Adapter) *url.URL { return adapter.GetDownloadURL() }, test)
testCacheOperation(t, "GetUploadURL", func(adapter cache.Adapter) *url.URL { return adapter.GetUploadURL() }, test)
testCacheOperation(
t,
"GetDownloadURL",
func(adapter cache.Adapter) *url.URL { return adapter.GetDownloadURL() },
test)
testCacheOperation(
t,
"GetUploadURL",
func(adapter cache.Adapter) *url.URL { return adapter.GetUploadURL() },
test)
})
}
}
......
......@@ -13,7 +13,12 @@ import (
const DefaultAWSS3Server = "s3.amazonaws.com"
type minioClient interface {
PresignedGetObject(bucketName string, objectName string, expires time.Duration, reqParams url.Values) (*url.URL, error)
PresignedGetObject(
bucketName string,
objectName string,
expires time.Duration,
reqParams url.Values,
) (*url.URL, error)
PresignedPutObject(bucketName string, objectName string, expires time.Duration) (*url.URL, error)
}
......
......@@ -176,24 +176,25 @@ func runOnFakeMinio(t *testing.T, test minioClientInitializationTest) func() {
func runOnFakeMinioWithCredentials(t *testing.T, test minioClientInitializationTest) func() {
oldNewMinioWithCredentials := newMinioWithCredentials
newMinioWithCredentials = func(endpoint string, creds *credentials.Credentials, secure bool, region string) (*minio.Client, error) {
if !test.expectedToUseIAM {
t.Error("Should not use minio with IAM client initializator")
}
newMinioWithCredentials =
func(endpoint string, creds *credentials.Credentials, secure bool, region string) (*minio.Client, error) {
if !test.expectedToUseIAM {
t.Error("Should not use minio with IAM client initializator")
}
if test.errorOnInitialization {
return nil, errors.New("test error")
}
if test.errorOnInitialization {
return nil, errors.New("test error")
}
assert.Equal(t, "s3.amazonaws.com", endpoint)
assert.True(t, secure)
assert.Empty(t, region)
assert.Equal(t, "s3.amazonaws.com", endpoint)
assert.True(t, secure)
assert.Empty(t, region)
client, err := minio.NewWithCredentials(endpoint, creds, secure, region)
require.NoError(t, err)
client, err := minio.NewWithCredentials(endpoint, creds, secure, region)
require.NoError(t, err)
return client, nil
}
return client, nil
}
return func() {
newMinioWithCredentials = oldNewMinioWithCredentials
......
......@@ -193,7 +193,9 @@ func (b *buildsHelper) removeBuild(deleteBuild *common.Build) bool {
b.lock.Lock()
defer b.lock.Unlock()
b.jobDurationHistogram.WithLabelValues(deleteBuild.Runner.ShortDescription()).Observe(deleteBuild.Duration().Seconds())
b.jobDurationHistogram.
WithLabelValues(deleteBuild.Runner.ShortDescription()).
Observe(deleteBuild.Duration().Seconds())
for idx, build := range b.builds {
if build == deleteBuild {
......
......@@ -202,6 +202,7 @@ func TestBuildsHelper_ListJobsHandler(t *testing.T) {
}
func TestCreateJobURL(t *testing.T) {
//nolint:lll
testCases := map[string]string{
"http://gitlab.example.com/my-namespace/my-project.git": "http://gitlab.example.com/my-namespace/my-project/-/jobs/1",
"http://gitlab.example.com/my-namespace/my-project": "http://gitlab.example.com/my-namespace/my-project/-/jobs/1",
......
......@@ -55,6 +55,7 @@ func (c *configOptions) RunnerByName(name string) (*common.RunnerConfig, error)
return nil, fmt.Errorf("could not find a runner with the name '%s'", name)
}
//nolint:lll
type configOptionsWithListenAddress struct {
configOptions
......
......@@ -22,7 +22,12 @@ const (
configurationFromConfig metricsServerConfigurationType = "from-config"
)
func testListenAddressSetting(t *testing.T, exampleName string, example metricsServerTestExample, testType metricsServerConfigurationType) {
func testListenAddressSetting(
t *testing.T,
exampleName string,
example metricsServerTestExample,
testType metricsServerConfigurationType,
) {
t.Run(fmt.Sprintf("%s-%s", exampleName, testType), func(t *testing.T) {
cfg := configOptionsWithListenAddress{}
cfg.config = &common.Config{}
......
......@@ -14,6 +14,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/network"
)
//nolint:lll
type ArtifactsDownloaderCommand struct {
common.JobCredentials
retryHelper
......@@ -82,11 +83,15 @@ func (c *ArtifactsDownloaderCommand) Execute(context *cli.Context) {
}
func init() {
common.RegisterCommand2("artifacts-downloader", "download and extract build artifacts (internal)", &ArtifactsDownloaderCommand{
network: network.NewGitLabClient(),
retryHelper: retryHelper{
Retry: 2,
RetryTime: time.Second,
common.RegisterCommand2(
"artifacts-downloader",
"download and extract build artifacts (internal)",
&ArtifactsDownloaderCommand{
network: network.NewGitLabClient(),
retryHelper: retryHelper{
Retry: 2,
RetryTime: time.Second,
},
},
})
)
}
......@@ -33,7 +33,11 @@ type testNetwork struct {
uploadedFiles []string
}
func (m *testNetwork) DownloadArtifacts(config common.JobCredentials, artifactsFile string, directDownload *bool) common.DownloadState {
func (m *testNetwork) DownloadArtifacts(
config common.JobCredentials,
artifactsFile string,
directDownload *bool,
) common.DownloadState {
m.downloadCalled++
if directDownload != nil && *directDownload {
......@@ -115,7 +119,11 @@ func (m *testNetwork) consumeRawUpload(reader io.Reader) common.UploadState {
return m.uploadState
}
func (m *testNetwork) UploadRawArtifacts(config common.JobCredentials, reader io.Reader, options common.ArtifactsOptions) common.UploadState {
func (m *testNetwork) UploadRawArtifacts(
config common.JobCredentials,
reader io.Reader,
options common.ArtifactsOptions,
) common.UploadState {
m.uploadCalled++
if m.uploadState == common.UploadSucceeded {
......
......@@ -172,8 +172,12 @@ func (c *ArtifactsUploaderCommand) Execute(*cli.Context) {
}
func init() {
common.RegisterCommand2("artifacts-uploader", "create and upload build artifacts (internal)", &ArtifactsUploaderCommand{
network: network.NewGitLabClient(),
Name: "artifacts",
})
common.RegisterCommand2(
"artifacts-uploader",
"create and upload build artifacts (internal)",
&ArtifactsUploaderCommand{
network: network.NewGitLabClient(),
Name: "artifacts",
},
)
}
......@@ -97,15 +97,21 @@ func (c *CacheArchiverCommand) Execute(*cli.Context) {
logrus.Fatalln(err)
}
} else {
logrus.Infoln("No URL provided, cache will be not uploaded to shared cache server. Cache will be stored only locally.")
logrus.Infoln(
"No URL provided, cache will be not uploaded to shared cache server. " +
"Cache will be stored only locally.")
}
}
func init() {
common.RegisterCommand2("cache-archiver", "create and upload cache artifacts (internal)", &CacheArchiverCommand{
retryHelper: retryHelper{
Retry: 2,
RetryTime: time.Second,
common.RegisterCommand2(
"cache-archiver",
"create and upload cache artifacts (internal)",
&CacheArchiverCommand{
retryHelper: retryHelper{
Retry: 2,
RetryTime: time.Second,
},
},
})
)
}
......@@ -116,7 +116,9 @@ func (c *CacheExtractorCommand) Execute(context *cli.Context) {
logrus.Fatalln(err)
}
} else {
logrus.Infoln("No URL provided, cache will not be downloaded from shared cache server. Instead a local version of cache will be extracted.")
logrus.Infoln(
"No URL provided, cache will not be downloaded from shared cache server. " +
"Instead a local version of cache will be extracted.")
}
err := archives.ExtractZipFile(c.File)
......@@ -126,10 +128,14 @@ func (c *CacheExtractorCommand) Execute(context *cli.Context) {
}
func init() {
common.RegisterCommand2("cache-extractor", "download and extract cache artifacts (internal)", &CacheExtractorCommand{
retryHelper: retryHelper{
Retry: 2,
RetryTime: time.Second,
common.RegisterCommand2(
"cache-extractor",
"download and extract cache artifacts (internal)",
&CacheExtractorCommand{
retryHelper: retryHelper{
Retry: 2,
RetryTime: time.Second,
},
},
})
)
}
......@@ -205,7 +205,11 @@ func TestFileArchiverFileIsNotChanged(t *testing.T) {
require.NoError(t, err)
require.NoError(t, os.Chtimes(fileArchiverUntrackedFile, now, now.Add(-time.Second)))
assert.False(t, f.isFileChanged(fileArchiverArchiveZipFile), "should return false if file was modified before the listed file")
assert.False(
t,
f.isFileChanged(fileArchiverArchiveZipFile),
"should return false if file was modified before the listed file",
)
}
func TestFileArchiverFileIsChanged(t *testing.T) {
......@@ -237,5 +241,9 @@ func TestFileArchiverFileDoesNotExist(t *testing.T) {
err := f.enumerate()
require.NoError(t, err)
assert.True(t, f.isFileChanged(fileArchiverNotExistingFile), "should return true if file doesn't exist")
assert.True(
t,
f.isFileChanged(fileArchiverNotExistingFile),
"should return true if file doesn't exist",
)
}
......@@ -178,5 +178,9 @@ func (c *ReadLogsCommand) openFileReader() (readSeekCloser, *bufio.Reader, error
}
func init() {
common.RegisterCommand2("read-logs", "reads job logs from a file, used by kubernetes executor (internal)", newReadLogsCommand())
common.RegisterCommand2(
"read-logs",
"reads job logs from a file, used by kubernetes executor (internal)",
newReadLogsCommand(),
)
}
......@@ -436,7 +436,11 @@ func (mr *RunCommand) processRunners(id int, stopWorker chan bool, runners chan
// GitLab instance and finally creates and finishes the job.
// To speed-up jobs handling before starting the job this method "requeues" the runner to another
// worker (by feeding the channel normally handled by feedRunners).
func (mr *RunCommand) processRunner(id int, runner *common.RunnerConfig, runners chan *common.RunnerConfig) (err error) {
func (mr *RunCommand) processRunner(
id int,
runner *common.RunnerConfig,
runners chan *common.RunnerConfig,
) (err error) {
provider := common.GetExecutorProvider(runner.Executor)
if provider == nil {
return
......@@ -526,7 +530,10 @@ func (mr *RunCommand) createSession(provider common.ExecutorProvider) (*session.
// requestJob will check if the runner can send another concurrent request to
// GitLab, if not the return value is nil.
func (mr *RunCommand) requestJob(runner *common.RunnerConfig, sessionInfo *common.SessionInfo) (common.JobTrace, *common.JobResponse, error) {
func (mr *RunCommand) requestJob(
runner *common.RunnerConfig,
sessionInfo *common.SessionInfo,
) (common.JobTrace, *common.JobResponse, error) {
if !mr.buildsHelper.acquireRequest(runner) {
mr.log().WithField("runner", runner.ShortDescription()).
Debugln("Failed to request job: runner requestConcurrency meet")
......@@ -857,12 +864,16 @@ func (mr *RunCommand) Collect(ch chan<- prometheus.Metric) {
func init() {
requestStatusesCollector := network.NewAPIRequestStatusesMap()
common.RegisterCommand2("run", "run multi runner service", &RunCommand{
ServiceName: defaultServiceName,
network: network.NewGitLabClientWithRequestStatusesMap(requestStatusesCollector),
networkRequestStatusesCollector: requestStatusesCollector,
prometheusLogHook: prometheus_helper.NewLogHook(),
failuresCollector: prometheus_helper.NewFailuresCollector(),
buildsHelper: newBuildsHelper(),
})
common.RegisterCommand2(
"run",
"run multi runner service",
&RunCommand{
ServiceName: defaultServiceName,
network: network.NewGitLabClientWithRequestStatusesMap(requestStatusesCollector),
networkRequestStatusesCollector: requestStatusesCollector,
prometheusLogHook: prometheus_helper.NewLogHook(),
failuresCollector: prometheus_helper.NewFailuresCollector(),
buildsHelper: newBuildsHelper(),
},
)
}
......@@ -59,6 +59,7 @@ func (c *configTemplate) loadConfigTemplate() error {
return nil
}
//nolint:lll
type RegisterCommand struct {
context *cli.Context
network common.Network
......@@ -193,7 +194,9 @@ func (s *RegisterCommand) askBasicDocker(exampleHelperImage string) {
s.Docker = &common.DockerConfig{}
}
s.Docker.Image = s.ask("docker-image", fmt.Sprintf("Please enter the default Docker image (e.g. %s):", exampleHelperImage))
s.Docker.Image = s.ask(
"docker-image",
fmt.Sprintf("Please enter the default Docker image (e.g. %s):", exampleHelperImage))
}
func (s *RegisterCommand) askParallels() {
......@@ -211,8 +214,16 @@ func (s *RegisterCommand) askSSHServer() {
func (s *RegisterCommand) askSSHLogin() {
s.SSH.User = s.ask("ssh-user", "Please enter the SSH user (e.g. root):")
s.SSH.Password = s.ask("ssh-password", "Please enter the SSH password (e.g. docker.io):", true)
s.SSH.IdentityFile = s.ask("ssh-identity-file", "Please enter path to SSH identity file (e.g. /home/user/.ssh/id_rsa):", true)
s.SSH.Password = s.ask(
"ssh-password",
"Please enter the SSH password (e.g. docker.io):",
true,
)
s.SSH.IdentityFile = s.ask(
"ssh-identity-file",
"Please enter path to SSH identity file (e.g. /home/user/.ssh/id_rsa):",
true,
)
}
func (s *RegisterCommand) addRunner(runner *common.RunnerConfig) {
......@@ -380,7 +391,12 @@ func (s *RegisterCommand) Execute(context *cli.Context) {
}
if s.config.Concurrent < s.Limit {
logrus.Warningf("Specified limit (%d) larger then current concurrent limit (%d). Concurrent limit will not be enlarged.", s.Limit, s.config.Concurrent)
logrus.Warningf(
"Specified limit (%d) larger then current concurrent limit (%d). "+
"Concurrent limit will not be enlarged.",
s.Limit,
s.config.Concurrent,
)
}
s.askExecutor()
......@@ -394,7 +410,9 @@ func (s *RegisterCommand) Execute(context *cli.Context) {
logrus.Panicln(err)
}
logrus.Printf("Runner registered successfully. Feel free to start it, but if it's running already the config should be automatically reloaded!")
logrus.Printf(
"Runner registered successfully. " +
"Feel free to start it, but if it's running already the config should be automatically reloaded!")
}
func (s *RegisterCommand) mergeTemplate() {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment