camptocamp / terraboard

:earth_africa: :clipboard: A web dashboard to inspect Terraform States
https://terraboard.io
Apache License 2.0
1.93k stars 162 forks source link

Feat/add Tencent Cloud Object Storage (COS) support #258

Closed lyu571 closed 1 year ago

lyu571 commented 1 year ago

Hi @hbollon , I've updated my codes based on your comments in previous PR(https://github.com/camptocamp/terraboard/pull/255). 1.added UT for COS. 2.updated document. make update-docs update the README.md 3.removed the commented code.

Pls start your review if you are free :)

hbollon commented 1 year ago

Hi @lyu571 Thanks for the update! I will try to have a look quickly :+1:

hbollon commented 1 year ago

So apparently the TestCosGetLocksWithNoLocks test case is failing with the following output:

--- FAIL: TestCosGetLocksWithNoLocks (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
    panic: runtime error: index out of range [0] with length 0

goroutine 36 [running]:
testing.tRunner.func1.2({0x14afa40, 0xc00004d470})
    /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1209 +0x24e
testing.tRunner.func1()
    /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1212 +0x218
panic({0x14afa40, 0xc00004d470})
    /opt/hostedtoolcache/go/1.17.13/x64/src/runtime/panic.go:1038 +0x215
github.com/camptocamp/terraboard/state.TestCosGetLocksWithNoLocks(0xc0000add40)
    /home/runner/work/terraboard/terraboard/state/cos_test.go:142 +0x325
testing.tRunner(0xc0000add40, 0x15d1348)
    /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1259 +0x102
created by testing.(*T).Run
    /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1306 +0x35a
FAIL    github.com/camptocamp/terraboard/state  0.054s

There is also some linter's issues (you can easily run golangci-lint locally with the lint command from the Makefile):

  Running [/home/runner/golangci-lint-1.40.1-linux-amd64/golangci-lint run --out-format=github-actions] in [] ...
  Warning: var-naming: struct field SecretId should be SecretID (revive)
  Warning: var-naming: var baseUrl should be baseURL (revive)
  Warning: var-naming: var verId should be verID (revive)
  Warning: cyclomatic: function Configure has cyclomatic complexity 19 (revive)
  Warning: exported: comment on exported function UseTencentCosClient should be of the form "UseTencentCosClient ..." (revive)

Can you have a look?

lyu571 commented 1 year ago

So apparently the TestCosGetLocksWithNoLocks test case is failing with the following output:

--- FAIL: TestCosGetLocksWithNoLocks (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
  panic: runtime error: index out of range [0] with length 0

goroutine 36 [running]:
testing.tRunner.func1.2({0x14afa40, 0xc00004d470})
  /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1209 +0x24e
testing.tRunner.func1()
  /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1212 +0x218
panic({0x14afa40, 0xc00004d470})
  /opt/hostedtoolcache/go/1.17.13/x64/src/runtime/panic.go:1038 +0x215
github.com/camptocamp/terraboard/state.TestCosGetLocksWithNoLocks(0xc0000add40)
  /home/runner/work/terraboard/terraboard/state/cos_test.go:142 +0x325
testing.tRunner(0xc0000add40, 0x15d1348)
  /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1259 +0x102
created by testing.(*T).Run
  /opt/hostedtoolcache/go/1.17.13/x64/src/testing/testing.go:1306 +0x35a
FAIL  github.com/camptocamp/terraboard/state  0.054s

There is also some linter's issues (you can easily run golangci-lint locally with the lint command from the Makefile):

  Running [/home/runner/golangci-lint-1.40.1-linux-amd64/golangci-lint run --out-format=github-actions] in [] ...
  Warning: var-naming: struct field SecretId should be SecretID (revive)
  Warning: var-naming: var baseUrl should be baseURL (revive)
  Warning: var-naming: var verId should be verID (revive)
  Warning: cyclomatic: function Configure has cyclomatic complexity 19 (revive)
  Warning: exported: comment on exported function UseTencentCosClient should be of the form "UseTencentCosClient ..." (revive)

Can you have a look?

Hi @hbollon, Thank you for your comments so quick. I've updated my PR. for the issue 1: I‘ve tested &passed all the cases of cos_test.go with real AKSK(set in ENV) and Bucket before. so, added the AKSK with default and fixed these following runtime error. However, it still cannot pass all cases with the mock AKSK.

for the issue2: fixed all the golangci-lint issues. And...i refactored the Configure method to avoid the too deep cyclomatic complexity, i'm not sure terraboard can support multi-provider(i mean the providers param) or not? So, i kept the previous logic while refactoring.

hbollon commented 1 year ago

Hi @lyu571 Sadly I cannot start reviewing that PR since all tests aren't passing. Atm we didn't have any Tencent COS ressources so I cannot test on my side. Additionally, it remains an issue on the golangci-lint side (bad formatting for a function comment block which must be of the form <function_name> ... to be correctly generated in the documentation).

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.