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

feat: Switch to viper for configuration #70

Merged
merged 11 commits into from
Jul 14, 2023
Merged

Conversation

JoseAngel1196
Copy link
Contributor

@JoseAngel1196 JoseAngel1196 commented Jul 7, 2023

Task

Fixes #32

Description

  • I’m representing the Viper library with a struct and creating the client using a public function called NewViper. To use it, I have implemented struct functions, similar to how we do it in the Slack package. We now have LoadConfig and LoadEnv.
  • When we run the project (e.g., go run scan...), we default the --config flag to the name of the config.toml. I have updated the flag value to include the current working directory (cwd) and the name of the config file. This change shouldn’t break anything, but I had to do it to make the LoadConfig dynamic and easy to test. We can now specify the configPath in the test file.
  • Additionally, I have renamed the TomlConfig to just Config, as this change is applicable to any type of config file.
  • I’m open to suggestions here, but I have created a /testdata folder to include two config files that I can use to test the LoadConfig and LoadEnv methods.

@phylum-io
Copy link

phylum-io bot commented Jul 7, 2023

Phylum OSS Supply Chain Risk Analysis - FAILED

This repository analyzes the risk of new dependencies. An
administrator of this repository has set requirements via Phylum policy.

If you see this comment, one or more dependencies have failed Phylum's risk analysis.

Package: golang.org/x/text@v0.3.3 failed.

golang.org/x/text@v0.3.3 is vulnerable to Denial of service

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

golang.org/x/text@v0.3.3 is vulnerable to golang.org/x/text/language Out-of-bounds Read vulnerability

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: golang.org/x/text@v0.3.4 failed.

golang.org/x/text@v0.3.4 is vulnerable to Denial of service

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

golang.org/x/text@v0.3.4 is vulnerable to golang.org/x/text/language Out-of-bounds Read vulnerability

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: golang.org/x/text@v0.3.6 failed.

golang.org/x/text@v0.3.6 is vulnerable to Denial of service

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

golang.org/x/text@v0.3.6 is vulnerable to golang.org/x/text/language Out-of-bounds Read vulnerability

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: golang.org/x/text@v0.3.7 failed.

golang.org/x/text@v0.3.7 is vulnerable to Denial of service

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.19.0 failed.

google.golang.org/grpc@v1.19.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.20.1 failed.

google.golang.org/grpc@v1.20.1 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.21.1 failed.

google.golang.org/grpc@v1.21.1 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.23.0 failed.

google.golang.org/grpc@v1.23.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.25.1 failed.

google.golang.org/grpc@v1.25.1 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.26.0 failed.

google.golang.org/grpc@v1.26.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.27.0 failed.

google.golang.org/grpc@v1.27.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.27.1 failed.

google.golang.org/grpc@v1.27.1 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.28.0 failed.

google.golang.org/grpc@v1.28.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.29.1 failed.

google.golang.org/grpc@v1.29.1 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.30.0 failed.

google.golang.org/grpc@v1.30.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.31.0 failed.

google.golang.org/grpc@v1.31.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.31.1 failed.

google.golang.org/grpc@v1.31.1 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.33.2 failed.

google.golang.org/grpc@v1.33.2 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.34.0 failed.

google.golang.org/grpc@v1.34.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: google.golang.org/grpc@v1.35.0 failed.

google.golang.org/grpc@v1.35.0 is vulnerable to Connection confusion

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

Package: gopkg.in/yaml.v2@v2.2.2 failed.

gopkg.in/yaml.v2@v2.2.2 is vulnerable to yaml package for Go can consume excessive amounts of CPU or memory

Risk Domain: Software Vulnerability
Risk Level: high

Reason: risk level cannot exceed medium

View this project in the Phylum UI

@JoseAngel1196 JoseAngel1196 marked this pull request as ready for review July 7, 2023 01:48
@JoseAngel1196 JoseAngel1196 requested a review from a team as a code owner July 7, 2023 01:48
@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #70 (a6d9945) into main (4f745c4) will increase coverage by 2.60%.
The diff coverage is 56.57%.

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   50.39%   52.99%   +2.60%     
==========================================
  Files          13       13              
  Lines         512      568      +56     
==========================================
+ Hits          258      301      +43     
- Misses        252      261       +9     
- Partials        2        6       +4     
Flag Coverage Δ
unittests 52.99% <56.57%> (+2.60%) ⬆️

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

Impacted Files Coverage Δ
internal/scan.go 0.00% <0.00%> (ø)
reporting/console.go 92.68% <ø> (ø)
config/config.go 76.81% <70.21%> (+12.52%) ⬆️
cmd/root.go 35.71% <100.00%> (+4.94%) ⬆️
internal/utils.go 31.57% <100.00%> (+31.57%) ⬆️
reporting/slack.go 95.23% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

@phylum-io
Copy link

phylum-io bot commented Jul 9, 2023

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and has passed the active policy.

View this project in the Phylum UI

@JoseAngel1196
Copy link
Contributor Author

@tarkatronic packages excluded 😊

@tarkatronic
Copy link
Contributor

@tarkatronic packages excluded 😊

Beautiful, thank you! Hmmm looks like now gosec is unhappy.

[/github/workspace/config/config.go:88] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    87: 	v.AutomaticEnv()
  > 88: 	v.ReadInConfig()
    89: 



[/github/workspace/config/config.go:71] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    70: 
  > 71: 	if v.Unmarshal(&config); err != nil {
    72: 		log.Fatal().Err(err).Msg("Unable to unmarshal config.")



[/github/workspace/config/config.go:69] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    68: 	v.SetConfigType(strings.TrimLeft(extension, "."))
  > 69: 	v.ReadInConfig()
    70: 

It looks like in both of those cases you are not capturing the return value of the function. That's especially problematic in the first instance, as your are attempting an error check, from a previous error value!

@JoseAngel1196 JoseAngel1196 requested review from tarkatronic and removed request for tarkatronic July 13, 2023 22:39
config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
cmd/root.go Outdated Show resolved Hide resolved
@tarkatronic
Copy link
Contributor

Strange, I definitely hit "Cancel" on that review just now. Ignore all that...

Copy link
Contributor

@tarkatronic tarkatronic left a comment

Choose a reason for hiding this comment

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

This looks like a great starting point for this switch -- this unlocks so many more possibilities for configuration! Thanks for tackling this @JoseAngel1196!

@tarkatronic tarkatronic merged commit cb02599 into main Jul 14, 2023
30 checks passed
@tarkatronic tarkatronic deleted the feature/viper-config branch July 14, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to viper for configuration
2 participants