-
Notifications
You must be signed in to change notification settings - Fork 2k
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
use allow/deny instead of the colored alternatives #9019
Conversation
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @greut!
As you can see there's a bunch of tiny backwards compatibility issues to getting this right, so we hadn't quite gotten around to doing this yet. So thank you so much for this PR!
I've left some remarks around ensuring backwards compat. Also, if you'd be willing to take a crack at updating the documentation that'd be great. Otherwise I'd be happy to do it myself in a follow-up PR once this gets merged.
@@ -43,7 +43,7 @@ func validateTask(task *structs.Task, taskEnv *taskenv.TaskEnv, conf *config.Con | |||
var mErr multierror.Error | |||
|
|||
// Validate the user | |||
unallowedUsers := conf.ReadStringListToMapDefault("user.blacklist", config.DefaultUserBlacklist) | |||
unallowedUsers := conf.ReadStringListToMapDefault("user.denylist", config.DefaultUserDenylist) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want to update ReadStringListToMapDefault
to do the same sort of thing we've done with ReadStringListToMap
so that we can keep backwards compatibility with user.blacklist
.
Marking this for the 0.13 milestone because I want to make sure we get this in. |
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks again for this @greut!
I'll follow-up on this with updating the docs to match! |
Signed-off-by: Yoan Blanc <[email protected]>
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Follow up of #8716 (on top of #8988)