8000 Fix aws_db_instance false-positives by lotoussa Β· Pull Request #545 Β· snyk/driftctl Β· GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix aws_db_instance false-positives #545

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

Merged
merged 3 commits into from
May 25, 2021
Merged

Fix aws_db_instance false-positives #545

merged 3 commits into from
May 25, 2021

Conversation

lotoussa
Copy link
Contributor
Q A
πŸ› Bug fix? yes
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #542
❓ Documentation no

Description

Fix false positive for aws_db_instance CharacterSetName
Add Acc test.
Maybe it's need to change pkg/iac/terraform/state/test/db_instance/terraform.tfstate and pkg/iac/terraform/state/test/db_instance/result.golden.json, let me know.

@lotoussa lotoussa added the kind/bug Something isn't working label May 24, 2021
@lotoussa lotoussa added this to the v0.9.0 milestone May 24, 2021
@lotoussa lotoussa self-assigned this May 24, 2021
@lotoussa lotoussa requested a review from a team as a code owner May 24, 2021 15:47
@codecov
Copy link
codecov bot commented May 24, 2021

Codecov Report

Merging #545 (6dda84a) into v0.9 (91eb23b) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             v0.9     #545      +/-   ##
==========================================
- Coverage   70.89%   70.85%   -0.05%     
==========================================
  Files         289      289              
  Lines        6549     6553       +4     
==========================================
  Hits         4643     4643              
- Misses       1526     1529       +3     
- Partials      380      381       +1     
Impacted Files Coverage Ξ”
pkg/resource/aws/aws_db_instance_ext.go 8.33% <0.00%> (-4.17%) ⬇️

@lotoussa lotoussa force-pushed the issue_542_aws_db_instance branch from 509e846 to 6bddc5e Compare May 25, 2021 08:06
sundowndev
sundowndev previously approved these changes May 25, 2021
Copy link
Contributor
@sundowndev sundowndev left a comment

Choose a reason for hiding this comment

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

great job πŸ‘πŸΌ

@lotoussa lotoussa force-pushed the issue_542_aws_db_instance branch from 9ea5f40 to 1acee71 Compare 8000 May 25, 2021 10:03
@lotoussa lotoussa requested a review from wbeuil May 25, 2021 10:14
@lotoussa lotoussa requested a review from wbeuil May 25, 2021 10:22
@wbeuil wbeuil merged commit b933a9b into v0.9 May 25, 2021
@wbeuil wbeuil deleted the issue_542_aws_db_instance branch May 25, 2021 12:47
This was referenced May 26, 2021
@@ -14,9 +14,15 @@ func (r *AwsDbInstance) NormalizeForState() (resource.Resource, error) {
if r.ApplyImmediately != nil && !*r.ApplyImmediately {
r.ApplyImmediately = nil
}
if r.CharacterSetName != nil && *r.CharacterSetName == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

This fix need to be applied to the new infrastructure design in main branch @lotoussa @wbeuil

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch will do it now !

Copy link
Contributor

Choose a reason for hiding this comment

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

Done d427632

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0