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

Created wrapper function around GetOkExists and all GetOk function usage has been replaced with GetFieldExists wrapper function #5965

Merged
merged 23 commits into from
Feb 11, 2025

Conversation

prajwal-jarali
Copy link
Contributor

@prajwal-jarali prajwal-jarali commented Feb 4, 2025

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

Output of make build

==> Checking that code complies with gofmt requirements...
go vet .
go install

Manual testing

  • Manual Testing results are attached below in the comment
  • From the results it can be observed the some attributes were not getting updated at all, the PR binary is fixing that issue

Go code test file test runs

=== RUN   TestAccIbmAppConfigCollectionDataSource
--- PASS: TestAccIbmAppConfigCollectionDataSource (80.47s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        81.897s

=== RUN   TestAccIbmAppConfigCollectionsDataSource
--- PASS: TestAccIbmAppConfigCollectionsDataSource (59.97s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        61.374s

=== RUN   TestAccIbmAppConfigEnvironmentDataSourceBasic
--- PASS: TestAccIbmAppConfigEnvironmentDataSourceBasic (58.86s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        60.244s

=== RUN   TestAccIbmAppConfigEnvironmentsDataSourceBasic
--- PASS: TestAccIbmAppConfigEnvironmentsDataSourceBasic (28.95s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        30.782s

=== RUN   TestAccIbmAppConfigFeatureDataSource
--- PASS: TestAccIbmAppConfigFeatureDataSource (56.84s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        58.237s

=== RUN   TestAccIbmAppConfigFeaturesDataSourceBasic
--- PASS: TestAccIbmAppConfigFeaturesDataSourceBasic (60.76s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        62.159s

=== RUN   TestAccIbmAppConfigPropertiesDataSourceBasic
--- PASS: TestAccIbmAppConfigPropertiesDataSourceBasic (58.62s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        59.991s

=== RUN   TestAccIbmAppConfigPropertyDataSource
--- PASS: TestAccIbmAppConfigPropertyDataSource (58.15s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        59.555s

=== RUN   TestAccIbmAppConfigSegmentDataSource
--- PASS: TestAccIbmAppConfigSegmentDataSource (57.07s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        58.477s

=== RUN   TestAccIbmAppConfigSegmentsDataSourceBasic
--- PASS: TestAccIbmAppConfigSegmentsDataSourceBasic (58.57s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        59.986s

=== RUN   TestAccIbmIbmAppConfigCollectionBasic
--- PASS: TestAccIbmIbmAppConfigCollectionBasic (72.80s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        74.190s

=== RUN   TestAccIbmAppConfigEnvironmentBasic
--- PASS: TestAccIbmAppConfigEnvironmentBasic (82.26s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        83.667s

=== RUN   TestAccIbmIbmAppConfigFeatureBasic
--- PASS: TestAccIbmIbmAppConfigFeatureBasic (84.73s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        86.141s

=== RUN   TestAccIbmIbmAppConfigPropertyBasic
--- PASS: TestAccIbmIbmAppConfigPropertyBasic (72.33s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        73.751s

=== RUN   TestAccIbmIbmAppConfigSegmentBasic
--- PASS: TestAccIbmIbmAppConfigSegmentBasic (78.74s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/appconfiguration        80.142s

resolved struct name changes from go-admin-sdk 0.3.0 to 0.4.4 upgrade
made code compliant with go fmt requirements
solving errors in struct type matching and string array conversion
added switch cases for hrefUrl
enabled toggle of feature to be set by user
…age has been replaced with GetFieldExists wrapper function
@prajwal-jarali
Copy link
Contributor Author

Issue of using GetOk function in terraform source code

Parent: https://github.com/IBM-Cloud/terraform-provider-ibm

How GetOk function is being used in the plugin :-

When user writes terraform code and runs it, terraform internally creates a instance of Struct of Schema.ResourceData where it stores attribute data in a map. Then it identifies the particular resource being used and sends the ResourceData to the particular CRUD function defined on that resource in provider.go file. Then the function wants to check what attributes are defined by user so they use the GetOk function provided by terraform plugin SDK to fetch the value and boolean indicating whether attribute is present or absent in attribute map of the data instance.

Implementation of GetOk function :-

// GetOk returns the data for the given key and whether or not the key
// has been set to a non-zero value at some point.
//
// The first result will not necessarily be nil if the value doesn't exist.
// The second result should be checked to determine this information.
func (d *ResourceData) GetOk(key string) (interface{}, bool) {
	r := d.getRaw(key, getSourceSet)
	exists := r.Exists && !r.Computed
	if exists {
		// If it exists, we also want to verify it is not the zero-value.
		value := r.Value
		zero := r.Schema.Type.Zero()

		if eq, ok := value.(Equal); ok {
			exists = !eq.Equal(zero)
		} else {
			exists = !reflect.DeepEqual(value, zero)
		}
	}

	return r.Value, exists
}

Issue with GetOk function implementation :-

If user sets attribute value to Zero value

Type String = ""
Type Int = 0
Type Boolean = false

Then GetOk function reports the attribute doesn't exists at all which is wrong behaviour and leads to issues like attribute being not present in resource during create time and also resource attribute value cannot be updated to the zero value

Current work-around

GetOkExists function can be used as replacement for GetOk function which gives correct behaviour, but it is deprecated

Implementation of GetOkExists :-

// GetOkExists can check if TypeBool attributes that are Optional with
// no Default value have been set.
//
// Deprecated: usage is discouraged due to undefined behaviors and may be
// removed in a future version of the SDK
func (d *ResourceData) GetOkExists(key string) (interface{}, bool) {
	r := d.getRaw(key, getSourceSet)
	exists := r.Exists && !r.Computed
	return r.Value, exists
}

@prajwal-jarali
Copy link
Contributor Author

Testing Terraform plugin on AppConfiguration

Procedure:-

  • main.tf file is created with the required values and later updated to check for updates
  • instance data is being fetched from the app config instance making API call

main.tf file

resource "ibm_app_config_environment" "app_config_environment" {
  guid = variable.guid
  environment_id = variable.environment
  name = variable.environment
  description = "description"
  tags = "tags"
}

resource "ibm_app_config_collection" "app_config_collection" {
  guid = variable.guid
  name = variable.collection
  tags = "tags"
  description = "description"
  collection_id = variable.collection
}

resource "ibm_app_config_feature" "app_config_feature" {
  guid = variable.feature
  name = "name"
  type = "STRING"
  tags = "tags"
  format="TEXT"
  feature_id = variable.feature
  enabled_value = "enabled_value"
  environment_id = variable.environment_def
  disabled_value = "disabled_value"
  rollout_percentage = 80
  enabled = false
}

resource "ibm_app_config_property" "app_config_property" {
  guid = variable.guid
  environment_id = variable.environment_def
  name = variable.property
  property_id = variable.property
  type = "STRING"
  value = "value"
  description = "description"
  tags = "tags"
}

resource "ibm_app_config_segment" "app_config_segment" {
  guid = variable.guid
  name = variable.segment
  description = "description"
  tags = "tags"
  segment_id = variable.segment
  rules {
    attribute_name = "email"
    operator = "ends_with"
    values = ".ibm.com"
  }
}

updated main.tf to check for updates

resource "ibm_app_config_environment" "app_config_environment" {
  guid = var.guid
  environment_id = var.environment
  name = var.environment
  description = ""
  tags = ""
}

resource "ibm_app_config_collection" "app_config_collection" {
  guid = var.guid
  name = var.collection
  tags = ""
  description = ""
  collection_id = var.collection
}

resource "ibm_app_config_feature" "app_config_feature" {
  guid = var.guid
  name = var.feature
  type = "STRING"
  tags = ""
  format="TEXT"
  feature_id = var.feature
  enabled_value = ""
  environment_id = var.environment_def
  disabled_value = ""
  rollout_percentage = 0
  enabled = false
}

resource "ibm_app_config_property" "app_config_property" {
  guid = var.guid
  environment_id = var.environment_def
  name = var.property
  property_id = var.property
  type = "STRING"
  format = "TEXT"
  value = ""
  description = ""
  tags = ""
}

resource "ibm_app_config_segment" "app_config_segment" {
  guid = var.guid
  name = var.segment
  description = ""
  tags = ""
  segment_id = var.segment
  rules {
    attribute_name = "email"
    operator = "endsWith"
    values = [".ibm.com"]
  }
}

Using 1.75.0 latest release

Before

{
	"environments": [
		{
			"name": "Dev",
			"environment_id": "dev",
			"description": "Environment created on instance creation",
			"color_code": "#FDD13A",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"tags": "tags",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "enabled_value",
					"disabled_value": "disabled_value",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 80,
					"isOverridden": true
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "description",
					"tags": "tags",
					"type": "STRING",
					"format": "TEXT",
					"value": "value",
					"segment_rules": [],
					"collections": [],
					"isOverridden": true
				}
			]
		},
		{
			"name": "stage",
			"environment_id": "stage",
			"description": "description",
			"tags": "tags",
			"color_code": "#fd6af0",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "",
					"disabled_value": "",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 100,
					"isOverridden": false
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "description",
					"type": "STRING",
					"format": "TEXT",
					"value": "",
					"segment_rules": [],
					"collections": [],
					"isOverridden": false
				}
			]
		}
	],
	"collections": [
		{
			"name": "test_collection",
			"collection_id": "test_collection",
			"description": "description",
			"tags": "tags"
		}
	],
	"segments": [
		{
			"name": "segment_test",
			"segment_id": "segment_test",
			"description": "description",
			"tags": "tags",
			"rules": [
				{
					"values": [
						".ibm.com"
					],
					"operator": "endsWith",
					"attribute_name": "email"
				}
			]
		}
	]
}

After

{
	"environments": [
		{
			"name": "Dev",
			"environment_id": "dev",
			"description": "Environment created on instance creation",
			"color_code": "#FDD13A",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"tags": "tags",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "",
					"disabled_value": "",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 80,
					"isOverridden": true
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "description",
					"tags": "tags",
					"type": "STRING",
					"format": "TEXT",
					"value": "",
					"segment_rules": [],
					"collections": [],
					"isOverridden": true
				}
			]
		},
		{
			"name": "stage",
			"environment_id": "stage",
			"description": "description",
			"tags": "tags",
			"color_code": "#fd6af0",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "",
					"disabled_value": "",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 100,
					"isOverridden": false
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "description",
					"type": "STRING",
					"format": "TEXT",
					"value": "",
					"segment_rules": [],
					"collections": [],
					"isOverridden": false
				}
			]
		}
	],
	"collections": [
		{
			"name": "test_collection",
			"collection_id": "test_collection",
			"description": "description",
			"tags": "tags"
		}
	],
	"segments": [
		{
			"name": "segment_test",
			"segment_id": "segment_test",
			"description": "description",
			"tags": "tags",
			"rules": [
				{
					"values": [
						".ibm.com"
					],
					"operator": "endsWith",
					"attribute_name": "email"
				}
			]
		}
	]
}

Using the binary built on this PR

Before

{
	"environments": [
		{
			"name": "Dev",
			"environment_id": "dev",
			"description": "Environment created on instance creation",
			"color_code": "#FDD13A",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "enabled_value",
					"disabled_value": "disabled_value",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 80,
					"isOverridden": true
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "description",
					"tags": "tags",
					"type": "STRING",
					"format": "TEXT",
					"value": "value",
					"segment_rules": [],
					"collections": [],
					"isOverridden": true
				}
			]
		},
		{
			"name": "stage",
			"environment_id": "stage",
			"description": "description",
			"tags": "tags",
			"color_code": "#db562a",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "",
					"disabled_value": "",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 100,
					"isOverridden": false
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "description",
					"type": "STRING",
					"format": "TEXT",
					"value": "",
					"segment_rules": [],
					"collections": [],
					"isOverridden": false
				}
			]
		}
	],
	"collections": [
		{
			"name": "test_collection",
			"collection_id": "test_collection",
			"description": "description",
			"tags": "tags"
		}
	],
	"segments": [
		{
			"name": "segment_test",
			"segment_id": "segment_test",
			"description": "description",
			"tags": "tags",
			"rules": [
				{
					"values": [
						".ibm.com"
					],
					"operator": "endsWith",
					"attribute_name": "email"
				}
			]
		}
	]
}

After

{
	"environments": [
		{
			"name": "Dev",
			"environment_id": "dev",
			"description": "Environment created on instance creation",
			"color_code": "#FDD13A",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"tags": "",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "",
					"disabled_value": "",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 0,
					"isOverridden": true
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "",
					"tags": "",
					"type": "STRING",
					"format": "TEXT",
					"value": "",
					"segment_rules": [],
					"collections": [],
					"isOverridden": true
				}
			]
		},
		{
			"name": "stage",
			"environment_id": "stage",
			"description": "",
			"tags": "",
			"color_code": "#db562a",
			"features": [
				{
					"name": "feature_test",
					"feature_id": "feature_test",
					"type": "STRING",
					"format": "TEXT",
					"enabled_value": "",
					"disabled_value": "",
					"segment_rules": [],
					"collections": [],
					"enabled": false,
					"rollout_percentage": 100,
					"isOverridden": false
				}
			],
			"properties": [
				{
					"name": "test_property",
					"property_id": "test_property",
					"description": "",
					"type": "STRING",
					"format": "TEXT",
					"value": "",
					"segment_rules": [],
					"collections": [],
					"isOverridden": false
				}
			]
		}
	],
	"collections": [
		{
			"name": "test_collection",
			"collection_id": "test_collection",
			"description": "",
			"tags": ""
		}
	],
	"segments": [
		{
			"name": "segment_test",
			"segment_id": "segment_test",
			"description": "",
			"tags": "",
			"rules": [
				{
					"values": [
						".ibm.com"
					],
					"operator": "endsWith",
					"attribute_name": "email"
				}
			]
		}
	]
}

@hkantare hkantare merged commit c61406f into IBM-Cloud:master Feb 11, 2025
1 check passed
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.

2 participants