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

Arbitrary validation rules in feature and variant keys #254

Closed
maarten-blokker opened this issue May 14, 2019 · 2 comments
Closed

Arbitrary validation rules in feature and variant keys #254

maarten-blokker opened this issue May 14, 2019 · 2 comments

Comments

@maarten-blokker
Copy link

Expected Behavior

I want to use numbers as variant keys and capital letters in my flag keys.

Current Behavior

I'm not allowed due to the current regular expression.

Possible Solution

Change the regular expression to be more permissive in Util.isSafeKey:
https://github.com/checkr/flagr/blob/master/pkg/util/util.go

Something like: ^[\w\d-]+$ would allow a user to use lowercase, uppercase, numbers, underscores and dashes in their keys.

Steps to Reproduce (for bugs)

  1. Create a new flag
  2. Change flag key to: "TEST"
  3. An error pops up saying: "cannot create flag due to invalid key. reason: key:TEST should have the format ^[a-z]+[a-z0-9_]*$"

Context

We are currently switching from a different system which allowed us to use any test key. We need to rename some of our flag and variant keys in order to work with Flagr. I don't really see the point of having the key names so restrictive.

@maarten-blokker maarten-blokker changed the title Arbitrary validation rules in test name and variant names Arbitrary validation rules in feature and variant keys May 14, 2019
@zhouzhuojie
Copy link
Collaborator

@maarten-blokker I think it makes sense to use ^[\w\d-]+$, I restricted it before and hoped that the key (flag_key and variant_key) can be shaped in a simple form like ^[a-z]+[a-z0-9_]*$, but didn't expect people are actually migrating from their existing system. We can work on fixing this.

@zhouzhuojie
Copy link
Collaborator

Thanks to @raviambati

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 a pull request may close this issue.

2 participants