OCM-10615 | Implement 'gcp wif-config update' command (#667)

* Implement gcp wif-config update command

* Accept name or id in wif commands

* Filter available versions when creating WIF cluster

* Create gcp wif helpers file and remove unnecessary validation functions

* Update WIF scripting logic

cmd/ocm/create/cluster/cmd.go

@@ -477,16 +477,21 @@ func GetDefaultClusterFlavors(connection *sdk.Connection, flavour string) (dMach
 }
 
 func getVersionOptions(connection *sdk.Connection) ([]arguments.Option, error) {
-	options, _, err := getVersionOptionsWithDefault(connection, "", "")
+	options, _, err := getVersionOptionsWithDefault(connection, "", "", "")
 	return options, err
 }
 
-func getVersionOptionsWithDefault(connection *sdk.Connection, channelGroup string, gcpMarketplaceEnabled string) (
+func getVersionOptionsWithDefault(
+	connection *sdk.Connection,
+	channelGroup string,
+	gcpMarketplaceEnabled string,
+	additionalFilters string,
+) (
 	options []arguments.Option, defaultVersion string, err error,
 ) {
 	// Check and set the cluster version
 	versionList, defaultVersion, err := c.GetEnabledVersions(
-		connection.ClustersMgmt().V1(), channelGroup, gcpMarketplaceEnabled)
+		connection.ClustersMgmt().V1(), channelGroup, gcpMarketplaceEnabled, additionalFilters)
 	if err != nil {
 		return
 	}
@@ -674,8 +679,9 @@ func preRun(cmd *cobra.Command, argv []string) error {
 	if isGcpMarketplace {
 		gcpMarketplaceEnabled = strconv.FormatBool(isGcpMarketplace)
 	}
+	additionalFilters := getVersionFilters()
 	versions, defaultVersion, err := getVersionOptionsWithDefault(connection, args.channelGroup,
-		gcpMarketplaceEnabled)
+		gcpMarketplaceEnabled, additionalFilters)
 	if err != nil {
 		return err
 	}
@@ -1573,3 +1579,13 @@ func promptAutoscaling(fs *pflag.FlagSet) error {
 	}
 	return nil
 }
+
+// getVersionFilters returns a version filter based on the current args
+func getVersionFilters() string {
+	filter := ""
+	// WIF filter
+	if args.gcpAuthentication.Type == c.AuthenticationWif {
+		filter = fmt.Sprintf("%s AND wif_enabled = 'true'", filter)
+	}
+	return filter
+}

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

@@ -27,7 +27,7 @@ var (
 // NewDeleteWorkloadIdentityConfiguration provides the "gcp delete wif-config" subcommand
 func NewDeleteWorkloadIdentityConfiguration() *cobra.Command {
 	deleteWifConfigCmd := &cobra.Command{
-		Use:     "wif-config [ID]",
+		Use:     "wif-config [ID|Name]",
 		Short:   "Delete workload identity configuration",
 		RunE:    deleteWorkloadIdentityConfigurationCmd,
 		PreRunE: validationForDeleteWorkloadIdentityConfigurationCmd,
@@ -42,21 +42,19 @@ func NewDeleteWorkloadIdentityConfiguration() *cobra.Command {
 }
 
 func validationForDeleteWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) error {
-	if len(argv) != 1 {
-		return fmt.Errorf(
-			"expected exactly one command line parameters containing the id " +
-				"of the WIF config",
-		)
+	var err error
+	DeleteWifConfigOpts.TargetDir, err = getPathFromFlag(DeleteWifConfigOpts.TargetDir)
+	if err != nil {
+		return err
 	}
 	return nil
 }
 
 func deleteWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) error {
 	ctx := context.Background()
-
-	wifConfigId := argv[0]
-	if wifConfigId == "" {
-		return fmt.Errorf("WIF config ID is required")
+	key, err := wifKeyFromArgs(argv)
+	if err != nil {
+		return err
 	}
 
 	// Create the client for the OCM API:
@@ -66,11 +64,11 @@ func deleteWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) e
 	}
 	defer connection.Close()
 
-	response, err := connection.ClustersMgmt().V1().GCP().WifConfigs().WifConfig(wifConfigId).Get().Send()
+	// Verify the WIF configuration exists
+	wifConfig, err := findWifConfig(connection.ClustersMgmt().V1(), key)
 	if err != nil {
 		return errors.Wrapf(err, "failed to get wif-config")
 	}
-	wifConfig := response.Body()
 
 	if DeleteWifConfigOpts.DryRun {
 		log.Printf("Writing script files to %s", DeleteWifConfigOpts.TargetDir)
@@ -96,11 +94,11 @@ func deleteWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) e
 	}
 
 	_, err = connection.ClustersMgmt().V1().GCP().WifConfigs().
-		WifConfig(wifConfigId).
+		WifConfig(wifConfig.ID()).
 		Delete().
 		Send()
 	if err != nil {
-		return errors.Wrapf(err, "failed to delete wif config %q", wifConfigId)
+		return errors.Wrapf(err, "failed to delete wif config %q", wifConfig.ID())
 	}
 	return nil
 }

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

@@ -6,27 +6,25 @@ import (
 	"text/tabwriter"
 
 	"github.com/openshift-online/ocm-cli/pkg/ocm"
-	"github.com/openshift-online/ocm-cli/pkg/urls"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
 // NewDescribeWorkloadIdentityConfiguration provides the "gcp describe wif-config" subcommand
 func NewDescribeWorkloadIdentityConfiguration() *cobra.Command {
 	describeWorkloadIdentityPoolCmd := &cobra.Command{
-		Use:     "wif-config [ID]",
-		Short:   "Show details of a wif-config.",
-		RunE:    describeWorkloadIdentityConfigurationCmd,
-		PreRunE: validationForDescribeWorkloadIdentityConfigurationCmd,
+		Use:   "wif-config [ID|Name]",
+		Short: "Show details of a wif-config.",
+		RunE:  describeWorkloadIdentityConfigurationCmd,
 	}
 
 	return describeWorkloadIdentityPoolCmd
 }
 
 func describeWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) error {
-	id, err := urls.Expand(argv)
+	key, err := wifKeyFromArgs(argv)
 	if err != nil {
-		return errors.Wrapf(err, "could not create URI")
+		return err
 	}
 
 	// Create the client for the OCM API:
@@ -36,11 +34,11 @@ func describeWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string)
 	}
 	defer connection.Close()
 
-	response, err := connection.ClustersMgmt().V1().GCP().WifConfigs().WifConfig(id).Get().Send()
+	// Verify the WIF configuration exists
+	wifConfig, err := findWifConfig(connection.ClustersMgmt().V1(), key)
 	if err != nil {
 		return errors.Wrapf(err, "failed to get wif-config")
 	}
-	wifConfig := response.Body()
 
 	// Print output
 	w := tabwriter.NewWriter(os.Stdout, 8, 0, 2, ' ', 0)
@@ -52,10 +50,3 @@ func describeWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string)
 
 	return w.Flush()
 }
-
-func validationForDescribeWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) error {
-	if len(argv) != 1 {
-		return fmt.Errorf("Expected exactly one command line parameters containing the id of the WIF config")
-	}
-	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 already 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 {
-		return errors.Errorf("workload identity provider %s already exists in pool %s", providerId, poolId)
 	}
 
 	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
 }