Skip to content

fix(template): Option with value as "on" is converted to true#1857

Merged
mergify[bot] merged 2 commits intoaws:mainlinefrom
kahirul:escape-option-value
Jan 21, 2021
Merged

fix(template): Option with value as "on" is converted to true#1857
mergify[bot] merged 2 commits intoaws:mainlinefrom
kahirul:escape-option-value

Conversation

@kahirul
Copy link
Copy Markdown
Contributor

@kahirul kahirul commented Jan 21, 2021

logging:
  image: amazon/aws-for-fluent-bit:latest
  destination:
    Name: datadog
    TLS: on

will produce something like "TLS": "true"

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kahirul kahirul requested a review from a team as a code owner January 21, 2021 05:09
@kahirul kahirul requested a review from uttarasridhar January 21, 2021 05:09
@kahirul kahirul changed the title Option with value as "on" is converted to true fix(template): Option with value as "on" is converted to true Jan 21, 2021
@iamhopaul123
Copy link
Copy Markdown
Contributor

Hello @kahirul. I'm using the latest binary and it seems like when I specified TLS: on, the CFN template was rendered correctly as following:

          LogConfiguration:
            LogDriver: awsfirelens
            Options:
              Name: datadog
              TLS: on
        - Name: firelens_log_router
          Image: amazon/aws-for-fluent-bit:latest
          FirelensConfiguration:
            Type: fluentbit
            Options:
              enable-ecs-log-metadata: true
              config-file-type: file
              config-file-value: /fluent-bit/configs/parse-json.conf

I was wondering how you produced "TLS": "true". Was it in the CFN template?

@kahirul
Copy link
Copy Markdown
Contributor Author

kahirul commented Jan 21, 2021

I was wondering how you produced "TLS": "true". Was it in the CFN template?

You're right. Both CFN template and copilot svc package output TLS: on but final task definition (json) have "TLS": "true"

Copy link
Copy Markdown
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.

Ship it! Thank you so much for contributing and fixing this bug!

@mergify mergify Bot merged commit 26aade8 into aws:mainline Jan 21, 2021
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
```
logging:
  image: amazon/aws-for-fluent-bit:latest
  destination:
    Name: datadog
    TLS: on
```
will produce something like `"TLS": "true"`

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
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.

4 participants