From 8bd16510bc6476edbe6ab593786feefbe2fb8f6f Mon Sep 17 00:00:00 2001 From: James Smith Date: Tue, 31 Jul 2018 03:20:38 -0500 Subject: [PATCH] Implement Golang template for stack file Append `latest` tag to all newly created functions. This fixes a bug where images being pushed without a tag will update all tags of the image. Now, only the current tag will be pushed. Fix is implemented with Golang template to allow for greater flexibility. Fixes: openfaas/faas#779 Signed-off-by: James Smith --- commands/faas.go | 1 + commands/new_function.go | 143 +++++++++++++-------------- commands/new_function_test.go | 176 ++++++++++++++++++++++------------ 3 files changed, 186 insertions(+), 134 deletions(-) diff --git a/commands/faas.go b/commands/faas.go index cba45f043..b47719224 100644 --- a/commands/faas.go +++ b/commands/faas.go @@ -49,6 +49,7 @@ func resetForTest() { filter = "" version.Version = "" shortVersion = false + appendFile = "" } func init() { diff --git a/commands/new_function.go b/commands/new_function.go index 89886bfe5..39419db4a 100644 --- a/commands/new_function.go +++ b/commands/new_function.go @@ -11,6 +11,7 @@ import ( "regexp" "sort" "strings" + "text/template" "github.com/openfaas/faas-cli/builder" "github.com/openfaas/faas-cli/stack" @@ -55,21 +56,20 @@ func validateFunctionName(functionName string) error { // Regex for RFC-1123 validation: // k8s.io/kubernetes/pkg/util/validation/validation.go var validDNS = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) - matched := validDNS.MatchString(functionName) - if matched { - return nil + if matched := validDNS.MatchString(functionName); !matched { + return fmt.Errorf(`function name can only contain a-z, 0-9 and dashes`) } - return fmt.Errorf(`function name can only contain a-z, 0-9 and dashes`) + return nil } // preRunNewFunction validates args & flags func preRunNewFunction(cmd *cobra.Command, args []string) error { - language, _ = validateLanguageFlag(language) - if list == true { return nil } + language, _ = validateLanguageFlag(language) + if len(args) < 1 { return fmt.Errorf("please provide a name for the function") } @@ -91,7 +91,6 @@ func runNewFunction(cmd *cobra.Command, args []string) error { var availableTemplates []string templateFolders, err := ioutil.ReadDir(templateDirectory) - if err != nil { return fmt.Errorf("no language templates were found. Please run 'faas-cli template pull'") } @@ -109,10 +108,11 @@ func runNewFunction(cmd *cobra.Command, args []string) error { PullTemplates(DefaultTemplateRepository) - if stack.IsValidTemplate(language) == false { + if !stack.IsValidTemplate(language) { return fmt.Errorf("%s is unavailable or not supported", language) } + var services *stack.Services appendMode := len(appendFile) > 0 if appendMode { if (strings.HasSuffix(appendFile, ".yml") || strings.HasSuffix(appendFile, ".yaml")) == false { @@ -123,91 +123,97 @@ func runNewFunction(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to find file: %s - %s", appendFile, statErr.Error()) } - duplicateError := duplicateFunctionName(functionName, appendFile) + var duplicateError error + services, duplicateError = duplicateFunctionName(functionName, appendFile) if duplicateError != nil { return duplicateError } + } else { + services = &stack.Services{ + Provider: stack.Provider{ + Name: "faas", + GatewayURL: gateway, + }, + Functions: make(map[string]stack.Function), + } } if _, err := os.Stat(functionName); err == nil { return fmt.Errorf("folder: %s already exists", functionName) } - if err := os.Mkdir(functionName, 0700); err == nil { - fmt.Printf("Folder: %s created.\n", functionName) - } else { + if err := os.Mkdir(functionName, 0700); err != nil { return fmt.Errorf("folder: could not create %s : %s", functionName, err) } + fmt.Printf("Folder: %s created.\n", functionName) if err := updateGitignore(); err != nil { return fmt.Errorf("got unexpected error while updating .gitignore file: %s", err) } - var imageName string - imagePrefix = strings.TrimSpace(imagePrefix) - if len(imagePrefix) > 0 { - imageName = imagePrefix + "/" + functionName - } else { - imageName = functionName - } - + // Create function directory from template. builder.CopyFiles(filepath.Join("template", language, "function"), functionName) + printFiglet() + fmt.Printf("\nFunction created in folder: %s\n", functionName) - var stackYaml string - - if !appendMode { - stackYaml += - `provider: - name: faas - gateway: ` + gateway + ` + // Define template of stack file. + const stackTmpl = `{{ if .Provider.Name -}} +provider: + name: {{ .Provider.Name }} + gateway: {{ .Provider.GatewayURL }} functions: -` - } - - stackYaml += - ` ` + functionName + `: - lang: ` + language + ` - handler: ./` + functionName + ` - image: ` + imageName + ` +{{- end }} +{{- range $name, $function := .Functions }} + {{ $name }}: + lang: {{ $function.Language }} + handler: ./{{ $name }} + image: {{ $function.Image }} +{{- end }} ` - printFiglet() - fmt.Println() - fmt.Printf("Function created in folder: %s\n", functionName) + var imageName string + if imagePrefix = strings.TrimSpace(imagePrefix); len(imagePrefix) > 0 { + imageName = fmt.Sprintf("%s/%s:latest", imagePrefix, functionName) + } else { + imageName = fmt.Sprintf("%s:latest", functionName) + } - var stackWriteErr error + function := stack.Function{ + Name: functionName, + Language: language, + Image: imageName, + } + services.Functions[functionName] = function + var fileName string if appendMode { - originalBytes, readErr := ioutil.ReadFile(appendFile) - if readErr != nil { - fmt.Printf("unable to read %s to append, %s", appendFile, readErr) - } - - buffer := string(originalBytes) + stackYaml - - stackWriteErr = ioutil.WriteFile(appendFile, []byte(buffer), 0600) - if stackWriteErr != nil { - return fmt.Errorf("error writing stack file %s", stackWriteErr) - } - - fmt.Printf("Stack file updated: %s\n", appendFile) + fileName = appendFile } else { + fileName = functionName + ".yml" + } + f, err := os.OpenFile("./"+fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + if err != nil { + return fmt.Errorf("could not open file '%s' %s", fileName, err) + } - stackWriteErr = ioutil.WriteFile("./"+functionName+".yml", []byte(stackYaml), 0600) - if stackWriteErr != nil { - return fmt.Errorf("error writing stack file %s", stackWriteErr) - } + t := template.Must(template.New("stack").Parse(stackTmpl)) + if err := t.Execute(f, services); err != nil { + return fmt.Errorf("could not parse functions into stack template %s", err) + } - fmt.Printf("Stack file written: %s\n", functionName+".yml") + if appendMode { + fmt.Printf("Stack file updated: %s\n", fileName) + } else { + fmt.Printf("Stack file written: %s\n", fileName) } if !quiet { languageTemplate, _ := stack.LoadLanguageTemplate(language) if languageTemplate.WelcomeMessage != "" { - fmt.Println("\nNotes:") + fmt.Printf("\nNotes:\n") fmt.Printf("%s\n", languageTemplate.WelcomeMessage) } } @@ -217,37 +223,32 @@ functions: func printAvailableTemplates(availableTemplates []string) string { var result string - sort.Sort(StrSort(availableTemplates)) + sort.Slice(availableTemplates, func(i, j int) bool { + return availableTemplates[i] < availableTemplates[j] + }) for _, template := range availableTemplates { result += fmt.Sprintf("- %s\n", template) } return result } -func duplicateFunctionName(functionName string, appendFile string) error { +func duplicateFunctionName(functionName string, appendFile string) (*stack.Services, error) { fileBytes, readErr := ioutil.ReadFile(appendFile) if readErr != nil { - return fmt.Errorf("unable to read %s to append, %s", appendFile, readErr) + return nil, fmt.Errorf("unable to read %s to append, %s", appendFile, readErr) } services, parseErr := stack.ParseYAMLData(fileBytes, "", "") if parseErr != nil { - return fmt.Errorf("Error parsing %s yml file", appendFile) + return nil, fmt.Errorf("Error parsing %s yml file", appendFile) } if _, ok := services.Functions[functionName]; ok { - return fmt.Errorf(` + return nil, fmt.Errorf(` Function %s already exists in %s file. Cannot have duplicate function names in same yml file`, functionName, appendFile) } - return nil + return services, nil } - -// StrSort sort strings -type StrSort []string - -func (a StrSort) Len() int { return len(a) } -func (a StrSort) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a StrSort) Less(i, j int) bool { return a[i] < a[j] } diff --git a/commands/new_function_test.go b/commands/new_function_test.go index 77597fd33..7945b2993 100644 --- a/commands/new_function_test.go +++ b/commands/new_function_test.go @@ -5,7 +5,6 @@ package commands import ( "fmt" - "io/ioutil" "os" "reflect" "regexp" @@ -16,16 +15,23 @@ import ( "github.com/openfaas/faas-cli/test" ) -const SuccessMsg = `(?m:Function created in folder)` -const InvalidYAMLMsg = `is not valid YAML` -const InvalidYAMLMap = `map is empty` -const IncludeUpperCase = "function name can only contain a-z, 0-9 and dashes" -const ListOptionOutput = `Languages available as templates: +const ( + SuccessMsg = `(?m:Function created in folder)` + InvalidYAMLMsg = `is not valid YAML` + InvalidYAMLMap = `map is empty` + IncludeUpperCase = "function name can only contain a-z, 0-9 and dashes" + NoFunctionName = "please provide a name for the function" + NoLanguage = "you must supply a function language with the --lang flag" + NoTemplates = "no language templates were found. Please run 'faas-cli template pull'" + InvalidFileSuffix = "when appending to a stack the suffix should be .yml or .yaml" + InvalidFile = "unable to find file: (.+)? - (.+)?" + ListOptionOutput = `Languages available as templates: - dockerfile - ruby` -const LangNotExistsOutput = `(?m:is unavailable or not supported)` -const FunctionExistsOutput = `(Function (.+)? already exists in (.+)? file)` + LangNotExistsOutput = `(?m:is unavailable or not supported)` + FunctionExistsOutput = `(Function (.+)? already exists in (.+)? file)` +) type NewFunctionTest struct { title string @@ -41,21 +47,21 @@ var NewFunctionTests = []NewFunctionTest{ title: "new_1", funcName: "new-test-1", funcLang: "ruby", - expectedImage: "new-test-1", + expectedImage: "new-test-1:latest", expectedMsg: SuccessMsg, }, { title: "lowercase-dockerfile", funcName: "lowercase-dockerfile", funcLang: "dockerfile", - expectedImage: "lowercase-dockerfile", + expectedImage: "lowercase-dockerfile:latest", expectedMsg: SuccessMsg, }, { title: "uppercase-dockerfile", funcName: "uppercase-dockerfile", funcLang: "dockerfile", - expectedImage: "uppercase-dockerfile", + expectedImage: "uppercase-dockerfile:latest", expectedMsg: SuccessMsg, }, { @@ -63,7 +69,7 @@ var NewFunctionTests = []NewFunctionTest{ funcName: "func-with-prefix", prefix: " username ", funcLang: "dockerfile", - expectedImage: "username/func-with-prefix", + expectedImage: "username/func-with-prefix:latest", expectedMsg: SuccessMsg, }, { @@ -71,7 +77,7 @@ var NewFunctionTests = []NewFunctionTest{ funcName: "func-with-whitespace-only-prefix", prefix: " ", funcLang: "dockerfile", - expectedImage: "func-with-whitespace-only-prefix", + expectedImage: "func-with-whitespace-only-prefix:latest", expectedMsg: SuccessMsg, }, { @@ -86,6 +92,18 @@ var NewFunctionTests = []NewFunctionTest{ funcLang: "dockerfile", expectedMsg: IncludeUpperCase, }, + { + title: "no-function-name", + funcName: "", + funcLang: "", + expectedMsg: NoFunctionName, + }, + { + title: "no-language", + funcName: "no-language", + funcLang: "", + expectedMsg: NoLanguage, + }, } func runNewFunctionTest(t *testing.T, nft NewFunctionTest) { @@ -95,19 +113,15 @@ func runNewFunctionTest(t *testing.T, nft NewFunctionTest) { var funcYAML string funcYAML = funcName + ".yml" - // Cleanup the created directory - defer func() { - os.RemoveAll(funcName) - os.Remove(funcYAML) - }() - cmdParameters := []string{ "new", - funcName, "--lang=" + funcLang, "--gateway=" + defaultGateway, "--prefix=" + imagePrefix, } + if len(funcName) != 0 { + cmdParameters = append(cmdParameters, funcName) + } faasCmd.SetArgs(cmdParameters) fmt.Println("Executing command") @@ -162,10 +176,10 @@ func Test_newFunctionTests(t *testing.T) { // Download templates templatePullLocalTemplateRepo(t) defer tearDownFetchTemplates(t) - defer tearDownNewFunction(t) for _, testcase := range NewFunctionTests { t.Run(testcase.title, func(t *testing.T) { + defer tearDownNewFunction(t, testcase.funcName) runNewFunctionTest(t, testcase) }) } @@ -175,7 +189,6 @@ func Test_newFunctionListCmds(t *testing.T) { // Download templates templatePullLocalTemplateRepo(t) defer tearDownFetchTemplates(t) - defer tearDownNewFunction(t) cmdParameters := []string{ "new", @@ -187,17 +200,31 @@ func Test_newFunctionListCmds(t *testing.T) { faasCmd.Execute() }) - // Validate new function output + // Validate command output if !strings.HasPrefix(stdOut, ListOptionOutput) { t.Fatalf("Output is not as expected: %s\n", stdOut) } } +func Test_newFunctionListNoTemplates(t *testing.T) { + cmdParameters := []string{ + "new", + "--list", + } + + faasCmd.SetArgs(cmdParameters) + stdOut := faasCmd.Execute().Error() + + // Validate command output + if !strings.HasPrefix(stdOut, NoTemplates) { + t.Fatalf("Output is not as expected: %s\n", stdOut) + } +} + func Test_languageNotExists(t *testing.T) { // Download templates templatePullLocalTemplateRepo(t) defer tearDownFetchTemplates(t) - defer tearDownNewFunction(t) // Attempt to create a function with a non-existing language cmdParameters := []string{ @@ -217,70 +244,93 @@ func Test_languageNotExists(t *testing.T) { } } -func Test_duplicateFunctionName(t *testing.T) { +func Test_appendInvalidSuffix(t *testing.T) { + const functionName = "samplefunc" + const functionLang = "ruby" + templatePullLocalTemplateRepo(t) defer tearDownFetchTemplates(t) - defer tearDownNewFunction(t) + // Create function + parameters := []string{ + "new", + functionName, + "--lang=" + functionLang, + "--append=" + functionName + ".txt", + } + faasCmd.SetArgs(parameters) + stdOut := faasCmd.Execute().Error() + + if found, err := regexp.MatchString(InvalidFileSuffix, stdOut); err != nil || !found { + t.Fatalf("Output is not as expected: %s\n", stdOut) + } +} + +func Test_appendInvalidFile(t *testing.T) { const functionName = "samplefunc" const functionLang = "ruby" - defer func() { - if _, err := os.Stat(functionName + ".yml"); err == nil { - err := os.Remove(functionName + ".yml") - if err != nil { - t.Log(err) - } - } - }() - - // Create a yml file with the same function name and language - writeFunctionYmlFile(functionName, functionLang, t) + templatePullLocalTemplateRepo(t) + defer tearDownFetchTemplates(t) - appendParameters := []string{ + // Create function + parameters := []string{ "new", functionName, "--lang=" + functionLang, - "--append=" + functionName + ".yml", + "--append=" + functionLang + ".yml", } - - faasCmd.SetArgs(appendParameters) + faasCmd.SetArgs(parameters) stdOut := faasCmd.Execute().Error() - if found, err := regexp.MatchString(FunctionExistsOutput, stdOut); err != nil || !found { + if found, err := regexp.MatchString(InvalidFile, stdOut); err != nil || !found { t.Fatalf("Output is not as expected: %s\n", stdOut) } } -func writeFunctionYmlFile(functionName string, lang string, t *testing.T) { - var stackYaml string +func Test_duplicateFunctionName(t *testing.T) { + resetForTest() - stackYaml += - `provider: - name: faas - gateway: ` + defaultGateway + ` + const functionName = "samplefunc" + const functionLang = "ruby" + + templatePullLocalTemplateRepo(t) + defer tearDownFetchTemplates(t) + defer tearDownNewFunction(t, functionName) -functions: -` + // Create function + parameters := []string{ + "new", + functionName, + "--lang=" + functionLang, + } + faasCmd.SetArgs(parameters) + faasCmd.Execute() - stackYaml += - ` ` + functionName + `: - lang: ` + lang + ` - handler: ./` + functionName + ` - image: ` + functionName + ` -` + // Attempt to create duplicate function + parameters = append(parameters, "--append="+functionName+".yml") + faasCmd.SetArgs(parameters) + stdOut := faasCmd.Execute().Error() - stackWriteErr := ioutil.WriteFile("./"+functionName+".yml", []byte(stackYaml), 0600) - if stackWriteErr != nil { - t.Log(fmt.Errorf("error writing stack file %s", stackWriteErr)) + if found, err := regexp.MatchString(FunctionExistsOutput, stdOut); err != nil || !found { + t.Fatalf("Output is not as expected: %s\n", stdOut) } } -func tearDownNewFunction(t *testing.T) { - // Remove existing archive file if it exists +func tearDownNewFunction(t *testing.T, functionName string) { if _, err := os.Stat(".gitignore"); err == nil { - err := os.Remove(".gitignore") - if err != nil { + if err := os.Remove(".gitignore"); err != nil { + t.Log(err) + } + } + if _, err := os.Stat(functionName); err == nil { + if err := os.RemoveAll(functionName); err != nil { + t.Log(err) + } + } + functionYaml := functionName + ".yml" + if _, err := os.Stat(functionYaml); err == nil { + if err := os.Remove(functionYaml); err != nil { t.Log(err) } }