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

helper/schema: Disallow validation+diff suppression on computed fields #13878

Merged
merged 4 commits into from
Apr 24, 2017

Conversation

radeksimko
Copy link
Member

Although we seem to run ValidateFunc and DiffSuppressFunc for computed-only fields, there is (AFAIK) no value in either validation or diff suppression.

For validation - even if the API response contained structures that didn't comply with validation restrictions (and I admit it may happen as some APIs may just change) and we raised that as error there's nothing the user can do about it. They can't change or fix the API. The validation should (IMO) be on the other side - in any fields where we might reference these computed fields.

For diff suppression - This feature was designed to suppress differences/drifts between config and state, not between state and state. That is why it has no effect in the context of compute-only field which has no config value to compare the value from state against.

Related #11005

cc @catsby @djosephsen

Test plan

make testacc TEST=./builtin/providers/circonus
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/23 13:13:42 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/circonus -v  -timeout 120m
=== RUN   TestAccDataSourceCirconusAccount
--- PASS: TestAccDataSourceCirconusAccount (6.45s)
=== RUN   TestAccDataSourceCirconusCollector
--- PASS: TestAccDataSourceCirconusCollector (2.82s)
=== RUN   Test_MetricChecksum
--- PASS: Test_MetricChecksum (0.00s)
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccCirconusCheckCAQL_basic
--- PASS: TestAccCirconusCheckCAQL_basic (7.93s)
=== RUN   TestAccCirconusCheckCloudWatch_basic
--- FAIL: TestAccCirconusCheckCloudWatch_basic (0.01s)
	testing.go:280: Step 0 error: Configuration is invalid.

		Warnings: []string(nil)

		Errors: []string{"circonus_check.rds_metrics: Invalid api_key specified (\"\"): regexp failed to match string", "circonus_check.rds_metrics: Invalid api_secret specified (\"\"): regexp failed to match string"}
=== RUN   TestAccCirconusCheckConsul_node
--- PASS: TestAccCirconusCheckConsul_node (8.45s)
=== RUN   TestAccCirconusCheckConsul_service
--- PASS: TestAccCirconusCheckConsul_service (8.04s)
=== RUN   TestAccCirconusCheckConsul_state
--- PASS: TestAccCirconusCheckConsul_state (9.92s)
=== RUN   TestAccCirconusCheckHTTP_basic
--- PASS: TestAccCirconusCheckHTTP_basic (7.67s)
=== RUN   TestAccCirconusCheckHTTPTrap_basic
--- PASS: TestAccCirconusCheckHTTPTrap_basic (7.80s)
=== RUN   TestAccCirconusCheckICMPPing_basic
--- PASS: TestAccCirconusCheckICMPPing_basic (8.95s)
=== RUN   TestAccCirconusCheckJSON_basic
--- PASS: TestAccCirconusCheckJSON_basic (14.39s)
=== RUN   TestAccCirconusCheckMySQL_basic
--- PASS: TestAccCirconusCheckMySQL_basic (7.58s)
=== RUN   TestAccCirconusCheckPostgreSQL_basic
--- PASS: TestAccCirconusCheckPostgreSQL_basic (8.96s)
=== RUN   TestAccCirconusCheckStatsd_basic
--- PASS: TestAccCirconusCheckStatsd_basic (8.98s)
=== RUN   TestAccCirconusCheckTCP_basic
--- PASS: TestAccCirconusCheckTCP_basic (11.33s)
=== RUN   TestAccCirconusContactGroup_basic
--- PASS: TestAccCirconusContactGroup_basic (6.85s)
=== RUN   TestAccCirconusGraph_basic
--- PASS: TestAccCirconusGraph_basic (13.16s)
=== RUN   TestAccCirconusMetricCluster_basic
--- PASS: TestAccCirconusMetricCluster_basic (6.05s)
=== RUN   TestAccCirconusMetric_basic
--- PASS: TestAccCirconusMetric_basic (0.04s)
=== RUN   TestAccCirconusMetric_tagsets
--- PASS: TestAccCirconusMetric_tagsets (0.07s)
=== RUN   TestAccCirconusRuleSet_basic
--- PASS: TestAccCirconusRuleSet_basic (13.52s)

That one failure is a known issue.

make testacc TEST=./builtin/providers/consul
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/23 11:59:24 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/consul -v -timeout 120m
TestAccDataConsulAgentSelf_basic
TestAccDataConsulCatalogNodes_basic
TestAccDataConsulCatalogService_basic
TestAccDataConsulCatalogServices_basic
TestAccDataConsulKeys_basic
TestAccConsulAgentService_basic
TestAccConsulCatalogEntry_basic
TestAccConsulKeyPrefix_basic
TestConsulKeysMigrateState
TestConsulKeysMigrateState_empty
TestAccConsulKeys_basic
TestAccConsulNode_basic
TestAccConsulPreparedQuery_basic
TestAccConsulService_basic
TestResourceProvider
TestResourceProvider_impl
TestResourceProvider_Configure
TestResourceProvider_ConfigureTLS
PASS
ok github.com/hashicorp/terraform/builtin/providers/consul	0.443s

We don't have any Rancher environment to test in for the moment.

Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me!

@radeksimko radeksimko merged commit 5681a47 into master Apr 24, 2017
@radeksimko radeksimko deleted the f-core-computed-fields-validation branch April 24, 2017 18:04
@ghost
Copy link

ghost commented Apr 13, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants