Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject policy with comma in name #5091

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jinapurapu
Copy link
Contributor

Community Contribution License

All community contributions in this pull request are licensed to the project maintainers
under the terms of the Apache 2 license.
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.

Description

Adds check to throw error if policy name contains comma.

Motivation and Context

Policy lists are comma delimited, so policy names may not contain commas. Before, mc allowed creation of policy with comma in name.

How to test this PR?

Attempt to create a policy with a comma in the policy name.

/mc admin policy create minio "testPolicy,withcomma" ~/mytestpolicy.json
mc: <ERROR> Only a single policy may be specified here. policy name may not include comma.

Types of changes

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

@taran-p
Copy link
Contributor

taran-p commented Dec 3, 2024

This change looks fine, but should we add it on the server side instead? That way the user gets a consistent error in console as well as mc

@taran-p
Copy link
Contributor

taran-p commented Dec 3, 2024

Actually, creating a policy with a comma in console doesn't actually fail, it just creates a broken policy so this should definitely be added server side

@cniackz
Copy link

cniackz commented Dec 3, 2024

We should not support policy names with commas.

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.

3 participants