-
Notifications
You must be signed in to change notification settings - Fork 89
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
Use state for unknown for computed attributes which won't change during plan application #677
Conversation
…ng plan application These values may potentially change throughout a deployment lifecycle, but those changes will be picked up during the refresh phase, updating state prior to computing the plan These values, when unset, should be 'known' as they are provided in the API response
@@ -44,6 +44,7 @@ func readElasticsearchConfig(in *models.ElasticsearchConfiguration) (*Elasticsea | |||
return &ElasticsearchConfig{}, nil | |||
} | |||
|
|||
config.Plugins = []string{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can lead to the inconsistency error if config is empty. Also it's not needed - append
can append to nil
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean if TF configuration doesn't have config
.
Hopefully should address elastic/terraform-provider-elasticstack#370 |
@@ -44,6 +44,7 @@ func readElasticsearchConfig(in *models.ElasticsearchConfiguration) (*Elasticsea | |||
return &ElasticsearchConfig{}, nil | |||
} | |||
|
|||
config.Plugins = []string{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config.Plugins = []string{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like acceptance tests failed due to the mentioned change to ES config's plugins
initialization.
Besides that issue, LGTM 👍 . Thank you for the PR!
@jamesagarside correct, I've been explicitly testing that issue locally with this change as well. |
🥳🥳🥳 |
Will need to explicitly set cloud_id to computed on name changes or when kibana is added/removed. |
…deployment resource
Description
These values may potentially change throughout a deployment lifecycle, but those changes will be picked up during the refresh phase, updating state prior to computing the plan
These values, when unset, should be 'known' as they are provided in the API response
This PR does not result in completely clean plans due to how traffic filters currently work. I'm not sure there's a straightforward solution there, as the traffic_filter_association resource can lead to traffic filters changing during the course of a plan application. Simply using state for unknown on this attribute leads to TF crashing as a result of an unexpected new value after apply.
I need to bump the TF version used in the acceptance tests here too.
Related Issues
Fixes #599
How Has This Been Tested?
Manually, unit tests
Types of Changes
Readiness Checklist