feat: [M3-8953] - Object Storage Gen2 Updates and Improvements (#11355)

* feat: [M3-8953] - OBJ Gen2 Updates * Add changeset * fix duplicate buckets landing page, may rework * Update e2e tests * Remove redundant beforeEach * Small adjustment to request * fix access keys e2e test * Fix bucket-create-gen2.spec.ts failing tests @linode/frontend-sdet * E2E review updates @cliu-akamai @AzureLatte * Fix issue where properties content was overriding SSL for non-gen2 capability users @coliu-akamai * Remove CORS from object drawers @bnussman-akamai * Update packages/manager/.changeset/pr-11355-upcoming-features-1733237339445.md Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com> * Update packages/manager/.changeset/pr-11355-upcoming-features-1733237339445.md Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com> * Remove double copy icon @dwiley-akamai * Add changesets * Update packages/manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyTable/HostNameTableCell.tsx Co-authored-by: Banks Nussman <115251059+bnussman-akamai@users.noreply.github.com> * Pluralize the regions * Fix E2E based in pluralize * Fix unit tests * Fix e2e from removing toggle --------- Co-authored-by: Jaalah Ramos <jaalah.ramos@gmail.com> Co-authored-by: Connie Liu <coliu@akamai.com> Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com> Co-authored-by: Banks Nussman <115251059+bnussman-akamai@users.noreply.github.com>
linode · Dec 4, 2024 · 7609507 · 7609507
1 parent 6f6d303
commit 7609507
Show file tree

Hide file tree

Showing 14 changed files with 153 additions and 66 deletions.
diff --git a/packages/api-v4/src/object-storage/buckets.ts b/packages/api-v4/src/object-storage/buckets.ts
@@ -262,6 +262,11 @@ export const updateBucketAccess = (
     setData(params, UpdateBucketAccessSchema)
   );
 
+/**
+ * getObjectStorageEndpoints
+ *
+ * Returns a list of Object Storage Endpoints.
+ */
 export const getObjectStorageEndpoints = ({ filter, params }: RequestOptions) =>
   Request<Page<ObjectStorageEndpoint>>(
     setMethod('GET'),

diff --git a/packages/manager/.changeset/pr-11355-fixed-1733342647044.md b/packages/manager/.changeset/pr-11355-fixed-1733342647044.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Fixed
+---
+
+Remove CORS toggle for Object Storage bucket objects ([#11355](https://github.com/linode/manager/pull/11355))
diff --git a/packages/manager/.changeset/pr-11355-upcoming-features-1733237339445.md b/packages/manager/.changeset/pr-11355-upcoming-features-1733237339445.md
@@ -0,0 +1,6 @@
+---
+"@linode/manager": Upcoming Features
+---
+
+Update Regions/S3 Hostnames interface to match new design guidelines with
+improved visualization of multiple storage regions ([#11355](https://github.com/linode/manager/pull/11355))
diff --git a/packages/manager/.changeset/pr-11355-upcoming-features-1733342861906.md b/packages/manager/.changeset/pr-11355-upcoming-features-1733342861906.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Upcoming Features
+---
+
+Remove Properties tab visibility for users without Gen2 capabilities, and fix duplicate bucket display issue ([#11355](https://github.com/linode/manager/pull/11355))
diff --git a/packages/manager/cypress/e2e/core/objectStorage/bucket-details.spec.ts b/packages/manager/cypress/e2e/core/objectStorage/bucket-details.spec.ts
@@ -0,0 +1,54 @@
+import { mockGetAccount } from 'support/intercepts/account';
+import { mockAppendFeatureFlags } from 'support/intercepts/feature-flags';
+import { ui } from 'support/ui';
+import {
+  accountFactory,
+  objectStorageBucketFactory,
+  regionFactory,
+} from 'src/factories';
+import { randomLabel } from 'support/util/random';
+
+describe('Object Storage Gen 1 Bucket Details Tabs', () => {
+  beforeEach(() => {
+    mockAppendFeatureFlags({
+      objMultiCluster: true,
+      objectStorageGen2: { enabled: false },
+    }).as('getFeatureFlags');
+    mockGetAccount(
+      accountFactory.build({
+        capabilities: ['Object Storage', 'Object Storage Access Key Regions'],
+      })
+    ).as('getAccount');
+  });
+
+  const mockRegion = regionFactory.build({
+    capabilities: ['Object Storage'],
+  });
+
+  const mockBucket = objectStorageBucketFactory.build({
+    label: randomLabel(),
+    region: mockRegion.id,
+  });
+
+  describe('Properties tab without required capabilities', () => {
+    it(`confirms the Properties tab does not exist for users without 'Object Storage Endpoint Types' capability`, () => {
+      const { region, label } = mockBucket;
+
+      cy.visitWithLogin(
+        `/object-storage/buckets/${region}/${label}/properties`
+      );
+
+      cy.wait(['@getFeatureFlags', '@getAccount']);
+
+      // Confirm that expected tabs are visible.
+      ui.tabList.findTabByTitle('Objects').should('be.visible');
+      ui.tabList.findTabByTitle('Access').should('be.visible');
+      ui.tabList.findTabByTitle('SSL/TLS').should('be.visible');
+
+      // Confirm that "Properties" tab is absent.
+      cy.findByText('Properties').should('not.exist');
+
+      // TODO Confirm "Not Found" notice is present.
+    });
+  });
+});
diff --git a/packages/manager/cypress/e2e/core/objectStorageGen2/bucket-access-keys-gen2.spec.ts b/packages/manager/cypress/e2e/core/objectStorageGen2/bucket-access-keys-gen2.spec.ts
@@ -59,12 +59,12 @@ describe('Object Storage gen2 access keys tests', () => {
     cy.findByText(mockAccessKey1.label).should('be.visible');
     cy.findByText(mockAccessKey2.label).should('be.visible');
     cy.findByText('US, Newark, NJ (E3): us-east.com').should('be.visible');
-    cy.findByText('US, Atlanta, GA (E3): us-southeast.com').should(
-      'be.visible'
-    );
 
-    // confirm endpoint types are present in the drawer
-    cy.findByText('and 3 more...').should('be.visible').click();
+    // Using contains since the text includes additional information, i.e. '| +2 regions | Show All'
+    cy.contains('US, Atlanta, GA (E3): us-southeast.com').should('be.visible');
+    cy.contains('+ 3 regions').should('be.visible');
+    cy.findByText('Show All').should('be.visible').click();
+
     ui.drawer
       .findByTitle('Regions / S3 Hostnames')
       .should('be.visible')

diff --git a/packages/manager/cypress/e2e/core/objectStorageGen2/bucket-create-gen2.spec.ts b/packages/manager/cypress/e2e/core/objectStorageGen2/bucket-create-gen2.spec.ts
@@ -233,9 +233,6 @@ describe('Object Storage Gen2 create bucket tests', () => {
           .click();
       });
 
-    // Wait for the newly 'created' mocked bucket to appear
-    cy.wait(['@getBuckets']);
-
     // Confirm request body has expected data
     cy.wait('@createBucket').then((xhr) => {
       const requestPayload = xhr.request.body;
@@ -356,9 +353,6 @@ describe('Object Storage Gen2 create bucket tests', () => {
           .click();
       });
 
-    // Wait for the newly 'created' mocked bucket to appear
-    cy.wait(['@getBuckets']);
-
     // Confirm request body has expected data
     cy.wait('@createBucket').then((xhr) => {
       const requestPayload = xhr.request.body;
@@ -483,9 +477,6 @@ describe('Object Storage Gen2 create bucket tests', () => {
           .click();
       });
 
-    // Wait for the newly 'created' mocked bucket to appear
-    cy.wait(['@getBuckets']);
-
     // Confirm request body has expected data
     cy.wait('@createBucket').then((xhr) => {
       const requestPayload = xhr.request.body;
@@ -608,9 +599,6 @@ describe('Object Storage Gen2 create bucket tests', () => {
           .click();
       });
 
-    // Wait for the newly 'created' mocked bucket to appear
-    cy.wait(['@getBuckets']);
-
     // Confirm request body has expected data
     cy.wait('@createBucket').then((xhr) => {
       const requestPayload = xhr.request.body;

diff --git a/packages/manager/cypress/e2e/core/objectStorageGen2/bucket-object-gen2.spec.ts b/packages/manager/cypress/e2e/core/objectStorageGen2/bucket-object-gen2.spec.ts
@@ -84,8 +84,8 @@ describe('Object Storage Gen2 bucket object tests', () => {
 
   const ACLNotification = 'Private: Only you can download this Object';
 
-  // For E0/E1, confirm CORS toggle and ACL selection are both present
-  // For E2/E3, confirm ACL and Cors are removed
+  // For E0/E1, ACL selection is present
+  // For E2/E3, confirm ACL is removed
   const checkBucketObjectDetailsDrawer = (
     bucketFilename: string,
     endpointType: string
@@ -95,16 +95,8 @@ describe('Object Storage Gen2 bucket object tests', () => {
         endpointType === 'Standard (E3)' ||
         endpointType === 'Standard (E2)'
       ) {
-        ui.toggle.find().should('not.exist');
-        cy.contains('CORS Enabled').should('not.exist');
         cy.findByLabelText('Access Control List (ACL)').should('not.exist');
       } else {
-        ui.toggle
-          .find()
-          .should('have.attr', 'data-qa-toggle', 'true')
-          .should('be.visible');
-        cy.contains('CORS Enabled').should('be.visible');
-
         cy.contains(ACLNotification).should('not.exist');
         // Verify that ACL selection show up as options
         cy.findByLabelText('Access Control List (ACL)')
@@ -131,7 +123,7 @@ describe('Object Storage Gen2 bucket object tests', () => {
   };
 
   /**
-  
+
      */
   it('can check Object details drawer with E0 endpoint type', () => {
     const endpointTypeE0 = 'Legacy (E0)';

diff --git a/...ger/src/features/ObjectStorage/AccessKeyLanding/AccessKeyTable/HostNameTableCell.test.tsx b/...ger/src/features/ObjectStorage/AccessKeyLanding/AccessKeyTable/HostNameTableCell.test.tsx
@@ -81,17 +81,19 @@ describe('HostNameTableCell', () => {
         return HttpResponse.json(makeResourcePage([region]));
       })
     );
-    const { findByText } = renderWithTheme(
+    const { getByText } = renderWithTheme(
       <HostNameTableCell
         setHostNames={vi.fn()}
         setShowHostNamesDrawers={vi.fn()}
         storageKeyData={storageKeyData}
       />
     );
-    const hostname = await findByText('US, Newark, NJ: alpha.test.com');
-    const moreButton = await findByText(/and\s+1\s+more\.\.\./);
-    await waitFor(() => expect(hostname).toBeInTheDocument());
 
-    await expect(moreButton).toBeInTheDocument();
+    await waitFor(() => {
+      expect(getByText('Newark', { exact: false })).toBeInTheDocument();
+      expect(getByText('alpha.test.com', { exact: false })).toBeInTheDocument();
+      expect(getByText(/\+ 1 region/, { exact: false })).toBeInTheDocument();
+      expect(getByText('Show All', { exact: false })).toBeInTheDocument();
+    });
   });
 });
diff --git a/.../manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyTable/HostNameTableCell.tsx b/.../manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyTable/HostNameTableCell.tsx
@@ -5,6 +5,7 @@ import React from 'react';
 import { CopyTooltip } from 'src/components/CopyTooltip/CopyTooltip';
 import { TableCell } from 'src/components/TableCell';
 import { useRegionsQuery } from 'src/queries/regions/regions';
+import { pluralize } from 'src/utilities/pluralize';
 import { getRegionsByRegionId } from 'src/utilities/regions';
 
 import type { ObjectStorageKey, ObjectStorageKeyRegions } from '@linode/api-v4';
@@ -27,27 +28,37 @@ export const HostNameTableCell = (props: Props) => {
   if (!regionsLookup || !regionsData || !regions || regions.length === 0) {
     return <TableCell>None</TableCell>;
   }
-  const label = regionsLookup[storageKeyData.regions[0].id]?.label;
-  const s3Endpoint = storageKeyData?.regions[0]?.s3_endpoint;
-  const endpointType = storageKeyData?.regions[0]?.endpoint_type;
+  const formatEndpoint = (region: ObjectStorageKeyRegions) => {
+    const label = regionsLookup[region.id]?.label;
+    const endpointType = region.endpoint_type
+      ? ` (${region.endpoint_type})`
+      : '';
+    return `${label}${endpointType}: ${region.s3_endpoint}`;
+  };
+
+  const firstRegion = regions[0];
+  const formattedFirstEndpoint = formatEndpoint(firstRegion);
+  const allEndpoints = regions.map(formatEndpoint).join('\n');
+  const showMultipleRegions = regions.length > 1;
 
   return (
     <TableCell>
-      {label}
-      {endpointType && ` (${endpointType})`}: {s3Endpoint}&nbsp;
-      {storageKeyData?.regions?.length === 1 && (
-        <StyledCopyIcon text={s3Endpoint} />
-      )}
-      {storageKeyData.regions.length > 1 && (
-        <StyledLinkButton
-          onClick={() => {
-            setHostNames(storageKeyData.regions);
-            setShowHostNamesDrawers(true);
-          }}
-          type="button"
-        >
-          and {storageKeyData.regions.length - 1} more...
-        </StyledLinkButton>
+      {formattedFirstEndpoint}&nbsp;
+      {showMultipleRegions ? (
+        <>
+          | + {pluralize('region', 'regions', regions.length - 1)} |&nbsp;
+          <StyledLinkButton
+            onClick={() => {
+              setHostNames(regions);
+              setShowHostNamesDrawers(true);
+            }}
+          >
+            Show All
+          </StyledLinkButton>
+          <StyledCopyIcon text={allEndpoints} />
+        </>
+      ) : (
+        <StyledCopyIcon text={firstRegion.s3_endpoint} />
       )}
     </TableCell>
   );
@@ -59,5 +70,5 @@ const StyledCopyIcon = styled(CopyTooltip)(({ theme }) => ({
     top: 1,
     width: 12,
   },
-  marginLeft: theme.spacing(),
+  marginLeft: theme.spacing(0.5),
 }));
diff --git a/packages/manager/src/features/ObjectStorage/BucketDetail/AccessSelect.test.tsx b/packages/manager/src/features/ObjectStorage/BucketDetail/AccessSelect.test.tsx
@@ -38,8 +38,8 @@ describe('AccessSelect', () => {
     ['bucket', 'E1', true],
     ['bucket', 'E2', false],
     ['bucket', 'E3', false],
-    ['object', 'E0', true],
-    ['object', 'E1', true],
+    ['object', 'E0', false],
+    ['object', 'E1', false],
     ['object', 'E2', false],
     ['object', 'E3', false],
   ])(

diff --git a/packages/manager/src/features/ObjectStorage/BucketDetail/AccessSelect.tsx b/packages/manager/src/features/ObjectStorage/BucketDetail/AccessSelect.tsx
@@ -52,10 +52,10 @@ export const AccessSelect = React.memo((props: Props) => {
 
   const { close: closeDialog, isOpen, open: openDialog } = useOpenClose();
   const label = capitalize(variant);
+
+  // CORS is only available at a bucket level, not at an object level.
   const isCorsAvailable =
-    (variant === 'bucket' || variant === 'object') &&
-    endpointType !== 'E2' &&
-    endpointType !== 'E3';
+    variant === 'bucket' && endpointType !== 'E2' && endpointType !== 'E3';
 
   const {
     data: bucketAccessData,

diff --git a/packages/manager/src/features/ObjectStorage/BucketDetail/index.tsx b/packages/manager/src/features/ObjectStorage/BucketDetail/index.tsx
@@ -64,7 +64,7 @@ export const BucketDetailLanding = React.memo((props: Props) => {
 
   const { endpoint_type } = bucket ?? {};
 
-  const isSSLEnabled = endpoint_type !== 'E2' && endpoint_type !== 'E3';
+  const isGen2Endpoint = endpoint_type === 'E2' || endpoint_type === 'E3';
 
   const tabs = [
     {
@@ -75,15 +75,15 @@ export const BucketDetailLanding = React.memo((props: Props) => {
       routeName: `${props.match.url}/access`,
       title: 'Access',
     },
-    ...(flags.objectStorageGen2?.enabled
+    ...(isObjectStorageGen2Enabled
       ? [
           {
             routeName: `${props.match.url}/properties`,
             title: 'Properties',
           },
         ]
       : []),
-    ...(isSSLEnabled
+    ...(!isGen2Endpoint
       ? [
           {
             routeName: `${props.match.url}/ssl`,
@@ -136,7 +136,7 @@ export const BucketDetailLanding = React.memo((props: Props) => {
                 endpointType={endpoint_type}
               />
             </SafeTabPanel>
-            {flags.objectStorageGen2?.enabled && bucket && (
+            {isObjectStorageGen2Enabled && bucket && (
               <SafeTabPanel index={2}>
                 <BucketProperties bucket={bucket} />
               </SafeTabPanel>

diff --git a/packages/manager/src/queries/object-storage/requests.ts b/packages/manager/src/queries/object-storage/requests.ts
@@ -166,13 +166,29 @@ export const getAllBucketsFromEndpoints = async (
     return { buckets: [], errors: [] };
   }
 
+  // Initialize a Map to group endpoints by region for better error handling and flexibility.
+  const endpointsByRegion = new Map<string, ObjectStorageEndpoint[]>();
+
+  for (const endpoint of endpoints) {
+    const existingEndpoint = endpointsByRegion.get(endpoint.region) || [];
+
+    // Update the Map with the current endpoint, maintaining all endpoints per region.
+    endpointsByRegion.set(endpoint.region, [...existingEndpoint, endpoint]);
+  }
+
   const results = await Promise.all(
-    endpoints.map((endpoint) =>
+    Array.from(endpointsByRegion.entries()).map(([region, regionEndpoints]) =>
       getAll<ObjectStorageBucket>((params) =>
-        getBucketsInRegion(endpoint.region, params)
+        getBucketsInRegion(region, params)
       )()
-        .then((data) => ({ buckets: data.data, endpoint }))
-        .catch((error) => ({ endpoint, error }))
+        .then((data) => ({
+          buckets: data.data,
+          endpoints: regionEndpoints,
+        }))
+        .catch((error) => ({
+          endpoints: regionEndpoints,
+          error,
+        }))
     )
   );
 
@@ -183,7 +199,10 @@ export const getAllBucketsFromEndpoints = async (
     if ('buckets' in result) {
       buckets.push(...result.buckets);
     } else {
-      errors.push({ endpoint: result.endpoint, error: result.error });
+      // For each endpoint in the region, log the error to provide detailed error information.
+      result.endpoints.forEach((endpoint) => {
+        errors.push({ endpoint, error: result.error });
+      });
     }
   });