Updated EC2 detector to use v2 aws sdk #6878
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6878 +/- ##
=======================================
- Coverage 76.1% 76.0% -0.1%
=======================================
Files 219 219
Lines 21299 21304 +5
=======================================
- Hits 16210 16208 -2
- Misses 4529 4534 +5
- Partials 560 562 +2
|
|
Side note: the other AWS detectors don't use |
| @@ -44,28 +46,39 @@ func (fn optionFunc) apply(c *config) { | |||
| } | |||
|
|
|||
| // WithClient sets the ec2metadata client in config. | |||
| func WithClient(t Client) Option { | |||
| func WithClient(t client) Option { | |||
There was a problem hiding this comment.
Hmm, this is going to be problematic.
The function signature here cannot change, that will be a breaking change
- This could be deprecated as well. It needs to be verified that there is never a need for a client to be set here. Likely this is not the case if a user is trying to stub out functionality for testing external to this package.
- The
Clientinterface could be extended and a type assertion can be made that will check if the passed client implements the prior functionality. This will mean that backwards compatible support for the existing API needs to be possible (even with the upgrade). - A
ec2/v2package is created to support the AWS v2 API and this package is deprecated - Maybe some other options ... (?)
There was a problem hiding this comment.
Hmm yeah I'm not sure if this just also should've been unexported along with Client or if there's a valid use case for a user to set their own? I see the commit that added it on the off chance you remember the rational behind it #1030 @MrAlias.
If we create a v2 package and deprecate this one, does the v2 package permanently stay as v2
There was a problem hiding this comment.
https://go.dev/blog/v2-go-modules
The recommended strategy is to develop v2+ modules in a directory named after the major version suffix.
This allows also nicely deprecating the existing v1 module.
There was a problem hiding this comment.
Plan from SIG meeting is to create a v2 of this module.
| @@ -4,12 +4,25 @@ go 1.22.0 | |||
|
|
|||
| require ( | |||
| github.com/aws/aws-sdk-go v1.55.6 | |||
There was a problem hiding this comment.
How do we plan on mitigating this https://nvd.nist.gov/vuln/detail/CVE-2020-8911 and https://nvd.nist.gov/vuln/detail/CVE-2020-8912 if we are still pulling in this dependency?
There was a problem hiding this comment.
Plan from SIG meeting is to create a v2 of this module.
This should solve it
There was a problem hiding this comment.
This package will be deprecated as is, a v2 will be created without this dependency
Resolves #6860