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.

Sign up for GitHub

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

Jump to bottom

#37 adding optional attributes parameter; #42

Merged
merged 3 commits into from
Sep 29, 2023
Merged

#37 adding optional attributes parameter; #42

merged 3 commits into from
Sep 29, 2023

Conversation

einsiedlerkrebs
Copy link
Contributor

List of colon separated KEY:VALUE pairs of jail attributes (e.g. devfs_ruleset:3)

@grembo
Copy link
Contributor

grembo commented Sep 28, 2023 •
edited
Loading

At first glance I see two improvements:

  1. Allow turning off the feature by config (as sending in attributes can be dangerous)
  2. It suffers from the same quoting problems as other parts of the plugin (see Env vars are inserted unquoted #36)
  3. Using the current syntax, it's not possible to have ':' in a value (could be solved easily by using SplitN with N being 2)

Maybe you could look into solving those? (for 2. there are various libraries available one could chose from). If you get stuck with 1. we're happy to help.

grembo
grembo requested changes Sep 28, 2023
View reviewed changes
driver/prepare.go Show resolved Hide resolved
driver/prepare.go Outdated Show resolved Hide resolved
RP and others added 2 commits September 29, 2023 10:10
@grembo
bsdpot#37 adding optional attributes parameter; List of colon separ…
470dbbe 
…ated `KEY:VALUE` pairs of jail attributes (e.g. `devfs_ruleset:3`)
@grembo
Escape attributes, improve splitting
1f4ec35
@grembo grembo merged commit dff6935 into bsdpot:master Sep 29, 2023
@grembo
Copy link
Contributor

grembo commented Sep 29, 2023

@einsiedlerkrebs As we want to release before the end of Q3, I did the most pressing changes myself and merged this now. I'll develop driver config to make this feature opt-in afterwards. Thanks for your contribution!

@einsiedlerkrebs
Copy link
Contributor Author

einsiedlerkrebs commented Sep 29, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grembo grembo grembo requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@einsiedlerkrebs @grembo