-
Notifications
You must be signed in to change notification settings - Fork 70
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 secret environment variables being updated with empty string #1618
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1618 +/- ##
=======================================
Coverage 86.56% 86.56%
=======================================
Files 114 114
Lines 16087 16087
=======================================
Hits 13925 13925
Misses 1297 1297
Partials 865 865 ☔ View full report in Codecov by Sentry. |
envVars = []astroplatformcore.DeploymentEnvironmentVariableRequest{} | ||
for i := range deploymentFromFile.Deployment.EnvVars { | ||
var envVar astroplatformcore.DeploymentEnvironmentVariableRequest | ||
envVar.IsSecret = deploymentFromFile.Deployment.EnvVars[i].IsSecret | ||
envVar.Key = deploymentFromFile.Deployment.EnvVars[i].Key | ||
envVar.Value = &deploymentFromFile.Deployment.EnvVars[i].Value | ||
// If the action is update, isSecret is true and value is empty, then the value should be left as nil | ||
if !(action == updateAction && deploymentFromFile.Deployment.EnvVars[i].IsSecret && deploymentFromFile.Deployment.EnvVars[i].Value == "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean, i cannot add an empty string for a secret env var? Not an ideal use-case but thinking what if i wanted to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that would be a strange case
i don't see why you would want to have a secret variable as ""
anyways if that were the case, i made some updates to the code to allow nil values for no-ops and to allow ""
to be updated
Description
Fix secret environment variables being updated. Nil values for existing secret env vars will no longer update the secret variables but keep them with the same value.
🎟 Issue(s)
#1593
🧪 Functional Testing
📸 Screenshots
Updated the secret variable via
./astro deployment update <deploymentId> --deployment-file deployment.yaml
Saw on airflow UI that the env var did not get set back to
""
📋 Checklist
make test
before taking out of draftmake lint
before taking out of draft