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.

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

AwsEc2Cache Refresh() fix #6

Merged
merged 1 commit into from
Dec 17, 2018
Merged

Conversation

faja
Copy link
Contributor

@faja faja commented Dec 14, 2018

Currently Refresh() function creates new AwsEc2Provider each time it is
executed. Morover it sets AwsConfig field to an "empty" aws.Config{}
causing aws.Config.Region to be . This forces REGION to be taken
from ~/.aws/config profile instead of .goship.yaml config file.

It could be fixed by:
"AwsConfig: config" -> "AwsConfig: g.Provider.AwsConfig":

-       config := aws.Config{}
-       provider := providers.AwsEc2Provider{AwsConfig: config, AwsRegion: g.Provider.AwsRegion, AwsProfileName: g.Provider.AwsProfileName}
+       provider := providers.AwsEc2Provider{AwsConfig: g.Provider.AwsConfig, AwsRegion: g.Provider.AwsRegion, AwsProfileName: g.Provider.AwsProfileName}

or (imo) even better to simply reuse existing provider without creating
a new struct. And that's what I'm suggesting in this commit.

Currently Refresh() function creates new AwsEc2Provider each time it is
executed. Morover it sets AwsConfig field to an "empty" aws.Config{}
causing aws.Config.Region to be <nil>. This forces REGION to be taken
from ~/.aws/config profile instead of .goship.yaml config file.

It could be fixed by:
"AwsConfig: config" -> "AwsConfig: g.Provider.AwsConfig":
```
-       config := aws.Config{}
-       provider := providers.AwsEc2Provider{AwsConfig: config, AwsRegion: g.Provider.AwsRegion, AwsProfileName: g.Provider.AwsProfileName}
+       provider := providers.AwsEc2Provider{AwsConfig: g.Provider.AwsConfig, AwsRegion: g.Provider.AwsRegion, AwsProfileName: g.Provider.AwsProfileName}
```
or (imo) even better to simply reuse existing provider without creating
a new struct. And that's what I'm suggesting in this commit.
@emate emate merged commit d3d7565 into zendesk:master Dec 17, 2018
@emate
Copy link
Contributor

emate commented Dec 17, 2018

thanks @faja!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants