Fix update proxy in cluster rosa

terraform-redhat · Sep 26, 2023 · bc172c4 · bc172c4
1 parent 0a0cd6d
commit bc172c4
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 60 deletions.
diff --git a/provider/clusterrosaclassic/cluster_rosa_classic_resource.go b/provider/clusterrosaclassic/cluster_rosa_classic_resource.go
@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"reflect"
 	"sort"
 	"strings"
 	"time"
@@ -715,24 +716,26 @@ func buildProxy(state *ClusterRosaClassicState, builder *cmv1.ClusterBuilder) (*
 	if state.Proxy != nil {
 		httpsProxy := ""
 		httpProxy := ""
+		httpNoProxy := ""
 		additionalTrustBundle := ""
 
 		if !common.IsStringAttributeEmpty(state.Proxy.HttpProxy) {
 			httpProxy = state.Proxy.HttpProxy.ValueString()
-			proxy.HTTPProxy(httpProxy)
 		}
+		proxy.HTTPProxy(httpProxy)
 		if !common.IsStringAttributeEmpty(state.Proxy.HttpsProxy) {
 			httpsProxy = state.Proxy.HttpsProxy.ValueString()
-			proxy.HTTPSProxy(httpsProxy)
 		}
+		proxy.HTTPSProxy(httpsProxy)
 		if !common.IsStringAttributeEmpty(state.Proxy.NoProxy) {
-			proxy.NoProxy(state.Proxy.NoProxy.ValueString())
+			httpNoProxy = state.Proxy.NoProxy.ValueString()
 		}
+		proxy.NoProxy(httpNoProxy)
 
 		if !common.IsStringAttributeEmpty(state.Proxy.AdditionalTrustBundle) {
 			additionalTrustBundle = state.Proxy.AdditionalTrustBundle.ValueString()
-			builder.AdditionalTrustBundle(additionalTrustBundle)
 		}
+		builder.AdditionalTrustBundle(additionalTrustBundle)
 
 		builder.Proxy(proxy)
 	}
@@ -1036,7 +1039,7 @@ func (r *ClusterRosaClassicResource) Update(ctx context.Context, request resourc
 		return
 	}
 
-	clusterBuilder, _, err = updateProxy(state, plan, clusterBuilder)
+	clusterBuilder, err = updateProxy(state, plan, clusterBuilder)
 	if err != nil {
 		response.Diagnostics.AddError(
 			"Can't update cluster",
@@ -1292,30 +1295,21 @@ func scheduleUpgrade(ctx context.Context, client *cmv1.ClustersClient, clusterID
 	return nil
 }
 
-func updateProxy(state, plan *ClusterRosaClassicState, clusterBuilder *cmv1.ClusterBuilder) (*cmv1.ClusterBuilder, bool, error) {
-	shouldUpdateProxy := false
-	if (state.Proxy == nil && plan.Proxy != nil) || (state.Proxy != nil && plan.Proxy == nil) {
-		shouldUpdateProxy = true
-	} else if state.Proxy != nil && plan.Proxy != nil {
-		_, patchNoProxy := common.ShouldPatchString(state.Proxy.NoProxy, plan.Proxy.NoProxy)
-		_, patchHttpProxy := common.ShouldPatchString(state.Proxy.HttpProxy, plan.Proxy.HttpProxy)
-		_, patchHttpsProxy := common.ShouldPatchString(state.Proxy.HttpsProxy, plan.Proxy.HttpsProxy)
-		_, patchAdditionalTrustBundle := common.ShouldPatchString(state.Proxy.AdditionalTrustBundle, plan.Proxy.AdditionalTrustBundle)
-		if patchNoProxy || patchHttpProxy || patchHttpsProxy || patchAdditionalTrustBundle {
-			shouldUpdateProxy = true
-		}
-	}
-
-	if shouldUpdateProxy {
+func updateProxy(state, plan *ClusterRosaClassicState, clusterBuilder *cmv1.ClusterBuilder) (*cmv1.ClusterBuilder, error) {
+	if !reflect.DeepEqual(state.Proxy, plan.Proxy) {
 		var err error
+		if plan.Proxy == nil {
+			plan.Proxy = &proxy.Proxy{}
+		}
 		clusterBuilder, err = buildProxy(plan, clusterBuilder)
 		if err != nil {
-			return nil, false, err
+			return nil, err
 		}
 	}
 
-	return clusterBuilder, shouldUpdateProxy, nil
+	return clusterBuilder, nil
 }
+
 func updateNodes(ctx context.Context, state, plan *ClusterRosaClassicState, clusterBuilder *cmv1.ClusterBuilder) (*cmv1.ClusterBuilder, bool, error) {
 	// Send request to update the cluster:
 	shouldUpdateNodes := false
@@ -1627,6 +1621,8 @@ func populateRosaClassicClusterState(ctx context.Context, object *cmv1.Cluster,
 		if ok {
 			state.Proxy.NoProxy = types.StringValue(noProxy)
 		}
+	} else {
+		state.Proxy = nil
 	}
 
 	trustBundle, ok := object.GetAdditionalTrustBundle()

diff --git a/provider/proxy/proxy_validator.go b/provider/proxy/proxy_validator.go
@@ -3,12 +3,9 @@ package proxy
 import (
 	"context"
 	"fmt"
+	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
-	"github.com/openshift/rosa/pkg/helper"
 	"github.com/terraform-redhat/terraform-provider-rhcs/provider/common"
-	"strings"
-
-	"github.com/hashicorp/terraform-plugin-framework/schema/validator"
 )
 
 // atLeastValidator validates that an integer Attribute's value is at least a certain value.
@@ -40,31 +37,18 @@ func (v proxyValidator) ValidateObject(ctx context.Context, request validator.Ob
 	errSum := "Invalid proxy's attribute assignment"
 	httpsProxy := ""
 	httpProxy := ""
-	additionalTrustBundle := ""
-	var noProxySlice []string
 
 	if !common.IsStringAttributeEmpty(proxy.HttpProxy) {
 		httpProxy = proxy.HttpProxy.ValueString()
 	}
 	if !common.IsStringAttributeEmpty(proxy.HttpsProxy) {
 		httpsProxy = proxy.HttpsProxy.ValueString()
 	}
-	if !common.IsStringAttributeEmpty(proxy.NoProxy) {
-		noProxySlice = helper.HandleEmptyStringOnSlice(strings.Split(proxy.NoProxy.ValueString(), ","))
-	}
 
-	if !common.IsStringAttributeEmpty(proxy.AdditionalTrustBundle) {
-		additionalTrustBundle = proxy.AdditionalTrustBundle.ValueString()
-	}
-
-	if httpProxy == "" && httpsProxy == "" && noProxySlice != nil && len(noProxySlice) > 0 {
+	if httpProxy == "" && httpsProxy == "" {
 		response.Diagnostics.AddError(errSum, "Expected at least one of the following: http-proxy, https-proxy")
 		return
 	}
-	if httpProxy == "" && httpsProxy == "" && additionalTrustBundle == "" {
-		response.Diagnostics.AddError(errSum, "Expected at least one of the following: http-proxy, https-proxy, additional-trust-bundle")
-		return
-	}
 }
 
 func ProxyValidator() validator.Object {

diff --git a/subsystem/cluster_resource_rosa_test.go b/subsystem/cluster_resource_rosa_test.go
@@ -1586,7 +1586,7 @@ var _ = Describe("rhcs_cluster_rosa_classic - create", func() {
 				Expect(resource).To(MatchJQ(`.attributes.proxy.no_proxy`, "test"))
 				Expect(resource).To(MatchJQ(`.attributes.proxy.additional_trust_bundle`, "123"))
 			})
-			It("Creates cluster without http proxy and update trust bundle", func() {
+			It("Creates cluster without http proxy and update trust bundle - should fail", func() {
 				// Prepare the server:
 				server.AppendHandlers(
 					CombineHandlers(
@@ -1717,9 +1717,7 @@ var _ = Describe("rhcs_cluster_rosa_classic - create", func() {
 			}
 		  }
 		`)
-				Expect(terraform.Apply()).To(BeZero())
-				resource := terraform.Resource("rhcs_cluster_rosa_classic", "my_cluster")
-				Expect(resource).To(MatchJQ(`.attributes.proxy.additional_trust_bundle`, "123"))
+				Expect(terraform.Apply()).ToNot(BeZero())
 			})
 		})
 		It("Creates cluster with default_mp_labels and update them", func() {
@@ -1914,23 +1912,23 @@ var _ = Describe("rhcs_cluster_rosa_classic - create", func() {
 
 			// Expected at least one of the following: http-proxy, https-proxy, additional-trust-bundle
 			terraform.Source(`
-		  resource "rhcs_cluster_rosa_classic" "my_cluster" {
-		    name           = "my-cluster"
-		    cloud_region   = "us-west-1"
-			aws_account_id = "123"
-			proxy = {
-			}
-			sts = {
-				operator_role_prefix = "test"
-				role_arn = "",
-				support_role_arn = "",
-				instance_iam_roles = {
-					master_role_arn = "",
-					worker_role_arn = "",
+			 resource "rhcs_cluster_rosa_classic" "my_cluster" {
+			   name           = "my-cluster"
+			   cloud_region   = "us-west-1"
+				aws_account_id = "123"
+				proxy = {
 				}
-			}
-		  }
-		`)
+				sts = {
+					operator_role_prefix = "test"
+					role_arn = "",
+					support_role_arn = "",
+					instance_iam_roles = {
+						master_role_arn = "",
+						worker_role_arn = "",
+					}
+				}
+			 }
+			`)
 			Expect(terraform.Apply()).NotTo(BeZero())
 		})
 		It("Creates private cluster with aws subnet ids without private link", func() {