Skip to content

ARSN-672: export the destination ARN parser and names - #2745

Open
anurag4DSB wants to merge 2 commits into
development/8.7from
improvement/ARSN-672-arn-parser-name-grammar
Open

anurag4DSB wants to merge 2 commits into
development/8.7from
improvement/ARSN-672-arn-parser-name-grammar

Conversation

@anurag4DSB

@anurag4DSB anurag4DSB commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

adds one shared helper, models.NotificationDestination (parseArn, isValidName, isGlobal). With it, CloudServer and backbeat read a bucketnotif destination ARN and check a destination name with the same rules, and existing exports keep their behavior.

Intent: why does this change exist?

CloudServer and backbeat must read a bucket notification destination ARN and check a destination name the same way, and Arsenal has neither: ARN.createFromString rejects bucketnotif, and the only check is a private prefix test in NotificationConfiguration.

System impact: what's affected, including downstream?

Additive only: one new module and one new export, models.NotificationDestination, with parseArn, isValidName and isGlobal. ARN.createFromString and NotificationConfiguration do not change, so no component that bumps Arsenal 8.7 gets a different answer.

Preserved behavior: what explicitly stays the same?

Every existing export and its answers. A global ARN (empty account) is read as today, with any region and any non-empty name, so stored global ARNs stay valid.

Intended change: what's different after this PR?

A tenant ARN, arn:scality:bucketnotif::<12-digit account>:<name>, parses to its account and name, with an empty region and a name in Vault's IAM name grammar, 1 to 128 characters. A stored ARN with an account outside those rules returns null: before tenant destinations, readers used only the name field, so how CloudServer and backbeat treat such stored configurations is an open spec question for their tasks.

Verification: how do we know this worked, or how would we know if it didn't?

The new spec fails on the test commit and passes with the code; the full unit suite, tsc, lint and the prettier check pass. As on the base, yarn test exits 1 after the open-handle warning of requestUtils.spec.js.

Tenant destination ARNs must have six fields, an empty region, a
12-digit account and a name in the IAM group and policy grammar, 1 to
128 characters. Global ARNs keep being read as today, with any region
and any non-empty name. The new module is reachable from models, and
the existing models do not change. The tests fail on the base, because
the module does not exist yet.
Add models.NotificationDestination, with parseArn, isValidName and
isGlobal, so that CloudServer and backbeat read a destination ARN and
check a name the same way. The name pattern is the one of Vault's IAM
name check. ARN.createFromString and NotificationConfiguration do not
change, so no existing caller gets a different answer.
@bert-e

bert-e commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Hello anurag4dsb,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval ⭐
/bypass_build_status Bypass the build and test status ⭐
/bypass_commit_size Bypass the check on the size of the changeset TBA ⭐
/bypass_incompatible_branch Bypass the check on the source branch prefix ⭐
/bypass_jira_check Bypass the Jira issue check ⭐
/bypass_peer_approval Bypass the pull request peers' approval ⭐
/bypass_leader_approval Bypass the pull request leaders' approval ⭐
/bypass_source_branch_lineage Bypass the cross-branch contamination check ⭐
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request.
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e

bert-e commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • 2 peers

@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.41%. Comparing base (3e799f5) to head (9c57ff1).
⚠️ Report is 4 commits behind head on development/8.7.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           development/8.7    #2745      +/-   ##
===================================================
+ Coverage            75.39%   75.41%   +0.01%     
===================================================
  Files                  226      227       +1     
  Lines                18634    18656      +22     
  Branches              3895     3868      -27     
===================================================
+ Hits                 14049    14069      +20     
- Misses                4580     4582       +2     
  Partials                 5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anurag4DSB
anurag4DSB marked this pull request as ready for review November 3, 2026 12:04
@anurag4DSB
anurag4DSB requested review from a team, nicolas2bert and tmacro November 3, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants