Web: Add alternate EC2 auto discover flow using AWS Systems Manager (SSM)#42038
Web: Add alternate EC2 auto discover flow using AWS Systems Manager (SSM)#42038
Conversation
| // kubeAppDiscovery specifies if Kubernetes App Discovery should be enabled for a discovered cluster. | ||
| kubeAppDiscovery?: boolean; | ||
| /** | ||
| * InstallParams sets the join method when installing on |
There was a problem hiding this comment.
| * InstallParams sets the join method when installing on | |
| * install sets the join method when installing on |
JSDocs above have similar minor issues where the name in the comment doesn't quite match the name in the code.
| 'TeleportDiscoveryInstaller' | ||
| ); | ||
| const [scriptUrl, setScriptUrl] = useState(''); | ||
| const [createdToken, setCreatedToken] = useState<JoinToken>(); |
There was a problem hiding this comment.
If it's not used for rendering but merely in event handlers, then it could be a ref. This saves an unnecessary re-render and paints a better picture as to how this piece of state is used.
This also simplifies the code a tiny tiny bit, because someone reading the code doesn't have to understand how joinToken differs from createdToken. With a ref there can be a single joinTokenRef.
Patch
diff --git a/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx b/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx
index 32c34ddefa..c622a6607d 100644
--- a/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx
+++ b/web/packages/teleport/src/Discover/Server/DiscoveryConfigSsm/DiscoveryConfigSsm.tsx
@@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-import React, { useState } from 'react';
+import React, { useState, useRef } from 'react';
import {
Box,
Link as ExternalLink,
@@ -70,7 +70,7 @@ export function DiscoveryConfigSsm() {
'TeleportDiscoveryInstaller'
);
const [scriptUrl, setScriptUrl] = useState('');
- const [createdToken, setCreatedToken] = useState<JoinToken>();
+ const joinTokenRef = useRef<JoinToken>();
const [showRestOfSteps, setShowRestOfSteps] = useState(false);
const [attempt, createJoinTokenAndDiscoveryConfig, setAttempt] = useAsync(
@@ -80,14 +80,12 @@ export function DiscoveryConfigSsm() {
// Don't create another token if token was already created.
// This can happen if creating discovery config attempt failed
// and the user retries.
- let joinToken = createdToken;
- if (!joinToken) {
- joinToken = await joinTokenService.fetchJoinToken({
+ if (!joinTokenRef.current) {
+ joinTokenRef.current = await joinTokenService.fetchJoinToken({
roles: ['Node'],
method: 'iam',
rules: [{ awsAccountId }],
});
- setCreatedToken(joinToken);
}
const config = await createDiscoveryConfig(clusterId, {
@@ -105,7 +103,7 @@ export function DiscoveryConfigSsm() {
install: {
enrollMode: InstallParamEnrollMode.Script,
installTeleport: true,
- joinToken: joinToken.id,
+ joinToken: joinTokenRef.current.id,
},
},
],
@@ -138,7 +136,7 @@ export function DiscoveryConfigSsm() {
function clear() {
setAttempt(makeEmptyAttempt);
- setCreatedToken(null);
+ joinTokenRef.current = undefined;
}
return (
| Link as ExternalLink, | ||
| Text, | ||
| Flex, | ||
| ButtonSecondary, | ||
| Link, |
| <Validation> | ||
| {({ validator }) => ( | ||
| <StyledBox mt={4}> | ||
| <Text bold>Step 4</Text> | ||
| <Box> | ||
| <Text typography="subtitle1" mb={1}> | ||
| Give a name for the{' '} | ||
| <Link | ||
| target="_blank" | ||
| href="https://docs.aws.amazon.com/systems-manager/latest/userguide/documents.html" | ||
| > | ||
| AWS SSM Document | ||
| </Link>{' '} | ||
| that will be created on your behalf. Required to run the | ||
| installer script on each discovered instances. | ||
| </Text> | ||
| <FieldInput | ||
| rule={requiredSsmDocument} | ||
| label="SSM Document Name" | ||
| value={ssmDocumentName} | ||
| onChange={e => setSsmDocumentName(e.target.value)} | ||
| placeholder="ssm-document-name" | ||
| disabled={!!scriptUrl} | ||
| /> | ||
| </Box> | ||
| <ButtonSecondary | ||
| onClick={() => | ||
| scriptUrl ? setScriptUrl('') : generateScriptUrl(validator) | ||
| } | ||
| disabled={!selectedRegion} | ||
| > | ||
| {scriptUrl ? 'Edit' : 'Next'} | ||
| </ButtonSecondary> | ||
| </StyledBox> | ||
| )} | ||
| </Validation> |
There was a problem hiding this comment.
This could be an actual <form> with onSubmit and with type="submit" on the button. This way it could be submitted either by clicking the button or inputting some text in the field and then pressing enter.
| install?: { | ||
| /** | ||
| * EnrollMode indicates the mode used to enroll the node into Teleport. | ||
| * Valid values: script, eice. |
There was a problem hiding this comment.
Does this mean that Unspecified is not allowed?
If so, I'd specify the type as InstallParamEnrollMode.Script | InstallParamEnrollMode.Eice and remove the comment.
There was a problem hiding this comment.
I instead removed the Unspecified enum value instead
| }; | ||
| }; | ||
|
|
||
| const SharedText = () => ( |
There was a problem hiding this comment.
It would be good to have more meaningful name here.
| export enum InstallParamEnrollMode { | ||
| 'Unspecified' = 0, | ||
| 'Script' = 1, | ||
| 'Eice' = 2, | ||
| } |
There was a problem hiding this comment.
| export enum InstallParamEnrollMode { | |
| 'Unspecified' = 0, | |
| 'Script' = 1, | |
| 'Eice' = 2, | |
| } | |
| export enum InstallParamEnrollMode { | |
| Unspecified = 0, | |
| Script = 1, | |
| Eice = 2, | |
| } |
|
We now have two very similar-looking EC2 tiles on the integrations page. How do we expect users to know which one to click? Do we have a preference? Should we guide users towards our preference in some way? If I'm new to Teleport and I'm trying to add an EC2 instance, I would probably click "EC2 instance" because it sounds simpler than "EC2 Auto Discover with SSM" |
we had a discussion about this today and concluded to remove all support for ec2 instance enrolling with eice method (since it has limitations scale wise) and opted for this single tile instead (since we want auto enrollment to be the default) 
when user clicks on this tile, the user is presented with an info box that lets user know how to install a single instance if they wish 
|
kimlisa
force-pushed
the
lisa/ssm-flow
branch
4 times, most recently
from
722ebb2 to
c339061
Compare
- Make refreshing optional for aws region selector - Define ssm ec2 flow
4 participants
closes #41002
There are two version of the flow:
new selectable tile:
creating a discovery config step (cloud + self hosted):
configuring a discovery service step for self hosted only
changelog: Add an alternate EC2 auto discover flow using AWS Systems Manager as a more scalable method than EICE in the "Enroll New Resource" view in the web UI