Update WIF scripting logic

cmd/ocm/gcp/create-wif-config.go

@@ -4,8 +4,6 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"os"
-	"path/filepath"
 	"strconv"
 
 	"github.com/openshift-online/ocm-cli/pkg/gcp"
@@ -65,26 +63,10 @@ func validationForCreateWorkloadIdentityConfigurationCmd(cmd *cobra.Command, arg
 		return fmt.Errorf("Project is required")
 	}
 
-	if CreateWifConfigOpts.TargetDir == "" {
-		pwd, err := os.Getwd()
-		if err != nil {
-			return errors.Wrapf(err, "failed to get current directory")
-		}
-
-		CreateWifConfigOpts.TargetDir = pwd
-	}
-
-	fPath, err := filepath.Abs(CreateWifConfigOpts.TargetDir)
+	var err error
+	CreateWifConfigOpts.TargetDir, err = getPathFromFlag(CreateWifConfigOpts.TargetDir)
 	if err != nil {
-		return errors.Wrapf(err, "failed to resolve full path")
-	}
-
-	sResult, err := os.Stat(fPath)
-	if os.IsNotExist(err) {
-		return fmt.Errorf("directory %s does not exist", fPath)
-	}
-	if !sResult.IsDir() {
-		return fmt.Errorf("file %s exists and is not a directory", fPath)
+		return err
 	}
 	return nil
 }
@@ -116,7 +98,7 @@ func createWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) e
 		if err != nil {
 			return errors.Wrapf(err, "failed to get project number from id")
 		}
-		err = createScript(CreateWifConfigOpts.TargetDir, wifConfig, projectNum)
+		err = createCreateScript(CreateWifConfigOpts.TargetDir, wifConfig, projectNum)
 		if err != nil {
 			return errors.Wrapf(err, "Failed to create script files")
 		}

cmd/ocm/gcp/delete-wif-config.go

@@ -42,6 +42,11 @@ func NewDeleteWorkloadIdentityConfiguration() *cobra.Command {
 }
 
 func validationForDeleteWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) error {
+	var err error
+	DeleteWifConfigOpts.TargetDir, err = getPathFromFlag(DeleteWifConfigOpts.TargetDir)
+	if err != nil {
+		return err
+	}
 	return nil
 }
 

cmd/ocm/gcp/gcp-client-shim.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"sort"
 	"strings"
 	"time"
 
@@ -86,8 +87,6 @@ func (c *shim) CreateWorkloadIdentityPool(
 		} else {
 			return errors.Wrapf(err, "failed to check if there is existing workload identity pool %s", poolId)
 		}
-	} else {
-		log.Printf("Workload identity pool %s exists", poolId)
 	}
 
 	return nil
@@ -136,8 +135,6 @@ func (c *shim) CreateWorkloadIdentityProvider(
 			return errors.Wrapf(err, "failed to check if there is existing workload identity provider %s in pool %s",
 				providerId, poolId)
 		}
-	} else {
-		log.Printf("Workload identity provider %s exists", providerId)
 	}
 
 	return nil
@@ -175,7 +172,6 @@ func (c *shim) GrantSupportAccess(
 	if err := c.bindRolesToGroup(ctx, support.Principal(), support.Roles()); err != nil {
 		return err
 	}
-	log.Printf("support access granted to %s", support.Principal())
 	return nil
 }
 
@@ -255,9 +251,11 @@ func (c *shim) createOrUpdateRoles(
 			log.Printf("Role %q undeleted", roleID)
 		}
 
-		// Update role if permissions have changed
-		if c.roleRequiresUpdate(permissions, existingRole.IncludedPermissions) {
-			existingRole.IncludedPermissions = permissions
+		if addedPermissions, needsUpdate := c.missingPermissions(permissions, existingRole.IncludedPermissions); needsUpdate {
+			// Add missing permissions
+			existingRole.IncludedPermissions = append(existingRole.IncludedPermissions, addedPermissions...)
+			sort.Strings(existingRole.IncludedPermissions)
+
 			_, err := c.updateRole(ctx, existingRole, c.fmtRoleResourceId(role))
 			if err != nil {
 				return errors.Wrap(err, fmt.Sprintf("Failed to update %s", roleID))
@@ -268,23 +266,27 @@ func (c *shim) createOrUpdateRoles(
 	return nil
 }
 
-func (c *shim) roleRequiresUpdate(
+// missingPermissions returns true if there are new permissions that are not in the existing permissions
+// and returns the list of missing permissions
+func (c *shim) missingPermissions(
 	newPermissions []string,
 	existingPermissions []string,
-) bool {
+) ([]string, bool) {
+	missing := []string{}
 	permissionMap := map[string]bool{}
 	for _, permission := range existingPermissions {
 		permissionMap[permission] = true
 	}
-	if len(permissionMap) != len(newPermissions) {
-		return true
-	}
 	for _, permission := range newPermissions {
 		if !permissionMap[permission] {
-			return true
+			missing = append(missing, permission)
 		}
 	}
-	return false
+	if len(missing) > 0 {
+		return missing, true
+	} else {
+		return missing, false
+	}
 }
 
 func (c *shim) bindRolesToServiceAccount(

cmd/ocm/gcp/gcp.go

@@ -30,7 +30,6 @@ func NewGcpCmd() *cobra.Command {
 	gcpCmd.AddCommand(NewGetCmd())
 	gcpCmd.AddCommand(NewListCmd())
 	gcpCmd.AddCommand(NewDescribeCmd())
-	gcpCmd.AddCommand(NewGenerateCommand())
 
 	return gcpCmd
 }

cmd/ocm/gcp/helpers.go

@@ -2,8 +2,11 @@ package gcp
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
+	"github.com/pkg/errors"
 )
 
 // Checks for WIF config name or id in input
@@ -45,3 +48,30 @@ func findWifConfig(client *cmv1.Client, key string) (*cmv1.WifConfig, error) {
 	}
 	return response.Items().Slice()[0], nil
 }
+
+// getPathFromFlag validates the filepath
+func getPathFromFlag(targetDir string) (string, error) {
+	if targetDir == "" {
+		pwd, err := os.Getwd()
+		if err != nil {
+			return "", errors.Wrapf(err, "failed to get current directory")
+		}
+
+		return pwd, nil
+	}
+
+	fPath, err := filepath.Abs(targetDir)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to resolve full path")
+	}
+
+	sResult, err := os.Stat(fPath)
+	if os.IsNotExist(err) {
+		return "", fmt.Errorf("directory %s does not exist", fPath)
+	}
+	if !sResult.IsDir() {
+		return "", fmt.Errorf("file %s exists and is not a directory", fPath)
+	}
+
+	return targetDir, nil
+}