Fix filter pipeline config files (#279)

closes #271 

- filter pipeline config folders for `.yml` and `.yaml` files
- improve `fetchConfig` tests
- update remote mock and correct wrong folder name `mock` => `mocks` to match package name
- fix: return correct filename for fallback
- improve config loading by checking if folder or not before sending api call

server/configFetcher.go

@@ -24,39 +24,58 @@ func NewConfigFetcher(remote remote.Remote, user *model.User, repo *model.Repo,
 	}
 }
 
-func (cf *configFetcher) Fetch() ([]*remote.FileMeta, error) {
+func (cf *configFetcher) Fetch() (files []*remote.FileMeta, err error) {
+	var file []byte
+
 	for i := 0; i < 5; i++ {
 		select {
 		case <-time.After(time.Second * time.Duration(i)):
+
 			// either a file
-			file, fileerr := cf.remote_.File(cf.user, cf.repo, cf.build, cf.repo.Config)
-			if fileerr == nil {
-				return []*remote.FileMeta{{
-					Name: cf.repo.Config,
-					Data: file,
-				}}, nil
+			if !strings.HasSuffix(cf.repo.Config, "/") {
+				file, err = cf.remote_.File(cf.user, cf.repo, cf.build, cf.repo.Config)
+				if err == nil {
+					return []*remote.FileMeta{{
+						Name: cf.repo.Config,
+						Data: file,
+					}}, nil
+				}
 			}
 
 			// or a folder
-			dir, direrr := cf.remote_.Dir(cf.user, cf.repo, cf.build, strings.TrimSuffix(cf.repo.Config, "/"))
-
-			if direrr == nil {
-				return dir, nil
-			} else if !cf.repo.Fallback {
-				return nil, direrr
+			if strings.HasSuffix(cf.repo.Config, "/") {
+				files, err = cf.remote_.Dir(cf.user, cf.repo, cf.build, strings.TrimSuffix(cf.repo.Config, "/"))
+				if err == nil {
+					return filterPipelineFiles(files), nil
+				}
 			}
 
 			// or fallback
-			file, fileerr = cf.remote_.File(cf.user, cf.repo, cf.build, ".drone.yml")
-			if fileerr != nil {
-				return nil, fileerr
+			if cf.repo.Fallback {
+				file, err = cf.remote_.File(cf.user, cf.repo, cf.build, ".drone.yml")
+				if err == nil {
+					return []*remote.FileMeta{{
+						Name: ".drone.yml",
+						Data: file,
+					}}, nil
+				}
 			}
 
-			return []*remote.FileMeta{{
-				Name: cf.repo.Config,
-				Data: file,
-			}}, nil
+			return nil, err
 		}
 	}
+
 	return []*remote.FileMeta{}, nil
 }
+
+func filterPipelineFiles(files []*remote.FileMeta) []*remote.FileMeta {
+	var res []*remote.FileMeta
+
+	for _, file := range files {
+		if strings.HasSuffix(file.Name, ".yml") || strings.HasSuffix(file.Name, ".yaml") {
+			res = append(res, file)
+		}
+	}
+
+	return res
+}

server/configFetcher_test.go

@@ -1,25 +1,189 @@
 package server_test
 
 import (
+	"errors"
 	"testing"
 
+	"github.com/stretchr/testify/mock"
 	"github.com/woodpecker-ci/woodpecker/model"
-	"github.com/woodpecker-ci/woodpecker/remote/github"
+	"github.com/woodpecker-ci/woodpecker/remote"
+	"github.com/woodpecker-ci/woodpecker/remote/mocks"
 	"github.com/woodpecker-ci/woodpecker/server"
 )
 
-func TestFetchGithub(t *testing.T) {
+func TestFetch(t *testing.T) {
 	t.Parallel()
 
-	github, err := github.New(github.Opts{URL: "https://github.com"})
-	if err != nil {
-		t.Fatal(err)
+	testTable := []struct {
+		name         string
+		repoConfig   string
+		repoFallback bool
+		fileMocks    []struct {
+			file []byte
+			err  error
+		}
+		dirMock struct {
+			files []*remote.FileMeta
+			err   error
+		}
+		expectedFileNames []string
+		expectedError     bool
+	}{
+		{
+			name:         "Single .woodpecker.yml file",
+			repoConfig:   ".woodpecker.yml",
+			repoFallback: false,
+			fileMocks: []struct {
+				file []byte
+				err  error
+			}{
+				{
+					file: []byte{},
+					err:  nil,
+				},
+			},
+			expectedFileNames: []string{
+				".woodpecker.yml",
+			},
+			expectedError: false,
+		},
+		{
+			name:         "Folder .woodpecker/",
+			repoConfig:   ".woodpecker/",
+			repoFallback: false,
+			dirMock: struct {
+				files []*remote.FileMeta
+				err   error
+			}{
+				files: []*remote.FileMeta{
+					{
+						Name: ".woodpecker/text.txt",
+						Data: []byte{},
+					},
+					{
+						Name: ".woodpecker/release.yml",
+						Data: []byte{},
+					},
+					{
+						Name: ".woodpecker/image.png",
+						Data: []byte{},
+					},
+				},
+				err: nil,
+			},
+			expectedFileNames: []string{
+				".woodpecker/release.yml",
+			},
+			expectedError: false,
+		},
+		{
+			name:         "Requesting woodpecker-file but using fallback",
+			repoConfig:   ".woodpecker.yml",
+			repoFallback: true,
+			fileMocks: []struct {
+				file []byte
+				err  error
+			}{
+				// first call requesting regular woodpecker.yml
+				{
+					file: nil,
+					err:  errors.New("File not found"),
+				},
+				// fallback file call
+				{
+					file: []byte{},
+					err:  nil,
+				},
+			},
+			expectedFileNames: []string{
+				".drone.yml",
+			},
+			expectedError: false,
+		},
+		{
+			name:         "Requesting folder but using fallback",
+			repoConfig:   ".woodpecker/",
+			repoFallback: true,
+			fileMocks: []struct {
+				file []byte
+				err  error
+			}{
+				{
+					file: []byte{},
+					err:  nil,
+				},
+			},
+			dirMock: struct {
+				files []*remote.FileMeta
+				err   error
+			}{
+				files: []*remote.FileMeta{},
+				err:   errors.New("Dir not found"),
+			},
+			expectedFileNames: []string{
+				".drone.yml",
+			},
+			expectedError: false,
+		},
+		{
+			name:         "Not found and disabled fallback",
+			repoConfig:   ".woodpecker.yml",
+			repoFallback: false,
+			fileMocks: []struct {
+				file []byte
+				err  error
+			}{
+				// first call requesting regular woodpecker.yml
+				{
+					file: nil,
+					err:  errors.New("File not found"),
+				},
+				// fallback file call
+				{
+					file: []byte{},
+					err:  errors.New("File not found"),
+				},
+			},
+			expectedFileNames: []string{},
+			expectedError:     true,
+		},
+	}
+
+	for _, tt := range testTable {
+		t.Run(tt.name, func(t *testing.T) {
+			repo := &model.Repo{Owner: "laszlocph", Name: "drone-multipipeline", Config: tt.repoConfig, Fallback: tt.repoFallback}
+
+			r := new(mocks.Remote)
+			for _, fileMock := range tt.fileMocks {
+				r.On("File", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(fileMock.file, fileMock.err).Once()
+			}
+			r.On("Dir", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tt.dirMock.files, tt.dirMock.err)
+
+			configFetcher := server.NewConfigFetcher(
+				r,
+				&model.User{Token: "xxx"},
+				repo,
+				&model.Build{Commit: "89ab7b2d6bfb347144ac7c557e638ab402848fee"},
+			)
+			files, err := configFetcher.Fetch()
+			if tt.expectedError && err == nil {
+				t.Fatal("expected an error")
+			} else if !tt.expectedError && err != nil {
+				t.Fatal("error fetching config:", err)
+			}
+
+			matchingFiles := 0
+			for _, expectedFileName := range tt.expectedFileNames {
+				for _, file := range files {
+					if file.Name == expectedFileName {
+						matchingFiles += 1
+					}
+				}
+			}
+
+			if matchingFiles != len(tt.expectedFileNames) {
+				t.Fatal("expected some other pipeline files", tt.expectedFileNames, files)
+			}
+		})
 	}
-	configFetcher := server.NewConfigFetcher(
-		github,
-		&model.User{Token: "xxx"},
-		&model.Repo{Owner: "laszlocph", Name: "drone-multipipeline", Config: ".drone"},
-		&model.Build{Commit: "89ab7b2d6bfb347144ac7c557e638ab402848fee"},
-	)
-	configFetcher.Fetch()
 }