Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

separate cobra constructors: lapi, machines, bouncers, postoverflows #1945

Merged
merged 4 commits into from
Dec 30, 2022

Conversation

mmetc
Copy link
Contributor

@mmetc mmetc commented Dec 23, 2022

I just moved around stuff like I did for "cscli config". The aim is to have

  • smaller functions, fewer indentation levels
  • fewer global variables
  • more testable code (eventually in pkg/ or cmd/internal/)

}
log.Printf("Local API credentials dumped to '%s'", dumpFile)
} else {
fmt.Printf("%s\n", string(apiConfigDump))

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information

[Sensitive data returned by an access to Password](1) flows to a logging call. [Sensitive data returned by an access to password](2) flows to a logging call.
}
log.Printf("API credentials dumped to '%s'", dumpFile)
} else {
fmt.Printf("%s\n", string(apiConfigDump))

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information

[Sensitive data returned by an access to machinePassword](1) flows to a logging call. [Sensitive data returned by an access to machinePassword](2) flows to a logging call. [Sensitive data returned by an access to password](3) flows to a logging call. [Sensitive data returned by an access to Password](4) flows to a logging call. [Sensitive data returned by an access to password](5) flows to a logging call. [Sensitive data returned by an access to password](6) flows to a logging call. [Sensitive data returned by an access to password](3) flows to a logging call.
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2022

Codecov Report

Merging #1945 (2024028) into master (c022eb1) will increase coverage by 0.21%.
The diff coverage is 58.77%.

❗ Current head 2024028 differs from pull request most recent head 75f4e71. Consider uploading reports for the commit 75f4e71 to get more accurate results

@@            Coverage Diff             @@
##           master    #1945      +/-   ##
==========================================
+ Coverage   54.61%   54.83%   +0.21%     
==========================================
  Files         152      156       +4     
  Lines       20149    20622     +473     
==========================================
+ Hits        11005    11308     +303     
- Misses       7961     8081     +120     
- Partials     1183     1233      +50     
Flag Coverage Δ
func-crowdsec 47.00% <ø> (ø)
func-cscli 47.61% <58.77%> (+0.24%) ⬆️
unit-linux 58.67% <ø> (+<0.01%) ⬆️
unit-windows 53.91% <ø> (?)

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

Impacted Files Coverage Δ
cmd/crowdsec-cli/capi.go 53.78% <ø> (ø)
cmd/crowdsec-cli/machines.go 50.31% <55.26%> (+0.49%) ⬆️
cmd/crowdsec-cli/lapi.go 62.19% <58.66%> (+1.74%) ⬆️
cmd/crowdsec-cli/bouncers.go 54.16% <64.83%> (+3.13%) ⬆️
pkg/leakybucket/manager_run.go 67.02% <0.00%> (-1.42%) ⬇️
pkg/cwhub/path_separator_windows.go 53.84% <0.00%> (ø)
pkg/csplugin/utils_windows.go 53.54% <0.00%> (ø)
pkg/acquisition/modules/file/tailline_windows.go 100.00% <0.00%> (ø)
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mmetc mmetc changed the title refactor "cscli lapi", "cscli machines" refactor "cscli lapi", "cscli machines", "cscli bouncers" Dec 23, 2022
@mmetc mmetc marked this pull request as draft December 26, 2022 12:26
@mmetc mmetc changed the title refactor "cscli lapi", "cscli machines", "cscli bouncers" separate cobra constructors: lapi, machines, bouncers, postoverflows Dec 26, 2022
@mmetc mmetc marked this pull request as ready for review December 26, 2022 12:53
@mmetc mmetc added this to the 1.5.0 milestone Dec 29, 2022
Copy link
Contributor

@buixor buixor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mmetc mmetc merged commit 59f6610 into master Dec 30, 2022
@mmetc mmetc deleted the cscli-ref-lapi branch December 30, 2022 09:13
nitescuc added a commit that referenced this pull request Jan 9, 2023
* backoff on refresh token error

* fix tls communication with lapi and user/pw auth (#1956)

allow self-signed TLS encryption with user/pw auth

docker:
 - remove defaults for certificate file locations
 - new envvar INSECURE_SKIP_VERIFY
 - register agent before TLS settings (cscli machine add removes them
   from the credentials file)

* separate cscli cobra constructors:  lapi, machines, bouncers, postoverflows (#1945)

* use feature toggling to improve testability with http retry backoff

* Add parse unix to dateparse enricher (#1958)

Add parse unix is we do have a strTime but wasnt parsed using convential golang time

* func tests: redirect stderr to filter extra logs (#1961)

* backoff on refresh token error

* use feature toggling to improve testability with http retry backoff

* refactor feature backoff toggle for tests

Co-authored-by: mmetc <92726601+mmetc@users.noreply.github.com>
Co-authored-by: Laurence Jones <laurence.jones@live.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants