Implement retry for eventual consistency in IAM updates #235
Conversation
AlexVulaj
force-pushed
the
backplane-assume-implement-retry
branch
from
624f36b to
4b81bba
Compare
cmd/ocm-backplane/cloud/assume.go
Outdated
| EnvProd Environment = "prod" | ||
| ProdPayerAccountID = 922711891673 | ||
| EnvStg Environment = "stg" | ||
| StgPayerAccountId = 277304166082 | ||
| EnvInt Environment = "int" | ||
| IntPayerAccountId = 277304166082 |
There was a problem hiding this comment.
What if moving the payerAccountID into the backplane config file [~/.config/backplane] ?
Otherwise, this exposes the payer accountID in a public repo for all envs .
There may be more supported env soon, [eg: fedramp], and this approach may need to change then.
May be PayerAccountId match to here
There was a problem hiding this comment.
I spoke about this with @jharrington22 way back when we began this work and we determined that the ARNs aren't secret so there's no issue with having them in GitHub.
Would you still like me to move these to a config file?
cmd/ocm-backplane/cloud/assume.go
Outdated
|
|
||
| func (e *Environment) Set(env string) error { | ||
| switch strings.ToLower(env) { | ||
| case "int", "stg", "prod": |
There was a problem hiding this comment.
This may not always true
There was a problem hiding this comment.
That's fair. Though we can change this as needed.
|
/lgtm |
openshift-ci
bot
added
lgtm
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
Putting a hold on this momentarily. For SRE convenience, we're going to remove the "env" flag entirely and have it pulled from the backplane config. More context here: https://redhat-internal.slack.com/archives/CFJD1NZFT/p1698096531994639?thread_ts=1698096531.994639&cid=CFJD1NZFT |
AlexVulaj
force-pushed
the
backplane-assume-implement-retry
branch
from
4b81bba to
35b220f
Compare
|
/unhold Moved the initial assume ARN to a config field. |
openshift-ci
bot
removed
the
do-not-merge/hold
AlexVulaj
force-pushed
the
backplane-assume-implement-retry
branch
from
35b220f to
37c5820
Compare
AlexVulaj
force-pushed
the
backplane-assume-implement-retry
branch
from
37c5820 to
710436c
Compare
|
@AlexVulaj: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexVulaj, samanthajayasinghe, wanghaoran1988 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
feature/bug fix
What this PR does / Why we need it?
The retry mechanism built into the
sts.Clientfrom the AWS SDK doesn't properly handle issues of IAM updates requiring a few seconds to resolve, so we have to roll our own retry that recreates the client on every failure. See aws/aws-sdk-go-v2#2332Which Jira/Github issue(s) does this PR fix?
OSD-18992
Pre-checks (if applicable)