fix golints G114 and package-comments #5037

shalier · 2022-08-24T20:52:22Z

Description:
changes:

Suppress for demo apps G114: Use of net/http serve function that has no support for setting timeouts
Ignore package-comments, known false positive https://golangci-lint.run/usage/false-positives/#exc0015
Change golangci-lint timeout to 20m
Remove pkg/mesh and moves isValidUUID() to pkg/cli/proxy_get.go
Remove unused pkg/k8s/fake folder

Signed-off-by: Shalier Xia shalierxia@microsoft.com

Testing done:

Affected area:

Functional Area
Other	[x ]

Please answer the following questions with yes/no.

Does this change contain code from or inspired by another project?
- Did you notify the maintainers and provide attribution?
Is this a breaking change?
Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?

codecov-commenter · 2022-08-24T21:11:01Z

Codecov Report

Merging #5037 (45286c8) into main (2d94011) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5037      +/-   ##
==========================================
- Coverage   68.71%   68.69%   -0.03%     
==========================================
  Files         215      215              
  Lines       16118    16118              
==========================================
- Hits        11076    11072       -4     
- Misses       4982     4986       +4     
  Partials       60       60

Flag	Coverage Δ
unittests	`68.69% <ø> (-0.03%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/certificate/castorage/k8s/k8s.go	`81.25% <ø> (ø)`
pkg/ticker/ticker.go	`86.66% <ø> (ø)`
pkg/workerpool/workerpool.go	`100.00% <ø> (ø)`
pkg/messaging/workqueue.go	`89.28% <0.00%> (-10.72%)`	⬇️
pkg/certificate/manager.go	`78.05% <0.00%> (-0.36%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

demo/cmd/bookbuyer/bookbuyer.go

jsturtevant · 2022-08-24T23:19:16Z

I think this linter issue occurs because we are running latest linter in CI. There is lots of value to that, but the UI is pretty rough since it doesn't show you all the linter failures in one go and it takes a really long time locally. I fixed a bunch of these last month #4969.

We have the timeout set to 10m in the

osm/.golangci.yml

Line 3 in 157a15e

timeout: 10m

but looks like it only running for 5. maybe add args: --timeout=10m to

osm/.github/workflows/main.yml

Lines 55 to 58 in 157a15e

    
           uses: golangci/golangci-lint-action@v3 
        
           with: 
        
             version: latest 
        
             skip-pkg-cache: true

and it will not timeout?

Thanks for addressing them @shalier, this was frustrating when I did it last month :-/

jsturtevant · 2022-08-25T20:21:32Z

That did make it run for 10 mins but looks like it still timed out :-(

golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

Signed-off-by: Shalier Xia shalierxia@microsoft.com Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

.github/workflows/main.yml

pkg/cli/proxy_get.go

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

Fix ingress backend broken SAN (openservicemesh#4914) Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN. This adds an e2e test to catch that going forward. This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain) Signed-off-by: nshankar13 <nshankar@microsoft.com> [backport] cherry-pick 961c865 to release-v1.2 fix golints G114 and package-comments (openservicemesh#5037) golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

Fix ingress backend broken SAN (openservicemesh#4914) Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN. This adds an e2e test to catch that going forward. This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain) Signed-off-by: nshankar13 <nshankar@microsoft.com> [backport] cherry-pick 961c865 to release-v1.2 fix golints G114 and package-comments (openservicemesh#5037) golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com> [backport] cherry-pick df502a7 to release-v1.2 bump version of go to 1.19 (openservicemesh#4972) Signed-off-by: Sean Teeling <seanteeling@microsoft.com>

Fix ingress backend broken SAN (openservicemesh#4914) Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN. This adds an e2e test to catch that going forward. This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain) [backport] cherry-pick 961c865 to release-v1.2 fix golints G114 and package-comments (openservicemesh#5037) golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

Fix ingress backend broken SAN (openservicemesh#4914) Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN. This adds an e2e test to catch that going forward. This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain) [backport] cherry-pick 961c865 to release-v1.2 fix golints G114 and package-comments (openservicemesh#5037) golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com> Signed-off-by: nshankar13 <nshankar@microsoft.com>

shalier requested review from shashankram, snehachhabria, nojnhuh, draychev, jaellio, trstringer, keithmattix and steeling as code owners August 24, 2022 20:52

shashankram reviewed Aug 24, 2022

View reviewed changes

demo/cmd/bookbuyer/bookbuyer.go Outdated Show resolved Hide resolved

shalier force-pushed the g114Lint branch 2 times, most recently from df6264c to 138c030 Compare August 24, 2022 22:25

shalier force-pushed the g114Lint branch from 3e686b3 to fa3ac5f Compare August 25, 2022 20:07

shalier force-pushed the g114Lint branch 2 times, most recently from 17fb7e4 to 4e9f5bb Compare August 26, 2022 18:26

shalier added 3 commits August 26, 2022 11:26

fix golints G114 and package-comments

fddffb7

golints addressed: 1. G114: Use of net/http serve function that has no support for setting timeouts 2. package-comments 3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

change the timeout to 20m, address current lints

9a5fc20

Signed-off-by: Shalier Xia shalierxia@microsoft.com Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

ignore package-comments lint error

6269656

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

shalier force-pushed the g114Lint branch from 4e9f5bb to 6269656 Compare August 26, 2022 18:26

shalier requested a review from shashankram August 26, 2022 18:58

keithmattix reviewed Aug 26, 2022

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

pkg/cli/proxy_get.go Outdated Show resolved Hide resolved

rm timeout arg from main yaml, revert pkg/mesh changes

45286c8

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

shalier requested a review from keithmattix August 26, 2022 20:00

keithmattix approved these changes Aug 26, 2022

View reviewed changes

shashankram approved these changes Aug 26, 2022

View reviewed changes

shashankram merged commit 961c865 into openservicemesh:main Aug 26, 2022

shalier deleted the g114Lint branch November 16, 2022 00:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix golints G114 and package-comments #5037

fix golints G114 and package-comments #5037

Uh oh!

shalier commented Aug 24, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 24, 2022 •

edited

Loading

Uh oh!

Uh oh!

jsturtevant commented Aug 24, 2022

Uh oh!

jsturtevant commented Aug 25, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix golints G114 and package-comments #5037

fix golints G114 and package-comments #5037

Uh oh!

Conversation

shalier commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jsturtevant commented Aug 24, 2022

Uh oh!

jsturtevant commented Aug 25, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shalier commented Aug 24, 2022 •

edited

Loading

codecov-commenter commented Aug 24, 2022 •

edited

Loading