Skip to content

Commit

Permalink
fix: tolerate required delay between service account creation and con…
Browse files Browse the repository at this point in the history
…figuration

It was discovered through testing that service accounts created on GCP
need a duration of time between creation and being referenced, otherwise
a BadRequest error occurs. A delayed retry logic is introduced to ensure
the service account is available before making additional configuration
calls.
  • Loading branch information
renan-campos committed Sep 16, 2024
1 parent 46bb9dc commit 5217d70
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 10 deletions.
18 changes: 13 additions & 5 deletions cmd/ocm/gcp/gcp-client-shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"log"
"reflect"
"strings"
"time"

"cloud.google.com/go/iam/admin/apiv1/adminpb"
"github.com/googleapis/gax-go/v2/apierror"
Expand All @@ -17,6 +18,7 @@ import (
"google.golang.org/grpc/codes"

"github.com/openshift-online/ocm-cli/pkg/gcp"
"github.com/openshift-online/ocm-cli/pkg/utils"
)

type GcpClientWifConfigShim interface {
Expand Down Expand Up @@ -269,11 +271,17 @@ func (c *shim) bindRolesToServiceAccount(
serviceAccountId := serviceAccount.ServiceAccountId()
roles := serviceAccount.Roles()

return c.bindRolesToPrincipal(
ctx,
fmt.Sprintf("serviceAccount:%s@%s.iam.gserviceaccount.com", serviceAccountId, c.wifConfig.Gcp().ProjectId()),
roles,
)
// It was found that there is a window of time between when a service
// account creation call is made that the service account is not available
// in adjacent API calls. The call is therefore wrapped in retry logic to
// be robust to these types of synchronization issues.
return utils.DelayedRetry(func() error {
return c.bindRolesToPrincipal(
ctx,
fmt.Sprintf("serviceAccount:%s@%s.iam.gserviceaccount.com", serviceAccountId, c.wifConfig.Gcp().ProjectId()),
roles,
)
}, 10, 500*time.Millisecond)
}

func (c *shim) bindRolesToGroup(
Expand Down
13 changes: 8 additions & 5 deletions pkg/gcp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ import (
)

type GcpClient interface {
/*
ListServiceAccounts(ctx context.Context, project string, filter func(s string) bool) ([]string, error) //nolint:lll
DeleteRole(context.Context, *adminpb.DeleteRoleRequest) (*adminpb.Role, error)
ListRoles(context.Context, *adminpb.ListRolesRequest) (*adminpb.ListRolesResponse, error)
*/
AttachImpersonator(ctx context.Context, saId, projectId, impersonatorResourceId string) error
AttachWorkloadIdentityPool(ctx context.Context, sa *cmv1.WifServiceAccount, poolId, projectId string) error
CreateRole(context.Context, *adminpb.CreateRoleRequest) (*adminpb.Role, error)
Expand All @@ -32,6 +27,7 @@ type GcpClient interface {
DeleteWorkloadIdentityPool(ctx context.Context, resource string) (*iamv1.Operation, error) //nolint:lll
GetProjectIamPolicy(ctx context.Context, projectName string, request *cloudresourcemanager.GetIamPolicyRequest) (*cloudresourcemanager.Policy, error) //nolint:lll
GetRole(context.Context, *adminpb.GetRoleRequest) (*adminpb.Role, error)
GetServiceAccount(ctx context.Context, request *adminpb.GetServiceAccountRequest) (*adminpb.ServiceAccount, error)
GetWorkloadIdentityPool(ctx context.Context, resource string) (*iamv1.WorkloadIdentityPool, error) //nolint:lll
GetWorkloadIdentityProvider(ctx context.Context, resource string) (*iamv1.WorkloadIdentityPoolProvider, error) //nolint:lll
ProjectNumberFromId(ctx context.Context, projectId string) (int64, error)
Expand Down Expand Up @@ -195,6 +191,13 @@ func (c *gcpClient) GetRole(ctx context.Context, request *adminpb.GetRoleRequest
return c.iamClient.GetRole(ctx, request)
}

func (c *gcpClient) GetServiceAccount(
ctx context.Context,
request *adminpb.GetServiceAccountRequest,
) (*adminpb.ServiceAccount, error) {
return c.iamClient.GetServiceAccount(ctx, request)
}

//nolint:lll
func (c *gcpClient) GetWorkloadIdentityPool(ctx context.Context, resource string) (*iamv1.WorkloadIdentityPool, error) {
return c.oldIamClient.Projects.Locations.WorkloadIdentityPools.Get(resource).Context(ctx).Do()
Expand Down
13 changes: 13 additions & 0 deletions pkg/utils/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"os"
"regexp"
"time"
)

// the following regex defines four different patterns:
Expand Down Expand Up @@ -90,3 +91,15 @@ func HasDuplicates(valSlice []string) (string, bool) {
}
return "", false
}

func DelayedRetry(f func() error, maxRetries int, delay time.Duration) error {
var err error
for i := 0; i < maxRetries; i++ {
err = f()
if err == nil {
return nil
}
time.Sleep(delay)
}
return fmt.Errorf("Reached max retries. Last error: %s", err.Error())
}

0 comments on commit 5217d70

Please sign in to comment.