feedback PR meeting

swisscom · Sep 19, 2023 · 3c231f6 · 3c231f6
1 parent 4482809
commit 3c231f6
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 7 deletions.
diff --git a/pkg/controller/servicecatalog/provisionedproduct/lifecycle.go b/pkg/controller/servicecatalog/provisionedproduct/lifecycle.go
@@ -166,6 +166,8 @@ func (c *custom) Update(ctx context.Context, mg resource.Managed) (managed.Exter
 		return managed.ExternalUpdate{}, errors.New(errUnexpectedObject)
 	}
 
+	// TODO: move this to preUpdate
+
 	// We want to check if we are in a state where we can actually update it
 	ppStatus := pointer.StringDeref(cr.Status.AtProvider.Status, "")
 	if ppStatus == "" || ppStatus == string(svcapitypes.ProvisionedProductStatus_SDK_UNDER_CHANGE) {

diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup.go b/pkg/controller/servicecatalog/provisionedproduct/setup.go
@@ -41,6 +41,8 @@ const (
 	acceptLanguageEnglish = "en"
 )
 
+// TODO: merge utils and lifecycle.go into this file
+
 type customConnector struct {
 	kube client.Client
 }
@@ -88,6 +90,8 @@ func SetupProvisionedProduct(mgr ctrl.Manager, o controller.Options) error {
 }
 
 func (c *customConnector) Connect(ctx context.Context, mg resource.Managed) (managed.ExternalClient, error) {
+	// TODO: remove custom connector and move code from Update method to preUpdate
+
 	cr, ok := mg.(*svcapitypes.ProvisionedProduct)
 	if !ok {
 		return nil, errors.New(errUnexpectedObject)

diff --git a/pkg/controller/servicecatalog/provisionedproduct/utils.go b/pkg/controller/servicecatalog/provisionedproduct/utils.go
@@ -36,14 +36,10 @@ func provisioningParamsAreChanged(cfStackParams []cfsdkv2types.Parameter, curren
 
 // productOrArtifactAreChanged will attempt to determine whether or not the currently requested SC product and provisioning artifact IDs have changed when using names instead of IDs
 func (c *custom) productOrArtifactAreChanged(ds *svcapitypes.ProvisionedProductParameters, resp *svcsdk.ProvisionedProductDetail) (bool, error) { // nolint:gocyclo
-	var productID, artifactID string
+	productID := pointer.StringDeref(ds.ProductID, "")
+	artifactID := pointer.StringDeref(ds.ProvisioningArtifactID, "")
 
-	if ds.ProductID != nil {
-		productID = pointer.StringDeref(ds.ProductID, "")
-	}
-	if ds.ProvisioningArtifactID != nil {
-		artifactID = pointer.StringDeref(ds.ProvisioningArtifactID, "")
-	}
+	// TODO: write tests for this function before any refactoring
 
 	if productID == "" || artifactID == "" {
 		// If we are using names for either the product or artifact, we should