Feat: support addon helm repo skip tls verify (#4122) #4146

TIEDPAG · 2022-06-10T04:33:15Z

Description of your changes

Fixes #4122

I have:

Read and followed KubeVela's contribution process.
Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

add test case using ginkgo

Special notes for your reviewer

codecov · 2022-06-10T04:44:35Z

Codecov Report

Merging #4146 (e76d58a) into master (16dfc1b) will increase coverage by 0.05%.
The diff coverage is 57.89%.

@@            Coverage Diff             @@
##           master    #4146      +/-   ##
==========================================
+ Coverage   61.39%   61.45%   +0.05%     
==========================================
  Files         347      347              
  Lines       34239    34249      +10     
==========================================
+ Hits        21021    21047      +26     
+ Misses      10465    10457       -8     
+ Partials     2753     2745       -8

Flag	Coverage Δ
apiserver-e2etests	`27.20% <31.57%> (-0.03%)`	⬇️
apiserver-unittests	`40.03% <ø> (ø)`
core-unittests	`56.39% <42.10%> (+0.10%)`	⬆️
e2e-multicluster-test	`19.87% <0.00%> (+0.03%)`	⬆️
e2e-rollout-tests	`22.26% <0.00%> (-0.02%)`	⬇️
e2etests	`28.71% <0.00%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/addon/addon.go	`64.43% <0.00%> (-0.09%)`	⬇️
pkg/addon/source.go	`53.03% <ø> (ø)`
pkg/addon/utils.go	`72.15% <0.00%> (-0.42%)`	⬇️
pkg/utils/common/common.go	`52.34% <0.00%> (-0.36%)`	⬇️
pkg/addon/cache.go	`62.96% <100.00%> (+0.46%)`	⬆️
pkg/addon/helper.go	`69.89% <100.00%> (+0.62%)`	⬆️
pkg/apiserver/interfaces/api/oam_application.go	`63.07% <0.00%> (-6.16%)`	⬇️
pkg/resourcekeeper/resourcekeeper.go	`69.69% <0.00%> (-6.07%)`	⬇️
.../manualscalertrait/manualscalertrait_controller.go	`67.30% <0.00%> (-5.77%)`	⬇️
pkg/apiserver/utils/bcode/bcode.go	`50.00% <0.00%> (-2.95%)`	⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16dfc1b...e76d58a. Read the comment docs.

pkg/addon/versioned_registry_test.go

references/cli/addon-registry.go

pkg/utils/common/common.go

sonatype-lift · 2022-06-10T04:58:21Z

pkg/utils/common/common.go

+	if opts.InsecureSkipTls {
+		resp, err = insecureHTTPClient.Do(req)
+	} else {
+		resp, err = http.DefaultClient.Do(req)


bodyclose: response body must be closed

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

pkg/addon/source.go

pkg/utils/common/common.go

sonatype-lift · 2022-06-10T04:58:25Z

pkg/utils/common/common.go

-	resp, err := http.DefaultClient.Do(req)
+	var resp *http.Response
+	if opts.InsecureSkipTls {
+		resp, err = insecureHTTPClient.Do(req)


bodyclose: response body must be closed

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

lgtm-com · 2022-06-10T05:17:32Z

This pull request introduces 1 alert when merging e57ba5a into 6eca997 - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

sonatype-lift · 2022-06-10T06:47:47Z

pkg/addon/cache.go

-		Password: r.Helm.Password,
+		Username:        r.Helm.Username,
+		Password:        r.Helm.Password,
+		InsecureSkipTls: r.Helm.InsecureSkipTls,


typecheck: r.Helm.InsecureSkipTls undefined (type *HelmSource has no field or method InsecureSkipTls)

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

pkg/addon/cache.go

sonatype-lift · 2022-06-10T06:47:50Z

pkg/addon/utils.go

-					Password: registry.Helm.Password,
+					Username:        registry.Helm.Username,
+					Password:        registry.Helm.Password,
+					InsecureSkipTls: registry.Helm.InsecureSkipTls,


typecheck: registry.Helm.InsecureSkipTls undefined (type *HelmSource has no field or method InsecureSkipTls)

Reply with "@sonatype-lift help" for more info.
Reply with "@sonatype-lift ignore" to tell Liftbot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell Liftbot to leave out all the findings from this PR and from the status bar in Github.

When talking to Liftbot, you need to refresh the page to see its response. Click here to get to know more about Liftbot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

pkg/addon/addon.go

pkg/addon/helper.go

lgtm-com · 2022-06-10T07:30:18Z

This pull request introduces 1 alert when merging f183cc3 into 6eca997 - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

StevenLeiZhang

LGTM

wangyikewxgm

Good job! Please fix the CI first.

lgtm-com · 2022-06-13T05:37:36Z

This pull request introduces 1 alert when merging d9a7235 into f978519 - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

wangyikewxgm · 2022-06-15T02:57:37Z

Please sign the dco.

lgtm-com · 2022-06-15T06:52:00Z

This pull request introduces 1 alert when merging 15bb082 into 27ec48b - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

lgtm-com · 2022-06-16T09:32:49Z

This pull request introduces 1 alert when merging 61e987f into 6ed041c - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

lgtm-com · 2022-06-16T11:51:37Z

This pull request introduces 1 alert when merging 958b9c8 into 6ed041c - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

lgtm-com · 2022-06-18T09:45:52Z

This pull request introduces 1 alert when merging b971dd3 into e572235 - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

lgtm-com · 2022-06-18T11:45:56Z

This pull request introduces 1 alert when merging dd85529 into e572235 - view on LGTM.com

new alerts:

1 for Uncontrolled data used in network request

wangyikewxgm · 2022-07-11T05:39:48Z

@TIEDPAG Any progress about this pr？

charlie0129 · 2022-07-14T05:46:14Z

ping @TIEDPAG Hi, conflicts should be resolved, otherwise we cannot merge your PR.

TIEDPAG · 2022-07-15T06:14:55Z

@wangyikewxgm @charlie0129 Are you sure that this pr will be merged? I see it seems that another pr has done the problem

charlie0129 · 2022-07-15T06:18:14Z

You mean this one #4322 ? It seems to me that they solve different problems. Yes, your PR is still needed.

cc @wangyikewxgm for details

TIEDPAG · 2022-07-15T06:21:21Z

ok, I'll deal with it

Signed-off-by: damianqin <damianqin@tiedpag.club>

charlie0129 · 2022-07-16T04:22:56Z

Great! Please run make reviewable to pass CI.

Signed-off-by: damianqin <damianqin@tiedpag.club>

charlie0129

LGTM

charlie0129 · 2022-07-19T04:18:08Z

@TIEDPAG Hi, sorry there are conflicts again. Would you please take a look at it?

Signed-off-by: Jianbo Sun <jianbo.sjb@alibaba-inc.com>

TIEDPAG · 2022-07-19T07:32:53Z

@TIEDPAG Hi, sorry there are conflicts again. Would you please take a look at it?

@charlie0129 ok

wonderflow · 2022-07-19T07:33:21Z

@TIEDPAG I've merged the conflicts. I'll merge this PR after CI passed

TIEDPAG · 2022-07-19T07:43:49Z

@TIEDPAG I've merged the conflicts. I'll merge this PR after CI passed

@wonderflow ok

wangyikewxgm

LGTM

TIEDPAG requested review from Somefive, zzxwill, StevenLeiZhang, barnettZQG, wonderflow and leejanee as code owners June 10, 2022 04:33