Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

Added option to use AWS bundled ca cert Fixes #171 #246

Merged
merged 19 commits into from
Jul 26, 2016

Conversation

mattgartman
Copy link
Contributor

No description provided.

@@ -5,6 +5,7 @@ class CLI < Thor
class_option :tfstate, type: :boolean, desc: "Generate tfstate"
class_option :profile, type: :string, desc: "AWS credentials profile"
class_option :region, type: :string, desc: "AWS region"
class_option :"aws-use-bundled-cert", type: :boolean , desc: "Use the bundled CA certificate from AWS SDK"
Copy link
Owner

Choose a reason for hiding this comment

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

[rubocop]

  • Space found before comma.
  • Line is too long. [109/80] :ref

@coveralls
Copy link

coveralls commented Jul 12, 2016

Coverage Status

Coverage increased (+0.003%) to 97.338% when pulling a49118f on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@coveralls
Copy link

coveralls commented Jul 12, 2016

Coverage Status

Coverage increased (+0.003%) to 97.338% when pulling f9e7d91 on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@@ -5,6 +5,9 @@ class CLI < Thor
class_option :tfstate, type: :boolean, desc: "Generate tfstate"
class_option :profile, type: :string, desc: "AWS credentials profile"
class_option :region, type: :string, desc: "AWS region"
class_option :"aws-use-bundled-cert",
Copy link
Owner

Choose a reason for hiding this comment

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

class_option :use_bundled_cert looks better.

Underscores are converted to hyphens automatically.

  [--use-bundled-cert], [--no-use-bundled-cert]  # Use the bundled CA certificate from AWS SDK

@dtan4
Copy link
Owner

dtan4 commented Jul 19, 2016

Sorry for late review 🙇 IMO aws- prefix is unnecessary, the same as region, profile.

response = http.get(uri)

unless response.code == "200"
raise "Error downloading Lambda Code HTTP Res Code #{response.code} from #{url}"
Copy link
Owner

Choose a reason for hiding this comment

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

[rubocop]

  • Line is too long. [92/80] :ref

@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage decreased (-0.9%) to 96.42% when pulling ffad1a6 on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@mattgartman
Copy link
Contributor Author

ugh was working on another branch to add lambda support, must have committed to the wrong branch.

@coveralls
Copy link

coveralls commented Jul 21, 2016

Coverage Status

Coverage increased (+0.01%) to 97.347% when pulling b4e24b3 on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@mattgartman
Copy link
Contributor Author

ok, all better.. removed aws- prefix..

@@ -5,6 +5,9 @@ class CLI < Thor
class_option :tfstate, type: :boolean, desc: "Generate tfstate"
class_option :profile, type: :string, desc: "AWS credentials profile"
class_option :region, type: :string, desc: "AWS region"
class_option :"use-bundled-cert",
Copy link
Owner

Choose a reason for hiding this comment

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

Please use :use_bundled_cert.

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.01%) to 97.347% when pulling e765830 on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@dtan4
Copy link
Owner

dtan4 commented Jul 26, 2016

Just one more thing, please update README. There is still --aws-use-bundled-cert.

@coveralls
Copy link

coveralls commented Jul 26, 2016

Coverage Status

Coverage increased (+0.01%) to 97.347% when pulling 2a17a6f on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@@ -68,7 +68,7 @@ $ terraforming s3 --profile hoge
You can force the AWS SDK to utilize the CA certificate that is bundled with the SDK for systems where the default OpenSSL certificate is not installed (e.g. Windows) by utilizing the `--aws-use-bundled-cert` option.
Copy link
Owner

Choose a reason for hiding this comment

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

☝️ one more ☝️

@coveralls
Copy link

coveralls commented Jul 26, 2016

Coverage Status

Coverage increased (+0.01%) to 97.347% when pulling 21f027f on mattgartman:cert-bundle-fix into 8bc3835 on dtan4:master.

@dtan4
Copy link
Owner

dtan4 commented Jul 26, 2016

LGTM 👍 Thanks ❗

@dtan4 dtan4 merged commit 1596db2 into dtan4:master Jul 26, 2016
@mattgartman
Copy link
Contributor Author

Thanks @dtan4! Sorry for making you triple check this.

@dtan4
Copy link
Owner

dtan4 commented Jul 26, 2016

No problem 👌 ❗

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

Successfully merging this pull request may close these issues.

3 participants