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

feat: allow variable escaping in manifest #5516

Merged
merged 3 commits into from Dec 21, 2023

Conversation

bencehornak
Copy link
Contributor

@bencehornak bencehornak commented Dec 2, 2023

Allows escaping of interpolated variables:

command: echo hello \${name}
variable:
  name: world

Fixes #5501, fixes #5532

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

@bencehornak bencehornak requested a review from a team as a code owner December 2, 2023 09:45
@bencehornak bencehornak requested review from CaptainCarpensir and removed request for a team December 2, 2023 09:45
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a2c99e0) 70.04% compared to head (b3b92a9) 70.03%.

Additional details and impacted files
@@             Coverage Diff              @@
##           mainline    #5516      +/-   ##
============================================
- Coverage     70.04%   70.03%   -0.01%     
============================================
  Files           302      302              
  Lines         46493    46498       +5     
  Branches        301      301              
============================================
+ Hits          32565    32566       +1     
- Misses        12345    12348       +3     
- Partials       1583     1584       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 3, 2023

🍕 Here are the new binary sizes!

Name New size (kiB) size (kiB) Delta (%)
macOS (amd) 56976 56796 +0.32
macOS (arm) 58000 57796 +0.35
linux (amd) 49960 49804 +0.31
linux (arm) 49280 49092 +0.38
windows (amd) 47088 46940 +0.32

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.

Slick! Thank you for your contribution 🙇

@iamhopaul123 iamhopaul123 changed the title Allow variable escaping in manifest feat: allow variable escaping in manifest Dec 3, 2023
@iamhopaul123 iamhopaul123 added do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. and removed do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. labels Dec 13, 2023
Copy link
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

thank you!!

@mergify mergify bot merged commit 6baf5b9 into aws:mainline Dec 21, 2023
12 checks passed
Sprint 🏃‍♀️ automation moved this from In review to Pending release Dec 21, 2023
@ShanikaEdiriweera
Copy link

Will this fix resolve env variables from container environment?

I need to do environment variable substitution from a value from secret.

variables:
  TEST1: "https://\\${DATABASE_SERVER}:5000/ReportServer?"
  TEST2: https://\${DATABASE_SERVER}:5000/ReportServer?
secrets:                      # Pass secrets from AWS Systems Manager (SSM) Parameter Store.
  DATABASE_SERVER: /database/server

@iamhopaul123
Copy link
Contributor

Hello @ShanikaEdiriweera, I think your use case should be covered by this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Sprint 🏃‍♀️
  
Pending release
5 participants