Skip to content
New issue

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

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

Already on GitHub? # to your account

Feature/support resolve variable references #351

Merged
merged 19 commits into from
Oct 24, 2020

Conversation

kanchwala-yusuf
Copy link
Contributor

No description provided.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Oct 12, 2020

Codecov Report

Merging #351 into master will decrease coverage by 2.36%.
The diff coverage is 47.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #351      +/-   ##
==========================================
- Coverage   63.78%   61.42%   -2.37%     
==========================================
  Files          66       72       +6     
  Lines        1378     1602     +224     
==========================================
+ Hits          879      984     +105     
- Misses        419      532     +113     
- Partials       80       86       +6     
Impacted Files Coverage Δ
...g/iac-providers/terraform/v12/module-references.go 2.94% <2.94%> (ø)
pkg/iac-providers/terraform/v12/cty-converters.go 11.11% <11.11%> (ø)
...iac-providers/terraform/v12/variable-references.go 44.44% <44.44%> (ø)
...g/iac-providers/terraform/v12/lookup-references.go 60.00% <60.00%> (ø)
...kg/iac-providers/terraform/v12/local-references.go 63.33% <63.33%> (ø)
pkg/iac-providers/terraform/v12/references.go 77.55% <77.55%> (ø)
pkg/iac-providers/terraform/v12/load-dir.go 82.69% <100.00%> (+1.84%) ⬆️
... and 3 more


// create output.ResourceConfig from hclConfigs.Resource
resourceConfig, err := CreateResourceConfig(managedResource)
if err != nil {
return allResourcesConfig, fmt.Errorf("failed to create ResourceConfig")
}

// resolve references
Copy link
Contributor

Choose a reason for hiding this comment

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

if a variable cannot be resolved, does this now cause an error? i think it would be preferable to continue processing while ignoring any missing vars.

lookup := r.ResolveStrRef(table)

// check if lookup is a map
if reflect.TypeOf(lookup).String() != "map[string]interface {}" {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use a type assertion here like in the next if statement?

case vType == "[]interface {}" && vKind == reflect.Slice:

// case 3: config value is a []interface{}
sConfig, ok := v.([]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like a redundant check

@williepaul williepaul merged commit 2c964d5 into master Oct 24, 2020
@williepaul williepaul deleted the feature/support-resolve-variable-references branch October 24, 2020 09:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants