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

fix(aws-ecs): token is added to Options instead of SecretOptions in SplunkLogDriver #15408

Merged
merged 5 commits into from
Jul 9, 2021

Conversation

upparekh
Copy link
Contributor

@upparekh upparekh commented Jul 2, 2021


This PR closes #7264.

The token field of the Splunk log driver populates the Options property of the Log Configuration which leads to the secret being resolved to its value on deploying, and then the token is viewable in plain text in the console and may be stored in plain text elsewhere. Thus, we are deprecating the token field of the Splunk log driver and are introducing a new secretToken field. secretToken can be used to provide the Splunk token as a Secrets Manager Secret or a Systems Manager Parameter and will be populated in the SecretOptions property of the Log Configuration.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Jul 2, 2021

@upparekh upparekh self-assigned this Jul 2, 2021
@upparekh upparekh added @aws-cdk/aws-ecs Related to Amazon Elastic Container pr/do-not-merge This PR should not be merged at this time. labels Jul 2, 2021
iamhopaul123
iamhopaul123 previously approved these changes Jul 6, 2021
Copy link
Contributor

@iamhopaul123 iamhopaul123 left a comment

Choose a reason for hiding this comment

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

LGTM overall just one nit!

@mergify mergify bot dismissed iamhopaul123’s stale review July 7, 2021 17:06

Pull request has been modified.

Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

Please also add validation to the SplunkLogDriver constructor that verifies the user specified at least one of token or secretToken.

@madeline-k madeline-k changed the title fix(ecs): Add secretToken to Splunk log driver fix(aws-ecs): token is added to Options instead of SecretOptions in SplunkLogDriver Jul 7, 2021
@mergify mergify bot dismissed madeline-k’s stale review July 8, 2021 18:14

Pull request has been modified.

@upparekh upparekh force-pushed the upparekh/secret-token-for-splunk branch from 659ceaf to 1b9adf4 Compare July 8, 2021 18:31
@upparekh upparekh force-pushed the upparekh/secret-token-for-splunk branch from 1b9adf4 to e2c7f85 Compare July 8, 2021 18:37
@upparekh upparekh requested a review from madeline-k July 8, 2021 19:56
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

👍

@upparekh upparekh removed the pr/do-not-merge This PR should not be merged at this time. label Jul 9, 2021
@mergify
Copy link
Contributor

mergify bot commented Jul 9, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 3cd2b16
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 23abe22 into aws:master Jul 9, 2021
@mergify
Copy link
Contributor

mergify bot commented Jul 9, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@upparekh upparekh deleted the upparekh/secret-token-for-splunk branch July 9, 2021 19:04
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Aug 3, 2021
…plunkLogDriver (aws#15408)

----
This PR closes [aws#7264](aws#7264).

The `token` field of the Splunk log driver populates the `Options` property of the Log Configuration which leads to the secret being resolved to its value on deploying, and then the token is viewable in plain text in the console and may be stored in plain text elsewhere. Thus, we are deprecating the `token` field of the Splunk log driver and are introducing a new `secretToken` field. `secretToken` can be used to provide the Splunk token as a Secrets Manager Secret or a Systems Manager Parameter and will be populated in the `SecretOptions` property of the Log Configuration. 

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this pull request Aug 26, 2021
…plunkLogDriver (aws#15408)

----
This PR closes [aws#7264](aws#7264).

The `token` field of the Splunk log driver populates the `Options` property of the Log Configuration which leads to the secret being resolved to its value on deploying, and then the token is viewable in plain text in the console and may be stored in plain text elsewhere. Thus, we are deprecating the `token` field of the Splunk log driver and are introducing a new `secretToken` field. `secretToken` can be used to provide the Splunk token as a Secrets Manager Secret or a Systems Manager Parameter and will be populated in the `SecretOptions` property of the Log Configuration. 

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SplunkLogDriverProps adds splunk-token to logging options instead of secretoptions
6 participants