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

Add resource source for terraform state #897

Merged
merged 1 commit into from Jul 30, 2021

Conversation

eliecharra
Copy link
Contributor

@eliecharra eliecharra commented Jul 29, 2021

Q A
πŸ› Bug fix? no
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues Fix #873
❓ Documentation no

@eliecharra eliecharra added the kind/maintenance Refactoring or changes to the workspace label Jul 29, 2021
@eliecharra eliecharra requested a review from a team as a code owner July 29, 2021 09:47
@eliecharra eliecharra added this to Review in driftctl Jul 29, 2021
@codecov
Copy link

codecov bot commented Jul 29, 2021

Codecov Report

Merging #897 (82b8c1c) into main (360f727) will decrease coverage by 0.06%.
The diff coverage is 65.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
- Coverage   81.66%   81.59%   -0.07%     
==========================================
  Files         231      232       +1     
  Lines        7494     7520      +26     
==========================================
+ Hits         6120     6136      +16     
- Misses       1149     1159      +10     
  Partials      225      225              
Impacted Files Coverage Ξ”
pkg/resource/resource.go 72.95% <25.00%> (-2.55%) ⬇️
pkg/iac/terraform/state/terraform_state_reader.go 66.06% <66.66%> (-0.61%) ⬇️
pkg/iac/config/config.go 100.00% <100.00%> (ΓΈ)

for _, stateVal := range val {
res, err := r.deserializer.DeserializeOne(ty, stateVal.val)
if err != nil {
logrus.WithField("ty", ty).Warnf("Could not read from state: %+v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can display here in which state the error came from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good idea, that was not the original goal of this PR but it can't hurt. Let's do this !

@eliecharra eliecharra force-pushed the add_state_source_in_resources branch from 97ffad9 to 82b8c1c Compare July 30, 2021 15:22
@eliecharra eliecharra requested a review from wbeuil July 30, 2021 15:22
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

I'm good with that

@eliecharra eliecharra merged commit 261bf01 into main Jul 30, 2021
@eliecharra eliecharra deleted the add_state_source_in_resources branch July 30, 2021 15:48
driftctl automation moved this from Review to Done Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add resource source when scanning IaC
3 participants