Specify VCL path and load multiple files at once with vcl.load in cli #3906
Conversation
|
quick bugwash notes:
please do also add some more detailed documentation and changes.rst entry |
walid-git
force-pushed
the
vcl_path_534
branch
8 times, most recently
from
53023a4
to
e160358
Compare
|
Note: the pull request is currently failing deprecated persistent storage test cases in circleci, but with 53023a4 that has the feature jammed into We need to find what is causing this, but we weren't able to reproduce it locally. |
walid-git
force-pushed
the
vcl_path_534
branch
8 times, most recently
from
7c91254
to
58b0391
Compare
|
Thanks to @bsdphk 's commit 22f5e216fb22ef4ae17b877ed80a99dc090123ed, all the tests are now passing successfully. Reviews are welcome. |
This commit introduces a new vcl.load_files cli command, which is similar to vcl.load except that it turns the state argument into an optional [-s auto|cold|warm] argument, and adds the ability to specify multiple vcl files at once: vcl.load_files [-s auto|cold|warm] <configname> <file1> [file2, ...] The specified files are parsed in the order they were provided and treated as if they were concatenated and merged into a single vcl file
|
Bugwash: this triggered a CLI privilege discussion in need for an overview. |
This commit adds an optional -p argument to vcl.load_files cli command to allow the user to specify a path that will be prepended to the VCL_PATH parameter when loading the vcl files
|
Updated the doc to mention that -p fails when vcl_path is read only |
|
bugwash: @bsdphk wants to finish a review of the CLI privilege design before making a decision on this ticket. |
|
I found myself giving @walid-git to read what I still consider to date the best random outburst (ever since I first read it). https://varnish-cache.org/docs/trunk/phk/barriers.html Seeing the diagram, I noticed the ADMIN vs OPER distinction @bsdphk was making on IRC when we last discussed this. My claim was that this distinction was a bit too stringent, especially since our defaults require root privileges to read the CLI secret. According to the diagram, the CLI actually is an admin privilege and the "mere" operator only has VSM access (and even that requires specific credentials with our defaults). So overriding the |
| if (pp->flags & PROTECTED) { | ||
| VCLI_SetResult(cli, CLIS_AUTH); | ||
| VCLI_Out(cli, "parameter \"vcl_path\" is protected.\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
To clarify my previous comment, since both varnishd -r and vcl.load_files require admin privileges as per our security model, I would be fine relaxing this rule and allowing both a read-only global vcl_path and per-VCL additional path since they take precedence.
|
This is on hold until we have agreed if the 2010 vintage security barriers are still the right one for us. Input solicited, in particular on different "levels" of CLI access and how to determine which one to offer on a given connection. |
|
bugwash: Varnish Software will collect security barriers requirements and submit them for discussion. Note to self: #3729 should serve as input too. |
This PR adds two features to vcl.load cli command:
In order to have a way to accept multiple files, we need to be able to accept any number of arguments (>2), but with the current implementation there will be a confusion for the last argument as there is no way to tell if it is a state (auto|cold|warm) or a VCL file. To address that, the first commit turns the state argument into an optional [-s auto|cold|warm] argument, and the second one does the same for vcl.inline for compliance.
The new format for vcl.load command is the following:
vcl.load [-s auto|cold|warm] [-p vcl_path] <configname> <file1> [file2, ...]