From dc132a153de419b14ae66f0eca2090181e72365c Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 10:33:34 -0700 Subject: [PATCH 01/22] Add -f/--file flag to "app add" command --- cmd/argocd/commands/app.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 21b244088484c..1f8f14ad96869 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -41,6 +41,7 @@ func NewApplicationCommand(clientOpts *argocdclient.ClientOptions) *cobra.Comman // NewApplicationAddCommand returns a new instance of an `argocd app add` command func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { var ( + fileURL string repoURL string appPath string env string @@ -79,6 +80,7 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com errors.CheckError(err) }, } + command.Flags().StringVarP(&fileURL, "file", "f", "", "Filename, directory, or URL to Kubernetes manifests for the app") command.Flags().StringVar(&repoURL, "repo", "", "Repository URL") errors.CheckError(command.MarkFlagRequired("repo")) command.Flags().StringVar(&appPath, "path", "", "Path in repository to the ksonnet app directory") From 798e1c0ff3f5fcabed7755c2833c461bbd7f6d23 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 13:45:29 -0700 Subject: [PATCH 02/22] Add basic readLocalFile method --- cmd/argocd/commands/util.go | 15 +++++++++++++++ cmd/argocd/commands/util_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 cmd/argocd/commands/util.go create mode 100644 cmd/argocd/commands/util_test.go diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go new file mode 100644 index 0000000000000..11e5b379bf2ce --- /dev/null +++ b/cmd/argocd/commands/util.go @@ -0,0 +1,15 @@ +package commands + +import ( + "io/ioutil" + "log" +) + +// readLocalFile reads a file from disk and returns its contents as a string. +func readLocalFile(path string) string { + data, err := ioutil.ReadFile(path) + if err != nil { + log.Fatal(err) + } + return string(data) +} diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go new file mode 100644 index 0000000000000..f831b77a09a6f --- /dev/null +++ b/cmd/argocd/commands/util_test.go @@ -0,0 +1,27 @@ +package commands + +import ( + "io/ioutil" + "os" + "testing" +) + +func TestReadLocalFile(t *testing.T) { + sentinel := "Hello, world!" + + file, err := ioutil.TempFile(os.TempDir(), "") + if err != nil { + t.Errorf("Could not write test data") + } + defer os.Remove(file.Name()) + + file.WriteString(sentinel) + file.Sync() + + data := readLocalFile(file.Name()) + if data != sentinel { + t.Errorf("Test data did not match! Expected \"%s\" and received \"%s\"", sentinel, data) + } + + +} From b276d77c0dc0b3d889d9b02fbfe4807d0750ffac Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 13:54:10 -0700 Subject: [PATCH 03/22] Don't ignore return values --- cmd/argocd/commands/util_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index f831b77a09a6f..645f9c172f53d 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -13,15 +13,15 @@ func TestReadLocalFile(t *testing.T) { if err != nil { t.Errorf("Could not write test data") } - defer os.Remove(file.Name()) + defer func() { + _ = os.Remove(file.Name()) + }() - file.WriteString(sentinel) - file.Sync() + _, _ = file.WriteString(sentinel) + _ = file.Sync() data := readLocalFile(file.Name()) if data != sentinel { t.Errorf("Test data did not match! Expected \"%s\" and received \"%s\"", sentinel, data) } - - } From 15543229825d75807ebd7195a4b6434c13af852d Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 14:31:37 -0700 Subject: [PATCH 04/22] Don't read local file into string --- cmd/argocd/commands/util.go | 4 ++-- cmd/argocd/commands/util_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 11e5b379bf2ce..5b12a2310a1f7 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -6,10 +6,10 @@ import ( ) // readLocalFile reads a file from disk and returns its contents as a string. -func readLocalFile(path string) string { +func readLocalFile(path string) []byte { data, err := ioutil.ReadFile(path) if err != nil { log.Fatal(err) } - return string(data) + return data } diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index 645f9c172f53d..b7ecfc9c5e7ea 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -21,7 +21,7 @@ func TestReadLocalFile(t *testing.T) { _ = file.Sync() data := readLocalFile(file.Name()) - if data != sentinel { - t.Errorf("Test data did not match! Expected \"%s\" and received \"%s\"", sentinel, data) + if string(data) != sentinel { + t.Errorf("Test data did not match! Expected \"%s\" and received \"%s\"", sentinel, string(data)) } } From 77e3800eca9b88fd08ea0ef0b5ad19cb407c978e Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 14:33:51 -0700 Subject: [PATCH 05/22] Unmarshal read file once it's read successfully --- cmd/argocd/commands/app.go | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 1f8f14ad96869..59d5051a2eda1 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -3,6 +3,8 @@ package commands import ( "context" "fmt" + "gopkg.in/yaml.v2" + "log" "os" "text/tabwriter" @@ -56,17 +58,27 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com c.HelpFunc()(c, args) os.Exit(1) } - app := argoappv1.Application{ - ObjectMeta: metav1.ObjectMeta{ - Name: args[0], - }, - Spec: argoappv1.ApplicationSpec{ - Source: argoappv1.ApplicationSource{ - RepoURL: repoURL, - Path: appPath, - Environment: env, + var app argoappv1.Application + if fileURL != "" { + fileContents := readLocalFile(fileURL) + err := yaml.Unmarshal(fileContents, &app) + if err != nil { + log.Fatal(err) + os.Exit(1) + } + } else { + app = argoappv1.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: args[0], }, - }, + Spec: argoappv1.ApplicationSpec{ + Source: argoappv1.ApplicationSource{ + RepoURL: repoURL, + Path: appPath, + Environment: env, + }, + }, + } } if destServer != "" || destNamespace != "" { app.Spec.Destination = &argoappv1.ApplicationDestination{ From 7fc4bf162e8d58cbd98c7891ad44f4ee6b860af6 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 14:49:33 -0700 Subject: [PATCH 06/22] Update flag requirements to allow file flag --- cmd/argocd/commands/app.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 59d5051a2eda1..46f27dc14d6bf 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -46,18 +46,15 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com fileURL string repoURL string appPath string + appName string env string destServer string destNamespace string ) var command = &cobra.Command{ Use: "add", - Short: fmt.Sprintf("%s app add APPNAME", cliName), + Short: fmt.Sprintf("%s app add", cliName), Run: func(c *cobra.Command, args []string) { - if len(args) != 1 { - c.HelpFunc()(c, args) - os.Exit(1) - } var app argoappv1.Application if fileURL != "" { fileContents := readLocalFile(fileURL) @@ -67,9 +64,14 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com os.Exit(1) } } else { + // all these params are required if we're here + if repoURL == "" || appPath == "" || appName == "" { + c.HelpFunc()(c, args) + os.Exit(1) + } app = argoappv1.Application{ ObjectMeta: metav1.ObjectMeta{ - Name: args[0], + Name: appName, }, Spec: argoappv1.ApplicationSpec{ Source: argoappv1.ApplicationSource{ @@ -93,10 +95,9 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com }, } command.Flags().StringVarP(&fileURL, "file", "f", "", "Filename, directory, or URL to Kubernetes manifests for the app") - command.Flags().StringVar(&repoURL, "repo", "", "Repository URL") - errors.CheckError(command.MarkFlagRequired("repo")) - command.Flags().StringVar(&appPath, "path", "", "Path in repository to the ksonnet app directory") - errors.CheckError(command.MarkFlagRequired("path")) + command.Flags().StringVar(&appName, "name", "", "A name for the app, ignored if a file is set") + command.Flags().StringVar(&repoURL, "repo", "", "Repository URL, ignored if a file is set") + command.Flags().StringVar(&appPath, "path", "", "Path in repository to the ksonnet app directory, ignored if a file is set") command.Flags().StringVar(&env, "env", "", "Application environment to monitor") command.Flags().StringVar(&destServer, "dest-server", "", "K8s cluster URL (overrides the server URL specified in the ksonnet app.yaml)") command.Flags().StringVar(&destNamespace, "dest-namespace", "", "K8s target namespace (overrides the namespace specified in the ksonnet app.yaml)") From c68173cc76552d875da862c86ac60adaae7bcfb8 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 14:51:03 -0700 Subject: [PATCH 07/22] Re-add initial usage test --- cmd/argocd/commands/app.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 46f27dc14d6bf..7db1fb79bda5d 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -55,6 +55,10 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com Use: "add", Short: fmt.Sprintf("%s app add", cliName), Run: func(c *cobra.Command, args []string) { + if len(args) != 0 { + c.HelpFunc()(c, args) + os.Exit(1) + } var app argoappv1.Application if fileURL != "" { fileContents := readLocalFile(fileURL) From 89d88799d27b8ee0f862e67d0bf7b616d64413e0 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 15:40:30 -0700 Subject: [PATCH 08/22] Add functions, tests for reading remote file --- cmd/argocd/commands/util.go | 26 +++++++++++++++++---- cmd/argocd/commands/util_test.go | 40 +++++++++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 5b12a2310a1f7..266e449520301 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -3,13 +3,31 @@ package commands import ( "io/ioutil" "log" + "net/http" ) -// readLocalFile reads a file from disk and returns its contents as a string. -func readLocalFile(path string) []byte { - data, err := ioutil.ReadFile(path) +// readLocalFile reads a file from disk and returns its contents as a byte array. +func readLocalFile(path string) (data []byte, err error) { + data, err = ioutil.ReadFile(path) if err != nil { log.Fatal(err) } - return data + return +} + +// readRemoteFile issues a GET request to retrieve the contents of the specified URL as a byte array. +func readRemoteFile(url string) (data []byte, err error) { + resp, err := http.Get(url) + if err != nil { + log.Fatal(err) + } else { + defer func() { + _ = resp.Body.Close() + }() + data, err = ioutil.ReadAll(resp.Body) + if err != nil { + log.Fatal(err) + } + } + return } diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index b7ecfc9c5e7ea..f1d912875c4d1 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -1,7 +1,10 @@ package commands import ( + "fmt" "io/ioutil" + "net" + "net/http" "os" "testing" ) @@ -11,7 +14,7 @@ func TestReadLocalFile(t *testing.T) { file, err := ioutil.TempFile(os.TempDir(), "") if err != nil { - t.Errorf("Could not write test data") + panic(err) } defer func() { _ = os.Remove(file.Name()) @@ -20,8 +23,39 @@ func TestReadLocalFile(t *testing.T) { _, _ = file.WriteString(sentinel) _ = file.Sync() - data := readLocalFile(file.Name()) + data, err := readLocalFile(file.Name()) if string(data) != sentinel { - t.Errorf("Test data did not match! Expected \"%s\" and received \"%s\"", sentinel, string(data)) + t.Errorf("Test data did not match (err = %v)! Expected \"%s\" and received \"%s\"", err, sentinel, string(data)) + } +} + +func TestReadRemoteFile(t *testing.T) { + sentinel := "Hello, world!" + + server := func(c chan<- string) { + listener, err := net.Listen("tcp", ":0") + if err != nil { + panic(err) + } + + c <- listener.Addr().String() + + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, sentinel) + }) + + panic(http.Serve(listener, nil)) + } + + c := make(chan string, 1) + + // run a local webserver to test data retrieval + go server(c) + + address := <-c + data, err := readRemoteFile("http://" + address) + t.Logf("Listening at address: %s", address) + if string(data) != sentinel { + t.Errorf("Test data did not match (err = %v)! Expected \"%s\" and received \"%s\"", err, sentinel, string(data)) } } From 232b88535b8db3ccaeafe1d361ac327f9dc34c2c Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 15:41:19 -0700 Subject: [PATCH 09/22] Add ability to load file remotely in app.go --- cmd/argocd/commands/app.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 7db1fb79bda5d..af92d565fe49b 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -6,6 +6,7 @@ import ( "gopkg.in/yaml.v2" "log" "os" + "strings" "text/tabwriter" "github.com/argoproj/argo-cd/errors" @@ -61,8 +62,20 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com } var app argoappv1.Application if fileURL != "" { - fileContents := readLocalFile(fileURL) - err := yaml.Unmarshal(fileContents, &app) + var ( + fileContents []byte + err error + ) + if strings.HasPrefix(fileURL, "https://") || strings.HasPrefix(fileURL, "http://") { + fileContents, err = readRemoteFile(fileURL) + } else { + fileContents, err = readLocalFile(fileURL) + } + if err != nil { + log.Fatal(err) + os.Exit(1) + } + err = yaml.Unmarshal(fileContents, &app) if err != nil { log.Fatal(err) os.Exit(1) From c7f19bb56f7de91c90ca2061c2de013ffbd4bc9d Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 15:50:41 -0700 Subject: [PATCH 10/22] Don't need extra spaghetti logic when using log.Fatal --- cmd/argocd/commands/util.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 266e449520301..7532e87b9c1ef 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -20,14 +20,13 @@ func readRemoteFile(url string) (data []byte, err error) { resp, err := http.Get(url) if err != nil { log.Fatal(err) - } else { - defer func() { - _ = resp.Body.Close() - }() - data, err = ioutil.ReadAll(resp.Body) - if err != nil { - log.Fatal(err) - } + } + defer func() { + _ = resp.Body.Close() + }() + data, err = ioutil.ReadAll(resp.Body) + if err != nil { + log.Fatal(err) } return } From f7dfe33fb0ba993c0bfa24baebbb0484ae3b1d49 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 16:05:41 -0700 Subject: [PATCH 11/22] Support both JSON and YAML --- cmd/argocd/commands/app.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index af92d565fe49b..f0353b7a10744 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -2,6 +2,7 @@ package commands import ( "context" + "encoding/json" "fmt" "gopkg.in/yaml.v2" "log" @@ -75,10 +76,15 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com log.Fatal(err) os.Exit(1) } - err = yaml.Unmarshal(fileContents, &app) + + // first, try unmarshaling as JSON + err = json.Unmarshal(fileContents, &app) if err != nil { - log.Fatal(err) - os.Exit(1) + // next, try unmarshaling as YAML + err = yaml.Unmarshal(fileContents, &app) + if err != nil { + log.Fatalf("Could not decode valid JSON or YAML from Kubernetes manifest: %s", fileURL) + } } } else { // all these params are required if we're here From 5a76c1e4996b11bed0c52b0091a3a2a9d1abacbc Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 16:28:24 -0700 Subject: [PATCH 12/22] Handle JSON and YAML properly now --- cmd/argocd/commands/app.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index f0353b7a10744..20b82aed6e1b2 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -4,7 +4,7 @@ import ( "context" "encoding/json" "fmt" - "gopkg.in/yaml.v2" + "github.com/ghodss/yaml" "log" "os" "strings" @@ -78,13 +78,17 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com } // first, try unmarshaling as JSON + // Based on technique from Kubectl, which supports both YAML and JSON: + // https://mlafeldt.github.io/blog/teaching-go-programs-to-love-json-and-yaml/ + // Short version: JSON unmarshaling won't zero out null fields; YAML unmarshaling will. + // This may have unintended effects or hard-to-catch issues when populating our application object. + fileContents, err = yaml.YAMLToJSON(fileContents) + if err != nil { + log.Fatalf("Could not decode valid JSON or YAML from Kubernetes manifest: %s", fileURL) + } err = json.Unmarshal(fileContents, &app) if err != nil { - // next, try unmarshaling as YAML - err = yaml.Unmarshal(fileContents, &app) - if err != nil { - log.Fatalf("Could not decode valid JSON or YAML from Kubernetes manifest: %s", fileURL) - } + log.Fatalf("Could not unmarshal Kubrnetes manifest: %s", fileURL) } } else { // all these params are required if we're here From b0e2cf7e44e3a8612215a926e240103f7db25664 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 18:18:09 -0700 Subject: [PATCH 13/22] Add URL validation function and test case --- cmd/argocd/commands/app.go | 3 +-- cmd/argocd/commands/util.go | 11 +++++++++++ cmd/argocd/commands/util_test.go | 20 ++++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 20b82aed6e1b2..e8cdd5749a4a9 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -7,7 +7,6 @@ import ( "github.com/ghodss/yaml" "log" "os" - "strings" "text/tabwriter" "github.com/argoproj/argo-cd/errors" @@ -67,7 +66,7 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com fileContents []byte err error ) - if strings.HasPrefix(fileURL, "https://") || strings.HasPrefix(fileURL, "http://") { + if hasSupportedManifestURLScheme(fileURL) { fileContents, err = readRemoteFile(fileURL) } else { fileContents, err = readLocalFile(fileURL) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 7532e87b9c1ef..45fc7e22c9b9a 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -4,8 +4,19 @@ import ( "io/ioutil" "log" "net/http" + "strings" ) +// isSupportedURL checks if a URL is of a supported type for loading manifests. +func hasSupportedManifestURLScheme(url string) bool { + for _, scheme := range []string{"https://", "http://"} { + if lowercaseUrl := strings.ToLower(url); strings.HasPrefix(lowercaseUrl, scheme) { + return true + } + } + return false +} + // readLocalFile reads a file from disk and returns its contents as a byte array. func readLocalFile(path string) (data []byte, err error) { data, err = ioutil.ReadFile(path) diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index f1d912875c4d1..85bc3881732d9 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -9,6 +9,26 @@ import ( "testing" ) +func TestHasSupportedManifestURLScheme(t *testing.T) { + data := []struct { + input string + expected bool + }{ + {"http://www.example.com", true}, + {"HTTP://www.EXAMPLE.com", true}, + {"HTTPS://localhost:8443", true}, + {"http://www.example.org", true}, + {"ftp://www.example.com", false}, + {"gopher://gopher.something/", false}, + {"file:///etc/passwd", false}, + } + for _, datum := range data { + if output := hasSupportedManifestURLScheme(datum.input); output != datum.expected { + t.Errorf("Invalid output for URL \"%s\"; was expecting %v and received %v", datum.input, datum.expected, output) + } + } +} + func TestReadLocalFile(t *testing.T) { sentinel := "Hello, world!" From 1cc0575bdfdf3a69bb59eb06a710022fec2094e2 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Tue, 13 Mar 2018 18:19:06 -0700 Subject: [PATCH 14/22] Fix typo, blame @merenbach --- cmd/argocd/commands/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index e8cdd5749a4a9..e675bab7df1ac 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -87,7 +87,7 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com } err = json.Unmarshal(fileContents, &app) if err != nil { - log.Fatalf("Could not unmarshal Kubrnetes manifest: %s", fileURL) + log.Fatalf("Could not unmarshal Kubernetes manifest: %s", fileURL) } } else { // all these params are required if we're here From 6f26e2e1d3cdf89e703f22b9f6d2f291e3f532ce Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 09:16:31 -0700 Subject: [PATCH 15/22] Factor unmarshaling into util.go; no dir support --- cmd/argocd/commands/app.go | 18 ++---------------- cmd/argocd/commands/util.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index e675bab7df1ac..679fbe3ce20bd 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -2,9 +2,7 @@ package commands import ( "context" - "encoding/json" "fmt" - "github.com/ghodss/yaml" "log" "os" "text/tabwriter" @@ -75,20 +73,8 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com log.Fatal(err) os.Exit(1) } + unmarshalApplication(fileContents, &app) - // first, try unmarshaling as JSON - // Based on technique from Kubectl, which supports both YAML and JSON: - // https://mlafeldt.github.io/blog/teaching-go-programs-to-love-json-and-yaml/ - // Short version: JSON unmarshaling won't zero out null fields; YAML unmarshaling will. - // This may have unintended effects or hard-to-catch issues when populating our application object. - fileContents, err = yaml.YAMLToJSON(fileContents) - if err != nil { - log.Fatalf("Could not decode valid JSON or YAML from Kubernetes manifest: %s", fileURL) - } - err = json.Unmarshal(fileContents, &app) - if err != nil { - log.Fatalf("Could not unmarshal Kubernetes manifest: %s", fileURL) - } } else { // all these params are required if we're here if repoURL == "" || appPath == "" || appName == "" { @@ -120,7 +106,7 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com errors.CheckError(err) }, } - command.Flags().StringVarP(&fileURL, "file", "f", "", "Filename, directory, or URL to Kubernetes manifests for the app") + command.Flags().StringVarP(&fileURL, "file", "f", "", "Filename or URL to Kubernetes manifests for the app") command.Flags().StringVar(&appName, "name", "", "A name for the app, ignored if a file is set") command.Flags().StringVar(&repoURL, "repo", "", "Repository URL, ignored if a file is set") command.Flags().StringVar(&appPath, "path", "", "Path in repository to the ksonnet app directory, ignored if a file is set") diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 45fc7e22c9b9a..72e498c2f0afc 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -1,12 +1,33 @@ package commands import ( + "encoding/json" + argoappv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" + "github.com/ghodss/yaml" "io/ioutil" "log" "net/http" "strings" ) +// unmarshalApplication tries to convert a YAML or JSON byte array into an Application struct. +func unmarshalApplication(data []byte, app *argoappv1.Application) { + // first, try unmarshaling as JSON + // Based on technique from Kubectl, which supports both YAML and JSON: + // https://mlafeldt.github.io/blog/teaching-go-programs-to-love-json-and-yaml/ + // http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/ + // Short version: JSON unmarshaling won't zero out null fields; YAML unmarshaling will. + // This may have unintended effects or hard-to-catch issues when populating our application object. + data, err := yaml.YAMLToJSON(data) + if err != nil { + log.Fatalf("Could not decode valid JSON or YAML from Kubernetes manifest: %s", string(data)) + } + err = json.Unmarshal(data, &app) + if err != nil { + log.Fatalf("Could not unmarshal Kubernetes manifest: %s", string(data)) + } +} + // isSupportedURL checks if a URL is of a supported type for loading manifests. func hasSupportedManifestURLScheme(url string) bool { for _, scheme := range []string{"https://", "http://"} { From ffe93476a14fbbd0ab799af03247f4f141269f98 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 09:17:32 -0700 Subject: [PATCH 16/22] Fix casing on var name --- cmd/argocd/commands/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 72e498c2f0afc..11569bf373177 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -31,7 +31,7 @@ func unmarshalApplication(data []byte, app *argoappv1.Application) { // isSupportedURL checks if a URL is of a supported type for loading manifests. func hasSupportedManifestURLScheme(url string) bool { for _, scheme := range []string{"https://", "http://"} { - if lowercaseUrl := strings.ToLower(url); strings.HasPrefix(lowercaseUrl, scheme) { + if lowercaseURL := strings.ToLower(url); strings.HasPrefix(lowercaseURL, scheme) { return true } } From 44aa67588a390003cf8d02bcf894726f8107683c Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 09:54:09 -0700 Subject: [PATCH 17/22] Use URL validation instead of scheme checking --- cmd/argocd/commands/app.go | 8 +++++--- cmd/argocd/commands/util.go | 31 ++++++++----------------------- cmd/argocd/commands/util_test.go | 20 -------------------- 3 files changed, 13 insertions(+), 46 deletions(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 679fbe3ce20bd..8af203352028d 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "net/url" "os" "text/tabwriter" @@ -64,10 +65,11 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com fileContents []byte err error ) - if hasSupportedManifestURLScheme(fileURL) { - fileContents, err = readRemoteFile(fileURL) - } else { + _, err = url.ParseRequestURI(fileURL) + if err != nil { fileContents, err = readLocalFile(fileURL) + } else { + fileContents, err = readRemoteFile(fileURL) } if err != nil { log.Fatal(err) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 11569bf373177..189ad96994699 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -20,7 +20,7 @@ func unmarshalApplication(data []byte, app *argoappv1.Application) { // This may have unintended effects or hard-to-catch issues when populating our application object. data, err := yaml.YAMLToJSON(data) if err != nil { - log.Fatalf("Could not decode valid JSON or YAML from Kubernetes manifest: %s", string(data)) + log.Fatal("Could not decode valid JSON or YAML Kubernetes manifest") } err = json.Unmarshal(data, &app) if err != nil { @@ -28,37 +28,22 @@ func unmarshalApplication(data []byte, app *argoappv1.Application) { } } -// isSupportedURL checks if a URL is of a supported type for loading manifests. -func hasSupportedManifestURLScheme(url string) bool { - for _, scheme := range []string{"https://", "http://"} { - if lowercaseURL := strings.ToLower(url); strings.HasPrefix(lowercaseURL, scheme) { - return true - } - } - return false -} - // readLocalFile reads a file from disk and returns its contents as a byte array. +// The caller is responsible for checking error return values. func readLocalFile(path string) (data []byte, err error) { data, err = ioutil.ReadFile(path) - if err != nil { - log.Fatal(err) - } return } // readRemoteFile issues a GET request to retrieve the contents of the specified URL as a byte array. +// The caller is responsible for checking error return values. func readRemoteFile(url string) (data []byte, err error) { resp, err := http.Get(url) - if err != nil { - log.Fatal(err) - } - defer func() { - _ = resp.Body.Close() - }() - data, err = ioutil.ReadAll(resp.Body) - if err != nil { - log.Fatal(err) + if err == nil { + defer func() { + _ = resp.Body.Close() + }() + data, err = ioutil.ReadAll(resp.Body) } return } diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index 85bc3881732d9..f1d912875c4d1 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -9,26 +9,6 @@ import ( "testing" ) -func TestHasSupportedManifestURLScheme(t *testing.T) { - data := []struct { - input string - expected bool - }{ - {"http://www.example.com", true}, - {"HTTP://www.EXAMPLE.com", true}, - {"HTTPS://localhost:8443", true}, - {"http://www.example.org", true}, - {"ftp://www.example.com", false}, - {"gopher://gopher.something/", false}, - {"file:///etc/passwd", false}, - } - for _, datum := range data { - if output := hasSupportedManifestURLScheme(datum.input); output != datum.expected { - t.Errorf("Invalid output for URL \"%s\"; was expecting %v and received %v", datum.input, datum.expected, output) - } - } -} - func TestReadLocalFile(t *testing.T) { sentinel := "Hello, world!" From 0041a4a4c867d320074d9373fc6e5b607eaa9eea Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 09:54:43 -0700 Subject: [PATCH 18/22] Rm unused import --- cmd/argocd/commands/util.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 189ad96994699..84565c14f65d8 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "log" "net/http" - "strings" ) // unmarshalApplication tries to convert a YAML or JSON byte array into an Application struct. From a6ba8ab260110d2ec7fe4a0336c21eba255d83be Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 09:56:27 -0700 Subject: [PATCH 19/22] Rename server=>serve to be more idiomatic --- cmd/argocd/commands/util_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index f1d912875c4d1..eea0ef05143b6 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -32,7 +32,7 @@ func TestReadLocalFile(t *testing.T) { func TestReadRemoteFile(t *testing.T) { sentinel := "Hello, world!" - server := func(c chan<- string) { + serve := func(c chan<- string) { listener, err := net.Listen("tcp", ":0") if err != nil { panic(err) @@ -50,7 +50,7 @@ func TestReadRemoteFile(t *testing.T) { c := make(chan string, 1) // run a local webserver to test data retrieval - go server(c) + go serve(c) address := <-c data, err := readRemoteFile("http://" + address) From f3e87e001f13274d378d0a19a4ce363f825a475a Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 10:00:52 -0700 Subject: [PATCH 20/22] Add some comments to util_test.go --- cmd/argocd/commands/util_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go index eea0ef05143b6..cf76c6768784c 100644 --- a/cmd/argocd/commands/util_test.go +++ b/cmd/argocd/commands/util_test.go @@ -33,14 +33,17 @@ func TestReadRemoteFile(t *testing.T) { sentinel := "Hello, world!" serve := func(c chan<- string) { + // listen on first available dynamic (unprivileged) port listener, err := net.Listen("tcp", ":0") if err != nil { panic(err) } + // send back the address so that it can be used c <- listener.Addr().String() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + // return the sentinel text at root URL fmt.Fprint(w, sentinel) }) From 5b5bd7eb5339cdffef60dab91539aeca4a7ac949 Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 13:50:58 -0700 Subject: [PATCH 21/22] Run goimports on util.go, thanks @alexmt --- cmd/argocd/commands/util.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go index 84565c14f65d8..5892364c760e9 100644 --- a/cmd/argocd/commands/util.go +++ b/cmd/argocd/commands/util.go @@ -2,11 +2,12 @@ package commands import ( "encoding/json" - argoappv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" - "github.com/ghodss/yaml" "io/ioutil" "log" "net/http" + + argoappv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" + "github.com/ghodss/yaml" ) // unmarshalApplication tries to convert a YAML or JSON byte array into an Application struct. From 4b9255b182ebc7ddb6d2c44a7e75ff895c9658ec Mon Sep 17 00:00:00 2001 From: Andrew Merenbach Date: Wed, 14 Mar 2018 13:51:49 -0700 Subject: [PATCH 22/22] Rm redundant os.Exit, thanks @alexmt --- cmd/argocd/commands/app.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index 8af203352028d..4b70e019e7b1e 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -73,7 +73,6 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com } if err != nil { log.Fatal(err) - os.Exit(1) } unmarshalApplication(fileContents, &app)