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

agent: reject config with invalid options #576

Merged
merged 2 commits into from
Jan 5, 2015
Merged

Conversation

ryanuber
Copy link
Member

@ryanuber ryanuber commented Jan 5, 2015

This helps by rejecting configurations with invalid keys. Mapstructure supports this already, but the way we parse services and checks doesn't use straight mapstructure decoding, so there are a few discarded fields in the config struct. AFAIK there wasn't a better way to allow this.

/cc @armon @mitchellh

@sethvargo
Copy link
Contributor

👍

Result: &result,
Metadata: &md,
Result: &result,
ErrorUnused: true,
Copy link
Member

Choose a reason for hiding this comment

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

We should leave this false and remove all the unused fields from Config. Instead use the Unused field in Metadata. We can scan that for fields that are not (services, checks, service, check) and error / warn on those. This way we have more fine grain control later when we deprecate flags and stuff too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that seems less ghetto - I'll fix this up.

@ryanuber
Copy link
Member Author

ryanuber commented Jan 5, 2015

Fixed in 42ace3a.

ryanuber added a commit that referenced this pull request Jan 5, 2015
agent: reject config with invalid options
@ryanuber ryanuber merged commit db3c502 into master Jan 5, 2015
@ryanuber ryanuber deleted the f-verify-config branch January 5, 2015 22:51
@ryanuber ryanuber mentioned this pull request Jan 6, 2015
duckhan pushed a commit to duckhan/consul that referenced this pull request Oct 24, 2021
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.

4 participants