Add prechecks for create and update params

Use plan to update local record instead of state
dell · Sep 5, 2023 · 364145d · 364145d
1 parent 0314220
commit 364145d
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 4 deletions.
diff --git a/powerscale/helper/ads_provider_helper.go b/powerscale/helper/ads_provider_helper.go
@@ -64,3 +64,27 @@ func DeleteAdsProvider(ctx context.Context, client *client.Client, adsID string)
 	_, err := client.PscaleOpenAPIClient.AuthApi.DeleteAuthv14ProvidersAdsById(ctx, adsID).Execute()
 	return err
 }
+
+// IsCreateAdsProviderParamInvalid Verify if create params contain params only for updating
+func IsCreateAdsProviderParamInvalid(plan models.AdsProviderResourceModel) bool {
+	if !plan.DomainController.IsNull() ||
+		!plan.ResetSchannel.IsNull() ||
+		!plan.Spns.IsUnknown() {
+		return true
+	}
+	return false
+}
+
+// IsUpdateAdsProviderParamInvalid Verify if update params contain params only for creating
+func IsUpdateAdsProviderParamInvalid(plan models.AdsProviderResourceModel, state models.AdsProviderResourceModel) bool {
+	if (!plan.DNSDomain.IsNull() && !state.DNSDomain.Equal(plan.DNSDomain)) ||
+		(!plan.Groupnet.IsUnknown() && !state.Groupnet.Equal(plan.Groupnet)) ||
+		(!plan.Instance.IsNull() && !state.Instance.Equal(plan.Instance)) ||
+		(!plan.KerberosHdfsSpn.IsNull() && !state.KerberosHdfsSpn.Equal(plan.KerberosHdfsSpn)) ||
+		(!plan.KerberosNfsSpn.IsNull() && !state.KerberosNfsSpn.Equal(plan.KerberosNfsSpn)) ||
+		(!plan.MachineAccount.IsUnknown() && !state.MachineAccount.Equal(plan.MachineAccount)) ||
+		(!plan.OrganizationalUnit.IsNull() && !state.OrganizationalUnit.Equal(plan.OrganizationalUnit)) {
+		return true
+	}
+	return false
+}
diff --git a/powerscale/helper/nfs_export_helper.go b/powerscale/helper/nfs_export_helper.go
@@ -209,7 +209,7 @@ func FilterExports(paths []types.String, ids []types.Int64, exports []powerscale
 
 // ResolvePersonaDiff implement state
 // For nfs export persona info, response may only contain UID while type/username is given
-// Need to manually copy plan info to state, or state would keep the type/username as null, which is inconsistent
+// Need to manually copy plan info to state, or state would keep the type/username as null, which is inconsistent.
 func ResolvePersonaDiff(ctx context.Context, plan models.NfsExportResource, state *models.NfsExportResource) {
 	state.MapAll = assignKnownObjectToUnknown(ctx, plan.MapAll, state.MapAll)
 	state.MapFailure = assignKnownObjectToUnknown(ctx, plan.MapAll, state.MapAll)

diff --git a/powerscale/provider/ads_provider_resource.go b/powerscale/provider/ads_provider_resource.go
@@ -446,7 +446,13 @@ func (r *AdsProviderResource) Create(ctx context.Context, req resource.CreateReq
 	if resp.Diagnostics.HasError() {
 		return
 	}
-
+	if helper.IsCreateAdsProviderParamInvalid(plan) {
+		resp.Diagnostics.AddError(
+			"Error creating ads provider",
+			"Should not provide parameters for updating",
+		)
+		return
+	}
 	adsToCreate := powerscale.V14ProvidersAdsItem{}
 	// Get param from tf input
 	err := helper.ReadFromState(ctx, plan, &adsToCreate)
@@ -593,6 +599,13 @@ func (r *AdsProviderResource) Update(ctx context.Context, req resource.UpdateReq
 		"adsState": adsState,
 	})
 
+	if helper.IsUpdateAdsProviderParamInvalid(adsPlan, adsState) {
+		resp.Diagnostics.AddError(
+			"Error updating ads provider",
+			"Should not provide parameters for creating",
+		)
+		return
+	}
 	adsID := adsState.ID.ValueString()
 	adsPlan.ID = adsState.ID
 	var adsToUpdate powerscale.V14ProvidersAdsIdParams
@@ -640,7 +653,7 @@ func (r *AdsProviderResource) Update(ctx context.Context, req resource.UpdateReq
 		return
 	}
 
-	err = helper.CopyFieldsToNonNestedModel(ctx, updatedAds.Ads[0], &adsState)
+	err = helper.CopyFieldsToNonNestedModel(ctx, updatedAds.Ads[0], &adsPlan)
 	if err != nil {
 		errStr := constants.ReadAdsProviderErrorMsg + "with error: "
 		message := helper.GetErrorString(err, errStr)
@@ -650,7 +663,7 @@ func (r *AdsProviderResource) Update(ctx context.Context, req resource.UpdateReq
 		)
 		return
 	}
-	diags = resp.State.Set(ctx, adsState)
+	diags = resp.State.Set(ctx, adsPlan)
 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return

diff --git a/powerscale/provider/ads_provider_resource_test.go b/powerscale/provider/ads_provider_resource_test.go
@@ -154,6 +154,10 @@ func TestAccAdsProviderResourceErrorUpdate(t *testing.T) {
 				Config:      ProviderConfig + AdsProviderInvalidResourceConfig,
 				ExpectError: regexp.MustCompile(".*Bad Request*."),
 			},
+			{
+				Config:      ProviderConfig + AdsProviderUpdatePreCheckConfig,
+				ExpectError: regexp.MustCompile(".*Should not provide parameters for creating*."),
+			},
 		},
 	})
 }
@@ -167,6 +171,10 @@ func TestAccAdsProviderResourceErrorCreate(t *testing.T) {
 				Config:      ProviderConfig + AdsProviderInvalidResourceConfig,
 				ExpectError: regexp.MustCompile(".*Bad Request*."),
 			},
+			{
+				Config:      ProviderConfig + AdsProviderCreatePreCheckConfig,
+				ExpectError: regexp.MustCompile(".*Should not provide parameters for updating*."),
+			},
 		},
 	})
 }
@@ -244,6 +252,15 @@ resource "powerscale_adsprovider" "ads_test" {
 }
 `, adsName)
 
+var AdsProviderCreatePreCheckConfig = fmt.Sprintf(`
+resource "powerscale_adsprovider" "ads_test" {
+	name = "%s"
+	user = "administrator"
+	password = "Password123!"
+	reset_schannel = true
+}
+`, adsName)
+
 var AdsProviderUpdatedResourceConfig = fmt.Sprintf(`
 resource "powerscale_adsprovider" "ads_test" {
 	name = "%s"
@@ -262,3 +279,12 @@ resource "powerscale_adsprovider" "ads_test" {
 	check_online_interval = 290
 }
 `, adsName)
+
+var AdsProviderUpdatePreCheckConfig = fmt.Sprintf(`
+resource "powerscale_adsprovider" "ads_test" {
+	name = "%s"
+	user = "administrator"
+	password = "Password123!"
+	kerberos_hdfs_spn = true
+}
+`, adsName)