fix(core): windows cloudwatch agent install script #296
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
horsmand
commented
Jan 22, 2021
packages/aws-rfdk/lib/core/scripts/powershell/configureCloudWatchAgent.ps1
Outdated
Show resolved
Hide resolved
ddneilson
reviewed
Jan 22, 2021
| # Download Amazon's public key and import it to GPG's keyring | ||
| $cloudwatch_pub_key = "$env:temp\amazon-cloudwatch-agent.gpg" | ||
| Read-S3Object -BucketName amazoncloudwatch-agent -Key assets/amazon-cloudwatch-agent.gpg -File $cloudwatch_pub_key -Region us-east-1 | ||
| gpg --import $cloudwatch_pub_key |
There was a problem hiding this comment.
Mind modifying this to use a temporary keyring instead of mutating the default keyring?
See the --no-default-keyring option usage in:
ddneilson
reviewed
Jan 22, 2021
| Exit 1 | ||
| } | ||
| # Checksum information found at https://gnupg.org/download/integrity_check.html | ||
| $sha1 = Get-FileHash $gpg_installer -Algorithm SHA1 |
There was a problem hiding this comment.
sha1 is considered to be insecure due to vulnerability to collision attacks; sha256 is more secure.
Mind manually calculating the sha256 of an installer that you've verified, and putting a sha256 check in here instead?
horsmand
force-pushed
the
cloudwatch-install-fix
branch
2 times, most recently
from
0853f7b
to
0d64add
Compare
ddneilson
previously approved these changes
Jan 22, 2021
jericht
requested changes
Jan 22, 2021
packages/aws-rfdk/lib/core/scripts/powershell/configureCloudWatchAgent.ps1
Outdated
Show resolved
Hide resolved
packages/aws-rfdk/lib/core/scripts/powershell/configureCloudWatchAgent.ps1
Outdated
Show resolved
Hide resolved
horsmand
force-pushed
the
cloudwatch-install-fix
branch
from
0d64add
to
c1136e8
Compare
ddneilson
previously approved these changes
Jan 22, 2021
Fixes #295 This fixes a bug that is preventing the deployment of any worker instance that is using an AMI with a Windows OS.
horsmand
force-pushed
the
cloudwatch-install-fix
branch
from
c1136e8
to
b808303
Compare
jericht
approved these changes
Jan 22, 2021
ddneilson
approved these changes
Jan 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #295
This fixes a bug that is preventing the deployment of any worker
instance that is using an AMI with a Windows OS.
To test this fix, I modified the All-In-AWS-Infrastructure-Basic example app to deploy a Windows worker fleet and made sure it could properly deploy.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license