Web: Require S3 fields when creating/editing AWS OIDC Integration by kimlisa · Pull Request #39285 · gravitational/teleport

kimlisa · 2024-03-13T02:57:54Z

part of #38782 (a bug fix)

recommend reviewing by commit

Before merge:

requires backend PRs listed ^ issue
test e2e

Require filling out S3 fields when creating:
http://192.168.0.103:9002/?path=/story/teleport-integrations-enroll-awsoidc--flow&globals=theme:Light%20Theme

Render warning with tooltip if integration is missing s3 fields:

Require missing s3 fields if editing integration:

Give user ability to edit s3 fields if editing:

Stories under Teleport/Integrations

changelog: Require AWS S3 bucket fields when creating/editing AWS OIDC integration in the web UI

github-actions · 2024-03-13T02:58:32Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

github-actions · 2024-03-13T02:59:05Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

github-actions · 2024-03-13T03:03:05Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

kimlisa · 2024-03-14T17:01:19Z

friendly ping 🙏 @ryanclark @ibeckermayer

ryanclark · 2024-03-15T14:08:31Z

+            <Box
+              requiresS3={requiresS3}
+              px={3}
+              pt={2}
+              css={`
+                border-radius: ${p => p.theme.space[1]}px;
+                border: 2px solid
+                  ${p => {
+                    if (p.requiresS3) {
+                      return p.theme.colors.warning.main;
+                    }
+                    return p.theme.colors.spotBackground[1];
+                  }};
+                background-color: ${p => {
+                  if (p.requiresS3) {
+                    return p.theme.colors.interactive.tonal.alert[0];
+                  }
+                  return p.theme.colors.spotBackground[0];
+                }};
+              `}
+            >


At this point, why not use const Whatever = styled(Box)``;

ryanclark · 2024-03-15T14:09:12Z

+
+import { requiredBucketName, requiredPrefixName } from './Shared/utils';
+
+export function S3Bucket({


S3Bucket is a bit ambiguous. Maybe S3BucketConfiguration or S3BucketForm?

ryanclark · 2024-03-15T14:10:32Z

+  expect(screen.queryByText(script)).not.toBeInTheDocument();
+  expect(screen.queryByText(checkbox)).not.toBeInTheDocument();


Is there a better way to match these elements vs text? Even resorting to data-testid will ensure these tests don't break if any copy changes

ryanclark · 2024-03-15T14:13:38Z

+  if (inputVal.length < 3 || inputVal.length > 63) {
+    return {
+      valid: false,
+      message: 'only 3-63 characters long',


How does this read in the UI? Should it be Field should be 3-63 characters?

ryanclark · 2024-03-15T14:13:53Z

+  if (!bucketNameRegex.test(inputVal)) {
+    return {
+      valid: false,
+      message: 'only - (in between words), a-z, and 0-9',


This could do with rewording also

i opted to add a tooltip for the error message because it's kinda long. i have too much tooltips now but this is temporary. in another PR i'll have the FieldXXX.tsx to conditionally render tooltip for errors only

avatus · 2024-03-15T15:32:06Z

Is it really arbitrary or some cap for something? hurts my heart seeing 63 instead of 64

Yeah, I was wondering about all these random checks and it's all real

https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html

oh AWS, you rascal 🤣

public-teleport-github-review-bot · 2024-03-18T16:41:29Z

@kimlisa See the table below for backport results.

Branch	Result
branch/v14	Failed
branch/v15	Create PR

…9285) * Add s3 fields to URL param * Add s3 fields to integration awsoidc type * Create S3Bucket component, utils, fixtures * Allow s3 fields to be editable * Give warning status if s3 fields are not defined * Require s3 fields when creating * Update types, story, language, address CRs

…on (#39513) * Web: Require S3 fields when creating/editing AWS OIDC Integration (#39285) * Add s3 fields to URL param * Add s3 fields to integration awsoidc type * Create S3Bucket component, utils, fixtures * Allow s3 fields to be editable * Give warning status if s3 fields are not defined * Require s3 fields when creating * Update types, story, language, address CRs * Fix import

github-actions Bot requested review from ibeckermayer and ryanclark March 13, 2024 02:58

github-actions Bot added size/lg ui labels Mar 13, 2024

kimlisa added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v14 labels Mar 13, 2024

kimlisa force-pushed the lisa/web-update-aws-oidc branch from 5882a8d to 027fe43 Compare March 13, 2024 19:04

marcoandredinis mentioned this pull request Mar 14, 2024

AWS OIDC Integration: use S3 bucket as IdP issuer #38782

Closed

ryanclark reviewed Mar 15, 2024

View reviewed changes

kimlisa requested a review from ryanclark March 15, 2024 14:58

kimlisa force-pushed the lisa/web-update-aws-oidc branch from 9bad88b to 3b49cb2 Compare March 15, 2024 15:16

ryanclark approved these changes Mar 15, 2024

View reviewed changes

avatus approved these changes Mar 15, 2024

View reviewed changes

public-teleport-github-review-bot Bot removed the request for review from ibeckermayer March 15, 2024 15:33

This was referenced Mar 18, 2024

[v15] AWS OIDC Integration: use S3 bucket as IdP issuer #39489

Merged

[v14] AWS OIDC Integration: use S3 bucket as IdP issuer #39490

Merged

kimlisa removed the no-changelog Indicates that a PR does not require a changelog entry label Mar 18, 2024

kimlisa enabled auto-merge March 18, 2024 15:47

kimlisa disabled auto-merge March 18, 2024 15:53

kimlisa added 6 commits March 18, 2024 08:53

Add s3 fields to URL param

a8ccfd3

Add s3 fields to integration awsoidc type

e165cf2

Create S3Bucket component, utils, fixtures

6181a09

Allow s3 fields to be editable

c42479d

Give warning status if s3 fields are not defined

11d1630

Require s3 fields when creating

b131c2e

Update types, story, language, address CRs

af8e4f5

kimlisa force-pushed the lisa/web-update-aws-oidc branch from bdefe93 to af8e4f5 Compare March 18, 2024 15:55

kimlisa enabled auto-merge March 18, 2024 15:55

kimlisa added this pull request to the merge queue Mar 18, 2024

Merged via the queue into master with commit 49ac0c5 Mar 18, 2024

kimlisa deleted the lisa/web-update-aws-oidc branch March 18, 2024 16:40

kimlisa mentioned this pull request Mar 18, 2024

[v15] Web: Require S3 fields when creating/editing AWS OIDC Integration #39510

Merged

kimlisa mentioned this pull request Mar 18, 2024

[v14] Web: Require S3 fields when creating/editing AWS OIDC Integration #39513

Merged

fheinecke mentioned this pull request Mar 19, 2024

Release 15.1.9 #39586

Merged


		import { requiredBucketName, requiredPrefixName } from './Shared/utils';

		export function S3Bucket({

		expect(screen.queryByText(script)).not.toBeInTheDocument();
		expect(screen.queryByText(checkbox)).not.toBeInTheDocument();

Conversation

kimlisa commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 13, 2024

Uh oh!

github-actions Bot commented Mar 13, 2024

Uh oh!

github-actions Bot commented Mar 13, 2024

Uh oh!

kimlisa commented Mar 14, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimlisa Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

public-teleport-github-review-bot Bot commented Mar 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kimlisa commented Mar 13, 2024 •

edited

Loading

kimlisa Mar 15, 2024 •

edited

Loading