Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebDiscover: Finish auto deploy screen (iam configure script) #28621

Merged
merged 11 commits into from Jul 10, 2023

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Jul 3, 2023

do not merge until the following has merged:

This pr:

  • enables the auto deploy flow
  • defines the script endpoint
  • emit events that differentiates between manual and auto deploy
  • previously, we were passing around integration.name but now we pass around the full object because we also need to access the roleArn for the cfg script

@kimlisa kimlisa force-pushed the lisa/discover-auto-part-2 branch from a5bd10f to 7ebc9d9 Compare July 4, 2023 01:23
@kimlisa kimlisa marked this pull request as ready for review July 4, 2023 02:09
@kimlisa kimlisa requested review from ryanclark and removed request for ravicious July 4, 2023 02:09
Base automatically changed from lisa/discover-auto-agent to master July 4, 2023 03:57
@kimlisa kimlisa force-pushed the lisa/discover-auto-part-2 branch 2 times, most recently from 2d52141 to 9814e87 Compare July 5, 2023 05:58
@kimlisa
Copy link
Contributor Author

kimlisa commented Jul 6, 2023

friendly ping @ryanclark @ibeckermayer


updateAgentMeta({ integrationName } as DbMeta);
updateAgentMeta({ integration } as DbMeta);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type system isn't complaining but this cast doesn't seem like it should work without making db and selectedAwsRdsDb optional.

export type DbMeta = BaseMeta & {
  // TODO(lisa): when we can enroll multiple RDS's, turn this into an array?
  // The enroll event expects num count of enrolled RDS's, update accordingly.
  db?: Database;
  integration?: Integration;
  selectedAwsRdsDb?: AwsRdsDatabase;
  // serviceDeployedMethod flag will be undefined if user skipped
  // deploying service (service already existed).
  serviceDeployedMethod?: ServiceDeployMethod;
};

Comment on lines 308 to 310
toolTipContent={`Amazon Resource Names (ARNs) uniquely identifies AWS \
resources, in this case you will be naming an IAM role that this \
deployed service will be using`}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is exactly correct

Suggested change
toolTipContent={`Amazon Resource Names (ARNs) uniquely identifies AWS \
resources, in this case you will be naming an IAM role that this \
deployed service will be using`}
toolTipContent={`Amazon Resource Names (ARNs) uniquely identify AWS \
resources. In this case you will be giving an ARN to the IAM role that this \
deployed service will be using`}

If not then

Suggested change
toolTipContent={`Amazon Resource Names (ARNs) uniquely identifies AWS \
resources, in this case you will be naming an IAM role that this \
deployed service will be using`}
toolTipContent={`Amazon Resource Names (ARNs) uniquely identify AWS \
resources. In this case you will naming an IAM role that this \
deployed service will be using`}

Comment on lines 242 to 243
You will only need to do this once for all databases per geographical
region. <br />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You will only need to do this once for all databases per geographical
region. <br />
You will only need to do this once per geographical region.<br />

<FeaturesContextProvider value={[]}>
<DiscoverProvider mockCtx={discoverCtx}>
<PingTeleportProvider
interval={TEST_PING_INTERVAL}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this actually try to ping something?

Copy link
Contributor Author

@kimlisa kimlisa Jul 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, the test is too short (default was like 3 seconds), though i added a success state testing that will hit it once

@kimlisa kimlisa added this pull request to the merge queue Jul 10, 2023
Merged via the queue into master with commit f693861 Jul 10, 2023
21 checks passed
@kimlisa kimlisa deleted the lisa/discover-auto-part-2 branch July 10, 2023 15:27
@public-teleport-github-review-bot

@kimlisa See the table below for backport results.

Branch Result
branch/v13 Failed

kimlisa added a commit that referenced this pull request Jul 26, 2023
* 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
kimlisa added a commit that referenced this pull request Aug 3, 2023
* 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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants