Skip to content

Commit

Permalink
tfschema: Fix rollup/mapping rule interval diff after apply [sc-11120…
Browse files Browse the repository at this point in the history
…4] (#78)

[sc-111204]

When neither interval or storage_policy is specified, a server-side
default is returned. This causes a diff-after-apply, so mark the field
as computed.

Note: When using a computed field, if the user sets an explicitly value,
say `interval=60s`, and then removes that field, Terraform cannot
differentiate whether the value in the state is a server-side default or
previously set, and will continue to use the existing value as-is.
See hashicorp/terraform-plugin-sdk#282
  • Loading branch information
prashantv authored Oct 7, 2024
1 parent d82fce1 commit cf2391c
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

Fixed:
* Allow creating `chronosphere_slack_notifier` with actions without `action_confirm_text`.
* Fix `chronosphere_rollup_rule` and `chronosphere_mapping_rule` diff after applying
a rule without an interval or storage policy.

## v1.5.0

Expand Down
2 changes: 1 addition & 1 deletion chronosphere/intschema/mapping_rule.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions chronosphere/intschema/rollup_rule.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 15 additions & 6 deletions chronosphere/resource_mapping_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ func (mappingRuleConverter) toModel(
if err := validateMappingRule(r); err != nil {
return nil, fmt.Errorf("invalid mapping rule: %v", err)
}
// The schema doesn't allow interval and policy to be set together.
// However, we receive both because interval is an optional computed field
// which will send the last server-returned value even if the user doesn't have it set
// in the configuration. Hence we clear it manually to workaround sc-81013.
if r.Interval != "" && r.StoragePolicy != nil {
r.Interval = ""
}
filter, err := aggregationfilter.StringToModel(r.Filter, aggregationfilter.MappingRuleDelimiter)
if err != nil {
return nil, err
Expand Down Expand Up @@ -155,23 +162,25 @@ func mappingStoragePolicyFromModel(
}

func validateMappingRule(rule *intschema.MappingRule) error {
hasInterval := rule.Interval != "" || rule.StoragePolicy != nil

if rule.Drop {
if len(rule.Aggregations) > 0 {
return fmt.Errorf("cannot set aggregations when drop is true")
}
if rule.StoragePolicy != nil {
return fmt.Errorf("cannot set storage_policy when drop is true")
if hasInterval {
return fmt.Errorf("cannot set interval or storage_policy when drop is true")
}
return nil
}

if rule.StoragePolicy == nil && len(rule.Aggregations) == 0 {
return fmt.Errorf("must set aggregations and storage_policy when drop is not set")
if !hasInterval && len(rule.Aggregations) == 0 {
return fmt.Errorf("must set aggregations and interval when drop is not set")
}

// Aggregations and StoragePolicies must be set together.
if rule.StoragePolicy != nil && len(rule.Aggregations) == 0 {
return fmt.Errorf("must set aggregations")
if hasInterval && len(rule.Aggregations) == 0 {
return fmt.Errorf("must set aggregations when setting interval")
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion chronosphere/resource_rollup_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (rollupRuleConverter) toModel(
// which will send the last server-returned value even if the user doesn't have it set
// in the configuration. Hence we clear it manually to workaround sc-81013.
if r.Interval != "" && r.StoragePolicies != nil {
r.StoragePolicies = nil
r.Interval = ""
}

filter, err := aggregationfilter.StringToModel(r.Filter, aggregationfilter.RollupRuleDelimiter)
Expand Down
3 changes: 3 additions & 0 deletions chronosphere/tfschema/mapping_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ var MappingRule = map[string]*schema.Schema{
"interval": {
Type: schema.TypeString,
Optional: true,
// When no interval is specified, a server-side default is used.
Computed: true,
ConflictsWith: []string{"storage_policy"},
},
"mode": Enum{
Value: enum.MappingModeType.ToStrings(),
Expand Down
8 changes: 4 additions & 4 deletions chronosphere/tfschema/rollup_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ var RollupRule = map[string]*schema.Schema{
}.Schema(),
},
},
// When no policies are specified, the server-side will set the defaults.
Computed: true,
Deprecated: "use `interval` instead",
},
"interval": {
Type: schema.TypeString,
Optional: true,
Type: schema.TypeString,
Optional: true,
// When no interval is specified, a server-side default is used.
Computed: true,
ConflictsWith: []string{"storage_policies"},
},
"group_by": {
Expand Down

0 comments on commit cf2391c

Please sign in to comment.