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

Check type casts to void crashing when casting explicit null values #534

Merged
merged 8 commits into from
Oct 5, 2022

Conversation

tobio
Copy link
Member

@tobio tobio commented Sep 14, 2022

Description

Repro in the linked issue.

Some fields may be sourced from Terraform variables, if those variables are optional then it's not uncommon to explicitly set fields to null, which the provider should treat as if the field was left unset.

Related Issues

Fixes #522

How Has This Been Tested?

Unit tests
Manually

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (improves code quality but has no user-facing effect)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation

Readiness Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@tobio tobio self-assigned this Sep 14, 2022
@tobio tobio requested a review from a team as a code owner September 14, 2022 07:28
@tobio tobio force-pushed the guard-explicit-nulls branch from 017de81 to df013c8 Compare September 22, 2022 04:10
@dimuon
Copy link
Contributor

dimuon commented Oct 4, 2022

@tobio can you please check why acceptance tests failed

@tobio
Copy link
Member Author

tobio commented Oct 4, 2022

This test appears to be failing due to hashicorp/terraform-plugin-sdk#1066 and hashicorp/terraform-plugin-sdk#1073 (seem to be duplicates).

I've updated the acceptance tests to run against 1.2.9 in CI until the TF issue is fixed.

@tobio tobio force-pushed the guard-explicit-nulls branch 2 times, most recently from cc2811f to 4f9cec1 Compare October 5, 2022 03:16
Copy link

@webfella webfella left a comment

Choose a reason for hiding this comment

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

Code LGTM

@tobio tobio force-pushed the guard-explicit-nulls branch from 4f9cec1 to f76d470 Compare October 5, 2022 09:21
@tobio tobio merged commit 00075aa into elastic:master Oct 5, 2022
@tobio tobio deleted the guard-explicit-nulls branch October 5, 2022 10:14
dimuon pushed a commit to dimuon/terraform-provider-ec that referenced this pull request Feb 2, 2023
…lastic#534)

* Ignore fields explicitly set to null in Kibana resources

* Ignore fields explicitly set to null in APM resources

* Ignore fields explicitly set to null in ES resources

* Ignore fields explicitly set to null in Enterprise Search resources

* Ignore fields explicitly set to null in Integrations Server  resources

* Ignore fields explicitly set to null in observability resources

* Ignore fields explicitly set to null in azure private endpoint resources

* Changelog
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.

The provider crashes when fields are explicitly set to null
3 participants