Fix deployment variable list --save with secrets
#1558
Conversation
ashb
requested review from
neel-astro,
kushalmalani,
sunkickr and
vandyliu
as code owners
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1558 +/- ##
==========================================
+ Coverage 86.16% 86.18% +0.01%
==========================================
Files 112 112
Lines 14952 14944 -8
==========================================
- Hits 12883 12879 -4
+ Misses 1251 1248 -3
+ Partials 818 817 -1 ☔ View full report in Codecov by Sentry. |
|
This will need backporting to 1.23 too (it applies cleanly) |
| varTab.AddRow([]string{strconv.Itoa(nbEnvVarFound), environmentVariablesObjects[i].Key, printValue, strconv.FormatBool(environmentVariablesObjects[i].IsSecret)}, false) | ||
| for i, variable := range environmentVariablesObjects { | ||
| if variable.Value != nil && !variable.IsSecret { | ||
| printValue = *variable.Value |
There was a problem hiding this comment.
do we not have to reset this to ****
eg. the first variable is not secret then the next one is, won't do the next printValue be the same as the first one?
There was a problem hiding this comment.
if that is indeed the case, we should add a test case to cover that
if it's not, maybe you can help me fill in the missing gaps im missing
There was a problem hiding this comment.
seems to be working as intended
There was a problem hiding this comment.
what if theres another secret var after the third one
There was a problem hiding this comment.
Agree with Vandy's comment here, can we just declare a temporary variable inside the loop? A unit test case is required here.
There was a problem hiding this comment.
I didn't actually change this behaviour -- before it was
printValue = *environmentVariablesObjects[i].ValueI agree though, while we're here lets fix it
ashb
force-pushed
the
variables-save-with-secrets-dont-panic
branch
from
53adbfa
to
2ea288c
Compare
After the migration to the REST API in #1425 it wasn't dealing with secret variables (which don't have a value anymore). And while I'm there do a bit of tidying up: - `_, x := range` instead of slice index access - Remove the use of `environmentVariablesObjects` global variable! - Refactoring of selecting the variable key to one place only (rather than in multiple)
ashb
force-pushed
the
variables-save-with-secrets-dont-panic
branch
from
2ea288c
to
022430f
Compare
Description
After the migration to the REST API in #1425 it wasn't dealing with secret variables (which don't have a value anymore).
And while I'm there do a bit of tidying up:
_, x := rangeinstead of slice index accessenvironmentVariablesObjectsglobal variable!than in multiple)
🎟 Issue(s)
Related #XXX
🧪 Functional Testing
I ran the command in question on a deployment with secret variables
📋 Checklist
make testbefore taking out of draftmake lintbefore taking out of draft