[WebDiscover] Determine if IAM policy setup step can be skipped #29724
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should I wait for #29721 to land to review this? |
|
@ibeckermayer @ryanclark ready for review |
web/packages/teleport/src/Discover/Database/CreateDatabase/useCreateDatabase.test.tsx
Show resolved
Hide resolved
web/packages/teleport/src/Discover/Database/CreateDatabase/useCreateDatabase.test.tsx
Show resolved
Hide resolved
web/packages/teleport/src/Discover/Database/CreateDatabase/useCreateDatabase.ts
Outdated
Show resolved
Hide resolved
web/packages/teleport/src/Discover/Database/CreateDatabase/useCreateDatabase.ts
Outdated
Show resolved
Hide resolved
web/packages/teleport/src/Discover/Database/DeployService/AutoDeploy/AutoDeploy.tsx
Show resolved
Hide resolved
kimlisa
force-pushed
the
lisa/iam-policy-status-web-ui
branch
from
1b4a151
to
099006f
Compare
|
friendly ping @ibeckermayer @ryanclark |
kimlisa
force-pushed
the
lisa/iam-policy-status-web-ui
branch
from
f4ce632
to
6a01dd5
Compare
r0mant
approved these changes
Aug 2, 2023
ibeckermayer
reviewed
Aug 2, 2023
Comment on lines
142
to
153
| const dbServer = res.agents[0]; | ||
| if ( | ||
| !isAws || | ||
| !dbServer.aws?.iamPolicyStatus || | ||
| dbServer.aws?.iamPolicyStatus === IamPolicyStatus.Unspecified | ||
| ) { | ||
| return dbServer; | ||
| } | ||
|
|
||
| if (dbServer.aws.iamPolicyStatus !== IamPolicyStatus.Pending) { | ||
| return dbServer; | ||
| } |
There was a problem hiding this comment.
I believe these are equivalent:
Suggested change
| const dbServer = res.agents[0]; | |
| if ( | |
| !isAws || | |
| !dbServer.aws?.iamPolicyStatus || | |
| dbServer.aws?.iamPolicyStatus === IamPolicyStatus.Unspecified | |
| ) { | |
| return dbServer; | |
| } | |
| if (dbServer.aws.iamPolicyStatus !== IamPolicyStatus.Pending) { | |
| return dbServer; | |
| } | |
| const dbServer = res.agents[0]; | |
| if ( | |
| !isAws || // If not AWS, then we return the first thing we get back. | |
| dbServer.aws?.iamPolicyStatus !== IamPolicyStatus.Pending // If AWS and aws.iamPolicyStatus is undefined or non-pending, we return the dbServer. | |
| ) { | |
| return dbServer; | |
| } | |
| // Otherwise continue polling |
ryanclark
approved these changes
Aug 3, 2023
An error can return if a user tried to deploy before the changes are fully propagated. Notify the user that this can be a reason why an error returned and try again.
Clarify poller callback with comment Make IAMPolicy screen optional
kimlisa
force-pushed
the
lisa/iam-policy-status-web-ui
branch
from
f05b5c4
to
99d940b
Compare
kimlisa
added a commit
that referenced
this pull request
Aug 3, 2023
* Prevent going back from access screen if deploying got skipped * Configuring perms for the user can take some time An error can return if a user tried to deploy before the changes are fully propagated. Notify the user that this can be a reason why an error returned and try again. * Check if IAM policy already configured with new dbs Clarify poller callback with comment Make IAMPolicy screen optional
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 3, 2023
…ondition (#29978) * WebDiscover: Finish auto deploy screen (iam configure script) (#28621) * Define the return type * Add endpoint for config script * Store the entire integration object instead of just the name * Build the correct script string, renames, emit event * Enable auto deploy as default * Fix script endpoint and update story * Add regex check, update story * Touch ups, add test * Address CR * Remove sudo from bash command * Make into ui friendly object * [WebDiscover] Determine if IAM policy setup step can be skipped (#29724) * Prevent going back from access screen if deploying got skipped * Configuring perms for the user can take some time An error can return if a user tried to deploy before the changes are fully propagated. Notify the user that this can be a reason why an error returned and try again. * Check if IAM policy already configured with new dbs Clarify poller callback with comment Make IAMPolicy screen optional
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
depends on #29721
Checks if the registered database has a configured IAM policy or not (it's a screen where you create and attach an policy to the ec2 instance you deployed the db service in). Depending on this state, a user can skip both the deploy service screen AND the IAM policy screen.
Relevant screenshots
make it apparent user can skip this screen:
if user encounters error with auto deploy, render a note that it could be b/c changes has not finished taking affect:
