Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix null reference when no env vars defined in yml #25

Merged
merged 6 commits into from
Aug 19, 2020
+29 −8
Merged

Fix null reference when no env vars defined in yml #25

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0ee4548
fix null reference when no env vars defined in yml
davidsiaw Apr 2, 2020
c9750c1
remove useless
davidsiaw Apr 9, 2020
e7d2fce
fix nullref
davidsiaw Apr 9, 2020
0c4e857
fix possible missing name
davidsiaw May 14, 2020
ee13ad1
do the recommended thing
davidsiaw Aug 19, 2020
be727b6
fix errors
davidsiaw Aug 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions cmd/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,24 @@ var RunCommand = cli.Command{
heritageName := c.String("heritage-name")
detach := c.Bool("detach")
envVars := c.StringSlice("envvar")
envVarMap := make(map[string]string)
envVarMap, loadEnvVarMapErr := loadEnvVars(envName)

if loadEnvVarMapErr != nil {
return cli.NewExitError(loadEnvVarMapErr.Error(), 1)
}

if len(envName) > 0 && len(heritageName) > 0 {
return cli.NewExitError("environment and heritage-name are exclusive", 1)
}
if len(envName) > 0 {

if len(heritageName) == 0 {
env, err := LoadEnvironment(envName)
if err != nil {
return cli.NewExitError(err.Error(), 1)
}
if err != nil {
return cli.NewExitError(err.Error(), 1)
}
heritageName = env.Name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like we might still need this heritageName = env.Name part.

Later in the file, it does this

api.DefaultClient.Post("/heritages/"+heritageName+"/oneoffs", bytes.NewBuffer(j))

which might not won't work right if len(envName) > 0 && len(heritageName) == 0.

Does that sound right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that code looks awfully suspicious. You are right. I'll try and fix it in a sane way

for k, v := range env.RunEnv.Vars {
envVarMap[k] = v
}
}

if len(envVars) > 0 {
varmap, err := checkEnvVars(envVars)
if err != nil {
Expand All @@ -72,6 +76,7 @@ var RunCommand = cli.Command{
envVarMap[k] = v
}
}

if len(c.Args()) == 0 {
return cli.NewExitError("Command is required", 1)
}
Expand Down Expand Up @@ -160,6 +165,22 @@ var RunCommand = cli.Command{
},
}

func loadEnvVars(envName string) (map[string]string, error) {
result := make(map[string]string)
if len(envName) > 0 {
env, err := LoadEnvironment(envName)
if err != nil {
return nil, err
}
if env.RunEnv != nil {
for k, v := range env.RunEnv.Vars {
result[k] = v
}
}
}
return result, nil
}

func checkEnvVars(envvarSlice []string) (map[string]string, error) {
var result = make(map[string]string)

Expand Down