From 179030fffd622c66f29279cabf84105cd3ed4a39 Mon Sep 17 00:00:00 2001
From: Reed Allman <reed@rdallman.com>
Date: Wed, 27 Feb 2019 14:35:19 -0800
Subject: [PATCH] add syslog_url from app.yaml (#543)

right now in the cli the only way to set syslog url is via update or create
with the --syslog-url flag. this adds support in deploy when syslog_url is
present in app.yaml
---
 commands/deploy.go    | 31 +++++++++++++++--------------
 common/appfile.go     |  4 ++--
 test/cli_misc_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/commands/deploy.go b/commands/deploy.go
index 3f7725ca..35bc4bfa 100644
--- a/commands/deploy.go
+++ b/commands/deploy.go
@@ -145,20 +145,24 @@ func (p *deploycmd) deploy(c *cli.Context) error {
 		return errors.New("App name must be provided, try `--app APP_NAME`")
 	}
 
+	// appfApp is used to create/update app, with app file additions if provided
+	appfApp := models.App{
+		Name: appName,
+	}
+	if appf != nil {
+		// set other fields from app file
+		appfApp.Config = appf.Config
+		appfApp.Annotations = appf.Annotations
+		if appf.SyslogURL != "" {
+			// TODO consistent with some other fields (config), unsetting in app.yaml doesn't unset on server. undecided policy for all fields
+			appfApp.SyslogURL = &appf.SyslogURL
+		}
+	}
+
 	// find and create/update app if required
 	app, err := apps.GetAppByName(p.clientV2, appName)
 	if _, ok := err.(apps.NameNotFoundError); ok && p.createApp {
-		app = &models.App{
-			Name: appName,
-		}
-
-		if appf != nil {
-			// set other fields from app file
-			app.Config = appf.Config
-			app.Annotations = appf.Annotations
-		}
-
-		app, err = apps.CreateApp(p.clientV2, app)
+		app, err = apps.CreateApp(p.clientV2, &appfApp)
 		if err != nil {
 			return err
 		}
@@ -166,10 +170,7 @@ func (p *deploycmd) deploy(c *cli.Context) error {
 		return err
 	} else if appf != nil {
 		// app exists, but we need to update it if we have an app file
-		app, err = apps.PutApp(p.clientV2, app.ID, &models.App{
-			Config:      appf.Config,
-			Annotations: appf.Annotations,
-		})
+		app, err = apps.PutApp(p.clientV2, app.ID, &appfApp)
 		if err != nil {
 			return fmt.Errorf("Failed to update app config: %v", err)
 		}
diff --git a/common/appfile.go b/common/appfile.go
index 36330995..27bfbe83 100644
--- a/common/appfile.go
+++ b/common/appfile.go
@@ -23,10 +23,10 @@ var (
 
 // AppFile defines the internal structure of a app.yaml/json/yml
 type AppFile struct {
-	Name string `yaml:"name,omitempty" json:"name,omitempty"`
-	// TODO: Config here is not yet used
+	Name        string                 `yaml:"name,omitempty" json:"name,omitempty"`
 	Config      map[string]string      `yaml:"config,omitempty" json:"config,omitempty"`
 	Annotations map[string]interface{} `yaml:"annotations,omitempty" json:"annotations,omitempty"`
+	SyslogURL   string                 `yaml:"syslog_url,omitempty" json:"syslog_url,omitempty"`
 }
 
 func findAppfile(path string) (string, error) {
diff --git a/test/cli_misc_test.go b/test/cli_misc_test.go
index a1d18e7e..d0ed61df 100644
--- a/test/cli_misc_test.go
+++ b/test/cli_misc_test.go
@@ -200,7 +200,53 @@ func TestAppYamlDeployFailNotExist(t *testing.T) {
 	h.Fn("deploy", "--all", "--local").AssertFailed().AssertStderrContains("app " + appName + " not found")
 }
 
+func TestAppYamlDeployInspect(t *testing.T) {
+	// this test only inspects, does not invoke! for syslog, mostly
+	t.Parallel()
+
+	h := testharness.Create(t)
+	defer h.Cleanup()
+
+	appName := h.NewAppName()
+	fnName := h.NewFuncName(appName)
+	h.WithFile("app.yaml", fmt.Sprintf(`
+name: %s
+syslog_url: tcp://example.com:42
+config:
+  animal: giraffe
+`, appName), 0644)
+	h.MkDir(fnName)
+	h.Cd(fnName)
+	withMinimalFunction(h)
+	h.Cd("")
+	h.Fn("deploy", "--all", "--local", "--create-app").AssertSuccess()
+	// check config from app.yaml is set
+	inspect := h.Fn("inspect", "app", appName).AssertSuccess()
+	inspect.AssertStdoutContains(fmt.Sprintf(`"name": "%s"`, appName))
+	inspect.AssertStdoutContains(`"giraffe"`)
+	inspect.AssertStdoutContains(`"tcp://example.com:42"`)
+
+	// now should exist, this should work too
+	h.WithFile("app.yaml", fmt.Sprintf(`
+name: %s
+syslog_url: tcp://example.com:443
+config:
+  animal: giraffe
+  tea: oolong
+`, appName), 0644)
+	h.Fn("deploy", "--all", "--local").AssertSuccess()
+	// make sure config was updated
+	inspect = h.Fn("inspect", "app", appName).AssertSuccess()
+	inspect.AssertStdoutContains(fmt.Sprintf(`"name": "%s"`, appName))
+	inspect.AssertStdoutContains(`"oolong"`)
+	inspect.AssertStdoutContains(`"tcp://example.com:443"`)
+}
+
 func TestAppYamlDeploy(t *testing.T) {
+	// this test makes sure that functions can be invoked after using app.yaml deploy,
+	// the other test that looks just like this just inspects the function b/c syslog config
+	// in tests is a bad idea (ie don't test syslog here)
+
 	t.Parallel()
 
 	h := testharness.Create(t)