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

Split the environment variable string in half using the first = character #2000

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

asungomez
Copy link

Description of changes

When an environment variable in the docker-compose.yml file of a container contains a = character, such as:

DB_CONNECTION_PARAMETERS=replicaSet=rs0&readPreference=secondaryPreferred&retryWrites=false

It splits the string by the = character and takes the first element as the variable key and the second as the value. For the previous example, it includes the following variable on the CF stack:

{
  "Name": "DB_CONNECTION_PARAMETERS",
  "Value": "replicaSet"
}

This Pull Request fixes that issue by splitting the string in half using the first = sign only, so the resulting environment variable is:

{
  "Name": "DB_CONNECTION_PARAMETERS",
  "Value": "replicaSet=rs0&readPreference=secondaryPreferred&retryWrites=false"
}
CDK / CloudFormation Parameters Changed
  • Fixed the generation of environment variables for ECS Task definitions, so they accept the character = in their value.

Description of how you validated changes

Created a Container instance passing to the constructor an environment variable string with the = sign in the value.

Checklist

  • PR description included
  • yarn test passes

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

@asungomez asungomez requested a review from a team as a code owner October 30, 2023 11:07
@dpilch
Copy link
Contributor

dpilch commented Dec 14, 2023

Hi @asungomez, the change looks good to me. Could you add a unit test to ensure the behavior?

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