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

Fix config parsing for structs and pointers to structs #345

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

asuresh4
Copy link
Contributor

Addresses the following -

  • Currently yamlTags from pointers to structs aren't accounted for
  • Capitalized yaml tags aren't applied recursively on structs

@asuresh4 asuresh4 requested a review from a team April 28, 2021 22:48
allSettings[updatedKey] = val
}
}
recursivelyCapitalizeConfigKeys(allSettings, yamlTags)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this should fix the issue reported in #322, not being able to set authType in kubelet.APIConfig.

@@ -131,6 +125,19 @@ func (cfg *Config) Unmarshal(componentParser *config.Parser) error {
return nil
}

func recursivelyCapitalizeConfigKeys(settings map[string]interface{}, yamlTags map[string]string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a viper-centric issue I wonder if yamlTagsFromStruct and recursivelyCapitalizeConfigKeys should be in a parent function (or receiver method)* like respectYamlTagsInAllSettings()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rmfitzpatrick - I've moved these methods to a helper and exposed RespectYamlTagsInAllSettings which can be used by both the receiver and the extension. Let me know if you think it's better.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do indeed!

Copy link
Contributor

@rmfitzpatrick rmfitzpatrick left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@rmfitzpatrick rmfitzpatrick left a comment

Choose a reason for hiding this comment

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

🎉

@asuresh4 asuresh4 merged commit e7c01af into main Apr 29, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix-config-parsing branch April 29, 2021 16:13
pjanotti referenced this pull request in pjanotti/splunk-otel-collector May 4, 2021
* Fix config parsing for structs

* Move common config utils to helper
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.

None yet

2 participants