upcoming: [DPS-42089] - label name update for object storage buckets - #655
3 commits merged into
Conversation
santoshp210-akamai
left a comment
santoshp210-akamai
left a comment
There was a problem hiding this comment.
Very simple and easy to follow logic. Looks good. But I feel like having UTs would be better.
Approval pending till UTs are added.
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared formatting helper to change how Object Storage resource hostnames are displayed in CloudPulse (e.g., simplifying bucket.region.linodeobjects.com into a shorter label), and applies it in both widget dimension labeling and the resources dropdown for Object Storage.
Changes:
- Added
formatObjectStorageUrlto transform Object Storage hostnames into a more compact label. - Applied formatting to Object Storage
entity_idlabels in widget dimension/legend naming. - Applied formatting to Object Storage resource labels in
CloudPulseResourcesSelect.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/manager/src/features/CloudPulse/Utils/utils.ts |
Adds formatObjectStorageUrl helper for Object Storage hostname label formatting. |
packages/manager/src/features/CloudPulse/Utils/CloudPulseWidgetUtils.ts |
Uses the formatter to display Object Storage entity labels in widget dimension names. |
packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx |
Uses the formatter to display formatted labels for Object Storage resources in the selector. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const formatObjectStorageUrl = (url: string): string => { | ||
| const parts = url.split('.'); | ||
|
|
||
| // Need at least 3 parts for a valid object storage URL | ||
| // e.g., 'bucket.region.linodeobjects.com' has 4 parts minimum | ||
| if (parts.length < 3) { | ||
| return url; // Return original if format doesn't match | ||
| } | ||
|
|
||
| // Remove 'linodeobjects' and 'com' from the end | ||
| const withoutDomain = parts.slice(0, -2); | ||
|
|
||
| if (withoutDomain.length === 0) { | ||
| return url; // Return original if nothing left after removing domain | ||
| } | ||
|
|
||
| // Get the region (last part after removing domain) | ||
| const region = withoutDomain[withoutDomain.length - 1]; | ||
|
|
||
| if (!region) { | ||
| return url; // Return original if no region found | ||
| } | ||
|
|
||
| // Get everything before the region | ||
| const prefix = withoutDomain.slice(0, -1).join('.'); | ||
|
|
||
| // If there's no prefix, just return the region in brackets | ||
| if (!prefix) { | ||
| return `[${region}]`; | ||
| } | ||
|
|
||
| // Combine prefix and region in brackets | ||
| return `${prefix}[${region}]`; |
There was a problem hiding this comment.
formatObjectStorageUrl currently formats any string with 3+ dot-separated segments, regardless of whether it actually ends with linodeobjects.com. This can produce incorrect output (e.g., us-east-1.linodeobjects.com becomes [us-east-1], and non-object-storage dotted labels could be mangled). Consider first verifying the hostname ends with .linodeobjects.com and requiring at least 4 segments (bucket.region.linodeobjects.com) before transforming; otherwise return the original input.
| // Need at least 3 parts for a valid object storage URL | ||
| // e.g., 'bucket.region.linodeobjects.com' has 4 parts minimum | ||
| if (parts.length < 3) { | ||
| return url; // Return original if format doesn't match | ||
| } |
There was a problem hiding this comment.
The comment says a valid object storage URL like bucket.region.linodeobjects.com has 4 parts minimum, but the guard only checks for < 3. This mismatch makes the function transform 3-part hostnames (endpoints) even though they don’t match the documented bucket format. Align the guard and logic with the documented expected format.
| const getResourcesList = React.useMemo<CloudPulseResources[]>(() => { | ||
| return filterUsingDependentFilters(resources, xFilter) ?? []; | ||
| }, [resources, xFilter]); | ||
| const filteredResources = | ||
| filterUsingDependentFilters(resources, xFilter) ?? []; | ||
| if (resourceType !== 'objectstorage') { | ||
| return filteredResources; | ||
| } | ||
|
|
||
| return filteredResources.map((resource: CloudPulseResources) => { | ||
| if (resource.label) { | ||
| return { | ||
| ...resource, | ||
| label: formatObjectStorageUrl(resource.label), | ||
| }; | ||
| } | ||
| return resource; | ||
| }); | ||
| }, [resourceType, resources, xFilter]); |
There was a problem hiding this comment.
New object storage label formatting behavior isn’t covered by this component’s existing test suite. Add a test case for resourceType="objectstorage" that verifies bucket/hostname labels are rendered in the formatted form (and that selection still uses the underlying id).
| if (key === 'entity_id') { | ||
| const resourceName = mapResourceIdToName(value, resources); | ||
| const index = groupBy.indexOf(key); | ||
| const formattedName = | ||
| serviceType === 'objectstorage' | ||
| ? formatObjectStorageUrl(resourceName) | ||
| : resourceName; | ||
|
|
||
| if (index !== -1) { | ||
| labels[index] = resourceName; | ||
| labels[index] = formattedName; | ||
| } else { | ||
| labels.push(resourceName); | ||
| labels.push(formattedName); |
There was a problem hiding this comment.
getDimensionName now formats all entity_id labels when serviceType === 'objectstorage'. Given useResourcesQuery can return object storage endpoints like us-east-1.linodeobjects.com, this will currently display as [us-east-1] due to formatObjectStorageUrl’s broad matching. Once formatObjectStorageUrl is tightened to only transform bucket-style hostnames, this call site should inherit the correct behavior; otherwise consider narrowing when formatting is applied.
|
run_cypress |
agorthi-akamai
closed this pull request by merging all changes
into
ACLPManager:aclp_develop
in
0925263
Description 📝
Highlight the Pull Request's context and intentions.
Changes 🔄
List any change(s) relevant to the reviewer.
Scope 🚢
Upon production release, changes in this PR will be visible to:
Target release date 🗓️
Please specify a release date (and environment, if applicable) to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.
Preview 📷
Include a screenshot
<img src="" />or video<video src="" />of the change.🔒 Use the Mask Sensitive Data setting for security.
💡 For changes requiring multiple steps to validate, prefer a video for clarity.
How to test 🧪
Prerequisites
(How to setup test environment)
Reproduction steps
(How to reproduce the issue, if applicable)
Verification steps
(How to verify changes)
Author Checklists
As an Author, to speed up the review process, I considered 🤔
Check all that apply
As an Author, before moving this PR from Draft to Open, I confirmed ✅