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

plugin: Make GetRuleConfigContent doesn't return an error even if config not found #1481

Merged
merged 1 commit into from
Aug 27, 2022

Conversation

wata727
Copy link
Member

@wata727 wata727 commented Aug 12, 2022

Currently, The GetRuleConfigContent returns an error when the passed rule config is not found. However, this behavior is inconvenient when calling DecodeRuleConfig on rules with default config.

If the rule config is not found, the rule should just adopt the default config, but DecodeRuleConfig will fail because GetRuleConfigContent will return an error. In order to properly adopt the default config in this situation, you must correctly handle the "rule config not found" error returned by GetRuleConfigContent.

This PR changes the behavior of the GetRuleConfigContent API to simply return an empty body if the rule config is not found. This allows the caller of DecodeRuleConfig to not care whether the rule config exists.

Be careful with the behavior when there are required attributes. It will behave like this:

  • If the rule config does not exist, no error will be returned even if the rule has required attributes.
    • This behavior contradicts the name of the "required" attribute, but here we interpret it as "required if a config exists". Rules must be implemented taking into account that required attributes are not necessarily set. If you want to avoid that, disable the rule by default.
    • Previously, it always returns an error.
  • Rules with required attributes will fail if the rule is enabled via the CLI, such as the --enable-rule option.
    • This is because if a user explicitly enables it, not properly passing the required config is likely to cause unexpected behavior.
    • Previously, GetRuleConfigContent called for a rule that enabled by CLI would return an error, regardless of whether the config was required or not.
  • If a rule is enabled in the config file, it will fail if the required attributes are not set.

This behavior is expected to be guaranteed by E2E testing. This behavior is the same as the old DecodeRuleConfig:
https://github.com/terraform-linters/tflint/blob/v0.39.3/tflint/runner.go#L391-L408

@wata727 wata727 force-pushed the return_empty_body_if_rule_config_not_found branch from 1cb5c1d to 881acd1 Compare August 13, 2022 13:01
@wata727 wata727 marked this pull request as ready for review August 13, 2022 13:02
@wata727 wata727 merged commit 3fbef3f into master Aug 27, 2022
@wata727 wata727 deleted the return_empty_body_if_rule_config_not_found branch August 27, 2022 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant