Skip to content

Commit 2baa2cd

Browse files
alexellisalexellis
committed
Fix broken unit tests for overwriting protection of templates
Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>
1 parent b839c8a commit 2baa2cd

File tree

7 files changed

+60
-97
lines changed

7 files changed

+60
-97
lines changed

‎commands/build.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ func pullTemplates(templateURL, templateName string) error {
381381
}
382382

383383
templatePath := filepath.Join(cwd, TemplateDirectory)
384+
384385
if _, err := os.Stat(templatePath); err != nil {
385386
if os.IsNotExist(err) {
386387
fmt.Printf("No templates found in current directory.\n")

‎commands/fetch_templates.go‎

Lines changed: 21 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ const DefaultTemplateRepository = "https://github.com/openfaas/templates.git"
2424
const TemplateDirectory = "./template/"
2525

2626
// fetchTemplates fetch code templates using git clone.
27-
func fetchTemplates(templateURL, refName, templateName string, overwrite bool) error {
27+
func fetchTemplates(templateURL, refName, templateName string, overwriteTemplates bool) error {
2828
if len(templateURL) == 0 {
2929
return fmt.Errorf("pass valid templateURL")
3030
}
3131

32-
log.Printf("Fetching templates from %s]", templateURL)
32+
log.Printf("Fetching templates from %s", templateURL)
3333

3434
targetDir, err := os.MkdirTemp("", "openfaas-templates-*")
3535
if err != nil {
@@ -68,7 +68,7 @@ func fetchTemplates(templateURL, refName, templateName string, overwrite bool) e
6868
return err
6969
}
7070

71-
protectedLanguages, fetchedLanguages, err := moveTemplates(targetDir, templateName, overwrite, templateURL, refName, sha)
71+
protectedLanguages, fetchedLanguages, err := moveTemplates(targetDir, templateName, overwriteTemplates, templateURL, refName, sha)
7272
if err != nil {
7373
return err
7474
}
@@ -85,17 +85,18 @@ func fetchTemplates(templateURL, refName, templateName string, overwrite bool) e
8585
// canWriteLanguage tells whether the language can be expanded from the zip or not.
8686
// availableLanguages map keeps track of which languages we know to be okay to copy.
8787
// overwrite flag will allow to force copy the language template
88-
func canWriteLanguage(existingLanguages []string, language string, overwrite bool) bool {
89-
if overwrite {
88+
func canWriteLanguage(existingLanguages []string, language string, overwriteTemplate bool) bool {
89+
if overwriteTemplate {
9090
return true
9191
}
92-
return slices.Contains(existingLanguages, language)
92+
93+
return !slices.Contains(existingLanguages, language)
9394
}
9495

9596
// moveTemplates moves the templates from the repository to the template directory
9697
// It returns the existing languages and the fetched languages
9798
// It also returns an error if the templates cannot be read
98-
func moveTemplates(repoPath, templateName string, overwrite bool, repository string, refName string, sha string) ([]string, []string, error) {
99+
func moveTemplates(repoPath, templateName string, overwriteTemplate bool, repository string, refName string, sha string) ([]string, []string, error) {
99100

100101
var (
101102
existingLanguages []string
@@ -110,23 +111,25 @@ func moveTemplates(repoPath, templateName string, overwrite bool, repository str
110111
}
111112

112113
destTemplatePath := filepath.Join(cwd, TemplateDirectory)
113-
if _, err := os.Stat(destTemplatePath); err != nil && !os.IsNotExist(err) {
114-
os.MkdirAll(destTemplatePath, 0755)
114+
if _, err := os.Stat(destTemplatePath); err != nil && os.IsNotExist(err) {
115+
if err := os.MkdirAll(destTemplatePath, 0755); err != nil {
116+
return nil, nil, fmt.Errorf("can't create template directory: %s", err)
117+
}
115118
}
116119

117-
// availableLanguages := make(map[string]bool)
118-
119-
templateEntries, _ := os.ReadDir(destTemplatePath)
120+
templateEntries, err := os.ReadDir(destTemplatePath)
121+
if err != nil {
122+
return nil, nil, fmt.Errorf("unable to read directory: %s", destTemplatePath)
123+
}
120124

121125
// OK if nothing exists yet
122-
123126
for _, entry := range templateEntries {
124127
if !entry.IsDir() {
125128
continue
126129
}
127130

128131
templateFile := filepath.Join(destTemplatePath, entry.Name(), "template.yml")
129-
if _, err := os.Stat(templateFile); err != nil {
132+
if _, err := os.Stat(templateFile); err != nil && !os.IsNotExist(err) {
130133
return nil, nil, fmt.Errorf("can't find template.yml in: %s", templateFile)
131134
}
132135

@@ -148,7 +151,7 @@ func moveTemplates(repoPath, templateName string, overwrite bool, repository str
148151
refSuffix = "@" + refName
149152
}
150153

151-
if canWriteLanguage(existingLanguages, language, overwrite) {
154+
if canWriteLanguage(existingLanguages, language, overwriteTemplate) {
152155
// Do cp here
153156
languageSrc := filepath.Join(repoPath, TemplateDirectory, language)
154157
languageDest := filepath.Join(destTemplatePath, language)
@@ -167,53 +170,11 @@ func moveTemplates(repoPath, templateName string, overwrite bool, repository str
167170
return nil, nil, err
168171
}
169172
} else {
170-
171173
protectedLanguages = append(protectedLanguages, language+refSuffix)
172174
continue
173175
}
174176
}
175177

176-
// if len(templateName) == 0 {
177-
178-
// canWrite := canWriteLanguage(availableLanguages, language, overwrite)
179-
// if canWrite {
180-
// fetchedLanguages = append(fetchedLanguages, language)
181-
// // Do cp here
182-
// languageSrc := filepath.Join(templatePath, language)
183-
// languageDest := filepath.Join(templateDirectory, language)
184-
// if refName != "" {
185-
// languageDest += "@" + refName
186-
// }
187-
// builder.CopyFiles(languageSrc, languageDest)
188-
189-
// if err := writeTemplateMeta(languageDest, repository, refName, sha); err != nil {
190-
// return nil, nil, err
191-
// }
192-
193-
// } else {
194-
// existingLanguages = append(existingLanguages, language)
195-
// continue
196-
// }
197-
// } else if language == templateName {
198-
199-
// if canWriteLanguage(availableLanguages, language, overwrite) {
200-
// fetchedLanguages = append(fetchedLanguages, language)
201-
// // Do cp here
202-
// languageSrc := filepath.Join(templatePath, language)
203-
// languageDest := filepath.Join(templateDirectory, language)
204-
// if refName != "" {
205-
// languageDest += "@" + refName
206-
// }
207-
// builder.CopyFiles(languageSrc, languageDest)
208-
209-
// if err := writeTemplateMeta(languageDest, repository, refName, sha); err != nil {
210-
// return nil, nil, err
211-
// }
212-
213-
// }
214-
// }
215-
// }
216-
217178
return protectedLanguages, fetchedLanguages, nil
218179
}
219180

@@ -238,7 +199,7 @@ func writeTemplateMeta(languageDest, repository, refName, sha string) error {
238199
return nil
239200
}
240201

241-
func pullTemplate(repository, templateName string) error {
202+
func pullTemplate(repository, templateName string, overwriteTemplates bool) error {
242203

243204
baseRepository := repository
244205

@@ -275,8 +236,8 @@ func pullTemplate(repository, templateName string) error {
275236
}
276237
}
277238

278-
if err := fetchTemplates(repository, refName, templateName, overwrite); err != nil {
279-
return fmt.Errorf("error while fetching templates: %s", err)
239+
if err := fetchTemplates(repository, refName, templateName, overwriteTemplates); err != nil {
240+
return fmt.Errorf("error while fetching templates: %w", err)
280241
}
281242

282243
return nil

‎commands/fetch_templates_test.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,21 @@ func setupLocalTemplateRepo(t *testing.T) string {
106106
// tearDownFetchTemplates cleans all files and directories created by the test
107107
func tearDownFetchTemplates(t *testing.T) {
108108

109+
cwd, err := os.Getwd()
110+
if err != nil {
111+
t.Logf("Could not get current working directory: %s", err)
112+
return
113+
}
114+
115+
fullPath := filepath.Join(cwd, TemplateDirectory)
109116
// Remove existing templates folder, if it exist
110-
if _, err := os.Stat("./template/"); err == nil {
111-
t.Log("Found a ./template/ directory, removing it.")
117+
if _, err := os.Stat(fullPath); err == nil {
118+
t.Log("Found a template directory, removing it.")
112119

113-
if err := os.RemoveAll("./template/"); err != nil {
120+
if err := os.RemoveAll(fullPath); err != nil {
114121
t.Log(err)
115122
}
116123
} else {
117-
t.Logf("Directory template was not created: %s", err)
124+
t.Logf("Template directory %s was not created: %s", fullPath, err)
118125
}
119126
}

‎commands/new_function.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ Download templates:
160160
templateName += "#" + ref
161161
}
162162

163-
if err := pullTemplate(templateInfo.Repository, templateName); err != nil {
163+
if err := pullTemplate(templateInfo.Repository, templateName, overwrite); err != nil {
164164
return fmt.Errorf("error while pulling template: %s", err)
165165
}
166166

‎commands/template_pull.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ directory from the root of the repo, if it exists.
3535
faas-cli template pull https://github.com/openfaas/templates
3636
faas-cli template pull https://github.com/openfaas/templates#1.0
3737
`,
38-
RunE: runTemplatePull,
38+
RunE: runTemplatePull,
39+
SilenceErrors: true,
40+
SilenceUsage: true,
3941
}
4042

4143
func runTemplatePull(cmd *cobra.Command, args []string) error {
@@ -44,10 +46,15 @@ func runTemplatePull(cmd *cobra.Command, args []string) error {
4446
repository = args[0]
4547
}
4648

47-
templateName := "" // templateName may not be known at this point
49+
return templatePull(repository, overwrite)
50+
}
51+
52+
func templatePull(repository string, overwriteTemplates bool) error {
53+
templateName := ""
4854

4955
repository = getTemplateURL(repository, os.Getenv(templateURLEnvironment), DefaultTemplateRepository)
50-
return pullTemplate(repository, templateName)
56+
57+
return pullTemplate(repository, templateName, overwriteTemplates)
5158
}
5259

5360
func pullDebugPrint(message string) {

‎commands/template_pull_stack.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func pullStackTemplates(missingTemplates []string, templateSources []stack.Templ
9191
fmt.Printf("Pulling template: %s from %s\n", val, templateConfig.Source)
9292

9393
templateName := templateConfig.Name
94-
if err := pullTemplate(templateConfig.Source, templateName); err != nil {
94+
if err := pullTemplate(templateConfig.Source, templateName, overwrite); err != nil {
9595
return err
9696
}
9797
}

‎commands/template_pull_test.go‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33
package commands
44

55
import (
6-
"bytes"
76
"fmt"
8-
"log"
97
"os"
10-
"regexp"
8+
"os/exec"
119
"strings"
1210
"testing"
1311
)
@@ -20,50 +18,39 @@ func Test_templatePull(t *testing.T) {
2018
defer tearDownFetchTemplates(t)
2119

2220
faasCmd.SetArgs([]string{"template", "pull", localTemplateRepository})
23-
err := faasCmd.Execute()
24-
if err != nil {
21+
if err := faasCmd.Execute(); err != nil {
2522
t.Fatalf("unexpected error while puling valid repo (%q): %s", localTemplateRepository, err.Error())
2623
}
2724

2825
// Verify created directories
2926
if _, err := os.Stat("template"); err != nil {
3027
t.Fatalf("The directory %s was not created", "template")
3128
}
29+
3230
})
3331

3432
t.Run("WithOverwriting", func(t *testing.T) {
3533
defer tearDownFetchTemplates(t)
36-
faasCmd.SetArgs([]string{"template", "pull", "--overwrite=false", localTemplateRepository})
37-
if err := faasCmd.Execute(); err != nil {
38-
t.Errorf("unexpected error while executing initial template pull: %s", err.Error())
39-
}
4034

41-
var buf bytes.Buffer
42-
log.SetOutput(&buf)
43-
44-
overwrite = false
45-
faasCmd.SetArgs([]string{"template", "pull", "--overwrite=false", localTemplateRepository})
46-
if err := faasCmd.Execute(); err != nil {
47-
t.Errorf("unexpected error while executing template pull: %s", err.Error())
35+
if err := templatePull(localTemplateRepository, true); err != nil {
36+
t.Fatalf("unexpected error while executing initial template pull: %s", err.Error())
4837
}
4938

50-
r := regexp.MustCompile(`(?m:Cannot overwrite the following \d+ template\(s\):)`)
51-
if !r.MatchString(buf.String()) {
52-
t.Fatalf("Output from stdio didn't match expression: %s\nvs: %s", buf.String(), r.String())
53-
}
39+
//execute command to ls in the template directory
5440

55-
buf.Reset()
41+
lsCmd := exec.Command("ls", "template")
42+
_, err := lsCmd.CombinedOutput()
43+
if err != nil {
44+
t.Fatalf("error while listing template directory: %s", err.Error())
45+
}
5646

57-
overwrite = true
5847

59-
faasCmd.SetArgs([]string{"template", "pull", localTemplateRepository, "--overwrite"})
60-
if err := faasCmd.Execute(); err != nil {
61-
t.Errorf("unexpected error while executing template pull with --overwrite: %s", err.Error())
48+
if err := templatePull(localTemplateRepository, false); err == nil {
49+
t.Fatalf("error expected overwriting existing templates with --overwrite=false:")
6250
}
6351

64-
str := buf.String()
65-
if r.MatchString(str) {
66-
t.Fatal()
52+
if err := templatePull(localTemplateRepository, true); err != nil {
53+
t.Fatalf("unexpected error while executing template pull with --overwrite: %s", err.Error())
6754
}
6855

6956
// Verify created directories

0 commit comments

Comments
 (0)