feat: CBR regenerate provider #5724

zhenwan · 2024-10-14T19:38:03Z

Per request from https://ibm-cloudplatform.slack.com/archives/C05CL2TVBC6/p1712696668957339

Acceptance Tests:

=== RUN   TestAccIBMCbrRuleDataSourceBasic
--- PASS: TestAccIBMCbrRuleDataSourceBasic (15.30s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        17.060s

=== RUN   TestAccIBMCbrRuleDataSourceAllArgs
--- PASS: TestAccIBMCbrRuleDataSourceAllArgs (14.56s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        16.372s

=== RUN   TestAccIBMCbrZoneAddressesDataSourceBasic
--- PASS: TestAccIBMCbrZoneAddressesDataSourceBasic (14.24s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        16.019s

=== RUN   TestAccIBMCbrZoneAddressesDataSourceMultiple
--- PASS: TestAccIBMCbrZoneAddressesDataSourceMultiple (16.91s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        18.723s

=== RUN   TestAccIBMCbrZoneDataSourceBasic
--- PASS: TestAccIBMCbrZoneDataSourceBasic (16.08s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        (cached)

=== RUN   TestAccIBMCbrRuleBasic
--- PASS: TestAccIBMCbrRuleBasic (12.66s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        14.449s

=== RUN   TestAccIBMCbrRuleAllArgs
--- PASS: TestAccIBMCbrRuleAllArgs (28.25s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        30.003s

=== RUN   TestAccIBMCbrZoneAddressesBasic
--- PASS: TestAccIBMCbrZoneAddressesBasic (34.39s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        36.193s

=== RUN   TestAccIBMCbrZoneAddressesUpdate
--- PASS: TestAccIBMCbrZoneAddressesUpdate (48.98s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        50.796s

=== RUN   TestAccIBMCbrZoneBasic
--- PASS: TestAccIBMCbrZoneBasic (14.40s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        16.186s

=== RUN   TestAccIBMCbrZoneAllArgs
--- PASS: TestAccIBMCbrZoneAllArgs (24.97s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/contextbasedrestrictions        26.747s

mtbrandy

Initial feedback.

mtbrandy · 2024-10-18T22:22:03Z

ibm/service/contextbasedrestrictions/data_source_ibm_cbr_rule.go

 	}

 	d.SetId(fmt.Sprintf("%s", *getRuleOptions.RuleID))

 	if err = d.Set("crn", rule.CRN); err != nil {
-		return diag.FromErr(fmt.Errorf("Error setting crn: %s", err))
+		return flex.DiscriminatedTerraformErrorf(err, fmt.Sprintf("Error setting crn: %s", err), "(Data) ibm_cbr_rule", "read", "set-crn").GetDiag()


Why not log the debug message for this error? What determines when a debug message is logged vs when it is not?

This comment/question applies to everywhere you construct a TerraformProblem and had to decide whether or not to log the value of GetDebugMessage().

mtbrandy · 2024-10-18T22:38:50Z

ibm/service/contextbasedrestrictions/data_source_ibm_cbr_zone_addresses.go

 	}

 	var zone *contextbasedrestrictionsv1.Zone
 	var found bool
 	zone, _, found, err = getZone(contextBasedRestrictionsClient, context, zoneId)
 	if err != nil {
-		return diag.FromErr(err)
+		tfErr := flex.TerraformErrorf(err, fmt.Sprintf("getZone failed: %s", err.Error()), "(Data) ibm_cbr_zone_addresses", "read")


It feels wrong to include the local function (getZone) in the error message.

mtbrandy · 2024-10-18T22:39:09Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone_addresses.go

@@ -152,7 +153,9 @@ func resourceIBMCbrZoneAddressesCreate(context context.Context, d *schema.Resour
 	addressesId := fmt.Sprintf("TF-%s", newUuid)
 	err := resourceReplaceZoneAddresses(context, d, meta, zoneId, addressesId, false)
 	if err != nil {
-		return diag.FromErr(err)
+		tfErr := flex.TerraformErrorf(err, fmt.Sprintf(" resourceReplaceZoneAddresses failed: %s", err.Error()), "ibm_cbr_zone_addresses", "create")


It feels wrong to include the local function (resourceReplaceZoneAddresses) in the error message.

mtbrandy · 2024-10-18T22:39:35Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone_addresses.go

@@ -171,29 +176,31 @@ func resourceIBMCbrZoneAddressesRead(context context.Context, d *schema.Resource
 	var found bool
 	zone, _, found, err = getZone(contextBasedRestrictionsClient, context, zoneId)
 	if err != nil {
-		return diag.FromErr(err)
+		tfErr := flex.TerraformErrorf(err, fmt.Sprintf("getZone failed: %s", err.Error()), "ibm_cbr_zone_addresses", "read")


It feels wrong to include the local function (getZone) in the error message.

mtbrandy · 2024-10-18T22:48:54Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone_addresses.go

@@ -203,7 +210,9 @@ func resourceIBMCbrZoneAddressesUpdate(context context.Context, d *schema.Resour
 	zoneId, addressesId := decomposeZoneAddressesId(d.Id())
 	err := resourceReplaceZoneAddresses(context, d, meta, zoneId, addressesId, false)
 	if err != nil {
-		return diag.FromErr(err)
+		tfErr := flex.TerraformErrorf(err, fmt.Sprintf("resourceReplaceZoneAddresses failed: %s", err.Error()), "ibm_cbr_zone_addresses", "update")


It feels wrong to include the local function (resourceReplaceZoneAddresses) in the error message.

mtbrandy · 2024-10-18T23:04:02Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_rule.go

 			}
 			contexts = append(contexts, *contextsItem)
 		}
+		replaceRuleOptions.SetContexts(contexts)


What is the reason for this change? The API won't accept a zone create with missing contexts. The previous code ensured that it is present (and empty) if no contexts are specified.

mtbrandy · 2024-10-18T23:07:36Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go

 	}

 	zone, response, found, err := getZone(contextBasedRestrictionsClient, context, d.Id())
 	if err != nil {
-		return diag.FromErr(err)
+		tfErr := flex.TerraformErrorf(err, fmt.Sprintf("getZone failed: %s", err.Error()), "ibm_cbr_zone", "read")


It feels wrong to include the local function (getZone) in the error message.

mtbrandy · 2024-10-18T23:08:02Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go

@@ -412,7 +364,9 @@ func resourceIBMCbrZoneUpdate(context context.Context, d *schema.ResourceData, m

 	currentZone, response, found, err := getZone(contextBasedRestrictionsClient, context, zoneId)
 	if err != nil {
-		return diag.FromErr(err)
+		tfErr := flex.TerraformErrorf(err, fmt.Sprintf("getZone failed: %s", err.Error()), "ibm_cbr_zone", "update")


It feels wrong to include the local function (getZone) in the error message.

mtbrandy · 2024-10-18T23:08:48Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go

 	}

 	if err := ResourceIBMCbrZoneSetData(zone, response, d); err != nil {
-		return diag.FromErr(fmt.Errorf("Error setting zone's resource data: %s", err))
+		return flex.DiscriminatedTerraformErrorf(err, err.Error(), "ibm_cbr_zone", "read", "ResourceIBMCbrZoneSetData").GetDiag()


should be "update"

mtbrandy · 2024-10-18T23:13:32Z

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone.go

+	if err = d.Set("etag", response.Headers.Get("Etag")); err != nil {
+		return fmt.Errorf("Error setting etag: %s", err)
+	}
+	if err = d.Set("etag", response.Headers.Get("Etag")); err != nil {


"etag" is set twice here

zhenwan added 11 commits October 8, 2024 14:23

rule resource

080c71f

rule and zone resource logs

45cdfd5

zone addresses log

4580c06

updated some logs

7a08c22

copyright changes

d09048d

zone datasource logs

d400687

Update data_source_ibm_cbr_zone_addresses.go

a8b56f2

update example

b82ba2b

website updates

a102111

Update resource_ibm_cbr_zone_test.go

fe8be5f

to use generator's naming convention

3e5b76f

mtbrandy reviewed Oct 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: CBR regenerate provider #5724

feat: CBR regenerate provider #5724

zhenwan commented Oct 14, 2024 •

edited

Loading

mtbrandy left a comment

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

mtbrandy Oct 18, 2024

feat: CBR regenerate provider #5724

Are you sure you want to change the base?

feat: CBR regenerate provider #5724

Conversation

zhenwan commented Oct 14, 2024 • edited Loading

mtbrandy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhenwan commented Oct 14, 2024 •

edited

Loading