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

remote/mocks/Remote.go

@@ -1,11 +1,15 @@
-// Code generated by mockery v1.0.0. DO NOT EDIT.
+// Code generated by mockery v0.0.0-dev. DO NOT EDIT.
 
 package mocks
 
-import http "net/http"
+import (
-import mock "github.com/stretchr/testify/mock"
+	http "net/http"
-import model "github.com/woodpecker-ci/woodpecker/model"
+
-import remote "github.com/woodpecker-ci/woodpecker/remote"
+	mock "github.com/stretchr/testify/mock"
+	model "github.com/woodpecker-ci/woodpecker/model"
+
+	remote "github.com/woodpecker-ci/woodpecker/remote"
+)
 
 // Remote is an autogenerated mock type for the Remote type
 type Remote struct {
@@ -254,13 +258,13 @@ func (_m *Remote) Repos(u *model.User) ([]*model.Repo, error) {
 	return r0, r1
 }
 
-// Status provides a mock function with given fields: u, r, b, link
+// Status provides a mock function with given fields: u, r, b, link, proc
-func (_m *Remote) Status(u *model.User, r *model.Repo, b *model.Build, link string) error {
+func (_m *Remote) Status(u *model.User, r *model.Repo, b *model.Build, link string, proc *model.Proc) error {
-	ret := _m.Called(u, r, b, link)
+	ret := _m.Called(u, r, b, link, proc)
 
 	var r0 error
-	if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *model.Build, string) error); ok {
+	if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *model.Build, string, *model.Proc) error); ok {
-		r0 = rf(u, r, b, link)
+		r0 = rf(u, r, b, link, proc)
 	} else {
 		r0 = ret.Error(0)
 	}

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 !strings.HasSuffix(cf.repo.Config, "/") {
-			if fileerr == nil {
+				file, err = cf.remote_.File(cf.user, cf.repo, cf.build, cf.repo.Config)
-				return []*remote.FileMeta{{
+				if err == nil {
-					Name: cf.repo.Config,
+					return []*remote.FileMeta{{
-					Data: file,
+						Name: cf.repo.Config,
-				}}, nil
+						Data: file,
+					}}, nil
+				}
 			}
 
 			// or a folder
-			dir, direrr := cf.remote_.Dir(cf.user, cf.repo, cf.build, strings.TrimSuffix(cf.repo.Config, "/"))
+			if strings.HasSuffix(cf.repo.Config, "/") {
-
+				files, err = cf.remote_.Dir(cf.user, cf.repo, cf.build, strings.TrimSuffix(cf.repo.Config, "/"))
-			if direrr == nil {
+				if err == nil {
-				return dir, nil
+					return filterPipelineFiles(files), nil
-			} else if !cf.repo.Fallback {
+				}
-				return nil, direrr
 			}
 
 			// or fallback
-			file, fileerr = cf.remote_.File(cf.user, cf.repo, cf.build, ".drone.yml")
+			if cf.repo.Fallback {
-			if fileerr != nil {
+				file, err = cf.remote_.File(cf.user, cf.repo, cf.build, ".drone.yml")
-				return nil, fileerr
+				if err == nil {
+					return []*remote.FileMeta{{
+						Name: ".drone.yml",
+						Data: file,
+					}}, nil
+				}
 			}
 
-			return []*remote.FileMeta{{
+			return nil, err
-				Name: cf.repo.Config,
-				Data: file,
-			}}, nil
 		}
 	}
+
 	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"})
+	testTable := []struct {
-	if err != nil {
+		name         string
-		t.Fatal(err)
+		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()
 }