From 56387cf6855d397ab3d0a865349ab4b12c7801cc Mon Sep 17 00:00:00 2001 From: Vitaly Mosin <73104048+beevital@users.noreply.github.com> Date: Tue, 11 Jun 2024 10:05:01 +0200 Subject: [PATCH] Fix readonly fields issue (#319) * Fix readonly fields issue * tests and changelog --- CHANGELOG.md | 8 +- .../framework/core/model/connector_config.go | 2 +- fivetran/framework/provider.go | 2 +- .../framework/resources/destination_test.go | 132 ++++++++++++++++++ 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a69b105f..505237dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased](https://github.com/fivetran/terraform-provider-fivetran/compare/v1.1.24...HEAD) +## [Unreleased](https://github.com/fivetran/terraform-provider-fivetran/compare/v1.1.25...HEAD) + +## [1.1.25](https://github.com/fivetran/terraform-provider-fivetran/compare/v1.1.24...v1.1.25) + +## Fixed + +- Fix issue when provider tries to set readonly config fields to `null` on update for connector/destination. ## [1.1.24](https://github.com/fivetran/terraform-provider-fivetran/compare/v1.1.23...v1.1.24) diff --git a/fivetran/framework/core/model/connector_config.go b/fivetran/framework/core/model/connector_config.go index dd60db19..0df6ef80 100644 --- a/fivetran/framework/core/model/connector_config.go +++ b/fivetran/framework/core/model/connector_config.go @@ -26,7 +26,7 @@ func PrepareConfigAuthPatch(state, plan map[string]interface{}, service string, for k := range state { if _, ok := plan[k]; !ok { if f, ok := allFields[k]; ok { - if f.Nullable || f.FieldValueType == common.ObjectList || f.FieldValueType == common.StringList { + if (f.Nullable || f.FieldValueType == common.ObjectList || f.FieldValueType == common.StringList) && !f.Readonly { // If the field is not represented in plan (deleted from config) // And the field is nullable - it should be set to null explicitly result[k] = nil diff --git a/fivetran/framework/provider.go b/fivetran/framework/provider.go index bbdf040f..f160693e 100644 --- a/fivetran/framework/provider.go +++ b/fivetran/framework/provider.go @@ -17,7 +17,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" ) -const Version = "1.1.24" // Current provider version +const Version = "1.1.25" // Current provider version type fivetranProvider struct { mockClient httputils.HttpClient diff --git a/fivetran/framework/resources/destination_test.go b/fivetran/framework/resources/destination_test.go index 27d96b3f..bbdf94d2 100644 --- a/fivetran/framework/resources/destination_test.go +++ b/fivetran/framework/resources/destination_test.go @@ -13,6 +13,138 @@ import ( "github.com/hashicorp/terraform-plugin-testing/terraform" ) +func TestReadonlyFieldSetMock(t *testing.T) { + var testDestinationData map[string]interface{} + var destinationMappingDeleteHandler *mock.Handler + step1 := resource.TestStep{ + Config: ` + resource "fivetran_destination" "mydestination" { + provider = fivetran-provider + group_id = "group_id" + service = "new_s3_datalake" + time_zone_offset = "0" + region = "GCP_US_EAST4" + run_setup_tests = "false" + + config { + bucket = "bucket" + fivetran_role_arn = "fivetran_role_arn" + region = "region" + } + }`, + } + + step2 := resource.TestStep{ + Config: ` + resource "fivetran_destination" "mydestination" { + provider = fivetran-provider + group_id = "group_id" + service = "new_s3_datalake" + time_zone_offset = "0" + region = "GCP_US_EAST4" + run_setup_tests = "false" + + config { + bucket = "bucket1" + fivetran_role_arn = "fivetran_role_arn" + region = "region" + } + }`, + } + + resource.Test( + t, + resource.TestCase{ + PreCheck: func() { + tfmock.MockClient().Reset() + + tfmock.MockClient().When(http.MethodGet, "/v1/destinations/group_id").ThenCall( + func(req *http.Request) (*http.Response, error) { + return tfmock.FivetranSuccessResponse(t, req, http.StatusOK, "Success", testDestinationData), nil + }, + ) + + tfmock.MockClient().When(http.MethodPost, "/v1/destinations").ThenCall( + func(req *http.Request) (*http.Response, error) { + testDestinationData = tfmock.CreateMapFromJsonString(t, ` + { + "id":"group_id", + "group_id":"group_id", + "service":"new_s3_datalake", + "region":"GCP_US_EAST4", + "time_zone_offset":"0", + "setup_status":"connected", + "daylight_saving_time_enabled":true, + + "config":{ + "external_id": "group_id", + "bucket": "bucket", + "fivetran_role_arn": "fivetran_role_arn", + "region": "region" + } + } + `) + return tfmock.FivetranSuccessResponse(t, req, http.StatusCreated, "Success", testDestinationData), nil + }, + ) + + tfmock.MockClient().When(http.MethodPatch, "/v1/destinations/group_id").ThenCall( + func(req *http.Request) (*http.Response, error) { + body := tfmock.RequestBodyToJson(t, req) + + tfmock.AssertKeyExists(t, body, "config") + + config := body["config"].(map[string]interface{}) + tfmock.AssertKeyExistsAndHasValue(t, config, "bucket", "bucket1") + + tfmock.AssertKeyDoesNotExist(t, config, "external_id") + + testDestinationData = tfmock.CreateMapFromJsonString(t, ` + { + "id":"group_id", + "group_id":"group_id", + "service":"new_s3_datalake", + "region":"GCP_US_EAST4", + "time_zone_offset":"0", + "setup_status":"connected", + "daylight_saving_time_enabled":true, + + "config":{ + "external_id": "", + "bucket": "bucket1", + "fivetran_role_arn": "fivetran_role_arn", + "region": "region" + } + } + `) + return tfmock.FivetranSuccessResponse(t, req, http.StatusOK, "Success", testDestinationData), nil + }, + ) + + destinationMappingDeleteHandler = tfmock.MockClient().When(http.MethodDelete, "/v1/destinations/group_id").ThenCall( + func(req *http.Request) (*http.Response, error) { + testDestinationData = nil + response := tfmock.FivetranSuccessResponse(t, req, 200, + "Destination with id 'destionation_id' has been deleted", nil) + return response, nil + }, + ) + }, + ProtoV6ProviderFactories: tfmock.ProtoV6ProviderFactories, + CheckDestroy: func(s *terraform.State) error { + tfmock.AssertEqual(t, destinationMappingDeleteHandler.Interactions, 1) + tfmock.AssertEmpty(t, testDestinationData) + return nil + }, + + Steps: []resource.TestStep{ + step1, + step2, + }, + }, + ) +} + func TestResourceDestinationMappingMock(t *testing.T) { var testDestinationData map[string]interface{} var destinationMappingGetHandler *mock.Handler