Revert "pool usage after update (#888)" (#901)

This reverts commit dfb52ff.

Author: miguelhbrito

cmd/ocm/gcp/flag_descriptions.go

@@ -8,8 +8,7 @@ manual:         Commands necessary to modify GCP resources will be output
                 as a script to be run manually.
 `
 
-	targetDirFlagDescription                        = `Directory to place generated files (defaults to current directory)`
-	versionFlagDescription                          = `Version of OpenShift to configure the WIF resources for`
-	federatedProjectFlagDescription                 = `ID of the Google cloud project that will host the WIF pool`
-	skipFederatedProjectVerificationFlagDescription = `Skip federated project verification`
+	targetDirFlagDescription        = `Directory to place generated files (defaults to current directory)`
+	versionFlagDescription          = `Version of OpenShift to configure the WIF resources for`
+	federatedProjectFlagDescription = `ID of the Google cloud project that will host the WIF pool`
 )

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

@@ -10,7 +10,6 @@ import (
 
 	"cloud.google.com/go/iam/admin/apiv1/adminpb"
 	"github.com/googleapis/gax-go/v2/apierror"
-	sdk "github.com/openshift-online/ocm-sdk-go"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/pkg/errors"
 	cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1"
@@ -27,12 +26,6 @@ const (
 	IamApiRetrySeconds = 600
 )
 
-const (
-	// PromQL time range in minutes
-	newPoolUsageTimeRange = 5
-	oldPoolUsageTimeRange = 2
-)
-
 const (
 	impersonatorRole           = "roles/iam.serviceAccountTokenCreator"
 	workloadIdentityUserRole   = "roles/iam.workloadIdentityUser"
@@ -50,9 +43,6 @@ type GcpClientWifConfigShim interface {
 	DeleteServiceAccounts(ctx context.Context, log *log.Logger) error
 	DeleteWorkloadIdentityPool(ctx context.Context, log *log.Logger) error
 	UnbindServiceAccounts(ctx context.Context, log *log.Logger) error
-	HasAssociatedClusters(ctx context.Context, connection *sdk.Connection, wifConfigID string) (bool, error)
-	ValidateNewWifConfigPoolUsage(ctx context.Context, wifConfig *cmv1.WifConfig) error
-	ValidateOldWifConfigPoolUsage(ctx context.Context, wifConfig *cmv1.WifConfig) error
 }
 
 type shim struct {
@@ -1215,94 +1205,3 @@ func (c *shim) UnbindServiceAccounts(
 	}
 	return nil
 }
-
-// checkPoolUsageWithQuery executes a PromQL query and returns true if any metric has a non-zero value
-func (c *shim) checkPoolUsageWithQuery(ctx context.Context, projectId string, query string) (bool, error) {
-	response, err := c.gcpClient.QueryPrometheusTimeSeries(ctx, projectId, query)
-	if err != nil {
-		return false, errors.Wrapf(err, "failed to execute prometheus query")
-	}
-
-	// Check if the query was successful
-	if response.Status != "success" {
-		return false, errors.Errorf("prometheus query failed with status: %s", response.Status)
-	}
-
-	// Check if there are any results
-	if len(response.Data.Result) == 0 {
-		return false, nil
-	}
-
-	// Check if any result has a non-zero value
-	for _, result := range response.Data.Result {
-		value, err := result.GetFloatValue()
-		if err != nil {
-			continue
-		}
-		if value > 0 {
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
-// HasAssociatedClusters checks if a WIF config has any clusters associated with it
-func (c *shim) HasAssociatedClusters(
-	ctx context.Context,
-	connection *sdk.Connection,
-	wifConfigID string,
-) (bool, error) {
-	searchQuery := fmt.Sprintf("gcp.authentication.wif_config_id = '%s'", wifConfigID)
-	response, err := connection.ClustersMgmt().V1().Clusters().
-		List().
-		Search(searchQuery).
-		Send()
-	if err != nil {
-		return false, errors.Wrapf(err, "failed to search for clusters using wif-config %s", wifConfigID)
-	}
-
-	return response.Size() > 0, nil
-}
-
-// ValidateNewWifConfigPoolUsage validates that a WIF config has recent pool usage
-func (c *shim) ValidateNewWifConfigPoolUsage(ctx context.Context, wifConfig *cmv1.WifConfig) error {
-	projectId := wifConfig.Gcp().FederatedProjectId()
-	query := fmt.Sprintf(
-		WifQuery,
-		wifConfig.Gcp().WorkloadIdentityPool().PoolId(),
-		newPoolUsageTimeRange,
-	)
-
-	hasUsage, err := c.checkPoolUsageWithQuery(ctx, projectId, query)
-	if err != nil {
-		return errors.Wrapf(err, "failed to check workload identity pool usage")
-	}
-
-	if !hasUsage {
-		return errors.Errorf("wif-config pool '%s' has no recent traffic", wifConfig.Gcp().WorkloadIdentityPool().PoolId())
-	}
-
-	return nil
-}
-
-// ValidateOldWifConfigPoolUsage validates if the old WIF config pool is still being used
-func (c *shim) ValidateOldWifConfigPoolUsage(ctx context.Context, wifConfig *cmv1.WifConfig) error {
-	projectId := wifConfig.Gcp().ProjectId()
-	query := fmt.Sprintf(
-		WifQuery,
-		wifConfig.Gcp().WorkloadIdentityPool().PoolId(),
-		oldPoolUsageTimeRange,
-	)
-
-	hasUsage, err := c.checkPoolUsageWithQuery(ctx, projectId, query)
-	if err != nil {
-		return errors.Wrapf(err, "failed to check workload identity pool usage")
-	}
-
-	if hasUsage {
-		return errors.Errorf("wif-config pool '%s' is still being used", wifConfig.Gcp().WorkloadIdentityPool().PoolId())
-	}
-
-	return nil
-}

cmd/ocm/gcp/gcp.go

@@ -5,18 +5,17 @@ import (
 )
 
 type options struct {
-	Interactive                      bool
-	Mode                             string
-	Name                             string
-	OpenshiftVersion                 string
-	Project                          string
-	FederatedProject                 string
-	SkipFederatedProjectVerification bool
-	Region                           string
-	RolePrefix                       string
-	TargetDir                        string
-	WorkloadIdentityPool             string
-	WorkloadIdentityProvider         string
+	Interactive              bool
+	Mode                     string
+	Name                     string
+	OpenshiftVersion         string
+	Project                  string
+	FederatedProject         string
+	Region                   string
+	RolePrefix               string
+	TargetDir                string
+	WorkloadIdentityPool     string
+	WorkloadIdentityProvider string
 }
 
 // NewGcpCmd implements the "gcp" subcommand for the credentials provisioning

cmd/ocm/gcp/helpers.go

@@ -1,29 +1,18 @@
 package gcp
 
 import (
-	"context"
 	"fmt"
-	"log"
 	"os"
 	"path/filepath"
 	"regexp"
-	"strconv"
 
-	"github.com/openshift-online/ocm-cli/pkg/gcp"
-	"github.com/openshift-online/ocm-cli/pkg/utils"
-	sdk "github.com/openshift-online/ocm-sdk-go"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/pkg/errors"
 )
 
 const (
 	ModeAuto   = "auto"
 	ModeManual = "manual"
-	// See reference: https://cloud.google.com/monitoring/api/metrics_gcp_i_o
-	// iam_googleapis_com:workload_identity_federation_count and workload_identity_federation/key_usage_count
-	// Both metrics could be used,
-	// but the first metric was chosen because it was the default metric used by the GCP on Metrics Explorer
-	WifQuery = `sum(rate(iam_googleapis_com:workload_identity_federation_count{pool_id="%s",result="success"}[%dm]))`
 )
 
 var Modes = []string{ModeAuto, ModeManual}
@@ -122,80 +111,3 @@ func getFederatedProjectId(wifConfig *cmv1.WifConfig) string {
 	}
 	return wifConfig.Gcp().ProjectId()
 }
-
-// updateFederatedProjectIfChanged if federated project was provided,
-// and if it differs from main project, update the WIF config with the federated project.
-func updateFederatedProjectIfChanged(
-	ctx context.Context,
-	gcpClient gcp.GcpClient,
-	wifBuilder *cmv1.WifConfigBuilder,
-	wifConfig *cmv1.WifConfig,
-	federatedProject string,
-) (bool, error) {
-
-	if federatedProject == "" ||
-		wifConfig.Gcp().FederatedProjectId() == federatedProject {
-		return false, nil
-	}
-
-	projectNumInt64, err := gcpClient.ProjectNumberFromId(ctx, federatedProject)
-	if err != nil {
-		return false, errors.Wrapf(err, "failed to get GCP project number from project id")
-	}
-
-	wifBuilder.Gcp(cmv1.NewWifGcp().
-		FederatedProjectId(federatedProject).
-		FederatedProjectNumber(strconv.FormatInt(projectNumInt64, 10)))
-
-	return true, nil
-}
-
-func federatedProjectPoolUsageVerification(
-	ctx context.Context,
-	log *log.Logger,
-	connection *sdk.Connection,
-	wifConfig *cmv1.WifConfig,
-	gcpShim GcpClientWifConfigShim,
-) error {
-
-	clustersAssociated, err := gcpShim.HasAssociatedClusters(ctx, connection, wifConfig.ID())
-	if err != nil {
-		return errors.Wrapf(err, "failed to check if wif-config '%s' has associated clusters. "+
-			"Please try again or contact Red Hat support if the issue persists", wifConfig.ID())
-	}
-
-	// Only validate pool usage if there are associated clusters
-	if !clustersAssociated {
-		return nil
-	}
-
-	// Validate new pool usage
-	log.Println("Ensuring new workload identity pool has recent traffic...")
-	if err := utils.RetryWithBackoffandTimeout(func() (bool, error) {
-		if validationErr := gcpShim.ValidateNewWifConfigPoolUsage(ctx, wifConfig); validationErr != nil {
-			return true, validationErr
-		}
-		return false, nil
-	}, IamApiRetrySeconds, log); err != nil {
-		log.Printf("Timed out verifying workload identity pool usage\n" +
-			"Cannot determine if old identity pool may be removed." +
-			"Please consult user documentation for manually checking usage")
-		return nil
-	}
-
-	// Validate old pool usage
-	log.Println("Ensuring old workload identity pool is still there and not being used...")
-	if err := utils.RetryWithBackoffandTimeout(func() (bool, error) {
-		if err := gcpShim.ValidateOldWifConfigPoolUsage(ctx, wifConfig); err != nil {
-			return true, err
-		}
-		return false, nil
-	}, IamApiRetrySeconds, log); err != nil {
-		log.Printf("Timed out verifying old workload identity pool usage\n" +
-			"Cannot determine if old identity pool may be removed." +
-			"Please consult user documentation for manually checking usage")
-		return nil
-	}
-
-	return nil
-}

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

@@ -16,11 +16,10 @@ import (
 
 var (
 	UpdateWifConfigOpts = options{
-		Mode:                             ModeAuto,
-		TargetDir:                        "",
-		OpenshiftVersion:                 "",
-		FederatedProject:                 "",
-		SkipFederatedProjectVerification: false,
+		Mode:             ModeAuto,
+		TargetDir:        "",
+		OpenshiftVersion: "",
+		FederatedProject: "",
 	}
 )
 
@@ -65,13 +64,6 @@ the wif-config metadata and the GCP resources it represents.`,
 		federatedProjectFlagDescription,
 	)
 
-	updateWifConfigCmd.PersistentFlags().BoolVar(
-		&UpdateWifConfigOpts.SkipFederatedProjectVerification,
-		"skip-federated-project-verification",
-		false,
-		skipFederatedProjectVerificationFlagDescription,
-	)
-
 	return updateWifConfigCmd
 }
 
@@ -125,15 +117,16 @@ func updateWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) e
 		return errors.Wrapf(err, "failed to initiate GCP client")
 	}
 
-	wifProjectUpdated, err := updateFederatedProjectIfChanged(
-		ctx,
-		gcpClient,
-		wifBuilder,
-		wifConfig,
-		UpdateWifConfigOpts.FederatedProject,
-	)
-	if err != nil {
-		return err
+	if UpdateWifConfigOpts.FederatedProject != "" &&
+		wifConfig.Gcp().FederatedProjectId() != UpdateWifConfigOpts.FederatedProject {
+		projectNumInt64, err := gcpClient.ProjectNumberFromId(ctx, UpdateWifConfigOpts.FederatedProject)
+		if err != nil {
+			return errors.Wrapf(err, "failed to get GCP project number from project id")
+		}
+
+		wifBuilder.Gcp(cmv1.NewWifGcp().
+			FederatedProjectId(UpdateWifConfigOpts.FederatedProject).
+			FederatedProjectNumber(strconv.FormatInt(projectNumInt64, 10)))
 	}
 
 	updatedWifConfig, err := wifBuilder.Build()
@@ -204,16 +197,6 @@ func updateWorkloadIdentityConfigurationCmd(cmd *cobra.Command, argv []string) e
 			"or contact Red Hat support.", wifConfig.ID())
 	}
 
-	// If the federated project was updated and skip verification is not set,
-	// verify the pool usage
-	if wifProjectUpdated && !UpdateWifConfigOpts.SkipFederatedProjectVerification {
-		if err := federatedProjectPoolUsageVerification(
-			ctx, log, connection, wifConfig, gcpClientWifConfigShim,
-		); err != nil {
-			return err
-		}
-	}
-
 	log.Printf("wif-config '%s' updated successfully.", wifConfig.ID())
 	return nil
 }