Fix the password display problem when passing the chart link #1281
Conversation
NavesEdu
requested review from
mumoshu,
itscaro,
yxxhero and
xiaomudk
as code owners
NavesEdu
force-pushed
the
fix_chart_display
branch
from
876dd49
to
5641051
Compare
|
@NavesEdu please move the logic into a single func and add some unit tests. Thanks So much. |
NavesEdu
force-pushed
the
fix_chart_display
branch
from
5641051
to
2cdd064
Compare
|
@yxxhero I made the changes that you asked me! |
pkg/state/state.go
Outdated
| @@ -3192,6 +3192,15 @@ func renderValsSecrets(e vals.Evaluator, input ...string) ([]string, error) { | |||
| return output, nil | |||
| } | |||
|
|
|||
| func hideChartCredentials(chartCredentials string) string { | |||
| httpIndex := strings.Index(chartCredentials, "http") | |||
There was a problem hiding this comment.
why did only have http? thanks. @NavesEdu
There was a problem hiding this comment.
Only the charts that were passed the links through the helmfile presented this credentials exposure problem. So I throught it would be better to hide everything from "http" to "@".
There was a problem hiding this comment.
Fixed @yxxhero, thank you!
There was a problem hiding this comment.
The result looks a bit strange. sorry for the error display.
http://***@example.com or https://***@example will be better.
There was a problem hiding this comment.
I left the output at the default you asked for above, is there still anything I need to change @yxxhero?
There was a problem hiding this comment.
No more. Please add more test cases. @NavesEdu
pkg/state/state_test.go
Outdated
| input string | ||
| expected string | ||
| }{ | ||
| {"http://example.com@user", "http*****@user"}, |
There was a problem hiding this comment.
Normal url style should like this: http://username:password@example.com/. right? thanks so much. @NavesEdu
There was a problem hiding this comment.
Yes, I already change this in the test.
NavesEdu
force-pushed
the
fix_chart_display
branch
3 times, most recently
from
5b2ae8e
to
f3422ec
Compare
pkg/state/state_test.go
Outdated
| expected string | ||
| }{ | ||
| {"http://username:password@example.com/", "http@example.com/"}, | ||
| {"http://example.com@", "http@"}, |
There was a problem hiding this comment.
There was a problem hiding this comment.
Done! Also made a improvement to the function I created, thanks @yxxhero!
chart link Signed-off-by: Eduardo Naves <eduardonaves41@gmail.com>
NavesEdu
force-pushed
the
fix_chart_display
branch
from
f3422ec
to
272e981
Compare
| {"https://username::password@example.com/", "http@example.com/"}, | ||
| {"https://username:httpd@example.com/", "http@example.com/"}, | ||
| {"https://username:httpsd@example.com/", "http@example.com/"}, | ||
| } |
There was a problem hiding this comment.
pkg/state/state.go
Outdated
| @@ -3192,6 +3192,15 @@ func renderValsSecrets(e vals.Evaluator, input ...string) ([]string, error) { | |||
| return output, nil | |||
| } | |||
|
|
|||
| func hideChartCredentials(chartCredentials string) string { | |||
There was a problem hiding this comment.
url.Parse is better?
doc: https://gobyexample.com/url-parsing
|
@NavesEdu any updates? I wish this can be contained in v0.161.0. I will release v0.161.0 in ths weekend. |
chart link Signed-off-by: Eduardo Naves <eduardonaves41@gmail.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
|
@NavesEdu WDYT of the new code? |
I had a problem passing a chart's credentials through a helmfile.yaml.
When I ran the following commands:
I could see in the output table, the credentials passed within the helmfile.yaml. So, I did a treatment under the
DisplayAffectedReleasesfunction to make sure the credentials woudn't be exposed.